Keep Bloom's log and collection when an e2e test fails, and harden two new tests

.github/workflows/nightly.yml

@@ -323,7 +323,11 @@ jobs:
               run: pnpm test --reporter=list,junit,html
 
             # What explains an e2e failure: Playwright's HTML report, plus the trace and failure
-            # screenshot its config keeps (trace: retain-on-failure, screenshot: only-on-failure).
+            # screenshot its config keeps (trace: retain-on-failure, screenshot: only-on-failure),
+            # and what the fixture keeps beside them for a failed test (fixtures/bloomTest.ts,
+            # keepEvidenceOnFailure): Bloom's Log.txt and a copy of the collection folder as Bloom
+            # left it. Not the user-settings folder: this artifact is public, and user.config holds
+            # the Bloom Library login of a Bloom that signed in for real.
             # On failure only: a green run's report says nothing a green check has not already said,
             # the same reason the visual-regression screenshots below go up only on failure.
             #

src/BloomE2E/fixtures/bloomTest.ts

@@ -23,6 +23,9 @@ import {
     type ICollectionSpec,
 } from "./launchBloom";
 import { chromium } from "@playwright/test";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as Path from "node:path";
 import {
     describeProblems,
     startProblemDialogWatcher,
@@ -90,8 +93,21 @@ interface IBloomWorkerFixtures {
 interface IBloomTestFixtures {
     /** Fails the test when Bloom raised a problem dialog while it ran. Runs automatically. */
     failOnBloomProblem: void;
+    /**
+     * When a test fails, keeps what explains it beside Playwright's own trace and screenshot: Bloom's
+     * log, and a copy of the collection as Bloom left it. Runs automatically.
+     */
+    keepEvidenceOnFailure: void;
 }
 
+/**
+ * Where the Bloom under test writes its log. Bloom logs to %TEMP%\SIL\Bloom\Log.txt whatever
+ * folder its settings are in, so this is the developer's or the runner's temp folder, and a run's
+ * successive Blooms overwrite one another there: what is there when a test fails is the log of the
+ * Bloom that was running.
+ */
+const BLOOM_LOG_PATH = Path.join(os.tmpdir(), "SIL", "Bloom", "Log.txt");
+
 // How long we wait for Bloom's WebView2 to expose the shell document after the HTTP server is up.
 // The first navigation after launch is slow: WebView2 starts, the bundle loads, and React mounts.
 const SHELL_READY_TIMEOUT_MS = 90000;
@@ -321,8 +337,71 @@ export const test = base.extend<IBloomTestFixtures, IBloomWorkerFixtures>({
         await use(bloomApp.page);
     },
 
+    // Torn down after every other test-scoped fixture (failOnBloomProblem depends on it, so it is
+    // set up first), so a failure raised by one of them is still seen here. Everything goes under
+    // testInfo.outputDir, which is test-results/<test>/, the folder CI already uploads on failure.
+    //
+    // The collection folder is copied rather than described because the failures this is for are
+    // the ones where Bloom's state on disk disagrees with what the test saw: a title typed on the
+    // cover that the collection never learned (AUTOMATION-DEBT.md). The book's HTML and meta.json
+    // say which side lost it.
+    //
+    // The user-settings folder is deliberately NOT kept. These artifacts are public (the repository
+    // is), and user.config is where a Bloom that signed in to Bloom Library for real
+    // (helpers/bloomLibraryAccount.ts) saves its session token and account, and where the next
+    // secret-shaped setting would land too. Redacting known names would protect only against the
+    // ones we thought of. helpers/userSettings.ts reads that file for a test while it runs instead.
+    keepEvidenceOnFailure: [
+        async ({ bloomApp }, use, testInfo) => {
+            await use();
+            if (testInfo.status === testInfo.expectedStatus) return;
+            // Bloom may still be writing; a moment lets its last save land in the copy.
+            await delay(1000);
+            // Best effort throughout: this runs on a test that has already failed, and a file
+            // Bloom still holds open must cost only that file, never the rest of the evidence or
+            // a second error on top of the real one. Each step is contained on its own, and the
+            // copy filter really opens each file, because on Windows accessSync checks attributes,
+            // not whether another process has the file locked.
+            try {
+                if (fs.existsSync(BLOOM_LOG_PATH))
+                    await testInfo.attach("bloom-log", {
+                        body: fs.readFileSync(BLOOM_LOG_PATH),
+                        contentType: "text/plain",
+                    });
+            } catch (error) {
+                console.warn(`Could not keep Bloom's log: ${error}`);
+            }
+            try {
+                fs.cpSync(
+                    bloomApp.collectionDir,
+                    testInfo.outputPath("collection"),
+                    {
+                        recursive: true,
+                        errorOnExist: false,
+                        filter: (source) => {
+                            if (fs.statSync(source).isDirectory()) return true;
+                            try {
+                                fs.closeSync(fs.openSync(source, "r"));
+                                return true;
+                            } catch {
+                                console.warn(
+                                    `Left out of the collection copy (locked?): ${source}`,
+                                );
+                                return false;
+                            }
+                        },
+                    },
+                );
+            } catch (error) {
+                console.warn(`Could not copy the collection folder: ${error}`);
+            }
+        },
+        { auto: true },
+    ],
+
     failOnBloomProblem: [
-        async ({ problemDialogWatcher }, use) => {
+        async ({ problemDialogWatcher, keepEvidenceOnFailure }, use) => {
+            void keepEvidenceOnFailure;
             // Discard anything raised before this test started, so one test's problem is not
             // reported against the next.
             problemDialogWatcher.takeProblems();

src/BloomE2E/helpers/fontChooser.ts

@@ -403,7 +403,12 @@ export async function showFontDetails(page: Page): Promise<string> {
             resolve(text);
         });
     });
-    await icon.click({ timeout: 30000 });
+    // The pane re-renders while the mouse rests on the list (on the nightly runner the icon was
+    // "not stable", then detached, for the whole 30 seconds of one click attempt), so try the click
+    // afresh, against the icon as it is now, rather than waiting on one that is being replaced.
+    await expect(async () => {
+        await icon.click({ timeout: 3000 });
+    }).toPass({ timeout: 30000 });
     return message;
 }
 

src/BloomE2E/helpers/userSettings.ts

@@ -29,14 +29,29 @@ export async function getUserSettingsFolder(page: Page): Promise<string> {
  * This reads the disk rather than asking Bloom, because what a test usually wants to know is what
  * reached the file: that is what the next Bloom to use the folder starts from. Bloom writes some
  * settings a moment after they change (the zoom two seconds after the last change), so poll.
+ *
+ * Bloom holds the file exclusively for a moment while it saves, and reading it then fails with
+ * EBUSY (seen on the nightly runner, 2026-09-10), so a busy file is retried for a few seconds.
  */
-export function readSavedUserSetting(
+export async function readSavedUserSetting(
     folder: string,
     name: string,
-): string | undefined {
+): Promise<string | undefined> {
     const file = Path.join(folder, "user.config");
     if (!fs.existsSync(file)) return undefined;
-    const xml = fs.readFileSync(file, "utf8");
+    let xml = "";
+    const deadline = Date.now() + 5000;
+    for (;;) {
+        try {
+            xml = fs.readFileSync(file, "utf8");
+            break;
+        } catch (error) {
+            const code = (error as NodeJS.ErrnoException).code;
+            if ((code !== "EBUSY" && code !== "EPERM") || Date.now() > deadline)
+                throw error;
+            await new Promise((resolve) => setTimeout(resolve, 200));
+        }
+    }
     const match = new RegExp(
         `<setting name="${name}"[^>]*>\\s*<value>([^<]*)</value>`,
     ).exec(xml);

src/BloomE2E/tests/user-settings-isolation.spec.ts

@@ -42,7 +42,10 @@ test.describe("a test's Bloom keeps its user settings to itself", () => {
         // Bloom accepted the license for us at startup (there is nobody to click Accept) and saved
         // that, so the folder already holds a user.config, and this is what is in it.
         expect(
-            readSavedUserSetting(bloomApp.userSettingsDir, "LicenseAccepted"),
+            await readSavedUserSetting(
+                bloomApp.userSettingsDir,
+                "LicenseAccepted",
+            ),
         ).toBe("True");
     });
 
@@ -79,7 +82,7 @@ test.describe("a test's Bloom keeps its user settings to itself", () => {
         bloomApp,
     }) => {
         test.setTimeout(300000);
-        const savedZoom = readSavedUserSetting(
+        const savedZoom = await readSavedUserSetting(
             bloomApp.userSettingsDir,
             "PageZoom",
         );
@@ -96,8 +99,8 @@ test.describe("a test's Bloom keeps its user settings to itself", () => {
                 bloomApp.userSettingsDir,
             ),
         ).toBe(true);
-        expect(readSavedUserSetting(bloomApp.userSettingsDir, "PageZoom")).toBe(
-            savedZoom,
-        );
+        expect(
+            await readSavedUserSetting(bloomApp.userSettingsDir, "PageZoom"),
+        ).toBe(savedZoom);
     });
 });