pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.59k stars 343 forks source link

Provider states are not being setup #1037

Open iamchughmayank opened 1 year ago

iamchughmayank commented 1 year ago

Software versions

Issue Checklist

Please confirm the following:

Issue Description

I am new to Pact and I am trying to create an MVP for contract testing between two of our services.

I was able to setup a simple example test without any state, however, I am unable to setup multiple states using stateHandlers. I have tried following the example here for V3 and also here which uses setup in the providerState but to no avail.

When I run the tests, I am observing two issues with my tests:

  1. providerState is not being set properly before the requestFilter
  2. Even though the contract has the request.method as GET, the API call to the provider service is going with POST. If I manually override it in the requestFilter, I am able to convert it to GET

I have checked various forums/existing issues and Stackoverflow, however, I could not find anyone facing problems with stateHandlers basic implementation.

Any help in

Expected behaviour

I am expecting the stateHandler to change a variable value as per the state name which will be then picked up by the requestFilter to bring provider in desirable state.

Steps to reproduce

Demo repository: https://github.com/iamchughmayank/pact-stateHandler-issue-demo

Relevant log files

Pact File

{
    "consumer": {
      "name": "SupportService"
    },
    "interactions": [
      {
        "description": "a request to get client by id",
        "providerStates": [
          {
            "name": "valid id"
          }
        ],
        "request": {
          "method": "GET",
          "path": "/clients/26"
        },
        "response": {
          "body": {
            "data": [
              {
                "account_status": "test",
                "active": 1,
                "client_id": 24297,
                "client_name": "Rowe - Cummings",
                "created": "2017-07-14T16:32:29.000Z",
                "email_address": "Grover.McCullough@yahoo.com"
              }
            ],
            "result": 1,
            "status": "success"
          },
          "headers": {
            "Content-Type": "application/json"
          },
          "matchingRules": {
            "body": {
              "$": {
                "combine": "AND",
                "matchers": [
                  {
                    "match": "type"
                  }
                ]
              }
            }
          },
          "status": 200
        }
      }
    ],
    "metadata": {
      "pact-js": {
        "version": "10.4.0"
      },
      "pactRust": {
        "ffi": "0.3.19",
        "models": "1.0.3"
      },
      "pactSpecification": {
        "version": "3.0.0"
      }
    },
    "provider": {
      "name": "ClientService"
    }
  }

Provier Spec


/**
 * External dependencies
 */
import {Verifier}  from '@pact-foundation/pact';
import path, { dirname } from 'path';
import { fileURLToPath } from 'url';

const __dirname = dirname(fileURLToPath(import.meta.url));

describe( 'pact Verification', () => {
    let clientId;

    const baseOptions = {
        providerBaseUrl: 'http://localhost:3000',
        provider: 'ClientService',
        providerVersion: '1.0.0',
        log: path.resolve( process.cwd(), 'logs', 'pact.log' ),
        logLevel: 'DEBUG',
        pactUrls: [ path.resolve( __dirname, './SupportService-ClientService.json' ) ],
        stateHandlers: {
            'valid id': {
                setup: () => {
                    clientId = '26';
                    return Promise.resolve( 'client id set to 26' );
                },

            },
            // I have also tried without setup as follows:
            // 'valid id': () => {
            //  clientId = '26';
            //  return Promise.resolve( 'client id set to 26' );
            // },
        },
        requestFilter: ( req, res, next ) => {
            req.url = `/clients/${ clientId }`;
            req.method = 'GET';
            next();
        },
    };

    it( 'should validate the expectations of SupportService for fetching one client by id', () => {
        const verifier = new Verifier( baseOptions );
        return verifier.verifyProvider();
    } );
} );

In the above case, when I run the test, I get a GET request with clientId as undefined even though the stateHandler has been setup to change the id

ClientService | 172.17.0.1 - - [29/Dec/2022:15:46:56 +0000] "GET /clients/undefined HTTP/1.1" 404 67 "-" "-"

The pact tests fail with error:

Verifying a pact between SupportService and ClientService

  a request to get client by id
     Given valid id
      Request Failed - One or more of the setup state change handlers has failed

Failures:

1) Verifying a pact between SupportService and ClientService Given valid id - a request to get client by id - One or more of the setup state change handlers has failed

Debug logs

2022-12-29T19:22:58.585776Z ERROR ThreadId(02) verify_pact_internal{provider_info=ProviderInfo { name: "ClientService", protocol: "http", host: "127.0.0.1", port: Some(52068), path: "/", transports: [ProviderTransport { transport: "http", port: Some(52068), path: Some("/"), scheme: None }] } filter=None pact=RequestResponsePact { consumer: Consumer { name: "SupportService" }, provider: Provider { name: "ClientService" }, interactions: [RequestResponseInteraction { id: None, description: "a request to get client by id", provider_states: [ProviderState { name: "valid id", params: {} }], request: Request { method: "GET", path: "/clients/26", query: None, headers: None, body: Missing, matching_rules: MatchingRules { rules: {} }, generators: Generators { categories: {} } }, response: Response { status: 200, headers: Some({"Content-Type": ["application/json"]}), body: Present(b"{\"data\":[{\"account_status\":\"test\",\"active\":1,\"client_id\":24297,\"client_name\":\"Rowe - Cummings\",\"created\":\"2017-07-14T16:32:29.000Z\",\"email_address\":\"Grover.McCullough@yahoo.com\"}],\"result\":1,\"status\":\"success\"}", None, None), matching_rules: MatchingRules { rules: {BODY: MatchingRuleCategory { name: BODY, rules: {DocPath { path_tokens: [Root], expr: "$" }: RuleList { rules: [Type], rule_logic: And, cascaded: false }} }} }, generators: Generators { categories: {} } } }], metadata: {"pact-js": {"version": "10.4.0"}, "pactRust": {"ffi": "0.3.19", "models": "1.0.3"}, "pactSpecification": {"version": "3.0.0"}}, specification_version: V3 } options=VerificationOptions { request_filter: None, disable_ssl_verification: false, request_timeout: 30000, custom_headers: {}, coloured_output: true, no_pacts_is_error: true } provider_state_executor=HttpRequestProviderStateExecutor { state_change_url: Some("http://127.0.0.1:52068/_pactSetup"), state_change_teardown: true, state_change_body: true, reties: 3 } pending=false}:verify_interaction{interaction="a request to get client by id"}:verify_interaction{provider=ProviderInfo { name: "ClientService", protocol: "http", host: "127.0.0.1", port: Some(52068), path: "/", transports: [ProviderTransport { transport: "http", port: Some(52068), path: Some("/"), scheme: None }] } interaction=RequestResponseInteraction { id: None, description: "a request to get client by id", provider_states: [ProviderState { name: "valid id", params: {} }], request: Request { method: "GET", path: "/clients/26", query: None, headers: None, body: Missing, matching_rules: MatchingRules { rules: {} }, generators: Generators { categories: {} } }, response: Response { status: 200, headers: Some({"Content-Type": ["application/json"]}), body: Present(b"{\"data\":[{\"account_status\":\"test\",\"active\":1,\"client_id\":24297,\"client_name\":\"Rowe - Cummings\",\"created\":\"2017-07-14T16:32:29.000Z\",\"email_address\":\"Grover.McCullough@yahoo.com\"}],\"result\":1,\"status\":\"success\"}", None, None), matching_rules: MatchingRules { rules: {BODY: MatchingRuleCategory { name: BODY, rules: {DocPath { path_tokens: [Root], expr: "$" }: RuleList { rules: [Type], rule_logic: And, cascaded: false }} }} }, generators: Generators { categories: {} } } } pact=RequestResponsePact { consumer: Consumer { name: "SupportService" }, provider: Provider { name: "ClientService" }, interactions: [RequestResponseInteraction { id: None, description: "a request to get client by id", provider_states: [ProviderState { name: "valid id", params: {} }], request: Request { method: "GET", path: "/clients/26", query: None, headers: None, body: Missing, matching_rules: MatchingRules { rules: {} }, generators: Generators { categories: {} } }, response: Response { status: 200, headers: Some({"Content-Type": ["application/json"]}), body: Present(b"{\"data\":[{\"account_status\":\"test\",\"active\":1,\"client_id\":24297,\"client_name\":\"Rowe - Cummings\",\"created\":\"2017-07-14T16:32:29.000Z\",\"email_address\":\"Grover.McCullough@yahoo.com\"}],\"result\":1,\"status\":\"success\"}", None, None), matching_rules: MatchingRules { rules: {BODY: MatchingRuleCategory { name: BODY, rules: {DocPath { path_tokens: [Root], expr: "$" }: RuleList { rules: [Type], rule_logic: And, cascaded: false }} }} }, generators: Generators { categories: {} } } }], metadata: {"pact-js": {"version": "10.4.0"}, "pactRust": {"ffi": "0.3.19", "models": "1.0.3"}, "pactSpecification": {"version": "3.0.0"}}, specification_version: V3 } options=VerificationOptions { request_filter: None, disable_ssl_verification: false, request_timeout: 30000, custom_headers: {}, coloured_output: true, no_pacts_is_error: true } provider_state_executor=HttpRequestProviderStateExecutor { state_change_url: Some("http://127.0.0.1:52068/_pactSetup"), state_change_teardown: true, state_change_body: true, reties: 3 }}: pact_verifier: Provider setup state change for 'valid id' has failed - MismatchResult::Error("Invalid status code: 404", None)
TimothyJones commented 1 year ago

Looks like there's a bug where the request filters are inappropriately applied to the state handlers.

What's happening is the state handler request is being redirected by your request filter, which means it is never called. You can fix this by removing your request filter.

I think the reason this hasn't been picked up before is because that's not how you're supposed to use request filters. I'm not 100% sure what you're trying to do, but you almost never need request filters - they're for rare situations where you need to inject authentication headers because you don't control the auth part of the server.

If you need the state handler to set up a client ID that is used in the test, then you need to:

1) Generate the contract to expect a variable from the provider state in the path (you can use fromProviderState to do this). 2) Return the client ID in your state setup (return { clientId: 12 } or return Promise.resolve({ clientId: 12 }))

iamchughmayank commented 1 year ago

Thanks for a prompt response @TimothyJones :)

I'm not 100% sure what you're trying to do, but you almost never need request filters - they're for rare situations where you need to inject authentication headers because you don't control the auth part of the server.

In my actual provider service, we have fixtures that help us to create on-demand test data for our unit tests to be destroyed at the end of test session. For example, if the unit test is looking for clientId = 100, the fixtures help to create that clientId. This helps us to avoid using static data in our tests and test db which can go stale easily.

I am trying to reuse those fixtures in the contract tests as well by creating the data as per state.

From the test lifecycle in pact documentation, I figured that if I set the clientId for each Given state and then used requestFilter to use that clientId, it should become easy to test various cases like "valid id", "invalid id", "not found", etc. At the same time keeping the consumer test fairly intuitive that path to be tested will be /clients/:id and not something that will be generated by provider.

If you need the state handler to set up a client ID that is used in the test, then you need to:

Generate the contract to expect a variable from the provider state in the path (you can use fromProviderState to do this). Return the client ID in your state setup (return { clientId: 12 } or return Promise.resolve({ clientId: 12 }))

Thanks for pointing that out :) I was able to implement it and get my contract working here: https://github.com/iamchughmayank/pact-stateHandler-issue-demo

While my consumer and provider API teams DO NOT have any comms challenge and both the services are handled internally (no public API here), using both stateHandler and requestFilter felt intuitive. Do you think this issue will be picked up?

TimothyJones commented 1 year ago

That’s a question for a maintainer. I think they’re all taking a well-deserved rest over the Christmas and new year period at the moment.

The request filter isn’t intended for this use case- I think you would run in to other maintenance issues if you tried scaling up that approach (for example, the path rewrite approach only works for one interaction- what happens if you have two?).

Importantly- if you’re rewriting parts of the interaction in the request filter, how do you know that what was tested at the client side matches what is tested at the provider side? The ability to know that your consumer expectation matches your provider behaviour is a key part of pact, and you would lose this if you built a lot on top of rewriting requests in the filters.

mefellows commented 1 year ago

Thanks for the report @iamchughmayank. The request filters are expected to work on the state handlers and requests to your provider. The reason you're seeing the POST is that the framework core actually calls the state handlers over HTTP.

This could be documented better, so I will leave this request open as a documentation issue.

Request filters are a powerful and advanced feature that most users shouldn't need, and as Tim has mentioned you should be able to solve your problem using other features.

TimothyJones commented 1 year ago

In the case where pact is hosting the state handlers itself, I feel like the request filters would be expected not to be applied?

mefellows commented 1 year ago

Yes I think so too, however at this point it would be a breaking change to not also apply it to the provider states (you could perhaps argue it as a bug that is fixed, but the impact would be the same).

I think there is some small utility in preserving that behaviour but not enough to justify a separate feature.

So either we document the behaviour and/or rename it because it's misleading, or remove it and release a new major version.

iamchughmayank commented 1 year ago

Thanks @TimothyJones and @mefellows for the explanation and details :) I am able to use fromProviderState to solve for my use-case :)