CCM-14637 - Set up integration tests#43
Conversation
3c51a66 to
f7ed545
Compare
ad358c7 to
20d564f
Compare
There was a problem hiding this comment.
Pull request overview
This PR sets up integration tests for the NHS Notify Client Callbacks service. It introduces the infrastructure verification test (infrastructure-exists.test.ts) that checks the subscription config S3 bucket exists in a deployed PR environment, supporting AWS helper utilities, and updates the CI/CD pipeline to create a dynamic environment before running acceptance tests (via an internal infrastructure repo dispatch).
Changes:
- Adds new integration test (
infrastructure-exists.test.ts) that verifies the subscription config S3 bucket exists, along withaws-helpers.tsfor reusable AWS deployment utility functions. - Updates the acceptance stage workflow to dispatch to the internal infrastructure repo, adds a new
acceptance-testscomposite action for running tests in a deployed environment, and wires the acceptance stage to depend on the PR dynamic environment creation. - Adds a Terraform output for deployment details (region, account ID, project, environment, group, component) to the
callbackscomponent, with corresponding README documentation.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
tests/integration/helpers/aws-helpers.ts |
New helper functions for resolving AWS deployment details via STS and building S3 bucket names |
tests/integration/helpers/index.ts |
Re-exports new aws-helpers alongside existing helpers |
tests/integration/infrastructure-exists.test.ts |
New integration test verifying the subscription config S3 bucket exists |
tests/integration/jest.config.ts |
Adds moduleNameMapper for helpers alias to match tsconfig paths |
tests/integration/tsconfig.json |
Adds helpers TypeScript path alias |
tests/integration/package.json |
Adds @aws-sdk/client-s3 and @aws-sdk/client-sts dependencies |
package-lock.json |
Lock file updates for new dependencies |
.github/actions/acceptance-tests/action.yaml |
New composite action to run acceptance tests in a deployed environment |
.github/actions/node-install/action.yaml |
New composite action for Node.js setup and npm install |
.github/actions/test-types.json |
New JSON listing test types (currently unused in any workflow) |
.github/workflows/stage-4-acceptance.yaml |
Replaces stub jobs with a real dispatch to the internal infra repo |
.github/workflows/cicd-1-pull-request.yaml |
Adds pr-create-dynamic-environment as a dependency for the acceptance stage |
infrastructure/terraform/components/callbacks/outputs.tf |
Adds deployment output with environment details for post-deployment scripts |
infrastructure/terraform/components/callbacks/README.md |
Documents the new deployment Terraform output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
77e971a to
2df6ac9
Compare
6fbb295 to
4aa3c02
Compare
2df6ac9 to
b8a8bda
Compare
4aa3c02 to
489edbb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
tests/integration/package.json:25
@aws-sdk/client-sqsandasync-wait-untilare listed in bothdependencies(lines 17-18) anddevDependencies(lines 21, 25). Since these integration tests are only ever run in CI (not consumed as a library), all runtime packages should appear in only one section. Having them in both sections is redundant and could cause version resolution confusion. These packages should be removed fromdevDependenciessince they are already independencies, or vice-versa if they are only needed at test time.
"@aws-sdk/client-sqs": "^3.990.0",
"async-wait-until": "^2.0.12"
},
"devDependencies": {
"@aws-sdk/client-sqs": "^3.990.0",
"@nhs-notify-client-callbacks/models": "*",
"@tsconfig/node22": "^22.0.2",
"@types/jest": "^29.5.14",
"async-wait-until": "^2.0.12",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mjewildnhs
left a comment
There was a problem hiding this comment.
Looks good.
A few questions
I'm probably being dumb but I can't see how the integration tests actually run in the environment that is deployed.
I was expecting to see something like make test-integration or something to actually run the jest command.
Am I missing something?
ab8c326 to
b82b117
Compare
489edbb to
c1f1365
Compare
3a5545b to
5596d45
Compare
* CCM-14637 - Set up integration tests * CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
* CCM-14637 - Set up integration tests * CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
* CCM-14637 - Set up integration tests * CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
* CCM-14637 - Set up integration tests * CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.