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.63k stars 348 forks source link

Add ability to filter contracts by description through config #951

Open Maxim-Filimonov opened 2 years ago

Maxim-Filimonov commented 2 years ago

Feature description

I would like to be able to filter pacts by description the same way that is done now through PACT_DESCRIPTION environment variable. I guess it could be an option to verifier or to pact config.

Use case

We have separated our backend verification tests based on description convention on the client side. We tag contracts(Add [Auth] to description) in different ways and then backend runs contracts only with those tags. At this moment we are overriding env variable is error prone as it needs to be reset every time and removes ability for devs to override test behaviour. Would be great to not set env variable in our test code and use programmatic way to filter pacts instead.

TimothyJones commented 2 years ago

I'm not a maintainer any more, but I think this is actually missing on purpose: see some related discussion here - https://github.com/pact-foundation/pact-net/issues/105#issuecomment-323243832

We tag contracts(Add [Auth] to description) in different ways and then backend runs contracts only with those tags.

This definitely is not how you're supposed to use PACT_DESCRIPTION. What are you trying to achieve?

mefellows commented 2 years ago

Perhaps a bit more information on your workflow would help us understand if you should be looking at a different approach here. I don't believe we allow you to publish verification results in the case you filter in this way, so key broker functionality won't work (e.g. can-i-deploy will always fail because there won't ever be a successful result).

But I think it makes sense nonetheless to support the flag in code if we support it via environment variables anyway

Maxim-Filimonov commented 2 years ago

This definitely is not how you're supposed to use PACT_DESCRIPTION. What are you trying to achieve?

We are trying to make our verification tests on the backend manageable and not run out of jest 30 seconds timeout. Our test suite cover all the contracts and groups of contracts are run as individual tests. Like this:

it('validates the expectations of auth endpoint consumers', async () => {
   // this uses pact description to set filter
    filterContractsByDescription('Auth')

    const stateHandlers = { ...hasSeniorAccount }

    const requestFilters = pipeFilters([updateTokenFilter])

    const opts = {
      ...pactConfig,
      providerBaseUrl: gqlServer,
      stateHandlers: stateHandlers,
      requestFilter: requestFilters,
    }

    await new Verifier(opts).verifyProvider()
  })
  it('validates the expectations of calendar endpoint consumers', async () => {
    filterContractsByDescription('Calendar')

    const stateHandlers = { ...hasSeniorAccount, ...hasRepeatedEvent }

    const requestFilters = pipeFilters([updateTokenFilter])

    const opts = {
      ...pactConfig,
      providerBaseUrl: gqlServer,
      stateHandlers: stateHandlers,
      requestFilter: requestFilters,
    }

    await new Verifier(opts).verifyProvider()
  })
TimothyJones commented 2 years ago

I feel like this isn’t the right approach, as it would be possible to miss tests and not realise.

You can increase the timeout with jest.setTimeout (and if you use jest pact, this is done for you).

mefellows commented 2 years ago

Ah! Thanks this is helpful context/feedback. I think you could deal with timeouts separately (you can easily configure jest to go for longer, which I'm sure you know).

The issue I see with this is that any new consumers would need to be added here (not that big of a deal probably), and as I said earlier, the verification results published to the broker could be misleading/might not actually work (I'd need to check the implementation to see exactly what is done), but at the very least, might make it hard to reason about.

I can see why you might like to split them / get granular feedback rather than one giant test. This has been requested before I believe, there is probably an open feature request somewhere.

Maxim-Filimonov commented 2 years ago

I feel like this isn’t the right approach, as it would be possible to miss tests and not realise.

You can increase the timeout with jest.setTimeout (and if you use jest pact, this is done for you).

Timeout is not the primary reason. Granularity is. Ideally if pact can generate test per contact for us we wouldn't need to do this split.

We noticed that backend team gets overloaded with too much logs when we use one gigantic test to do everything.

Implementation is pretty simple:

function filterContractsByDescription(description) {
  if (!process.env.PACT_DESCRIPTION) {
    process.env.PACT_DESCRIPTION = description
  }
}

we also have reset function which runs after every test and does PACT_DESCRIPTION = null