Add note about pact tests #4643
Conversation
KludgeKML
force-pushed
the
update-pact-docs
branch
from
123aec7 to
59f2d4b
Compare
- note differences between consumer and provider - note gotchas in input variable - (aside) replace Imminence references with Places Manager
KludgeKML
force-pushed
the
update-pact-docs
branch
from
59f2d4b to
f73a4b8
Compare
There was a problem hiding this comment.
Thanks so much for this @KludgeKML, I'm sorry I've suggested quite such extensive changes. I've just found myself spending the last hour and a half in the pact worm hole re-understanding and that involved a lot of rephrasing.
| ## How GDS API Adapters retrieves tests from the Providers | ||
|
|
||
| Each provider defines some details of its tests in the `.github/workflows/pact-verify.yml` file. This workflow is used by both | ||
| the provider and the consumer (gds-api-adapters), and behaves differently based on the value of the pact_artifact input, which can | ||
| be confusing if you're unused to it. |
There was a problem hiding this comment.
I started reviewing this and struggled to understand it so had a go at rewriting it with some more detail:
How pacts operates as part of continuous integration (CI)
Consumers (for example GDS API Adapters) define the tests, and thus the contract, between the consumer and the provider (for example Places Manager API). These tests produce JSON files, known as pact artifcats, and describe how an API is expected to behave. These pact artifcats are then run against the provider codebase to confirm a pact.
As part of a CI process we need to test the pact whenever a consumer or provider changes. This involves executing a GitHub action defined on the Provider. This action recieves different inputs depending on whether it is executed by the consumer or by the provider. Where the consumer provides pact-artificats files generated as a result of changes to the consumer repository and where the provider downloads previously generated pact-artifacts based on the latest state of the consumer from a service, pact broker.
What do you think?
I'd also suggest we move this section to be above the guide: "Running Pact tests locally" since it seems key information in proceeding with the further sections
| When the provider runs its CI pipeline, it does not set the `pact_artifact` input when it runs the `pact_verify` job, which causes it | ||
| to retrieve the pact artifacts from Github and verify against them. |
There was a problem hiding this comment.
The pacts are downloaded from the pact broker rather than from GitHub. You probably want to include the detail that the pacts that are downloaded default to those produced by the main branch of the consumer.
May also want to include the detail that this branch can be configured with pact_consumer_version, but may be able to get away with just a very short note that links to a an existing section on making breaking changes.
|
|
||
| - generates the pact artifacts in the `generate_pacts` job. This generates the artifacts, but also uploads them to Github | ||
| - runs the `pact_verify` job from each provider in turn, setting the `pact_artifact` input, which causes it to download the | ||
| pact artifacts from the pact broker (ie it's not using the artifacts it just created, but the previous versions). At this |
There was a problem hiding this comment.
This is incorrect, the pacts that are used here are not from the Pact Broker - they are the ones uploaded to GitHub.
The GitHub upload/download is a bit of an annoying complex implementation detail - we just have to do that as a way of sharing files between actions and they're only uploaded temporarily for the duration of the CI run - I do wonder if going into much detail on that risks extra confusion.
| pact artifacts from the pact broker (ie it's not using the artifacts it just created, but the previous versions). At this | ||
| point it uses the `pact_artifact_file_to_verify` value from the provider's `pact-verify.yml` file, which must be explicitly | ||
| set to match the implicit artifact names used when getting the pacts from Github (see below) | ||
| - if all the verify jobs complete, it triggers the `publish_pacts` job, which publishes the generated pacts to the broker. |
There was a problem hiding this comment.
I imagine we might want to make this section a little bit less GDS API Adapters specific and more generic to any app using pact tests - it also seems to be quite specific on specific implementation naming which seems risky for this staying in date.
Might want to go along the lines of saying:
The consumer CI job is responsible for generating new pact artifacts, verifying them against the provider and then, if successful, uploading them to the pact broker. The means it uses to achieve this is:
- running a task to generate the pacts (link to task) and uploading them to GitHub
- running the provider CI job to execute pacts with an argument that tells the provider to download these new pact artifacts (link to call) from GitHub to use as part of verifying
- running a task to upload the pact artifacts to the pact broker (link to task)
In some ways I do think perhaps this suggestion and are text are still a bit TMI for the dev docs but I'd understand in this pact case given how hard it is to understand.
|
|
||
| You should be careful that the names in the pact_helper.rb files in both the provider and consumer match, and that the value of | ||
| `pact_artifact_file_to_verify` is "gds_api_adapters-<provider_id>.json", where the provider_id is the name from the pact_helper | ||
| file in snake case. |
There was a problem hiding this comment.
I'm not sure this is the right place for this gotcha, is this better included in the "Adding tests for a new app" ?
Add a little bit of documentation around how the CI pipeline for both provider and consumer runs.