Skip to content

CCM-14637 - Set up integration tests#43

Merged
rhyscoxnhs merged 2 commits intofeature/CCM-14201from
feature/CCM-14637
Mar 4, 2026
Merged

CCM-14637 - Set up integration tests#43
rhyscoxnhs merged 2 commits intofeature/CCM-14201from
feature/CCM-14637

Conversation

@rhyscoxnhs
Copy link

Description

Context

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I am familiar with the contributing guidelines
  • I have followed the code style of the project
  • I have added tests to cover my changes
  • I have updated the documentation accordingly
  • This PR is a result of pair or mob programming
  • If I have used the 'skip-trivy-package' label I have done so responsibly and in the knowledge that this is being fixed as part of a separate ticket/PR.

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.

  • I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

@rhyscoxnhs rhyscoxnhs requested review from a team as code owners February 25, 2026 09:05
@rhyscoxnhs rhyscoxnhs requested a review from a team as a code owner February 25, 2026 11:18
@rhyscoxnhs rhyscoxnhs force-pushed the feature/CCM-14637 branch 2 times, most recently from ad358c7 to 20d564f Compare February 25, 2026 11:50
@rhyscoxnhs rhyscoxnhs requested a review from Copilot March 3, 2026 10:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 with aws-helpers.ts for reusable AWS deployment utility functions.
  • Updates the acceptance stage workflow to dispatch to the internal infrastructure repo, adds a new acceptance-tests composite 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 callbacks component, 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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-sqs and async-wait-until are listed in both dependencies (lines 17-18) and devDependencies (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 from devDependencies since they are already in dependencies, 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.

Copy link
Contributor

@mjewildnhs mjewildnhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@rhyscoxnhs rhyscoxnhs merged commit f02ce04 into feature/CCM-14201 Mar 4, 2026
49 of 50 checks passed
@rhyscoxnhs rhyscoxnhs deleted the feature/CCM-14637 branch March 4, 2026 15:06
rhyscoxnhs added a commit that referenced this pull request Mar 4, 2026
* CCM-14637 - Set up integration tests

* CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
rhyscoxnhs added a commit that referenced this pull request Mar 4, 2026
* CCM-14637 - Set up integration tests

* CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
rhyscoxnhs added a commit that referenced this pull request Mar 4, 2026
* CCM-14637 - Set up integration tests

* CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
rhyscoxnhs added a commit that referenced this pull request Mar 4, 2026
* CCM-14637 - Set up integration tests

* CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants