pact-js icon indicating copy to clipboard operation
pact-js copied to clipboard

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

Open pdeszynski opened this issue 2 years ago • 6 comments

Software versions

Please provide at least OS and version of pact-js

Issue Checklist

Please confirm the following:

  • [x] I have upgraded to the latest
  • [ ] I have the read the FAQs in the Readme
  • [x] I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
  • [x] I have set my log level to debug and attached a log file showing the complete request/response cycle
  • [x] For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem (https://github.com/pdeszynski/pact-test)

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

pdeszynski avatar Feb 27 '23 21:02 pdeszynski

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

mefellows avatar Feb 28 '23 06:02 mefellows

@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

pdeszynski avatar Feb 28 '23 07:02 pdeszynski

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

mefellows avatar Feb 28 '23 07:02 mefellows

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
        },

TimothyJones avatar Feb 28 '23 12:02 TimothyJones

@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

pdeszynski avatar Feb 28 '23 12:02 pdeszynski

Ah right! Interesting use case

TimothyJones avatar Feb 28 '23 12:02 TimothyJones