diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 5d04ebc16292..5af856b19832 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -21,7 +21,6 @@ and ask its author. | Pull request | Branch | What it pays down | | --- | --- | --- | -| #8291 | `BL-16799-vr-collect-failures` | A visual-regression case collects every failed comparison and fails once at the end. | | #8292 | `BL-16799-component-tests-in-ci` | The component-tester Playwright suites get a nightly job. | | #8293 | `BL-16799-vite-port` | `BLOOM_E2E_VITE_PORT` makes a run test the working tree's front end. Adds a new entry for what remains. | | #8294 | `BL-16799-type-in-one-call` | Typing in a text box is one insertion, not one key press per character. Adds a new entry: typing now raises no key events. | @@ -118,17 +117,6 @@ below) has landed: with a settings folder of its own, a test's Bloom can be give account's login before it starts, and can sign out for real without touching anyone else's login. The test account, and where its credentials live, are the rest of this branch. -## Visual-regression cases stop at the first failed comparison - -Each case in `src/BloomVisualRegressionTests/index.spec.ts` throws on the first -mismatch, so later comparisons never capture their images; stale baselines surface one -layer per ~3-minute run (BL-16638 took three accept-and-rerun rounds). Fix direction: -accumulate per-comparison failures and fail once at the end — proven during BL-16638 -(~20–30 lines, confined to the spec). Loop at `index.spec.ts:426`, assertion at -`index.spec.ts:486` (pre-rewire line numbers). (Promoted from PAPERCUTS 2026-07-30.) - -being fixed on `BL-16799-vr-collect-failures` (#8291). - ## The top bar has no stable test ids, so tests match on localized text `TopBar.tsx` renders the workspace tabs as `` with a localized `` diff --git a/src/BloomVisualRegressionTests/index.spec.ts b/src/BloomVisualRegressionTests/index.spec.ts index e52cf21fb109..46e71a4a4b17 100644 --- a/src/BloomVisualRegressionTests/index.spec.ts +++ b/src/BloomVisualRegressionTests/index.spec.ts @@ -208,6 +208,12 @@ describe("All books", () => { let playerPage: Page; let browser: Browser; let context: BrowserContext; + // Every image comparison the CURRENT case has failed. A case compares one book-preview image + // and one image per bloom-player page, and a stale baseline usually affects several of them. + // Throwing on the first mismatch meant the later comparisons never even captured their + // images, so accepting a real layout change took one ~3-minute run per image (BL-16638 took + // three rounds). So collect them all and fail once, at the end of the case. + let comparisonFailures: string[] = []; beforeAll(async () => { await launchDedicatedBloom(); @@ -293,6 +299,7 @@ describe("All books", () => { }); test.each(cases)("$title", async (testCase) => { + comparisonFailures = []; // Park the capture pages before we mutate this book. Otherwise the previous case's still-open // book-preview / bloom-player page keeps requesting book and staged-BloomPUB files while this // case rewrites them, which caused mid-run "file is being used by another process" and @@ -330,6 +337,14 @@ describe("All books", () => { await selectTab("publish"); const stagedUrl = await makeBloomPubPreview(); await capturePlayerPages(stagedUrl, testCase.label, screenshotsDir); + + // One failure for the whole case, listing every image that did not match, so a single run + // shows all the baselines that need looking at. + if (comparisonFailures.length > 0) + throw new Error( + `${comparisonFailures.length} of this case's images did not match their ` + + `reference:\n ${comparisonFailures.join("\n ")}`, + ); }); // Create the reference image if it does not exist yet; otherwise capture a current image and @@ -641,10 +656,29 @@ describe("All books", () => { } } + // Compare one captured image against its reference. This does NOT throw on a mismatch: it + // appends a description to comparisonFailures, and the test body fails the case once it has + // compared every image. Anything thrown while comparing (notably Pixelmatch's "Image sizes do + // not match", which is itself a real failure) is recorded the same way. async function comparePreviewImage( referencePath: string, testPath: string, diffPath: string, + ) { + try { + await compareOrThrow(referencePath, testPath, diffPath); + } catch (error) { + comparisonFailures.push( + `${testPath}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + + // The comparison itself: write a diff image and throw when the two images differ at all. + async function compareOrThrow( + referencePath: string, + testPath: string, + diffPath: string, ) { const referenceImage = PNG.sync.read(fs.readFileSync(referencePath)); const testImage = PNG.sync.read(fs.readFileSync(testPath)); @@ -675,10 +709,10 @@ describe("All books", () => { ), ); // A thrown Error rather than expect(...).toBe(0), so the failure itself carries the - // diff path and, when we know one, the likely cause; the console lines above are lost in - // a long run's output. + // diff path; the console lines above are lost in a long run's output. + // comparePreviewImage catches this and records it. throw new Error( - `${testPath} differed from the reference by ${numberOfDifferentPixels} pixels. ` + + `${testPath} differed from ${referencePath} by ${numberOfDifferentPixels} pixels. ` + `The diff image is at ${diffPath}.`, ); }