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

Consumer Compatibility Suite (V1) #468

Merged
merged 14 commits into from
Nov 15, 2023
Merged

Conversation

JP-Ellis
Copy link
Contributor

@JP-Ellis JP-Ellis commented Nov 12, 2023

📝 Summary

This implements the first (of many) compatibility suite tests: V1/http_consumer.feature.

This PR focuses primarily on the compatibility suite, but also includes a few changes to the v3 module in order to implement features required for the compatibility suite, or bugs identified as a result of the compatibility suite.

A couple of scenarios from the feature file have been disabled for now, pending on a resolution from upstream.

Note to the reviewer

As this PR includes a few changes to other parts of the codebase, it might be best to review the PR on a per-commit basis.

🚨 Breaking Changes

None related to the existing v2 module

🔥 Motivation

🔨 Test Plan

Compatibility suite is the test plan.

🔗 Related issues/PRs

Add two methods to the Pact Server which allow for the verification of
interactions. By default, if there are any mismatches, the server will
raise an exception on exit. It is possible to bypass this if the
failures are expected (such as in unit testing).

Signed-off-by: JP-Ellis <josh@jpellis.me>
I have debated whether to include a submodule or a subtree. Given that
we purely want to use the compatibility suite as a reference and not
modify it, I thought it best to used a submodule. A subtree provides
better compatibility when the files need to subsequently be changed, but
has the downside of being more difficult to update.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
TD002 is a rule pertaining to TODO items and enforces the assignment of
an actor/owner for the task.

As this is an open source project, I don't think this is particularly
relevant, and it should hopefully suffice to have the URL of the related
issue.

Signed-off-by: JP-Ellis <josh@jpellis.me>
In order to more closely match the behaviour of the FFI, where the FFI
allows a `NULL` pointer, the Python wrapper allows a `None` to be
passed. This is change is also reflect in the methods of the user-facing
Pact class.

Signed-off-by: JP-Ellis <josh@jpellis.me>
To help ensure consistency with the `with_body`, I have renamed the
`with_binary_file` to `with_binary_body`, as the former implies that it
expects a path to a file, as opposed to some byte array.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Implement the compability test suite as defined in the V1 consumer
feature file from the Pact Compability Suite.

Signed-off-by: JP-Ellis <josh@jpellis.me>

Signed-off-by: JP-Ellis <josh@jpellis.me>
A recent change to the `content_type` type uncovered an accidental error
in the order of the arguments.

Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis JP-Ellis added type:feature New feature difficulty:hard A task requiring a lot of work and an in-depth understanding of the codebase area:tests Relating to the testing area:v3 Relating to the pact.v3 module type:chore Part of regular code upkeep labels Nov 12, 2023
@JP-Ellis JP-Ellis self-assigned this Nov 12, 2023
As I have opted to use a submodule for the compatibility suite, it needs
to be checked out explicitly when running tests.

Signed-off-by: JP-Ellis <josh@jpellis.me>
@rholshausen
Copy link

LGTM

Copy link
Member

@mefellows mefellows left a comment

Choose a reason for hiding this comment

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

Nice work! I mostly just browsed through it rather than fine tooth comb given the volume. The regex and other syntax for the parameterization is obscure/interesting, but it looks to have done the job!

@JP-Ellis
Copy link
Contributor Author

JP-Ellis commented Nov 13, 2023

It should is pretty standard Python regex, with the main uncommon feature being the naming of groups using (?P<name>...). This is required to ensure the group can be referenced by name 🙂

PS: I will look into what's going on with the failing tests tomorrow morning.

@mefellows
Copy link
Member

Oh, I just meant it's kind of funky to read it in the descriptions that way. The negation thing was also cool.

@YOU54F
Copy link
Member

YOU54F commented Nov 13, 2023

Might be worth adding in the docs that to run these, one would have to pull in the submodules

git submodule update --init

@YOU54F
Copy link
Member

YOU54F commented Nov 13, 2023

Nice work, I had assumed we would be able to see the python code, for each feature file example, which is why I kept talking about potentially using these as examples of lib usage for a particular scenario but I like the way it has been designed.

Great work

@YOU54F
Copy link
Member

YOU54F commented Nov 13, 2023

cirrus-ci looks like they are failing due to not checking out submodules 👍🏾

There are two hatch scripts which call pytest, `hatch run test` and
`hatch run example`. The scripts allow for additional arguments which
get passed along to `pytest` with an optional default. This was
misconfigured for the examples:

\## Before

`hatch run examples` becomes `pytest run examples/`
`hatch run examples --broker-url=...` becomes `pytest run --broker-url=...`

\## Now

`hatch run examples` becomes `pytest run examples/`
`hatch run examples --broker-url=...` becomes `pytest run examples/ --broker-url=...`

Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
With the addition of a git submodule for the compatibility suite, the
contributing docs needed to be updated to ensure the additional step is
documented.

Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis JP-Ellis enabled auto-merge (rebase) November 15, 2023 05:44
In order to reduce the burden for new contributors, I have added a
pytest fixture which checks that the submodule has been created. If it
is not, it will try and run `git submodule init`.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis JP-Ellis force-pushed the feat/consumer-compatibility-suite branch from 2de1197 to 5d16769 Compare November 15, 2023 06:12
@JP-Ellis JP-Ellis disabled auto-merge November 15, 2023 07:27
@JP-Ellis JP-Ellis merged commit e49ced9 into master Nov 15, 2023
26 of 32 checks passed
@JP-Ellis JP-Ellis deleted the feat/consumer-compatibility-suite branch November 15, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:tests Relating to the testing area:v3 Relating to the pact.v3 module difficulty:hard A task requiring a lot of work and an in-depth understanding of the codebase type:chore Part of regular code upkeep type:feature New feature
Projects
Status: ✅ Completed
Development

Successfully merging this pull request may close these issues.

v1/http_consumer.feature
4 participants