From c4fce048b3d43f19fa21233224dc9c4d1f11b779 Mon Sep 17 00:00:00 2001 From: Hatton Date: Thu, 3 Sep 2026 11:14:57 -0600 Subject: [PATCH] Let an e2e run test the working tree's front end A launched Bloom serves its React UI from the built output/browser, so an edit to a .tsx file did not reach the suite until somebody rebuilt that bundle, which AGENTS.md reserves for a developer or CI. Set BLOOM_E2E_VITE_PORT= and fixtures/launchBloom.ts passes --vite-port to the Bloom it launches, which then loads every React control from that dev server. Bloom already had the option; nothing passed it. Use 5173 and set the variable. The README, the add-e2e-test skill and the new AUTOMATION-DEBT.md entry all say why: the page list and the toolbox write http://localhost:5173 into their own imports, so on any other port those two frames come up empty, which reads as the feature being missing; and an unset variable does not mean "no dev server", because a dev build probes 5173 by itself, so a run can quietly test a bundle from yesterday. getViteDevPort rejects a value that is not a whole number from 1 to 65535, naming the variable and the value it holds. Passed through untouched, a typo reaches Bloom as --vite-port, and Bloom's ValidateStartupVitePort then stops with a dialog no test can dismiss: every test in the run waits out its launch timeout and reports that Bloom never came up, and nothing in that report names the variable. Adds two AUTOMATION-DEBT.md entries rather than retiring one: "Which front end the e2e suite tests depends on what else is running", for the choice the fixture still does not own, and "A Vite dev server only reaches the whole UI on port 5173", for the two pug files that ignore the port. Verified: type check clean. A full suite run against a dev server on the working tree is what found both entries above. Co-Authored-By: Claude Opus 5 (1M context) --- .github/skills/add-e2e-test/SKILL.md | 18 ++++++++++ src/BloomE2E/AUTOMATION-DEBT.md | 54 ++++++++++++++++++++++++++-- src/BloomE2E/README.md | 27 ++++++++++++++ src/BloomE2E/fixtures/launchBloom.ts | 32 +++++++++++++++++ 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/.github/skills/add-e2e-test/SKILL.md b/.github/skills/add-e2e-test/SKILL.md index d1fafb90738e..76db18f4c2be 100644 --- a/.github/skills/add-e2e-test/SKILL.md +++ b/.github/skills/add-e2e-test/SKILL.md @@ -354,6 +354,24 @@ see "Build Bloom whenever it helps") and the inputs at `output/testing-inputs`. `BLOOM_TESTING_INPUTS_DIR` at a bloom-testing-inputs checkout to use your own in-progress collections instead of the pinned ones. +The launched Bloom serves its React UI from the built `output/browser`, so **an edit to a `.tsx` +file does not reach a run until that bundle is rebuilt.** To test the working tree instead, start +a dev server and name its port in `BLOOM_E2E_VITE_PORT`; the fixture passes `--vite-port` and +Bloom loads every React control from it. Set `PORT` as well as `--port`, or the dev server's +HMR and React-Refresh URLs still point at 5173 and the page fails to load its entry module. + +```bash +PORT=5173 pnpm exec vite --port 5173 --strictPort # in src/BloomBrowserUI +BLOOM_E2E_VITE_PORT=5173 pnpm test # in src/BloomE2E +``` + +**Use 5173, and set the variable.** The page list and the toolbox write `http://localhost:5173` +into their own imports, so on any other port those two frames load nothing and come up empty, +which reads as the feature being missing. And leaving the variable unset does not mean "no dev +server": a dev build of Bloom probes 5173 by itself, so an unset variable and a server elsewhere +means the run quietly tests the built bundle, however old it is. Stop a Bloom that already holds +5173 rather than moving the dev server. See AUTOMATION-DEBT.md. + `.github/workflows/nightly.yml` does not run this suite yet. The step it will need is the same `pnpm test` in that folder, after the Release build and the testing-inputs fetch that the visual-regression job already does. diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 37fd16849dc3..69a752f2a48c 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -21,10 +21,8 @@ the identity here. Before you start on a marked entry, ask the owner of its bran | Branch | What it pays down | | --- | --- | -| `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. | -| `BL-16799-suite-docs` | The `add-e2e-test` rule that every step of a test is a helper call. | | `BL-16799-toolbox-registration` | One `registerAllToolboxTools()` that both the bootstrap and the test harness call. | | `BL-16799-shell-document` | A test can no longer attach to a shell document Bloom does not drive: one WebView2 environment per run, plus the `e2e/shellUrl` hook. | | `BL-16799-tab-test-ids` | `data-testid` on the workspace tabs, so no test matches a localized label. | @@ -110,6 +108,35 @@ refuses to upload at all rather than let an automated click publish under the de account. (Found 2026-09-02 automating Test Case ID 606, `upload-required-items.spec.ts`.) +## Which front end the e2e suite tests depends on what else is running + +A launched Bloom serves its React front end either from the built `output/browser` or from a Vite +dev server, and until the fixture is told which, the answer depends on the machine. Three facts, +established 2026-09-01: + +- **There is no way to point Bloom at another folder.** `BloomFileLocator.BrowserRoot` computes + `output/browser` (or `browser`) from where the app sits, with no environment variable and no + command-line option, so the isolated bundle that `build/agent-vite.ps1` writes under + `output/agent//browser` cannot be used by a launched Bloom. +- **A dev server is the supported route, and the fixture now takes it.** Set + `BLOOM_E2E_VITE_PORT=` and `fixtures/launchBloom.ts` passes `--vite-port `, so the suite + tests the working tree with no build at all. Start the server with `PORT` set as well as + `--port`: the port in `vite.config.mts` comes from `process.env.PORT`, so `--port` alone moves + the server but leaves its HMR and React-Refresh URLs pointing at 5173, and the page then fails + to load its entry module. +- **Leaving the variable unset does not mean "no dev server".** A dev build probes port 5173 by + itself (`ReactControl.TryGetActiveViteDevPort`), so a developer's own dev server silently + decides what the suite tests, and Bloom offers no option that means "ignore any dev server" + (`--vite-port` rejects 0, and `ValidateStartupVitePort` requires the port to answer). + +What remains: the fixture neither starts a dev server of its own nor checks that `output/browser` +is newer than `src/BloomBrowserUI`, so a run with the variable unset can still test a stale +bundle without saying so. Fix direction: have the fixture own the choice, either by starting a +dev server on a port of its own choosing, or by refusing to run against an `output/browser` older +than the source and naming the file that is newer. Bloom needs an explicit "no dev server" +option before the second half of that can be trusted. +(Found 2026-09-01 while fixing the top-bar test ids.) + ## The top bar has no stable test ids, so tests match on localized text `TopBar.tsx` renders the workspace tabs as `` with a localized `` @@ -400,3 +427,26 @@ meantime: run small and stay red, or stay large until the tests are fixed. (Written and measured on 2026-09-03 during BL-16804, then deliberately taken back out: the developer chose to record the plan here rather than carry a red suite. The code is not in the history, so rebuilding it from this entry is part of the job.) + +## A Vite dev server only reaches the whole UI on port 5173 + +`--vite-port` tells Bloom's shell which dev server to load the front end from, but two of the +Edit tab's frames ignore it. `bookEdit/pageThumbnailList/pageThumbnailList.vite-dev.pug` and +`bookEdit/toolbox/toolbox.vite-dev.pug` write `http://localhost:5173/...` into every import +they emit, so on any other port the page list and the toolbox load nothing and come up empty. + +That failure looks like the feature being missing, not like a port problem. A run on port 5199 +failed `duplicate-page.spec.ts` on 2026-09-02 with "waiting for +getByTestId('duplicate-page-button') to be visible", 30 seconds, because `#PageControls` had +never been filled. Nothing in the message points at the dev server. + +The same run showed the second half of it: `BLOOM_E2E_VITE_PORT` was unset, so Bloom fell back +to probing 5173 by itself, found nothing there, and served the built `output/browser` instead. +That bundle was a day old, so the suite silently tested yesterday's front end and reported the +new test id as absent. + +So both halves say the same thing: **serve the dev server on 5173 and set +`BLOOM_E2E_VITE_PORT=5173`.** Fix direction: emit the port into those two pug files the way the +shell gets it, so `--vite-port` means what it says; and give Bloom an option that means "ignore +any dev server", so a run can state which front end it is testing rather than inherit it from +the machine. (Found 2026-09-02.) diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index c7f58794d51d..d389171de629 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -208,6 +208,33 @@ BLOOM_TESTING_INPUTS_DIR=D:/bloom-testing-inputs pnpm test Either way the fixture copies the collection before Bloom opens it, so a run never modifies your inputs. +## Testing a front-end change + +The launched Bloom serves its React UI from the built `output/browser`, so an edit to a `.tsx` +file does not reach a run until somebody rebuilds that bundle. To test the working tree instead, +start a Vite dev server and name its port in `BLOOM_E2E_VITE_PORT`; the fixture then passes +`--vite-port` to Bloom, which loads every React control from the dev server. + +```bash +# In one terminal, in src/BloomBrowserUI. Set PORT as well as --port: the port in +# vite.config.mts comes from process.env.PORT, and --port alone leaves the HMR and +# React-Refresh URLs pointing at 5173, which makes the page fail to load its entry module. +PORT=5173 pnpm exec vite --port 5173 --strictPort + +# In another, in src/BloomE2E +BLOOM_E2E_VITE_PORT=5173 pnpm exec playwright test +``` + +**Use 5173, and set the variable.** The port is not free to choose: the page list and the toolbox +write `http://localhost:5173` into their own imports, so on any other port those two frames load +nothing and come up empty, which reads as the feature being missing rather than as a port +problem. And leaving `BLOOM_E2E_VITE_PORT` unset does not mean "no dev server": a dev build of +Bloom probes 5173 by itself, so an unset variable and a server somewhere else means the run +quietly tests the built bundle, however old it is. Both halves are in AUTOMATION-DEBT.md under +"A Vite dev server only reaches the whole UI on port 5173". + +So stop a Bloom that is already using 5173 before a run, rather than moving the dev server. + ## In CI `.github/workflows/nightly.yml` runs the whole suite every night against the Release build it has diff --git a/src/BloomE2E/fixtures/launchBloom.ts b/src/BloomE2E/fixtures/launchBloom.ts index 7fb7c640626f..041dd0c8af09 100644 --- a/src/BloomE2E/fixtures/launchBloom.ts +++ b/src/BloomE2E/fixtures/launchBloom.ts @@ -178,6 +178,34 @@ function environmentForBloom(): NodeJS.ProcessEnv { return process.env; } +/** + * The Vite dev server port the launched Bloom should load its React front end from, or undefined + * to leave the choice to Bloom. + * + * A launched Bloom serves its UI from the built output/browser unless it is told about a dev + * server, so an edit to a .tsx file does not reach the suite until somebody rebuilds the bundle, + * which AGENTS.md reserves for a developer or CI. Set BLOOM_E2E_VITE_PORT= and Bloom loads the + * front end from that dev server instead, so the suite tests the working tree. + * + * Leaving the variable unset is NOT the same as "no dev server". A dev build of Bloom probes port + * 5173 by itself (ReactControl.TryGetActiveViteDevPort), so a developer's own dev server silently + * changes what the suite tests, and Bloom has no option that means "ignore any dev server" + * (--vite-port rejects 0). See AUTOMATION-DEBT.md. + */ +function getViteDevPort(): string | undefined { + const value = process.env.BLOOM_E2E_VITE_PORT; + if (!value) return undefined; + // Fail here rather than hand a bad port to Bloom. Bloom's own ValidateStartupVitePort + // stops with a dialog no test can dismiss, so every test in the run would instead wait out + // its launch timeout and report that Bloom never came up. + const port = Number(value); + if (!Number.isInteger(port) || port < 1 || port > 65535) + throw new Error( + `BLOOM_E2E_VITE_PORT must be a whole number from 1 to 65535. It is "${value}".`, + ); + return String(port); +} + /** What common/instanceInfo tells us about a running Bloom. Only the fields we use. */ interface IInstanceInfo { editableCollectionFolder?: string; @@ -385,6 +413,10 @@ async function startBloomOn( // --automation: let this instance run alongside a Bloom the developer already has open, and // let BLOOM_AUTOMATION_MONITOR say where its windows go (see environmentForBloom). const args = [findCollectionFile(collectionDir), "--e2e", "--automation"]; + // --vite-port: serve the React front end from a dev server, so the suite tests the working + // tree rather than a stale output/browser (see getViteDevPort). + const vitePort = getViteDevPort(); + if (vitePort) args.push("--vite-port", vitePort); const bloomProcess: ChildProcess = execFile(exe, args, { env: environmentForBloom(), });