[GPCAPIM-397] Set up INT test provider requests#193
[GPCAPIM-397] Set up INT test provider requests#193davidhamill1-nhs wants to merge 101 commits intomainfrom
Conversation
| return requester | ||
|
|
||
| @pytest.fixture | ||
| def expected_response_message_for_simple_request( |
There was a problem hiding this comment.
I hadn't imagined a new integration test in this set of PRs. The problem will be that we'll currently have nothing to run them against (since the preview environment is mocks only).
There was a problem hiding this comment.
I think this was more for me to think how the data and test setup would get changed for the env.
And provides an easy sanity check over manually running Bruno.
Having done a bit of dev against this EMIS INT instance - it definitely has a cold start during working hours, and 404 response during non-working hours.
…-int-test-provider-requests
…cation in INT only in order to prove end-to-end flow in INT.
|
Deployment Complete
|
|
There was a problem hiding this comment.
Pull request overview
Sets up integration test configuration to exercise end-to-end calls against INT downstream/provider environments by introducing INT-specific test data, environment targets, and response fixtures.
Changes:
- Added
env-test-intsupport and new.env.testvariables (TEST_NHS_NUMBER,CONSUMER_ODS_CODE) for targeting INT flows. - Updated provider client to optionally disable TLS certificate verification via env (
VERIFY_PROVIDER_CERTS) and wired this into env generation. - Refactored integration tests to load expected responses from JSON fixtures keyed by the test NHS number.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/env/test/user.sh | Removed (test-user logic moved into new shared data script). |
| scripts/env/test/target.sh | Added int base URL target for running tests against a locally deployed gateway configured for INT downstreams. |
| scripts/env/test/env.sh | Now sources data.sh and writes new .env.test variables (TEST_NHS_NUMBER, CONSUMER_ODS_CODE). |
| scripts/env/test/data.sh | New: centralizes env-specific test user/NHS number/ODS code selection. |
| scripts/env/env.mk | Adds env-test-int make target and registers it in silent targets list. |
| scripts/env/app/sds.sh | Adjusts SDS token output behavior for int. |
| scripts/env/app/provider.sh | Switches `int |
| scripts/env/app/pds.sh | Changes how PDS secrets are handled for int (currently outputs empty string). |
| scripts/env/app/env.sh | Exports VERIFY_PROVIDER_CERTS into generated .env. |
| infrastructure/images/build-container/resources/dev-certificates/.gitignore | Removed/emptied gitignore content for dev-certificates path. |
| gateway-api/tests/integration/test_get_structured_record.py | Uses ODS header fixture; compares response to JSON fixtures loaded from disk; strips randomized fields. |
| gateway-api/tests/integration/data/emis_int_test_9692140466.json | New expected response fixture for INT provider response. |
| gateway-api/tests/integration/data/alice_jones_9999999999.json | New expected response fixture for default/local stub scenario. |
| gateway-api/tests/contract/stub/test_sds_stub_contract.py | Simplifies endpoint entry assertions to be ID-driven rather than fixed ordering. |
| gateway-api/tests/conftest.py | Builds payload from TEST_NHS_NUMBER; adds ods_header fixture; uses UUID trace ID. |
| gateway-api/tests/README.md | Documents env-test-int and adds “Testing INT locally” instructions. |
| gateway-api/stubs/stubs/sds/stub.py | Updates seeded consumer ASID value. |
| gateway-api/stubs/stubs/data/patients/int_9692140466.json | Updates ODS code in stub patient record. |
| gateway-api/src/gateway_api/provider/test_client.py | Updates fake post stub signature to accept kwargs. |
| gateway-api/src/gateway_api/provider/client.py | Passes verify= to provider POST and logs the setting. |
| gateway-api/pyproject.toml | Changes pytest config table name (potentially affecting pytest config discovery). |
| README.md | Documents new test env vars (TEST_NHS_NUMBER, CONSUMER_ODS_CODE). |
| .vscode/cspell-dictionary.txt | Adds dictionary entries for EMIS/HSCN. |
Comments suppressed due to low confidence (1)
gateway-api/pyproject.toml:72
- Pytest reads configuration from
pyproject.tomlunder[tool.pytest.ini_options]. Moving this block to[tool.pytest]is likely to make pytest ignorebdd_features_base_dirandmarkers, which can break acceptance/integration runs (unknown markers / pytest-bdd base dir not set). Consider reverting to[tool.pytest.ini_options](or confirm the project has explicitly adopted a pytest version/config that supports[tool.pytest]).
[tool.pytest]
bdd_features_base_dir = "tests/acceptance/features"
markers = [
"status_auth_headers",
"status_merged_auth_headers",
]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case "$env" in | ||
| int) | ||
| # A patient that is known to exist in PDS and SDS INT, and exists in EMIS INT, | ||
| # an provider INT service that does not require auth or mTLS certs - only HSCN |
There was a problem hiding this comment.
Grammar in comment: "an provider" should be "a provider".
| # an provider INT service that does not require auth or mTLS certs - only HSCN | |
| # a provider INT service that does not require auth or mTLS certs - only HSCN |
| with open(test_patient_file_path) as f: | ||
| expected_response = cast("dict[str, Any]", json.load(f)) | ||
| return expected_response |
There was a problem hiding this comment.
cast("dict[str, Any]", ...) passes a string as the first argument to typing.cast, which will be flagged by type checkers (and isn’t needed here). Prefer cast(dict[str, Any], json.load(f)) or assign expected_response: dict[str, Any] = json.load(f).
| headers: CaseInsensitiveDict[str], | ||
| data: str, | ||
| timeout: int, # NOQA ARG001 (unused in stub) | ||
| **_kwargs: str, |
There was a problem hiding this comment.
**_kwargs: str is too restrictive for a requests.post-like stub. The production call now passes verify (bool) and timeout (int|None), so this annotation will fail type-checking under strict mypy. Use **_kwargs: Any (or a typed **kwargs: object) to match the real signature more accurately.
| **_kwargs: str, | |
| **_kwargs: Any, |
| int) | ||
| if [[ -f "$secret_file" ]]; then | ||
| cat "$secret_file" | ||
| # cat "$secret_file" # TODO [GPCAPIM-395]: Remove stubbing | ||
| echo "" | ||
| return 0 | ||
| else |
There was a problem hiding this comment.
For int, these functions now require the secret files to exist but intentionally output an empty string instead of the secret value. This makes make env-int fail unless secrets are present, while still leaving PDS credentials blank, which is inconsistent with the env-int docs/intent and can prevent real PDS INT calls. Either read and output the secret values for INT, or (if intentionally stubbed) stop requiring the files / update the env-int description accordingly.
| def find_test_patient_file(nhs_number: str) -> str: | ||
| filenames = os.listdir("tests/integration/data/") | ||
| for filename in filenames: | ||
| if nhs_number in filename: | ||
| return "tests/integration/data/" + filename | ||
| raise ValueError(f"No test patient file defined for nhs_number {nhs_number}") |
There was a problem hiding this comment.
find_test_patient_file is nondeterministic and potentially ambiguous: os.listdir() order is unspecified, and if nhs_number in filename can match multiple files if more fixtures are added. Consider using a deterministic/sorted search and an exact naming convention (or a direct map from NHS number -> filename), and build the path relative to this test file (e.g., via Path(__file__).parent / "data").
| ;; | ||
| int) | ||
| # This is currently set to test a locally deployed application that is set to point | ||
| # at PDS, SDS and Provider INT environments. |
There was a problem hiding this comment.
The comment says the locally deployed app is configured to point at PDS, SDS, and Provider INT environments, but the current env-int scripts still point PDS to stub (see scripts/env/app/pds.sh). Please align this comment (and/or the env scripts) so env-test-int accurately describes what downstreams are actually exercised.
| # at PDS, SDS and Provider INT environments. | |
| # at PDS stub, and SDS and Provider INT environments. |
| To test a locally deployed application against the integration environments of external services, PDS, SDS and Provider systems: | ||
|
|
||
| * Set up secrets in `.secrets` as per the instructions in `.secrets/README.md` | ||
| * `make env-int` - writes the `.env` file to point the application at the integration environments for each external service. | ||
| * `make deploy` - will deploy the application, feeding the `.env` values in to its container environment. | ||
|
|
||
| ### Manual test | ||
|
|
||
| To manually test the application against the integration environments, run the Bruno collection using the `localInt.yml` environment. |
There was a problem hiding this comment.
This section states make env-int points the application at PDS/SDS/Provider integration environments, but the current env scripts still configure PDS as stub for int. Either update the scripts to actually target PDS INT or adjust this documentation to reflect the current behaviour so local INT testing expectations are correct.
| To test a locally deployed application against the integration environments of external services, PDS, SDS and Provider systems: | |
| * Set up secrets in `.secrets` as per the instructions in `.secrets/README.md` | |
| * `make env-int` - writes the `.env` file to point the application at the integration environments for each external service. | |
| * `make deploy` - will deploy the application, feeding the `.env` values in to its container environment. | |
| ### Manual test | |
| To manually test the application against the integration environments, run the Bruno collection using the `localInt.yml` environment. | |
| To test a locally deployed application with the current INT-style configuration for external services: | |
| * Set up secrets in `.secrets` as per the instructions in `.secrets/README.md` | |
| * `make env-int` - writes the `.env` file to point SDS and Provider at their integration environments; PDS remains configured as `stub`. | |
| * `make deploy` - will deploy the application, feeding those generated `.env` values in to its container environment. | |
| ### Manual test | |
| To manually test the application with this configuration, run the Bruno collection using the `localInt.yml` environment. |
|
|
||
| ### Automated tests | ||
|
|
||
| To run a subset the automated test suite, run the following commands |
There was a problem hiding this comment.
Grammar: "To run a subset the automated test suite" is missing "of" ("subset of the automated test suite").
| To run a subset the automated test suite, run the following commands | |
| To run a subset of the automated test suite, run the following commands |
Description
Set up assurance that Gateway API is able to talk to INT provider systems.
Context
By default, gateway talks to our own mocks instead of downstream services. We can configure it locally to point to sandbox environments. This is to take it one step further for the provider and to test against a test instance they've deployed.
We can do this by pointing the SDS endpoint for a known test org into the test provider system.
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.