Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements #40

Closed
wants to merge 4 commits into from
Closed

Conversation

SiimMardus
Copy link

@SiimMardus SiimMardus commented May 27, 2024

Description

Changes:

  • Modified add_pathz input in the pact_escript module to use the correct path.
  • Added logging of the Output from pact verification

Further details in commit messages.

Fixes: #39 #32

Checklist:

  • I have ensured that all new code is covered by tests
  • I have run make format to format the code according to guidelines
  • I have documented any new functions introduced
  • I have updated the changelog in accordance with https://keepachangelog.com/en/1.1.0/
  • I have updated the README, if required

Siim Mardus added 2 commits May 27, 2024 15:16
Previously the path to ebin did not work properly when a different
folder structure was used. This makes sure that it targets the correct
folder.

(In future commits, this could even be a parameter, for better configurability)
OutputLog contains useful information about what happens with Pact
verification.
@SiimMardus SiimMardus force-pushed the improvements branch 2 times, most recently from 5a403ed to 8a16791 Compare May 29, 2024 12:26
This is useful if the provider wants to use real authentication
flow for the contract tests.

Disclaimer:
`pactffi_verifier_add_custom_header` function unfortunately is not
able to override an existing header, so using this requires the pact
to not include the authorization header in the first place. This can
either be done by simply not using it on the consumer-side test or
removing it somehow before publishing the pact.

Although I worked through the following issues:
- pact-foundation/pact-net#460
- pact-foundation/pact-reference#275
from which the latter is actually marked as closed, and the fix is
allegedly included in the pact-ffi version that we use, it still did
not override an existing header. I tried bumping to a higher version
as well but without any luck. I will create an issue to pact-foundation
to try to get clarity on this.
This update should include the fix for overriding headers.

It was not as straightforward to track down, because it is not
marked in any release notes and the fix commit does not have a
concrete tag..., but:
- [This issue][1] was marked as completed on Feb 22
- [Pact FFI 0.4.17][2] was released on the same date
- 0.4.17 source code shows that the mentioned fix is present in
this version

[1]: pact-foundation/pact-reference#275
[2]: https://github.com/pact-foundation/pact-reference/releases/tag/libpact_ffi-v0.4.17
@SiimMardus
Copy link
Author

Closing as the improvements on original branch are actually not final yet

@SiimMardus SiimMardus closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print out the output from pact verification
1 participant