pdf updates#770
Merged
RyonLeightonDEFRA merged 2 commits intoFCRM-6803-AddP1TestsToResultsAndNextStepsfrom Apr 28, 2026
Merged
Conversation
Co-authored-by: Copilot <copilot@github.com>
|
RyonLeightonDEFRA
merged commit fc3b38a
into
FCRM-6803-AddP1TestsToResultsAndNextSteps
3 checks passed
garydawsonDEFRA
added a commit
that referenced
this pull request
Apr 28, 2026
* Removed Confirmation page test from Check your details spec file * added projects and tags to separate e2e test * Updated framework to handle P1 PDF validation and updates Results page tests * Added P1 PDF tests to Next Steps spec file, updated pdf driver to handle potentially conflicting pdf jobs between spec files * Sonar Qube fixes * Further Sonar Qube fixes * pdf updates (#770) * pdf updates Co-authored-by: Copilot <copilot@github.com> * removed test.describe.only from results spec file --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Gary Dawson <gary.dawson@defra.gov.uk> --------- Co-authored-by: RyonLeightonDEFRA <ryon.leighton@defra.gov.uk> Co-authored-by: Copilot <copilot@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested simplifications to PDF test implementation
Summary
Reviewed the P1 PDF test code and made some simplifications and improvements to the PdfDriver, test structure, and page object usage. These are suggestions to discuss, happy to adjust based on your thoughts.
Changes
PdfDriver simplified
Replaced the dual-strategy download (Promise.any racing a download event against response interception) with Playwright's standard awaitDownload() pattern, the server returns content-disposition: attachment so the download event is sufficient
Removed clearPdfFiles() / listPdfFiles(). Server filenames include timestamps so collisions aren't an issue, and the folder is gitignored
Merged expectRequiredLinks + expectAllLinksAreValid into a single expectLinks method
Added expectPdfContent() that wraps all four assertion methods into one call
Dropped async from assertion methods that had no awaits
Removed normalizedLinks from the parsePdf return, normalization happens inline in expectLinks
Tests consolidated
Removed PDF tests from next-steps spec
Reduced from 6 tests (2 per zone) to 4 (1 per zone + 1 for the unspecified reference case) — the "without reference" tests were duplicating all assertions except checking "unspecified"
Flattened the nested describe/beforeEach structure into a single loop with inline setup