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

Term/array on query parameters generate invalid matching rules and expected value #1063

Open pdeszynski opened 1 year ago

pdeszynski commented 1 year ago

Software versions

Please provide at least OS and version of pact-js

Issue Checklist

Please confirm the following:

Expected behaviour

When running test like:

import { MatchersV3, TemplateQuery } from '@pact-foundation/pact';
import { HTTPMethods } from '@pact-foundation/pact/src/common/request';
import { pactWith } from 'jest-pact/dist/v3';
import axios from 'axios';
import qs from 'querystring';

pactWith({ consumer: 'Test', provider: 'Test' }, (interaction) => {
  interaction('Test', ({ provider, execute }) => {
    const request = {
      ids: MatchersV3.eachLike('id', 1),
    };

    const response = [{ example: 'response' }];

    beforeEach(() => {
      provider
        .uponReceiving('test request')
        .given('test given')
        .withRequest({
          method: HTTPMethods.GET,
          path: '/expected/url',
          query: request as TemplateQuery,
        })
        .willRespondWith({
          status: 200,
          body: response,
        });
    });

    execute('it will work', async (mockserver) => {
      const result = await axios.get(mockserver.url + '/expected/url', {
        params: {
          ids: ['id'],
        },
        paramsSerializer: {
          serialize: (params: any) => qs.stringify(params),
        },
      });

      expect(result.status).toEqual(200);
    });
  });
});

Should generate a matching rule like:

"query": {
  "$.ids": { //...

Actual behaviour

Running previous test generate pact with a rule on $.ids[0] and an example value for ids. This makes stub-server not matching requests.

{
  "consumer": {
    "name": "Test"
  },
  "interactions": [
    {
      "description": "test request",
      "request": {
        "matchingRules": {
          "query": {
            "$.ids[0]": {
              "combine": "AND",
              "matchers": [
                {
                  "match": "type",
                  "min": 1
                }
              ]
            }
          }
        },
        "method": "GET",
        "path": "/expected/url",
        "query": {
          "ids": [
            "[\"id\"]"
          ]
        }
      },
      "response": {
        "body": [
          {
            "example": "response"
          }
        ],
        "headers": {
          "Content-Type": "application/json"
        },
        "status": 200
      }
    }
  ],
  "metadata": {
    "pact-js": {
      "version": "10.4.1"
    },
    "pactRust": {
      "ffi": "0.4.0",
      "models": "1.0.4"
    },
    "pactSpecification": {
      "version": "3.0.0"
    }
  },
  "provider": {
    "name": "Test"
  }
}

Whether such case is not described in the docs, then using a term should be:

Alternatively, if the order of the query parameters does not matter, you can specify the query as a hash. You can embed Pact::Terms or Pact::SomethingLike inside the hash.

Also an issue was already closed with some PRs being added https://github.com/pact-foundation/pact-reference/issues/205 so I'm assuming that this should work already?

This also leads to wrongly generated matchingRule ($.ids[0] instead of $ids)

Steps to reproduce

Run the snipped above / replace MatchersV3.eachLike with Matchers.term({generate: 'id', matcher: '(.+)'})

Relevant log files

None

mefellows commented 1 year ago

I think this is an allowed scenario.

Also an issue was already closed with some PRs being added https://github.com/pact-foundation/pact-reference/issues/205 so I'm assuming that this should work already?

Assuming this fixes it, we'll need to :

  1. Create another method like this https://github.com/pact-foundation/pact-js-core/blob/master/native/consumer.cc#L732 with the v2 variant of that method
  2. Expose that variant in the JS API
  3. Get a new release of Pact JS Core out
  4. Bump the version in Pact JS to use that release (it will come through transitively, but best to pull it in explicitly)
  5. Update Pact JS to send through the correct matcher structure as defined in that pact-reference issue
pdeszynski commented 1 year ago

@mefellows thank you for a fast answer.

Sorry, I didn't look whether it's already released 😞

Is there any suggested workaround to handle arrays right now? I've tried to use term / regex on the query field itself, but that does lead to serialize term as a json instead of adding matching rules.

The only way that comes into my mind right now is to change tests to be parametrized and generate pacts for multiple array lengths

mefellows commented 1 year ago

I don't know of a workaround just yet, but if you're up for a PR that might help shortcut a resolution?

TimothyJones commented 1 year ago

Aside, but why are you trying to put matchers in the query parameter?

        params: {
          ids: ['id'], <-- this is exactly what you're sending, you don't need a matcher
        },
pdeszynski commented 1 year ago

@TimothyJones I've explicitly passed there an exact value so the contract definition would generate, I know that the test does not make sense in any way.

I'm using matchers in most of the cases to have generic endpoints in pact stub server, this way I do not have to take care of different array lengths (for e.g. to handle IDs from a DataLoader)

I don't know of a workaround just yet, but if you're up for a PR that might help shortcut a resolution?

Yeah, I think that should be the way to go

TimothyJones commented 1 year ago

Ah right! Interesting use case