first commit with e2e test cleanup improvements#1368
first commit with e2e test cleanup improvements#1368avshetty1980 wants to merge 6 commits intomasterfrom
Conversation
|
This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket: VED-0000 |
| # This handles the case where the @when archive-wait assert raised before | ||
| # the @then step containing the old inline cleanup call could execute, | ||
| # leaving a "Failed" record in the next test run's DynamoDB query. | ||
| if hasattr(context, "filename") and context.filename and hasattr(context, "S3_env") and context.S3_env: |
There was a problem hiding this comment.
I thought we agreed not to clear the audit table after the test run, since we rely on that data for debugging — especially when we are doing cleanup before each test run
There was a problem hiding this comment.
you can filter by status = "Not processed - Automation testing" to see all test-generated failures. I've also added a cleaned_at timestamp to the update expression for when automation reset the record.
once at start of every test run cleanup_failed_audit_records_before_tests() does a scan with status = Failed AND timestamp >= now - 24h (reduced from 48h to limit scan cost on a large table). This clears any stale Failed records left by the previous CI run/test e.g. if the previous run was killed before teardown completed.
we can check for the cleaned_at timestamp to see where the update happened.
The allure report will also capture the failed test so should be good in terms of debugging.
There was a problem hiding this comment.
Thanks for the detailed explanation
One recommendation from my side: since this issue is only occurring in the Batch File Validation tests, it would be cleaner and more efficient to scope the after‑scenario cleanup using the Batch_File_Validation_Feature tag rather than running it for every scenario.
If we continue to run the cleanup globally in pytest_bdd_after_scenario, then the pre‑test execution cleanup becomes redundant and just adds extra delay to the test run. We also need to remember that this is test code, and some failures are expected by design — so we shouldn’t over‑clean in a way that hides useful debugging information.
Using the tag‑based approach keeps the behaviour targeted, avoids unnecessary scans, and still gives us the audit visibility we rely on for debugging.
|
|
Duplicate PR created for this implementation - so closing this |



Summary
e2e cleanup improvements so items don't end up in a failed state in the audit table
Pre-test sweep: on session start, scans the audit table for any Failed records from the last 48 hours and resets them to "Not processed - Automation testing", preventing the batch_processor_filter from being blocked before tests even begin.
Post-scenario safety net: unconditionally cleans up Failed records for the current test's filename after every scenario, catching cases where a mid-step failure skipped the inline cleanup.
Hardened conditional writes: [update_audit_table_for_failed_status] uses a DynamoDB [ConditionExpression] so the two cleanup paths can't race each other.
Bug fixes:
Restored the missing [mns_validation_required] guard in the Delete_cleanUp teardown (regression in original branch)
Expired Apigee tokens (401/403) during teardown DELETEs are now logged and skipped rather than failing the test
Apigee on-demand app setup failures no longer abort the entire session.