From 2a95d132cccb136742bf651934c181b3b768f9f3 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 11:12:50 -0600 Subject: [PATCH] Run the component-tester Playwright suites nightly Nothing ran src/BloomBrowserUI/react_components/component-tester, which is how its harness sat broken for weeks on a React 17 pin plus a config bug. The nightly workflow now runs it as a fifth suite, with its own check run and its own junit report, next to the front-end, C#, BloomE2E and visual-regression suites. It is the component config only. The sibling playwright.bloom-exe.config.ts attaches over CDP to a Bloom the developer already has running, so it needs the src/BloomE2E launch fixture before it can run unattended, and that config's testIgnore keeps its specs out of this one. The job needs neither build: the harness serves the components from its own Vite dev server, which playwright.config.ts starts. It does need the src/BloomBrowserUI install, for the axios and jquery that lib/localizationManager imports, and the "pnpm install" step above it provides that. So the group depends on nothing else in the workflow and runs even when the builds failed. --timeout=120000 raises the per-test 30 seconds in playwright.config.ts, which assumes a dev server that has already transformed the module graph. Every CI run starts cold and the first request for a component pays for that transform. A passing test still returns as soon as it passes, so this costs a green run nothing. Two tests that failed about once per full-suite run and passed on their own get a longer wait for the state they are waiting for: the LinkTargetChooser error message (1000ms was not enough on a loaded machine) and the registration opt-out button (a 2000ms buffer on top of the component's own 2-second delay). Both are waits, not sleeps, so a green run pays nothing for the larger number. Retires the AUTOMATION-DEBT.md entry "The component-tester Playwright suites are not in CI". Verified: the suite passes locally through the same command the job runs, 144 passed and 25 skipped, both cold and warm. The job itself is unverified until CI runs it. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/nightly.yml | 73 +++++++++++++++++-- .../component-tests/error-handling.uitest.ts | 27 ++++++- .../component-tests/test-helpers.ts | 8 +- src/BloomE2E/AUTOMATION-DEBT.md | 13 ---- 4 files changed, 97 insertions(+), 24 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index d5e120d1eeb5..c8dacac19d53 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -1,19 +1,19 @@ # Nightly build + full test run of everything on master. # # This is a health check, not a release: it builds the front-end and the C# solution and -# runs all four test suites — front-end vitest, C# NUnit, the BloomE2E suite and the visual -# regression suite — but produces no installer, does no signing, and publishes nothing. It -# exists to catch breakage that the PR checks miss — e.g. tests excluded from PR runs, or rot -# from dependency/runner drift — on a predictable cadence. +# runs all five test suites — front-end vitest, C# NUnit, the BloomE2E suite, the visual +# regression suite, and the component-tester Playwright suite — but produces no installer, does +# no signing, and publishes nothing. It exists to catch breakage that the PR checks miss — e.g. +# tests excluded from PR runs, or rot from dependency/runner drift — on a predictable cadence. # -# Each suite publishes its own check run / job-summary section, so the commit shows four +# Each suite publishes its own check run / job-summary section, so the commit shows five # independent results rather than one merged total. See the "Test reports" steps at the end. # # Schedule: 04:00 UTC daily. GitHub cron is always UTC (== GMT, no DST), so this is a # literal 4am GMT. Note GitHub may delay scheduled runs during peak load; exact timing is # best-effort. Can also be run on demand via the Actions "Run workflow" button. # -# A manual run can pick which of the four suites to run; all four are ticked by default, so the +# A manual run can pick which of the five suites to run; all five are ticked by default, so the # default manual run matches the scheduled one. Unticking the ones you don't need is how to iterate # quickly when chasing a failure in a single suite: the C# suite alone is ~9 minutes of a ~22-minute # run, so dropping it takes an attempt to ~12. That was added while hunting the intermittent @@ -54,6 +54,10 @@ on: description: "Visual-regression suite (drives a real Bloom)" type: boolean default: true + run_component_tests: + description: "Component-tester suite (Playwright against the Vite harness)" + type: boolean + default: true # Don't stack nightlies: if a manual run overlaps the scheduled one, let the first finish. concurrency: @@ -81,6 +85,7 @@ jobs: RUN_CSHARP_TESTS: ${{ github.event_name != 'workflow_dispatch' || inputs.run_csharp_tests }} RUN_E2E_TESTS: ${{ github.event_name != 'workflow_dispatch' || inputs.run_e2e_tests }} RUN_VISUAL_REGRESSION: ${{ github.event_name != 'workflow_dispatch' || inputs.run_visual_regression }} + RUN_COMPONENT_TESTS: ${{ github.event_name != 'workflow_dispatch' || inputs.run_component_tests }} steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -110,6 +115,7 @@ jobs: src/content/pnpm-lock.yaml src/BloomE2E/pnpm-lock.yaml src/BloomVisualRegressionTests/pnpm-lock.yaml + src/BloomBrowserUI/react_components/component-tester/pnpm-lock.yaml # ----- Dependencies (mirrors init.sh) ----- @@ -453,6 +459,51 @@ jobs: retention-days: 4 if-no-files-found: ignore + # ----- Component-tester tests ----- + # src/BloomBrowserUI/react_components/component-tester renders one React component at a + # time in a Vite dev server and drives it with Playwright (the *.uitest.ts files beside + # each component). Nothing ran these until now, which is how the harness sat broken for + # weeks — a React 17 pin plus a config bug — and it would rot again silently + # (AUTOMATION-DEBT.md, "The component-tester Playwright suites are not in CI"). + # + # This is the component config only (playwright.config.ts, which the package's own + # `pnpm test` uses). The sibling playwright.bloom-exe.config.ts attaches over CDP to a + # Bloom the developer already has running, so it needs the src/BloomE2E launch fixture + # before it can run unattended; its specs are excluded by that config's testIgnore. + # + # It needs neither build: the harness serves the components from its own Vite dev + # server, which playwright.config.ts starts as its webServer. So this group depends on + # nothing above it, and runs even when the builds failed. + - name: Set up component-tester tests + id: setup_component_tests + if: ${{ !cancelled() && env.RUN_COMPONENT_TESTS == 'true' }} + working-directory: src/BloomBrowserUI/react_components/component-tester + shell: bash + run: | + pnpm install --frozen-lockfile + pnpm exec playwright install chromium + + # Playwright takes its junit path from PLAYWRIGHT_JUNIT_OUTPUT_NAME, not from a CLI + # flag: it has no --outputFile (that is vitest's). The path is relative to the working + # directory, so it climbs back to the repo root's output/Tests like the other suites. + # + # --timeout raises the per-test 30s of playwright.config.ts, which is a developer's + # number: it assumes a dev server that has already transformed the module graph. Every + # CI run starts cold, and the first request for a component pays for transforming that + # whole graph. Locally, a cold first run failed 12 tests on `page.goto` timing out + # while the warm re-run of the same suite passed 142 in 58 seconds. So give the runner + # room rather than reporting a cold start as a broken component. A passing test still + # returns as soon as it passes, so this costs a green run nothing. + # John approved this timeout on 2026-09-03, as src/BloomBrowserUI/AGENTS.md asks. + - name: Run component-tester tests + id: component_tests + if: ${{ !cancelled() && steps.setup_component_tests.outcome == 'success' }} + working-directory: src/BloomBrowserUI/react_components/component-tester + shell: bash + env: + PLAYWRIGHT_JUNIT_OUTPUT_NAME: ../../../../output/Tests/component-tester-junit.xml + run: pnpm test --timeout=120000 --reporter=list,junit + # ----- Test reports, one per suite ----- # Each suite gets its OWN invocation of the publish action, and therefore its own # check run on the commit and its own section in the job summary: pass/fail/skip @@ -518,6 +569,15 @@ jobs: action_fail_on_inconclusive: true files: output/Tests/visual-regression-junit.xml + - name: Publish component-tester test results + if: ${{ !cancelled() && steps.component_tests.outcome != 'skipped' }} + uses: EnricoMi/publish-unit-test-result-action/windows@d0a4676d0e0b938bc201470d88276b7c74c712b3 # v2.24.0 + with: + check_name: "Nightly tests: component-tester (Playwright)" + comment_mode: "off" + action_fail_on_inconclusive: true + files: output/Tests/component-tester-junit.xml + - name: Upload test results if: ${{ always() }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -531,4 +591,5 @@ jobs: output/Tests/vitest-junit.xml output/Tests/e2e-junit.xml output/Tests/visual-regression-junit.xml + output/Tests/component-tester-junit.xml retention-days: 14 diff --git a/src/BloomBrowserUI/react_components/LinkTargetChooser/component-tests/error-handling.uitest.ts b/src/BloomBrowserUI/react_components/LinkTargetChooser/component-tests/error-handling.uitest.ts index 0661561e390f..e402dd06d950 100644 --- a/src/BloomBrowserUI/react_components/LinkTargetChooser/component-tests/error-handling.uitest.ts +++ b/src/BloomBrowserUI/react_components/LinkTargetChooser/component-tests/error-handling.uitest.ts @@ -7,6 +7,13 @@ import { test, expect } from "../../component-tester/playwrightTest"; import { setupLinkTargetChooser } from "./test-helpers"; +// How long to let the error message appear or clear. It was 1000ms, which was not enough on a +// loaded machine: two of these tests failed about once per full-suite run and passed when run on +// their own. These are waits for a state rather than sleeps, so the larger number costs a passing +// run nothing. +// John approved this timeout on 2026-09-03, as AGENTS.md asks. +const kErrorAppearsTimeoutMs = 10000; + test.describe("LinkTargetChooser - Error Handling for Missing Books/Pages", () => { test("Shows error when URL points to missing book", async ({ page }) => { const context = await setupLinkTargetChooser(page, { @@ -17,7 +24,10 @@ test.describe("LinkTargetChooser - Error Handling for Missing Books/Pages", () = // Wait for the error message to appear const errorMsgElement = await context.errorDisplay.getErrorMessage(); - await errorMsgElement.waitFor({ state: "visible", timeout: 1000 }); + await errorMsgElement.waitFor({ + state: "visible", + timeout: kErrorAppearsTimeoutMs, + }); // Error message should be visible const isErrorVisible = await context.errorDisplay.isVisible(); @@ -42,7 +52,10 @@ test.describe("LinkTargetChooser - Error Handling for Missing Books/Pages", () = // Wait for the error message to appear const errorMsgElement = await context.errorDisplay.getErrorMessage(); - await errorMsgElement.waitFor({ state: "visible", timeout: 1000 }); + await errorMsgElement.waitFor({ + state: "visible", + timeout: kErrorAppearsTimeoutMs, + }); // Error message should be visible const isErrorVisible = await context.errorDisplay.isVisible(); @@ -114,7 +127,10 @@ test.describe("LinkTargetChooser - Error Handling for Missing Books/Pages", () = // Wait for the error message to appear const errorMsgElement = await context.errorDisplay.getErrorMessage(); - await errorMsgElement.waitFor({ state: "visible", timeout: 1000 }); + await errorMsgElement.waitFor({ + state: "visible", + timeout: kErrorAppearsTimeoutMs, + }); // Verify error appears let isErrorVisible = await context.errorDisplay.isVisible(); @@ -124,7 +140,10 @@ test.describe("LinkTargetChooser - Error Handling for Missing Books/Pages", () = await context.urlEditor.setValue("/book/book1"); // Wait for error to disappear - await errorMsgElement.waitFor({ state: "hidden", timeout: 1000 }); + await errorMsgElement.waitFor({ + state: "hidden", + timeout: kErrorAppearsTimeoutMs, + }); isErrorVisible = await context.errorDisplay.isVisible(); expect(isErrorVisible).toBe(false); diff --git a/src/BloomBrowserUI/react_components/registration/component-tests/test-helpers.ts b/src/BloomBrowserUI/react_components/registration/component-tests/test-helpers.ts index 70d70241959f..0e0551d242bd 100644 --- a/src/BloomBrowserUI/react_components/registration/component-tests/test-helpers.ts +++ b/src/BloomBrowserUI/react_components/registration/component-tests/test-helpers.ts @@ -11,7 +11,13 @@ import { // Test timing constants export const kTestOptOutDelaySeconds = 2; -export const kTestOptOutTimeoutMs = kTestOptOutDelaySeconds * 1000 + 2000; // delay + buffer +// The delay the component waits before offering the opt-out button, plus a buffer for the machine +// running the suite. The buffer was 2000ms, which was not enough on a loaded machine: the last +// worker to start would miss the button by a fraction of a second, so one of these tests failed +// about once per full-suite run and passed when run on its own. This is a wait for a state, not a +// sleep, so a longer buffer costs a passing run nothing. +// John approved this timeout on 2026-09-03, as AGENTS.md asks. +export const kTestOptOutTimeoutMs = kTestOptOutDelaySeconds * 1000 + 8000; // Field helper type for registration form type FieldHelper = { diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 3d0bd9f2af0d..37fd16849dc3 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -21,7 +21,6 @@ the identity here. Before you start on a marked entry, ask the owner of its bran | Branch | What it pays down | | --- | --- | -| `BL-16799-component-tests-in-ci` | The component-tester Playwright suites get a nightly job. | | `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. | | `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. | | `BL-16799-page-screenshot` | A helper captures a whole book page, which absorbs the `captureBeyondViewport` footgun. | @@ -136,18 +135,6 @@ inline), so `src/BloomE2E/helpers/pageThumbnails.ts` has to find "Copy Page" and by their English labels, exactly as the top bar does. Same fix: a `data-testid` per command, taken from the `commandId` the menu already has. -## The component-tester Playwright suites are not in CI - -`nightly.yml` runs vitest, C#, visual-regression and BloomE2E; nothing runs -`react_components/component-tester`'s suites, which is how the harness sat broken -(React 17 pin + config bug) unnoticed until it was green again at 144 passed. It will -rot again silently. Fix direction: a nightly job mirroring the visual-regression one -(component config only; the bloom-exe config needs the e2e launch fixture first). -(Promoted from PAPERCUTS 2026-07-27.) - -being fixed on `BL-16799-component-tests-in-ci`, as that nightly job. The bloom-exe config -stays out of it, for the reason given above. - ## Toolbox tool registration is a side effect of toolboxBootstrap `ToolboxRoot` only renders tools registered via importing `toolboxBootstrap.ts`, which