Pact-js: Undeterministic content of the contract file

[ertrzyiks]

Issue Classification

• [ ] Feature request
• [x] Bug report

Bug Report

When a project uses multiple files with pact interactions and runs them with jest the content of the contract file
may change even the consumer tests did not. It's caused by undeterministic order of loading test files.

To spread interactions across files we need to use:

pactfileWriteMode: 'merge'

and then each spec file adds it's interactions to the contract file in the same order that spec files are executed.

Software versions

• OS: Mac OSX 10.15.4
• Consumer Pact library: "@pact-foundation/pact": "^9.13.0",
• Provider Pact library: n/a
• Node Version: v12.16.1

Issue Checklist

Confirm the following:

• [x] I have upgraded to the latest
• [x] I have created a reproducable git repository (see below) to illuminate the problem
• [x] I have the read the FAQs in the Readme
• [x] I have triple checked, that there are no unhandled promises in my code

Expected behaviour

When running consumer pact tests the output contract file should not change if there are no changes to the contract.

Actual behaviour

When running consumer pact tests the output contract file changes even without any code changes.

Steps to reproduce

https://github.com/ertrzyiks/pact-js-interactions-order

Relevant log files

All the details are in the repo

Workaround

We address this issue by a workaround. After the contract file is generated we read the content, parse JSON, sort interactions by description and only then publish to the broker.

[mefellows]

Thanks so much for providing a detailed description and repro. Much appreciated.

May I ask why that's an actual issue, however?

[ertrzyiks]

@mefellows for the same reason as random data in tests described here https://docs.pact.io/getting_started/matching/#random-data---avoid-it

The current behavior breaks optimization provided by the broker, which is smart enough to reuse verification if the contract is not changed. The problem I described leads to situation that contract_content_changed webhook triggers verification job all the time, even if nobody touched the interactions definition purely because the order inside the interactions array in the json file is different and it makes such contract unique.

[mefellows]

Thanks - I suspected that might have been the case.

I'll need to see about how the determinism can be addressed in Pact JS (if it's here, it's likely elsewhere as well).

I thought the broker was cleverer when it came to determining the hash of the contents (I perhaps incorrectly thought it first parsed the contents, ordered it and then hashed it). We'll get to the bottom of that, but that actually seems like a better place to solve it.

[mefellows]

Is it also possible that the determinism is unrelated to Pact, but more that the jests test may run in parallel (and therefore, Pact simply writes the interactions in the order received?).

EDIT: I can see you have --runInBand so probably not.

[ertrzyiks]

I noticed that jest picks test files in undeterministing order even if --runInBand is used. I think that's the case and then on the pact side it just acts in the order forced by jest. If we can sort interactions on the broker side, that would be amazing 👍

[ertrzyiks]

Actually, I see the interactions sorted on the broker side 🤔

https://github.com/pact-foundation/pact_broker/blob/master/spec/lib/pact_broker/pacts/sort_content_spec.rb

Let me double check if it's still the case for us

[ertrzyiks]

I removed our sorting and the contract_content_changed webhook is called while it shouldn't. We use broker 2.67.0

[bethesque]

You can look in the broker to see what it thinks has changed. https://docs.pact.io/pact_broker/advanced_topics/see_changes_pact

[ertrzyiks]

@bethesque
The diff is unreadable to me, I can't share it here but at first removing some interactions like:

-      "_id": "some id here",
-      "description": "My Description",

and then adding it back in another place

+      "_id": "another id here",
+      "description": "My Description",

There is also plenty of removed graphql variables or matchingRules.
I'm wondering why the _id field for the same interactions are different though 🤔

[bethesque]

The _id is a sha of the interaction. So if there is a single field in the interaction that has changed, the sha will be different. It looks like something has changed, but without seeing the diff, I can't help you more unfortunately.

[ertrzyiks]

I tested in a small, isolated app and it works as expected even if we send unordered interactions. There must be something else triggering too many jobs on our side.

[mefellows]

Thanks for clarifying @ertrzyiks. Really appreciate your care and detail with your PRs/Issues/questions 🙏