-
Notifications
You must be signed in to change notification settings - Fork 137
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
Pact Consumer #410
Conversation
a153cbb
to
9bd36b2
Compare
9bd36b2
to
be1cc1a
Compare
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome!!!
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) |
also added |
efa3c15
to
8217ab0
Compare
2dfe985
to
2ae3783
Compare
8217ab0
to
d8705e7
Compare
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>
2ae3783
to
2168d22
Compare
I linked to the wrong underlying issue 😅. Fixed. As with the related PRs, I had to rebase this before merging it. |
📝 Summary
Implements the core
Pact
class, along with a number of other classes required for this to all work.🚨 Breaking ChangesNo 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.