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

Docs inconsistency for PactFileWriteMode

Open dabfleming opened this issue 3 years ago • 4 comments

The Go docs for PactFileWriteMode describe values overwrite and merge.

https://github.com/pact-foundation/pact-go/blob/2354ed96613f7704d44d5e181ddaa018b2c02996/dsl/pact.go#L64-L70

However the linked docs show

Default value: :overwrite Options: :overwrite, :update, :smart, :none

With no mention of 'merge'.

Merge seems to work, but I'm working on internal tooling and documentation for using pact and would like to know for sure which values we can use.

dabfleming avatar Feb 22 '21 16:02 dabfleming

@bethesque am I confused about merge? I also have this document over at pact-js.

@dabfleming I'll see if I can dig up the conversation where I originally learned about this.

mefellows avatar Jun 12 '21 12:06 mefellows

@mefellows : jest-pact uses update, which means that the interactions are upserted into an existing pact file. I thought merge was a synonym for update, and I don't think there's an option that behaves like the pact-js readme says (picking up other pact files and merging them together).

The docs for the ruby standalone mention merge here, but don't mention update, smart or none.

overwrite or merge. Use merge when running multiple mock service instances in parallel for the same consumer/provider pair. Ensure the pact file is deleted before running tests when using this option so that interactions deleted from the code are not maintained in the file.

TimothyJones avatar Jun 14 '21 23:06 TimothyJones

The way the mock service originally worked, it would completely overwrite the pact file each time it got a request to the write endpoint. This meant that if there are any interactions that are not executed in that particular test run, they got deleted from the file. Back before we were using the broker, and we used the committed pact file as the contract, there was a codebase that had a really slow set of tests, I wanted to be able to run just one spec without it clearing out all the un-executed interactions. So I added the "update" write mode, and updated the logic so that when the pact write call came in, it would find the interaction with the matching description and provider state, and just update it, leaving any exiting interactions untouched.

The problem with the update mode is that if you change either the description or the provider state, it appends a new interaction and the old one is not removed. To help mitigate this, if update mode was used, the following message was printed:

*****************************************************************************
Updating existing file .#{pactfile_path.gsub(Dir.pwd, '')} as pactfile_write_mode is :update
Only interactions defined in this test run will be updated.
As interactions are identified by description and provider state, pleased note that if either of these have changed, the old interactions won't be removed from the pact file until the specs are next run with :pactfile_write_mode => :overwrite.
*****************************************************************************

To also help mitigate this, I then added the "smart" mode to the Pact Ruby DSL (not the mock service itself) that detected whether we were running the full rake test suite (in which case it used "overwrite") or a single test (in which case it used "update").

When the mock service started being used by node, we had the situation where we had tests running in parallel for the same consumer/provider. These required separate mock services to be run, one for each test file, as the Ruby mock service impl is stateful, and doesn't work for concurrent tests (there's no way of identifying which request would be coming from which test).

To support this, the "update" mode could be used, but (and I've forgotten the exact history of this) a new "merge" mode was added that operated in a similar way to update, but had a bit of logic to protect against the orphan interactions being left in the pact file. Here is the comment at the top of one of the files involved with this logic.

When running in pactfile_write_mode :overwrite, all interactions are cleared from the pact file, and all new interactions should be distinct (unique description and provider state). When running in pactfile_write_mode :update, an interaction with the same description and provider state as an existing one will just overwrite that one interaction. When running in pactfile_write_mode :merge, an interaction with the same description and provider state must be identical to the existing one, otherwise an exception will be raised.

bethesque avatar Jun 14 '21 23:06 bethesque

I can't remember why someone along the way wanted "none", but they wanted to be able to run a mock service without writing the interactions to the pact. I don't think it's something we should or need to support generally.

Smart mode is only relevant to Ruby.

So, this is why only "overwrite", "update" and "merge" are available in Pact JS. The other ones are not relevant.

bethesque avatar Jun 14 '21 23:06 bethesque