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

Feature Request - Introduce --provider-state-header-name in pact-verifier #398

Open
spockz opened this issue Mar 1, 2024 · 8 comments
Open
Labels
awaiting feedback Awaiting Feedback from OP enhancement Indicates new feature requests smartbear-supported SmartBear engineering team will support this issue. See https://docs.pact.io/help/smartbear

Comments

@spockz
Copy link

spockz commented Mar 1, 2024

The pact-stub-server has a --provider-state-header-name that allows for selecting a better matching interaction based on a header containing the requested provider state. The pact-verifier-cli (rust implementation) is missing this option. We should probably also add the analog header in the verifier.

@mefellows
Copy link
Member

The reason that flag exists, is because the stub server is loaded with the entire pact contents in advance, so state handlers and descriptions can't be used to delineate the incoming request - specifying a header that can override it makes sense in this context.

In the verification context, only one interaction should be coming in at a time, which can be disambiguated through the state name.

What problem are you trying to solve here?

@mefellows mefellows added the awaiting feedback Awaiting Feedback from OP label Mar 5, 2024
@spockz
Copy link
Author

spockz commented Mar 5, 2024

The short answer is twofold:

  1. speed gained by parallel execution.
  2. The docs state that requests should contain the provider state, and it would be symmetrical.

    NOTE: For verifying messages fetched via HTTP, the provider state is also passed in the request to fetch the message, so the state change URL is not required.

The long story, without going into the full details, is:

We are a pretty big organisation and attempted to adopt contract testing several times across different parts of the organisation. This failed in the past, mostly because the gains were largely for the consumer where the producer carried most of the burden and gained the least benefit. To address this (and other points) we reduce the amount of work required to maintain the setup for verifying the consumer contracts by the provider. Part of this is to re-use the contracts the provider has with its providers again as the backing, together with running the binary exactly as it goes to production.

This means that when verifying the contract the request from the consumer contract flows through the app and reaches the pact stub server where a header can be used to disambiguate between the interactions. We previously integrated with the pact-jvm library where we add the additional headers. And we are successful in enabling providers to easily support contract testing.

Because we were unable to upload pact verification results to the broker using the pact-jvm library we want to pivot to using the platform/language agnostic pact-verifier-cli. It works, except for that we lose the ability to send the provider state header. Therefore, we would like to see it added and are happy to contribute it even.

Update: using the --state-change-url would force us back to sequential verification, which is slow for the scale at which we verifies contracts. We have applications with 100s of consuming services, each with multiple interactions. Verifying these interactions takes a significant amount of time, which reduces the experience on the provider side.

@mjpieters
Copy link
Contributor

mjpieters commented May 17, 2024

The docs state that requests should contain the provider state, and it would be symmetrical.

NOTE: For verifying messages fetched via HTTP, the provider state is also passed in the request to fetch the message, so the state change URL is not required.

You misunderstood that section. It is explicitly talking about message queue verification where the normal producer - consumer interaction doesn't involve HTTP requests (a Pact V3 feature). See the separate section in the README: Verifying message pacts.

For Pact verification involving a backend service, I would avoid using Pact contract testing as a stub. Pact contract verification should really only verify your producer layers, not downstream service implementations, including their contracts. But, if you really must do this, then you already have instrumented your downstream API client and it should be triivial to add your own header in your state change handler.

Running pact verifications concurrently is fraught with issues because you have to confine the producer state for each test to a single request-response pair, but most mocking libraries operate on global state. If you have a lot of different consumers, I'd just run a test matrix on the CI with each job confined to a specific consumer and so run the verification in parallel against isolated producer servers.

@spockz
Copy link
Author

spockz commented May 17, 2024

But, if you really must do this, then you already have instrumented your downstream API client and it should be triivial to add your own header in your state change handler.

Indeed, we have done so, but it limits to executing/verifying a single interaction at a time.

Running pact verifications concurrently is fraught with issues because you have to confine the producer state for each test to a single request-response pair, but most mocking libraries operate on global state. If you have a lot of different consumers, I'd just run a test matrix on the CI with each job confined to a specific consumer and so run the verification in parallel against isolated producer servers.

We have this under control for downstream web services and it works with our custom command based on the pact-jvm. We prefer moving to the official pact-verifier-cli in order to gain the other features of that client (mainly communication with the broker).

Note that we have services that have over 100 consumers and setting these kind of CI jobs up manually is a pain which is why we take a lot of orchestration away. By verifying in parallel we also ensure that verification is feasible to run instead of it becoming a hurdle for teams.

@mjpieters
Copy link
Contributor

Note that we have services that have over 100 consumers

That's where your problem lies. Is there no SDK for your consumers? SDKs would let you reduce that number drastically, because you consolidate all consumer interactions into that SDK. You reduce the cardinality to the number of programming languages / ecosystems your SDK supports.

@mefellows mefellows added the smartbear-supported SmartBear engineering team will support this issue. See https://docs.pact.io/help/smartbear label Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

🤖 Great news! We've labeled this issue as smartbear-supported and created a tracking ticket in PactFlow's Jira (PACT-2020). We'll keep work public and post updates here. Meanwhile, feel free to check out our docs. Thanks for your patience!

@mefellows mefellows removed the smartbear-supported SmartBear engineering team will support this issue. See https://docs.pact.io/help/smartbear label Jun 14, 2024
@mefellows
Copy link
Member

So the pact stub server is used by consumers to stub the backend - i.e. it stubs the provider. The reason why we have that header, is to disambiguate requests - we need this because all of the interactions are loaded at once. It looks something like this:

sequenceDiagram
    User->>Stub Server: ./pact-stub-server {options}
    Stub Server->>User: PID 1234
    User->>Test: run tests
    Test->>Consumer: execute test
    Consumer->>+Stub Server: GET /a/request [x-state-change-header=state happy]
    Stub Server->>-Consumer: HTTP 200 {...}
    Consumer->>Test: results
    Test->>Consumer: execute test
    Consumer->>+Stub Server: GET /a/request [x-state-change-header=state unhapp]
    Stub Server->>-Consumer: HTTP 404 {...}
    Test->>Consumer: ...
Loading

The verifier is the reverse of this - it acts as the consumer. Each request from the verifier comes in to the API one at a time anyway, so we don't need to be able to disambiguate interactions at the request level:

sequenceDiagram
    User->>Verifier: ./pact-verifier {options}
    Verifier->>+API: POST /state/change/URL {"state": "state happy", "params": {...}}
    API->>-Verifier: state change response [OK|ERROR]
    Verifier->>+API: GET /a/request
    API->>V-erifier: HTTP 200
    Note right of Verifier: Next interaction to test...
    Verifier->>+API: POST /state/change/URL {"state": "state unhappy", "params": {...}}
    API->>-Verifier: state change response [OK|ERROR]
    Verifier->>+API: GET /a/request
    API->>-Verifier: HTTP 404
    Note right of Verifier: ...
    Verifier->>User: results
Loading

I think what you're proposing is to change this to:

sequenceDiagram
    User->>Verifier: ./pact-verifier {options}
    Verifier->>+API: GET /a/request [x-state-change-header=state happy]
    API->>-Verifier: HTTP 200
    Note right of Verifier: Next interaction to test...
    Verifier->>+API: GET /a/request [x-state-change-header=state unhappy]
    API->>-Verifier: HTTP 404
    Note right of Verifier: ...
    Verifier->>User: results
Loading

If so, I'd be supportive of this change. If not, I think I'm missing something - could you please perhaps draw out how you'd like to see it work?

@mefellows mefellows added smartbear-supported SmartBear engineering team will support this issue. See https://docs.pact.io/help/smartbear enhancement Indicates new feature requests labels Jul 11, 2024
@mefellows
Copy link
Member

One thing on the final diagram above, if we were to support that, we need to consider the impact on #441 - how would be support provider supplied parameters (if at all?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback Awaiting Feedback from OP enhancement Indicates new feature requests smartbear-supported SmartBear engineering team will support this issue. See https://docs.pact.io/help/smartbear
Projects
None yet
Development

No branches or pull requests

3 participants