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

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

Open
4 of 5 tasks
pdeszynski opened this issue Feb 27, 2023 · 6 comments
Open
4 of 5 tasks
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@pdeszynski
Copy link

pdeszynski commented Feb 27, 2023

Software versions

Please provide at least OS and version of pact-js

  • OS: Mac OSX 13.2.1
  • Consumer Pact library: @pact-foundation/pact-core@13.13.4, @pact-foundation/pact@10.4.1, jest-pact@0.10.2
  • Node Version: v19.7.0

Issue Checklist

Please confirm the following:

  • I have upgraded to the latest
  • I have the read the FAQs in the Readme
  • I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
  • I have set my log level to debug and attached a log file showing the complete request/response cycle
  • 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 pact-foundation/pact-reference#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 pdeszynski added the bug Indicates an unexpected problem or unintended behavior label Feb 27, 2023
@pdeszynski pdeszynski changed the title Arrays on query parameters generate invalid matching rules and expected value Term/array on query parameters generate invalid matching rules and expected value Feb 27, 2023
@mefellows
Copy link
Member

mefellows commented Feb 28, 2023

I think this is an allowed scenario.

Also an issue was already closed with some PRs being added pact-foundation/pact-reference#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
Copy link
Author

@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
Copy link
Member

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

@TimothyJones
Copy link
Contributor

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
Copy link
Author

pdeszynski commented Feb 28, 2023

@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
Copy link
Contributor

Ah right! Interesting use case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants