Fix functional test create-case retry handling - #2471
Conversation
createCase only retried unsuccessful HTTP status codes. A transport-level failure (TLS/socket) was not retried, and SerenityRest turned it into a ResponseStub while recording a failed step on the Serenity event bus. Serenity then skips subsequent @step calls and returns Mockito mocks from them, so a create-case call that was successfully retried still failed the test - surfacing as the misleading claimantPaymentRef NPE seen on master. - retry both unsuccessful responses and request exceptions - keep the final failure as the cause of the thrown IllegalStateException - include the status code and response body when a call returns non-201 - issue the request with plain RestAssured so a recovered retry no longer records a failed step and poisons later steps - assert the first fee payment reference is non-null before dereferencing it
CCD diff summary👉 Full report: https://github.com/hmcts/pcs-api/actions/runs/32866142823?check_suite_focus=true No change |
gergelykisshmcts
added
pr-values:ccd
enable_keep_helm
enable_e2e_test
tvr-hmcts
left a comment
tvr-hmcts
left a comment
There was a problem hiding this comment.
Cool. :)
Some thoughts ... perhaps beyond the fix though.
Somethings can not be retried so should they be checked for and just skip out earlier on? 400 / 401's etc
Also there is no back off / wait etc period before hitting again in that loop. Would that help at all in some circumstances?
Yep, both good points. I think it makes sense to tighten this while we’re here. |
POST /testing-support/{country}/create-case has no idempotency key and no
duplicate detection. The CCD case is created part-way through the server-side
orchestration by submitCaseCreation, and the resume event, court allocation,
issue date and access-code steps all run after that, so a failure response can
be returned with the case already created. Replaying the request could create a
second case.
- issue the request once, via SerenityRest so reporting is unchanged
- report the real status code and truncated response body on failure
- fail with a clear message when no usable response was read, rather than
replaying a request that may already have been processed
Conflict in ApiSteps.java: master reformatted the old createCase retry loop while this branch replaced it with a single-shot request. Kept the single-shot version and master's dataStoreUrl field, getInternalCaseDetails rename and new validateEventData step.
5 participants
Jira link
Personal contribution no jira link
Change description
createCase only retried unsuccessful HTTP status codes. A transport-level failure (TLS/socket) was not retried, and SerenityRest turned it into a ResponseStub while recording a failed step on the Serenity event bus. Serenity then skips subsequent @step calls and returns Mockito mocks from them, so a create-case call that was successfully retried still failed the test and surfacing as the misleading claimantPaymentRef NPE seen on master.
Testing done
Local test runs
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist
PCS checklist
@EnabledIfEnvironmentVariable(named = "CCD_ENABLED", ...)+ shutter guard) unless they must run on unlabelled PRs (HDPI-8084)