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

Pact Consumer #410

Merged
merged 6 commits into from Oct 12, 2023
Merged

Pact Consumer #410

merged 6 commits into from Oct 12, 2023

Conversation

JP-Ellis
Copy link
Contributor

@JP-Ellis JP-Ellis commented Oct 6, 2023

📝 Summary

Implements the core Pact class, along with a number of other classes required for this to all work.

🚨 Breaking Changes

No changes to the existing library, but introduces significant changes to pact.v3.

🔨 Test Plan

Unit tests have been included as part of this PR.

🔗 Related issues/PRs

👀 Notes for Reviewers

As this PR is rather substantial, I have split some of the less consequential commits from the major commit (feat(v3): implement pact class). While the PR as a whole can be reviewed, I would recommend reviewing each commit individually.

@JP-Ellis JP-Ellis self-assigned this Oct 6, 2023
@JP-Ellis JP-Ellis force-pushed the feat/ffi-pact branch 4 times, most recently from a153cbb to 9bd36b2 Compare October 10, 2023 02:14
@JP-Ellis JP-Ellis changed the base branch from master to feat/ffi-wrapper October 10, 2023 02:18
@JP-Ellis JP-Ellis marked this pull request as ready for review October 10, 2023 02:21
@JP-Ellis JP-Ellis requested a review from YOU54F October 10, 2023 02:32
Copy link

@rholshausen rholshausen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

users of the library, and avoid unnecessarily casting to and from possibly
complicated structs.

In the Rust library, the handles are thin wrappers around integers, and

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done to make it easier for Go and JS, but if it is a problem, we can change it to be a pointer type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not at all an issue. It's just that a struct { int } has the same in-memory representation as a bare int. I added the thin wrappers for two reasons:

  • Obfuscating the int. After all, it doesn't make sense to 'add' or 'subtract' handles (unlike integers). It also makes it explicit that the handles to a Pact or an Interaction are different.
  • This makes it easy to implement the __del__ method to ensure that memory is cleaned up when Python's garbage control executes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanation @JP-Ellis 👍🏾 thank you!

Copy link
Member

@YOU54F YOU54F left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!!!

@YOU54F
Copy link
Member

YOU54F commented Oct 12, 2023

🔗 Related issues/PRs
Resolves: #376

Should this be Add support and example of a HTTP consumer test without the plugin bit. I couldn't see a plugin being loaded in any of the unit tests.

The methods are exposed, but there isn't an example unit test as per linked ticket (that i've seen)

@YOU54F
Copy link
Member

YOU54F commented Oct 12, 2023

also added g++ to the linux image deps to support a couple of python versions that were failing to build

JP-Ellis and others added 6 commits October 13, 2023 09:03
Specifically want to make it clear how this module is going to be
implemented, and lay down guidelines to be followed going forward.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
This helps makes enums more manageable during logging and debugging.

The `D200` Ruff lint is ignored, ensuring that single-line docstrings
use the same formatting as the remaining docstrings.

Signed-off-by: JP-Ellis <josh@jpellis.me>
The `--broker-url` PyTest CLI argument is used by the examples to
determine whether a Broker is automatically launched, or whether an
existing broker is used.

All PyTest CLI arguments _must_ be defined in the root directory of the
project. As a result, the previous location where these arguments were
defined meant that the examples could only be launched of PyTest was
specifically pointed at the `examples/` directory.

This commit moves the definition to the root directory, thereby unifying
the `examples/` with the remaining tests.

Signed-off-by: JP-Ellis <josh@jpellis.me>
This (rather large) commit implements core functionality for the `Pact`
class, and the `Interaction` class to handle specific interactions
within a Pact.

This does _not_ implement every method that Pacts and/or interactions
might have (e.g., it is currently not possible to specify a Pact
version). The intent of this commit is to be a minimal working example
which can be improved upon more incrementally.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Base automatically changed from feat/ffi-wrapper to master October 12, 2023 22:04
@JP-Ellis
Copy link
Contributor Author

🔗 Related issues/PRs
Resolves: #376

Should this be Add support and example of a HTTP consumer test without the plugin bit. I couldn't see a plugin being loaded in any of the unit tests.

The methods are exposed, but there isn't an example unit test as per linked ticket (that i've seen)

I linked to the wrong underlying issue 😅. Fixed.

As with the related PRs, I had to rebase this before merging it.

@JP-Ellis JP-Ellis merged commit a070978 into master Oct 12, 2023
1 of 10 checks passed
@JP-Ellis JP-Ellis deleted the feat/ffi-pact branch October 12, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Completed
Development

Successfully merging this pull request may close these issues.

Add support and example for a HTTP consumer test using Rust engine
3 participants