[V3] feat: Pact should overwrite existing contract files automatically, or at least provide an option to do so

[lviana-menlosecurity]

Before making a feature request, I have:

• [X] Searched the issues to check that this feature hasn't been requested before
• [X] Checked the documentation to see if it is possible to do what I want already

Feature description

Pact should overwrite existing contract files automatically, or at least provide an option to do so.

Use case

I’m seeing some interesting things regarding pact file creation in pact-js v3 beta and I’m wondering if they're expected. I’ll describe the 2 cases I’m seeing:

1. Let’s say I have 2 consumer tests, and I’ve run them such that the pact file has already been generated locally with both interactions. Now, let’s say that I edit the uponReceiving string slightly in one of those tests and re-run them. It turns out that pact will actually add a third interaction with the updated uponReceiving string to the existing pact file. I was expecting pact to simply regenerate the pact (e.g. overwrite the existing file) with the uponReceiving string updated in the appropriate interaction (meaning, the pact file would still have 2 interactions, rather than 3).
2. Again, let’s say I have 2 consumer tests, and I’ve run them such that the pact file has already been generated locally with both interactions. Now, let’s say I change a boolean value in the request body of one of them from true to false, and re-run them. It turns out that pact will not regenerate the pact file, which will still have the old true value for the boolean.

The common thread here seems to be that pact files may not be re-generated properly if they already exist. I get it that removing pact files before running tests avoids these issues. However, is there any value in doing, for example, what’s described in issue 2 above, namely not updating the existing pact file with the new body values (that one seems like a bug to me)? I was just wondering why doesn’t Pact overwrite existing pact files automatically, or at least provide an option to do so. For example, in PactV3Options there could be a boolean field, called something like overwrite, which when set to true would automatically overwrite a previously existing pact file with the one generated by the current run.

[TimothyJones]

We do support this. I thought it was well documented, but perhaps it isn't as I can't find it right now.

The option you want is pactFileWriteMode. Have a look at https://github.com/pact-foundation/pact-js#parallel-tests for some discussion.

Part of the problem isn't pact, but rather the way that JS testing frameworks work. When Jest runs, it runs each spec separately and potentially in parallel, without context on whether other specs are being run. When mocha runs, it runs all tests in one execution. We need to be able to support both cases.

---

Can you elaborate on why you're expecting an update in your first case? Maybe this needs to be better documented, but uponReceiving exists to provide a key for the interaction (for convenience, the key is also intended to be human-readable). So, when you change the uponReceiving string, you're changing the key for the interaction.

I don't think we can implement the behaviour you're asking for in a way that would work for everyone. If we implemented logic that said "if this new interaction is exactly the same as another one, just rename the other one", we might stamp out interactions from other spec files.

Your second case sounds like a bug. Are you able to share some code that reproduces this?

The common thread here seems to be that pact files may not be re-generated properly if they already exist.

They should be. So, if they're not, let's dig deeper.

The advice to remove the spec files first isn't because interactions can't be updated correctly in place, it's because interactions that no longer exist can't always be removed if pactFileWriteMode is update (for example, in Jest tests we can't construct a list of all interactions delivered in this test run without being very restrictive about the way people write their tests).

[lviana-menlosecurity]

I wasn't aware of pactfileWriteMode so it’s good to know it already exists. And one of the values it can be set to is overwrite, which seems to be exactly what I was looking for. I do see it defined as part of the PactOptions interface: https://github.com/pact-foundation/pact-js/blob/feat/v3.0.0/src/dsl/options.ts#L50 However, I do not see a similar option for the equivalent PactV3Options interface: https://github.com/pact-foundation/pact-js/blob/feat/v3.0.0/src/v3/pact.ts#L11 Not only pactfileWriteMode, but many other fields also seem to be missing from PactV3Options. As mentioned in the description, I'm using pact-js v3 beta, so I'd need it defined there in order to be unblocked. So I guess this issue should be about adding the missing fields to PactV3Options.

[TimothyJones]

Ah! Apologies, I'm not sure how I missed that you were asking about v3. It seems clear in your question.

The rust core doesn't have the same lifecycle, so many of the options from the current release are not applicable to the beta release. Having said that, we're trying to get the next release as close as possible to a drop-in release.

One major difference in the lifecycle with the rust core is that the pact file is written at the end of each test (when you call executeTest). The rust core does support overwrite, but since it would overwrite at each test this is almost certainly not what you want - so we haven't exposed that function in the DSL. I don't think it's going to be practical to do so.

Your first use case is solved by clearing the pact files at the start of the run. I would like to better support the clearing of pact files automatically at the start of the test run, but in js land we have to do that on a per-framework basis. If you could let us know what framework you're using, we can prioritise that one first.

The second use case still sounds like a bug - could you provide an example that we can reproduce?

[lviana-menlosecurity]

We use Nx, which by default uses Jest under the covers. As for an example for the second case, it's trivial and exactly as I said. Let's say we have a simple test like the following:

    it('should get the policy revision', () => {
      const mockValue = {
        state: 'staged'
      };
      provider
        .uponReceiving('a policy revision request to get')
        .withRequest({
          method: 'GET',
          path: '/api/policy/v1/rules',
          query: { timestamp: MatchersV3.fromProviderState('${timestamp}', '1627488400629') }
        })
        .willRespondWith({
          status: 200,
          body: mockValue
        });

      return provider.executeTest(async (mockserver) => {
        const result = await service.v1.rules.getPolicyRevision().toPromise();
        expect(result).toEqual(mockValue);
      });
    });

Then, we run it and the pact file gets generated, with the response body containing a "state" of "staged". Then, without deleting the pact file, you make a simple change to the value of the "state" field to say "published", and re-run. The test will pass, the timestamp of the pact file will be updated, however the response body will still have the old "state" of "staged", instead of "published". This seems to happen no matter what kind of change you make, meaning that if the pact file already exists it will not be updated with the new values. Now, if I first delete the pact file before running the test the second time, then it works fine.

[TimothyJones]

 MatchersV3.like(mockValue)

^ This is why you're seeing that behaviour. The like matcher matches on type. So, you're saying that your expected response is an object that looks like this:

{
        state: string
}

If you want the pact file to exactly match staged, then change it to:

 body: mockValue

[lviana-menlosecurity]

Sorry, my bad, even without the like matcher, I still get the same result (meaning, the pact file is not updated with the new values). I'll remove the like from my previous response for clarity.

[mefellows]

Now, if I first delete the pact file before running the test the second time, then it works fine.

Because of the way the new core works, it can allow several concurrent processes (tests) to be running in parallel on different ports. If there is an interaction conflict (uniqueness is by the combination of a description, verb, path) it doesn't know what to do - i.e. which one should take priority? Neither, that could result in a bug.

I believe it should warn/error in this case, saying that there is a conflict. If not, that should be a bug.

As you've discovered, you should clear all pacts before running tests locally.

Apologies, this does appear to be documented (and is a divergence from the current mainline). There are ways to resolve it, but the trade off is we can't allow concurrency. It's analogous to this behaviour.

[mefellows]

Closing as not supported. The pact files can be cleared before the tests run.