From b2c8c528a10cc88602a43e2da5bd58578a470dc7 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 6 Jul 2026 15:22:44 -0600 Subject: [PATCH 01/24] Expand E2E testing --- cspell.json | 2 + e2e-tests/README.md | 19 +- e2e-tests/fixtures/helpers.ts | 305 +++++++++++++++++- .../tests/features/draft-persistence.spec.ts | 63 ++++ .../tests/features/gloss-roundtrip.spec.ts | 41 +++ .../tests/features/project-modals.spec.ts | 61 ++++ 6 files changed, 475 insertions(+), 16 deletions(-) create mode 100644 e2e-tests/tests/features/draft-persistence.spec.ts create mode 100644 e2e-tests/tests/features/gloss-roundtrip.spec.ts create mode 100644 e2e-tests/tests/features/project-modals.spec.ts diff --git a/cspell.json b/cspell.json index 3046472d..09ef9b56 100644 --- a/cspell.json +++ b/cspell.json @@ -20,6 +20,7 @@ "BBCCCVVV", "believ", "clickability", + "cmdk", "cullable", "deconflict", "deconfliction", @@ -65,6 +66,7 @@ "Stylesheet", "typedefs", "unanalyzed", + "unglossed", "unhover", "unobserves", "unphrased", diff --git a/e2e-tests/README.md b/e2e-tests/README.md index db053ae1..fae81cea 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -1,6 +1,9 @@ # e2e-tests -End-to-end tests for the interlinearizer extension using Playwright + Electron. The suite launches a real Platform.Bible instance with the extension loaded via `--extensions` and verifies the extension starts up correctly. Currently contains one smoke test confirming the extension activates and registers its PAPI command. +End-to-end tests for the interlinearizer extension using Playwright + Electron. The suite has two tiers: + +- **Smoke tests** (`tests/smoke/`, `app.fixture`) launch a fresh Platform.Bible instance with the extension loaded via `--extensions` and verify the extension starts up correctly. +- **Feature tests** (`tests/features/`, `cdp.fixture`) attach to an already-running `npm run start:cdp` instance and exercise interlinearizer UI flows (glossing, draft persistence, project modals). **Contents:** @@ -17,3 +20,17 @@ These tests are adapted from `paranext-core`'s e2e suite with changes to support - **Extension launch helper** — `fixtures/helpers.ts` uses `launchElectronWithExtension()` instead of `launchElectronApp()`. It passes `--extensions ` to the Electron process, resolves the Electron binary from paranext-core's `node_modules`, and polls `rpc.discover` for the extension's PAPI method to confirm activation. - **Window finding** — `fixtures/app.fixture.ts` manually polls `electronApp.windows()` by URL instead of calling `electronApp.firstWindow()`, because the extension injects content into an existing window rather than being the sole owner of the renderer. - **Renderer readiness** — `global-setup.ts` adds an HTTP GET probe after the TCP port check to wait for webpack compilation to finish, rather than assuming the port being open means the bundle is ready. + +## Writing feature tests + +Feature tests run with `npm run test:e2e:cdp` against a shared, long-lived `npm run start:cdp` instance, so they must assume nothing about its state and must leave nothing behind that could poison the next run. The protocol: + +- **Import from `cdp.fixture`, never `app.fixture`.** The CDP config already serializes execution (`workers: 1`), so tests never race each other on the shared instance. +- **The instance is only ever used with the WEB project.** This is an operating assumption, not something tests verify: `ensureInterlinearizerOpenOnWeb()` trusts an existing Interlinearizer tab and only picks WEB when opening fresh. Don't point `start:cdp` at other projects. +- **Mutating tests operate on the dedicated "E2E Test Project", never on a developer's own projects.** `ensureE2eProjectActive()` opens it (creating it on first use) at the start of each mutating test. Because the draft is the single per-source working buffer, replacing it could destroy unsaved developer work — so when the draft is dirty and the active project is *not* the e2e project, the helper first saves the draft into a new `e2e-rescued-work-` project. Rescue projects are backups, not junk: delete them manually once recovered. Dirty state left while the e2e project is active is treated as leftover test data and discarded. +- **Self-establish every precondition.** Each mutating test starts with `ensureInterlinearizerOpenOnWeb()` → `ensureE2eProjectActive()` → `navigateToScriptureRef()` (the scroll-group reference could be anywhere) → `wipeDraft()`. +- **Reset at the start, tidy at the end.** The start-of-test sequence is what guarantees correctness — it self-heals whatever a failed run left behind. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers; this is a courtesy so the next run doesn't misread test junk as rescuable developer work, not something correctness depends on. +- **Use unique per-run values** (e.g. `` `e2e-gloss-${Date.now()}` ``) for anything written into the draft, so a stale leftover can never satisfy an assertion. +- **Drive only the visible UI.** No JSON-RPC/WebSocket calls to set up or assert state (the rpc.discover readiness polls in the shared helpers are the one sanctioned exception). +- **Prefer existing accessible selectors** (roles, aria-labels like `Gloss for {word}`, ModalShell title ids) over adding new `data-testid`s to production code. +- **Mutating tests must not overwrite or delete projects, and must not create any beyond what `ensureE2eProjectActive()` creates** (the e2e project itself, plus rescue projects). The current modal coverage is a read-only cancel tour; a create/delete lifecycle test needs its own self-healing cleanup (e.g. deleting leftover `e2e-*` projects at start) before it's safe on a shared instance. diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index a8c604a0..4f626eac 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -1,5 +1,11 @@ // Adapted from paranext-core/e2e-tests/fixtures/helpers.ts -import { _electron as electron, ElectronApplication, expect, Page } from '@playwright/test'; +import { + _electron as electron, + ElectronApplication, + expect, + FrameLocator, + Page, +} from '@playwright/test'; import fs from 'fs'; import { createRequire } from 'module'; import os from 'os'; @@ -400,15 +406,18 @@ export async function waitForInterlinearizerReady(timeoutMs = 90_000): Promise { - // Focus the Scripture Editor tab (opens without a project at fresh start). - const editorTab = page.locator('.dock-tab', { hasText: 'Scripture Editor' }).first(); + // Focus the Scripture Editor tab. Its title (and its iframe's title) is "Scripture Editor" on a + // fresh app with no project loaded, and the project short name once one is loaded. + const editorTab = page + .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${projectName})\\b`) }) + .first(); await expect(editorTab).toBeVisible({ timeout: 15_000 }); await editorTab.click(); // The Scripture Editor renders its own toolbar inside its iframe. Click the ≡ ("Project") button. - const editorFrame = page.frameLocator('iframe[title*="Scripture Editor" i]'); + const editorFrame = page + .frameLocator(`iframe[title*="Scripture Editor" i], iframe[title^="${projectName}"]`) + .first(); await editorFrame.locator("button[aria-label='Project']").first().click(); // Click the "Open Interlinearizer for this Project" item contributed by this extension. @@ -435,16 +449,277 @@ export async function openInterlinearizerFromScriptureEditor( .first() .click(); - // The command calls papi.dialogs.selectProject (because no project is selected in a fresh start), - // which opens a floating "Open Interlinearizer" dock tab with the project list. + // When the editor has no project selected, the command calls papi.dialogs.selectProject, which + // opens a floating "Open Interlinearizer" dock tab with the project list. When the editor + // already has a project (a warm instance), the Interlinearizer tab opens directly instead. const selectProjectDialog = page.locator('.select-project-dialog'); - await expect(selectProjectDialog).toBeVisible({ timeout: 15_000 }); - const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); - await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); + const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + await expect(selectProjectDialog.or(interlinearizerTab)).toBeVisible({ timeout: 15_000 }); + if (await selectProjectDialog.isVisible()) { + const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); + await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); + } // Wait for the Interlinearizer tab to appear and focus it. - const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); await interlinearizerTab.click(); } + +/** + * Frame locator for the Interlinearizer WebView's iframe, where all of the extension's own UI + * (toolbar, token strips, modals) renders. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns A `FrameLocator` scoped to the Interlinearizer WebView iframe. + */ +export function getInterlinearizerFrame(page: Page): FrameLocator { + return page.frameLocator('iframe[title*="Interlinearizer" i]'); +} + +/** + * Ensure the Interlinearizer is open and focused, reusing an existing tab when one is present. + * Standard precondition for feature tests running against the shared CDP instance: an existing + * Interlinearizer tab is trusted to be on the WEB project (the shared instance is only ever used + * with WEB — see e2e-tests/README.md); otherwise the tab is opened fresh via the Scripture Editor + * menu flow. Resolves only once the extension's toolbar has rendered inside the iframe. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Interlinearizer tab is focused and its toolbar is interactive. + * @throws If the Interlinearizer cannot be opened or its toolbar does not render within the + * timeouts. + */ +export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise { + const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + if (await interlinearizerTab.isVisible()) { + await interlinearizerTab.click(); + } else { + await openInterlinearizerFromScriptureEditor(page); + } + const frame = getInterlinearizerFrame(page); + await expect(frame.locator("button[aria-label='Project']").first()).toBeVisible({ + timeout: 30_000, + }); +} + +/** + * Navigate the platform's book-chapter-verse control to the given scripture reference so tests can + * assert against known text. Opens the toolbar's reference combobox, types the reference, and + * submits it. Requires a fully-qualified reference (book, chapter, and verse — e.g. `"GEN 1:1"`); + * partial references are ambiguous and are not auto-submitted by the control. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param reference Fully-qualified scripture reference to navigate to (e.g. `"GEN 1:1"`). + * @returns Resolves when the reference popover has closed after submitting. + * @throws If the reference control does not open, or the popover does not close after submitting. + */ +export async function navigateToScriptureRef(page: Page, reference: string): Promise { + const trigger = page.locator('button[aria-label="book-chapter-trigger"]').first(); + await trigger.click(); + const input = page.locator('input[cmdk-input]').first(); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.fill(reference); + await input.press('Enter'); + // The popover closes when the reference is accepted. + await expect(input).not.toBeVisible({ timeout: 5_000 }); +} + +/** + * Open the Interlinearizer's ≡ ("Project") top menu inside its iframe and wait for the dropdown to + * appear. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns The frame locator for the Interlinearizer iframe, for chaining menu-item clicks. + * @throws If the menu button or the opened menu does not become visible within the timeouts. + */ +export async function openInterlinearizerProjectMenu(page: Page): Promise { + const frame = getInterlinearizerFrame(page); + const projectMenuButton = frame.locator("button[aria-label='Project']").first(); + await expect(projectMenuButton).toBeVisible({ timeout: 15_000 }); + await projectMenuButton.click(); + await expect(frame.locator('[role="menu"]')).toBeVisible({ timeout: 5_000 }); + return frame; +} + +/** Name of the dedicated interlinear project the mutating feature tests operate on. */ +export const E2E_PROJECT_NAME = 'E2E Test Project'; + +/** Name prefix for projects created to rescue a developer's unsaved draft work. */ +const RESCUE_PROJECT_PREFIX = 'e2e-rescued-work'; + +/** + * Glyph the Interlinearizer appends to its dock tab title while the draft has unsaved changes. Keep + * in sync with UNSAVED_TAB_MARKER in src/components/InterlinearizerLoader.tsx. + */ +const UNSAVED_TAB_MARKER = '●'; + +/** + * Read whether the draft has unsaved changes from the Interlinearizer dock tab's title marker (the + * only place the dirty state is observable outside the WebView). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns `true` when the tab title carries the unsaved-changes glyph. + */ +async function isDraftDirty(page: Page): Promise { + const tabText = await page + .locator('.dock-tab', { hasText: 'Interlinearizer' }) + .first() + .textContent(); + return (tabText ?? '').includes(UNSAVED_TAB_MARKER); +} + +/** + * Open the "Select Interlinear Project" modal from the Project menu and wait for its project list + * to finish loading (the modal's buttons are disabled while the fetch is in flight). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns The frame locator for the Interlinearizer iframe, for chaining clicks in the modal. + * @throws If the modal does not open or its list does not finish loading within the timeouts. + */ +async function openSelectProjectModal(page: Page): Promise { + const frame = await openInterlinearizerProjectMenu(page); + await frame + .getByRole('menuitem', { name: /Select Interlinear Project/i }) + .first() + .click(); + await expect(frame.locator('#select-project-modal-title')).toBeVisible({ timeout: 10_000 }); + await expect(frame.locator('dialog').getByRole('button', { name: 'Cancel' })).toBeEnabled({ + timeout: 10_000, + }); + return frame; +} + +/** + * Preserve the current draft by saving it as a brand-new, timestamped rescue project (Project menu + * → Save As… → "Save as New Project"). Used before the e2e project replaces a dirty draft that may + * hold a developer's unsaved work, so nothing is silently discarded. Clears the draft's + * unsaved-changes state as a side effect (the rescue project becomes the active Save target). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Save As modal has closed and the unsaved marker has cleared. + * @throws If the Save As modal does not open, close, or clear the unsaved marker within the + * timeouts. + */ +async function rescueDraftToNewProject(page: Page): Promise { + const frame = await openInterlinearizerProjectMenu(page); + await frame + .getByRole('menuitem', { name: /^Save As/i }) + .first() + .click(); + + const saveAsTitle = frame.locator('#save-as-modal-title'); + await expect(saveAsTitle).toBeVisible({ timeout: 10_000 }); + await frame.locator('#save-as-name').fill(`${RESCUE_PROJECT_PREFIX}-${Date.now()}`); + await frame.getByTestId('save-as-new').click(); + await expect(saveAsTitle).not.toBeVisible({ timeout: 10_000 }); + + // The save clears the unsaved marker; wait for it so later dirty checks read the new state. + await expect(page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first()).not.toContainText( + UNSAVED_TAB_MARKER, + { timeout: 10_000 }, + ); +} + +/** + * Make the dedicated e2e project ({@link E2E_PROJECT_NAME}) the active project, creating it if it + * does not exist yet, so mutating tests never touch a developer's own projects. Opening a project + * replaces the draft (the single per-source working buffer), so when the draft is dirty and the + * active project is NOT the e2e project — i.e. the unsaved work may be a developer's, not leftover + * test data — it is first rescued into a new `e2e-rescued-work-*` project instead of being + * discarded. Dirty state left while the e2e project is active is treated as leftover test data and + * discarded via the confirm dialog. + * + * Mutating tests call this at the START (with rescue on) to establish their precondition, and again + * at the END (with rescue off) to discard their own leftovers so the next run starts clean. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param opts Options object. + * @param opts.rescueDirtyDraft Whether a dirty draft not owned by the e2e project is rescued before + * being replaced (default `true`). Pass `false` only at the end of a test, where the dirty state + * is known to be the test's own leftovers. + * @returns Resolves when the e2e project is active and all modals have closed. + * @throws If the modals do not open/close or the project cannot be selected or created within the + * timeouts. + */ +export async function ensureE2eProjectActive( + page: Page, + opts: { rescueDirtyDraft?: boolean } = {}, +): Promise { + const { rescueDirtyDraft = true } = opts; + + let dirty = await isDraftDirty(page); + let frame = await openSelectProjectModal(page); + let dialog = frame.locator('dialog'); + + const activeEntry = dialog.locator('button[aria-current="true"]'); + const activeIsE2e = + (await activeEntry.count()) > 0 && + ((await activeEntry.first().textContent()) ?? '').includes(E2E_PROJECT_NAME); + + if (dirty && !activeIsE2e && rescueDirtyDraft) { + await dialog.getByRole('button', { name: 'Cancel' }).click(); + await expect(frame.locator('#select-project-modal-title')).not.toBeVisible({ timeout: 5_000 }); + await rescueDraftToNewProject(page); + dirty = false; + frame = await openSelectProjectModal(page); + dialog = frame.locator('dialog'); + } + + const selectTitle = frame.locator('#select-project-modal-title'); + const e2eEntry = dialog.getByRole('button', { name: new RegExp(E2E_PROJECT_NAME) }).first(); + if ((await e2eEntry.count()) > 0) { + await e2eEntry.click(); + if (dirty) { + // Replacing a dirty draft asks for confirmation; anything worth keeping was rescued above. + const discardConfirm = frame.getByTestId('discard-draft-confirm'); + await expect(discardConfirm).toBeVisible({ timeout: 5_000 }); + await discardConfirm.click(); + } + } else { + await dialog.getByRole('button', { name: 'Create New' }).click(); + const createTitle = frame.locator('#create-project-modal-title'); + await expect(createTitle).toBeVisible({ timeout: 5_000 }); + await frame.locator('#project-name').fill(E2E_PROJECT_NAME); + await frame.locator('dialog').getByRole('button', { name: 'Create' }).click(); + await expect(createTitle).not.toBeVisible({ timeout: 10_000 }); + } + await expect(selectTitle).not.toBeVisible({ timeout: 10_000 }); +} + +/** + * Wipe the entire draft's analysis via the visible UI (Project menu → Wipe… → "Entire draft" → + * Wipe). Standard reset step for mutating feature tests on the shared CDP instance: run it at the + * START of a test (never at the end) so a previously failed run self-heals instead of poisoning the + * next one. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the wipe dialog has closed after confirming. + * @throws If the wipe dialog does not open or does not close after confirming. + */ +export async function wipeDraft(page: Page): Promise { + const frame = await openInterlinearizerProjectMenu(page); + await frame.getByRole('menuitem', { name: /Wipe/i }).first().click(); + + const wipeDialogTitle = frame.locator('#wipe-modal-title'); + await expect(wipeDialogTitle).toBeVisible({ timeout: 5_000 }); + await frame.getByTestId('wipe-scope-all').check(); + await frame.getByTestId('wipe-confirm').click(); + await expect(wipeDialogTitle).not.toBeVisible({ timeout: 10_000 }); +} + +/** + * Close the Interlinearizer dock tab via its close button and wait for it to disappear. Used by + * tests that verify draft persistence across a close/reopen cycle. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when the Interlinearizer tab is gone. + * @throws If the tab or its close button is not visible, or the tab does not close within the + * timeout. + */ +export async function closeInterlinearizerTab(page: Page): Promise { + const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); + await interlinearizerTab.hover(); + await interlinearizerTab.locator('.dock-tab-close-btn').click(); + await expect(interlinearizerTab).not.toBeVisible({ timeout: 10_000 }); +} diff --git a/e2e-tests/tests/features/draft-persistence.spec.ts b/e2e-tests/tests/features/draft-persistence.spec.ts new file mode 100644 index 00000000..c6f9a68a --- /dev/null +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -0,0 +1,63 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + closeInterlinearizerTab, + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + waitForAppReady, + waitForInterlinearizerReady, + wipeDraft, +} from '../../fixtures/helpers'; + +test.describe('Draft persistence', () => { + test('a glossed draft survives closing and reopening the interlinearizer', async ({ + mainPage, + }) => { + // Two full open cycles (plus first-use project creation and book loading on a cold instance) + // legitimately exceed the default 120 s budget. + test.slow(); + await waitForAppReady(mainPage); + await waitForInterlinearizerReady(); + await ensureInterlinearizerOpenOnWeb(mainPage); + await ensureE2eProjectActive(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + await wipeDraft(mainPage); + + const frame = getInterlinearizerFrame(mainPage); + const glossInput = frame.getByLabel('Gloss for beginning', { exact: true }).first(); + await expect(glossInput).toBeVisible({ timeout: 30_000 }); + + // Unique per run so a leftover value from a previous run can never false-pass. + const gloss = `e2e-persist-${Date.now()}`; + await glossInput.click(); + await glossInput.fill(gloss); + await glossInput.press('Tab'); + await expect(glossInput).toHaveValue(gloss); + + // The draft auto-saves on a 300 ms debounce after the last keystroke, and there is no UI + // signal that the storage write has landed (the tab's dirty marker tracks draft-vs-saved- + // project, not draft-vs-storage). This fixed wait deliberately covers the debounce so the + // test exercises the debounced-save path; the unmount-flush (close immediately after + // typing) path is intentionally out of scope here. + await mainPage.waitForTimeout(1_000); + + await closeInterlinearizerTab(mainPage); + + await ensureInterlinearizerOpenOnWeb(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + + const reopenedFrame = getInterlinearizerFrame(mainPage); + const reopenedGlossInput = reopenedFrame + .getByLabel('Gloss for beginning', { exact: true }) + .first(); + await expect(reopenedGlossInput).toBeVisible({ timeout: 30_000 }); + await expect(reopenedGlossInput).toHaveValue(gloss); + + // Discard this test's leftover gloss (reload the e2e project into the draft) so the next run + // starts clean instead of triggering the dirty-draft rescue. Rescue must stay off here: the + // close/reopen dropped the active-project WebView state, so the dirty draft would otherwise + // look like developer work. + await ensureE2eProjectActive(mainPage, { rescueDirtyDraft: false }); + }); +}); diff --git a/e2e-tests/tests/features/gloss-roundtrip.spec.ts b/e2e-tests/tests/features/gloss-roundtrip.spec.ts new file mode 100644 index 00000000..2b073f10 --- /dev/null +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -0,0 +1,41 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + waitForAppReady, + waitForInterlinearizerReady, + wipeDraft, +} from '../../fixtures/helpers'; + +test.describe('Gloss round-trip', () => { + test('typing a gloss on a token renders it in the gloss field', async ({ mainPage }) => { + await waitForAppReady(mainPage); + await waitForInterlinearizerReady(); + await ensureInterlinearizerOpenOnWeb(mainPage); + await ensureE2eProjectActive(mainPage); + await navigateToScriptureRef(mainPage, 'GEN 1:1'); + await wipeDraft(mainPage); + + const frame = getInterlinearizerFrame(mainPage); + + // WEB Genesis 1:1: "In the beginning, God created the heavens and the earth." + const glossInput = frame.getByLabel('Gloss for beginning', { exact: true }).first(); + await expect(glossInput).toBeVisible({ timeout: 30_000 }); + // The wipe just cleared all analysis, so the field must start empty. + await expect(glossInput).toHaveValue(''); + + // Unique per run so a leftover value from a previous run can never false-pass. + const gloss = `e2e-gloss-${Date.now()}`; + await glossInput.click(); + await glossInput.fill(gloss); + await glossInput.press('Tab'); + + await expect(glossInput).toHaveValue(gloss); + + // Discard this test's leftover gloss (reload the e2e project into the draft) so the next run + // starts clean instead of triggering the dirty-draft rescue. + await ensureE2eProjectActive(mainPage, { rescueDirtyDraft: false }); + }); +}); diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts new file mode 100644 index 00000000..03709137 --- /dev/null +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -0,0 +1,61 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + openInterlinearizerProjectMenu, + waitForAppReady, + waitForInterlinearizerReady, +} from '../../fixtures/helpers'; + +/** + * The project-related modals reachable from the Interlinearizer's ≡ (Project) menu, each with the + * menu item that opens it and the title element that identifies it (from ModalShell's `titleId`). + * The tour is read-only: each modal is opened, verified, and canceled — no project is created, + * saved, or deleted, so the shared CDP instance is left untouched. + */ +const MODAL_TOURS = [ + { + name: 'Select Interlinear Project', + menuItem: /Select Interlinear Project/i, + titleSelector: '#select-project-modal-title', + }, + { + name: 'New Interlinear Project', + menuItem: /New Interlinear Project/i, + titleSelector: '#create-project-modal-title', + }, + { + name: 'Save As', + menuItem: /^Save As/i, + titleSelector: '#save-as-modal-title', + }, +]; + +test.describe('Project modals cancel tour', () => { + MODAL_TOURS.forEach((modal) => { + test(`the ${modal.name} modal opens from the Project menu and cancels cleanly`, async ({ + mainPage, + }) => { + await waitForAppReady(mainPage); + await waitForInterlinearizerReady(); + await ensureInterlinearizerOpenOnWeb(mainPage); + + const frame = await openInterlinearizerProjectMenu(mainPage); + await frame.getByRole('menuitem', { name: modal.menuItem }).first().click(); + + const modalTitle = frame.locator(modal.titleSelector); + await expect(modalTitle).toBeVisible({ timeout: 10_000 }); + + await frame.locator('dialog').getByRole('button', { name: 'Cancel' }).click(); + await expect(modalTitle).not.toBeVisible({ timeout: 5_000 }); + + // The underlying view must still be interactive after the modal unmounts — a stuck + // overlay is the most common real modal regression. + const projectMenuButton = getInterlinearizerFrame(mainPage) + .locator("button[aria-label='Project']") + .first(); + await expect(projectMenuButton).toBeVisible({ timeout: 5_000 }); + await expect(projectMenuButton).toBeEnabled(); + }); + }); +}); From ed2c07a203e8d01f1ab08ff366307d81fc708a48 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 7 Jul 2026 10:17:17 -0600 Subject: [PATCH 02/24] Fix test failures, lint errors --- e2e-tests/README.md | 2 +- e2e-tests/fixtures/helpers.ts | 57 +++++++-- package-lock.json | 224 +++++++++++++++++----------------- 3 files changed, 157 insertions(+), 126 deletions(-) diff --git a/e2e-tests/README.md b/e2e-tests/README.md index fae81cea..bb55ca6a 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -27,7 +27,7 @@ Feature tests run with `npm run test:e2e:cdp` against a shared, long-lived `npm - **Import from `cdp.fixture`, never `app.fixture`.** The CDP config already serializes execution (`workers: 1`), so tests never race each other on the shared instance. - **The instance is only ever used with the WEB project.** This is an operating assumption, not something tests verify: `ensureInterlinearizerOpenOnWeb()` trusts an existing Interlinearizer tab and only picks WEB when opening fresh. Don't point `start:cdp` at other projects. -- **Mutating tests operate on the dedicated "E2E Test Project", never on a developer's own projects.** `ensureE2eProjectActive()` opens it (creating it on first use) at the start of each mutating test. Because the draft is the single per-source working buffer, replacing it could destroy unsaved developer work — so when the draft is dirty and the active project is *not* the e2e project, the helper first saves the draft into a new `e2e-rescued-work-` project. Rescue projects are backups, not junk: delete them manually once recovered. Dirty state left while the e2e project is active is treated as leftover test data and discarded. +- **Mutating tests operate on the dedicated "E2E Test Project", never on a developer's own projects.** `ensureE2eProjectActive()` opens it (creating it on first use) at the start of each mutating test. Because the draft is the single per-source working buffer, replacing it could destroy unsaved developer work — so when the draft is dirty and the active project is _not_ the e2e project, the helper first saves the draft into a new `e2e-rescued-work-` project. Rescue projects are backups, not junk: delete them manually once recovered. Dirty state left while the e2e project is active is treated as leftover test data and discarded. - **Self-establish every precondition.** Each mutating test starts with `ensureInterlinearizerOpenOnWeb()` → `ensureE2eProjectActive()` → `navigateToScriptureRef()` (the scroll-group reference could be anywhere) → `wipeDraft()`. - **Reset at the start, tidy at the end.** The start-of-test sequence is what guarantees correctness — it self-heals whatever a failed run left behind. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers; this is a courtesy so the next run doesn't misread test junk as rescuable developer work, not something correctness depends on. - **Use unique per-run values** (e.g. `` `e2e-gloss-${Date.now()}` ``) for anything written into the draft, so a stale leftover can never satisfy an assertion. diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 4f626eac..49c7c4ba 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -408,20 +408,33 @@ export async function waitForInterlinearizerReady(timeoutMs = 90_000): Promise { - // Focus the Scripture Editor tab. Its title (and its iframe's title) is "Scripture Editor" on a - // fresh app with no project loaded, and the project short name once one is loaded. + // A Scripture Editor tab is titled by the project short name once a project is loaded (e.g. + // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. const editorTab = page .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${projectName})\\b`) }) .first(); + const homeTab = page.locator('.dock-tab', { hasText: 'Home' }).first(); + + // Wait for the dock layout to actually mount before deciding which path to take — a fresh profile + // briefly reports zero tabs, and a non-waiting `count()` would misread that as "no editor". + await expect(editorTab.or(homeTab)).toBeVisible({ timeout: 30_000 }); + + // If the layout came up without a Scripture Editor (single-tab Home layout), open the project + // from Home so the editor (and its ≡ menu) exists before we try to focus it. + if ((await editorTab.count()) === 0) { + await homeTab.click(); + const homeFrame = page.frameLocator('iframe[title="Home"]'); + await homeFrame.locator(`tr:has-text("${projectName}") button:has-text("Open")`).click(); + } + await expect(editorTab).toBeVisible({ timeout: 15_000 }); await editorTab.click(); @@ -453,7 +480,11 @@ export async function openInterlinearizerFromScriptureEditor( // opens a floating "Open Interlinearizer" dock tab with the project list. When the editor // already has a project (a warm instance), the Interlinearizer tab opens directly instead. const selectProjectDialog = page.locator('.select-project-dialog'); - const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + // Match the Interlinearizer WebView tab ("Interlinearizer") but NOT the project-picker dialog's + // own dock tab ("Open Interlinearizer"), whose title also contains "Interlinearizer". + const interlinearizerTab = page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(); await expect(selectProjectDialog.or(interlinearizerTab)).toBeVisible({ timeout: 15_000 }); if (await selectProjectDialog.isVisible()) { const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); diff --git a/package-lock.json b/package-lock.json index 12c9754d..5d3961fc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3916,17 +3916,17 @@ "license": "MIT" }, "node_modules/@typescript-eslint/eslint-plugin": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.62.1.tgz", - "integrity": "sha512-4EQM77WgVNxj7OkL/5b/D/xZsw00G577+UriYTC7JF5opcF3T2AuoeY7ueLaZgSVjSgCS6yOAJB5bRGLPSJUzA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.63.0.tgz", + "integrity": "sha512-rvwSgqT+DHpWdzfSzPatRLm02a0GlESt++9iy3hLCDY4BgkaLcl8LBi9Yh7XGFBpwcBE/K3024QuXWTpbz4FfQ==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/regexpp": "^4.12.2", - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/type-utils": "8.62.1", - "@typescript-eslint/utils": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/type-utils": "8.63.0", + "@typescript-eslint/utils": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "ignore": "^7.0.5", "natural-compare": "^1.4.0", "ts-api-utils": "^2.5.0" @@ -3939,22 +3939,22 @@ "url": "https://opencollective.com/typescript-eslint" }, "peerDependencies": { - "@typescript-eslint/parser": "^8.62.1", + "@typescript-eslint/parser": "^8.63.0", "eslint": "^8.57.0 || ^9.0.0 || ^10.0.0", "typescript": ">=4.8.4 <6.1.0" } }, "node_modules/@typescript-eslint/parser": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.62.1.tgz", - "integrity": "sha512-sPhE4iHuJDSvoAiec+Ro8JyXw8f0ql13HFR82P99nCm9GwTEKG0KYLvDe6REk8BCXuit6vJAv/Yxg5ABaNS2rA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-8.63.0.tgz", + "integrity": "sha512-gwh4gvvlaVDKKxyfxMG+Gnu1u9X0OQBwyGLkbwB65dIzBKnxeRiJlNFqlI3zwVhNXJIs6qV7mlFCn/BIajlVig==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "debug": "^4.4.3" }, "engines": { @@ -3970,14 +3970,14 @@ } }, "node_modules/@typescript-eslint/project-service": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.62.1.tgz", - "integrity": "sha512-yQ3RgY5RkSBpsNS1Bx/JQEcA24FOSdfGktoyprAr5u18390UQdtVcfnEv4nIrIshNnavlVyZBKxQwT1fIAE6cg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/project-service/-/project-service-8.63.0.tgz", + "integrity": "sha512-e5dh0/UI0ok53AlZ5wRkXCB32z/f2jUZqPR/ygAw5WYaSw8j9EoJWlS7wQjr/dmOaqWjnPIn2m+HhVPCMWGZVQ==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/tsconfig-utils": "^8.62.1", - "@typescript-eslint/types": "^8.62.1", + "@typescript-eslint/tsconfig-utils": "^8.63.0", + "@typescript-eslint/types": "^8.63.0", "debug": "^4.4.3" }, "engines": { @@ -3992,14 +3992,14 @@ } }, "node_modules/@typescript-eslint/scope-manager": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.62.1.tgz", - "integrity": "sha512-r4d249KbQ1SFdpeStvob8Ih6aPPIzfqllPVOtvhve6ZcpuVcYo5/7zUWckKpHE7StASX4kTKZTLf0WQm/wPkcg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.63.0.tgz", + "integrity": "sha512-uUyfMWCnDSN8bCpcrY8nGP2BLkQ9Xn0GsipcONcpIDWhwhO4ZSyHvyS14U3X75mzxWxL3I2UZIrenTzdzcJO8A==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1" + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4010,9 +4010,9 @@ } }, "node_modules/@typescript-eslint/tsconfig-utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.62.1.tgz", - "integrity": "sha512-xadytJqX9vJVQ2fdQjkcIVigwaOJNWkpjdLt6cEQ+xPnrI1fkp+/jZE/I97k9KUjqtpd25i0HeyZf3T6dutv2g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/tsconfig-utils/-/tsconfig-utils-8.63.0.tgz", + "integrity": "sha512-sUAbkulqBAsncKnbRP3+7CtQFRKicexnj7ZwNC6ddCR7EmrXvjvdCYMJbUIqMd6lwoEriZjwLo08aS5tSjVMHg==", "dev": true, "license": "MIT", "engines": { @@ -4027,15 +4027,15 @@ } }, "node_modules/@typescript-eslint/type-utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.62.1.tgz", - "integrity": "sha512-aXM5xlqXiTxPibXB93cLAURfT3rlizf7uMXISCXy66Isr/9hISJx3yDsKl0L7lKa51b8JpFuNKby0/O0pEm9jg==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-8.63.0.tgz", + "integrity": "sha512-Nzzh/OGxVCOjObjaj1CQF2RUasyYy2Jfuh+zZ3PjLzG2fYRriAiZLib9UKtO+CpQAS3YHiAS+ckZDclwqI1TPA==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1", - "@typescript-eslint/utils": "8.62.1", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0", + "@typescript-eslint/utils": "8.63.0", "debug": "^4.4.3", "ts-api-utils": "^2.5.0" }, @@ -4052,9 +4052,9 @@ } }, "node_modules/@typescript-eslint/types": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.62.1.tgz", - "integrity": "sha512-ooCzJFaf+Hg+uG6fA3NRFGuFjlfNlDhBthbv4ZPU/0elCAFUfnyXUvf/WOpHz/jYwSmvU2GkR2LtyUfy1AxZ1Q==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-8.63.0.tgz", + "integrity": "sha512-xyLtl9DUBBFrcJS4x2pIqGLH68/tC2uOa4Z7pUteW09D3bXnnXUom4dyPikzWgB7llmIc1zoeI3aoUdC4rPK/Q==", "dev": true, "license": "MIT", "engines": { @@ -4066,16 +4066,16 @@ } }, "node_modules/@typescript-eslint/typescript-estree": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.62.1.tgz", - "integrity": "sha512-xMcW9oP9u7fAMXYs9A65CVmtLQe2r//oXINHfi8HV+oiqhih17sbLdhXr4540YWlgpDKQdY854OL5ZrdCiQsAA==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-8.63.0.tgz", + "integrity": "sha512-ygBkU+B7ex5UI/gKhaqexWev79uISfIv7XQCRNYO/jmD8rGLPyWLAb3KMRT6nd8Gt9bmUBi9+iX6tBdYfOY81Q==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/project-service": "8.62.1", - "@typescript-eslint/tsconfig-utils": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/visitor-keys": "8.62.1", + "@typescript-eslint/project-service": "8.63.0", + "@typescript-eslint/tsconfig-utils": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/visitor-keys": "8.63.0", "debug": "^4.4.3", "minimatch": "^10.2.2", "semver": "^7.7.3", @@ -4094,16 +4094,16 @@ } }, "node_modules/@typescript-eslint/utils": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.62.1.tgz", - "integrity": "sha512-sHtbPfuKNZCG+ih8SyjjucqRntSVmp8XgL5u6o9mAhiSn8ds5o/M/XdM0abweme2Tln3szOstOrZ9OXitvPh0g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-8.63.0.tgz", + "integrity": "sha512-fUKaeAvrTuQg/Tgt3nliAUSZHJM6DlCcfyEmxCvlX8kieWSStBX+5O5Fnidtc3i2JrH+9c/GL4RY2iasd/GPTA==", "dev": true, "license": "MIT", "dependencies": { "@eslint-community/eslint-utils": "^4.9.1", - "@typescript-eslint/scope-manager": "8.62.1", - "@typescript-eslint/types": "8.62.1", - "@typescript-eslint/typescript-estree": "8.62.1" + "@typescript-eslint/scope-manager": "8.63.0", + "@typescript-eslint/types": "8.63.0", + "@typescript-eslint/typescript-estree": "8.63.0" }, "engines": { "node": "^18.18.0 || ^20.9.0 || >=21.1.0" @@ -4118,13 +4118,13 @@ } }, "node_modules/@typescript-eslint/visitor-keys": { - "version": "8.62.1", - "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.62.1.tgz", - "integrity": "sha512-4g3BLxfdTMy8iZG0MaBkadnlRrCJ74cQiFbyEVMrkwIoqdyaXXQM22cotDvrl4x28wgIZ9rEJRoM+mmhSJpJ1g==", + "version": "8.63.0", + "resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-8.63.0.tgz", + "integrity": "sha512-UexrHGnGTpbuQHct2ExOc2ZcFbGUS9FOesCxxqdBGcpI1BxYu/LZ6U8Aq6/72XtF/qRBk9nhuGHFJIXXMhPMdw==", "dev": true, "license": "MIT", "dependencies": { - "@typescript-eslint/types": "8.62.1", + "@typescript-eslint/types": "8.63.0", "eslint-visitor-keys": "^5.0.0" }, "engines": { @@ -5488,9 +5488,9 @@ "license": "MIT" }, "node_modules/baseline-browser-mapping": { - "version": "2.10.40", - "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.40.tgz", - "integrity": "sha512-BSSLZ9/Cjjv7Gtj5B68ZzXcXUg8iOf3fme+FCuh8rC/Go+Kmh8cox7M3A8dolou16s64QjLPOSdngh7GxXvkSw==", + "version": "2.10.42", + "resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.42.tgz", + "integrity": "sha512-c/jurFrDLyui7o1J86yLkRu4LMsTYcBohveus7/I2Hzdn9KIP2bdJPTue/lR1KH46enoPbD77GKeSYNdyPoD3Q==", "dev": true, "license": "Apache-2.0", "bin": { @@ -5585,9 +5585,9 @@ } }, "node_modules/browserslist": { - "version": "4.28.4", - "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.4.tgz", - "integrity": "sha512-MTc8i/x9jBQd1iMw2CFGS+rwMa07eYjLR0CCTLDACl9xhxy+nIs3KeML/biicXtk9JrZ6dnnTatmc7ErPXIxqw==", + "version": "4.28.5", + "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.5.tgz", + "integrity": "sha512-Cu2E6QejHWzuDMTkuwgpABFgDfZrXLQq5V13YOACZx4mFAG4IwGTbTfHPMr4WtxlHoXSM8FIuRwYYCz5XiabaQ==", "dev": true, "funding": [ { @@ -5605,10 +5605,10 @@ ], "license": "MIT", "dependencies": { - "baseline-browser-mapping": "^2.10.38", - "caniuse-lite": "^1.0.30001799", - "electron-to-chromium": "^1.5.376", - "node-releases": "^2.0.48", + "baseline-browser-mapping": "^2.10.42", + "caniuse-lite": "^1.0.30001800", + "electron-to-chromium": "^1.5.387", + "node-releases": "^2.0.50", "update-browserslist-db": "^1.2.3" }, "bin": { @@ -5804,9 +5804,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001800", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001800.tgz", - "integrity": "sha512-MMHtuAz9Ys840zAY5F4k6fV5GaivZ9sPk+nz0mY+GYVzRBnYkN0mpqkSR92oWRQ19yQWo4HvBV/FnC16AJX8MA==", + "version": "1.0.30001803", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001803.tgz", + "integrity": "sha512-g/uHREV2ZpK9qMalCsWaxmA6ol+DX8GYhuf3T40RKoP+oL7vhRJh8LNt73PCjpnR6l14FzfPrB5Yux4PKm2meg==", "dev": true, "funding": [ { @@ -6970,9 +6970,9 @@ "license": "MIT" }, "node_modules/electron-to-chromium": { - "version": "1.5.382", - "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.382.tgz", - "integrity": "sha512-8ETaWbV6SZOrno+G93Ffd9ENsMtetqdnqj4nlfxFW90Sm5GgnuV28Kf62hqQVD6VUgzm7qFQKsTsAPmeUiU3Ug==", + "version": "1.5.388", + "resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.388.tgz", + "integrity": "sha512-Pl/aJaqOOxYxda3vcx1IKSJimwYXHDkEnGn0F+kG2EE68dDtx2uCinaS+Vih8Z91B9t8CSAbiF/HKyWcnXjhzw==", "dev": true, "license": "ISC" }, @@ -7227,9 +7227,9 @@ } }, "node_modules/es-module-lexer": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.2.0.tgz", - "integrity": "sha512-3lGxdTXCLfe1MYfTz1y2ksAAUM4NAOP6rPEjxGJVKO7TZ5+tvHCaQWGpC4Y3IXvW3ece0Cz1cIP4FWBxOnGCTQ==", + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/es-module-lexer/-/es-module-lexer-2.3.0.tgz", + "integrity": "sha512-KLdwQm2NvGLDkQDCGvmiQrhkd0JbMzXthwQAUgWjQuQdBLFa3eiBP5arXZyA+f8x+x7OXgud6bq2rxjGtHV2tw==", "dev": true, "license": "MIT" }, @@ -7506,9 +7506,9 @@ } }, "node_modules/eslint-module-utils": { - "version": "2.13.0", - "resolved": "https://registry.npmjs.org/eslint-module-utils/-/eslint-module-utils-2.13.0.tgz", - "integrity": "sha512-bLohSkT6469rRs8czj0tLTD8vaeIS/whvPRJVjDr7IuoTT1k5DYDERlNycjDj/HkOlvQdYurmfZ/g3fG5bgeLQ==", + "version": "2.14.0", + "resolved": "https://registry.npmjs.org/eslint-module-utils/-/eslint-module-utils-2.14.0.tgz", + "integrity": "sha512-W2WCRZ9Dqntd+2u8jJcVMV2PKulc6RdLgUUoh/yQr3uB6lo/ZOeGx11sv60/8S4QFFKNslAlWhr9u0Ef7ZW6Ig==", "dev": true, "license": "MIT", "dependencies": { @@ -8394,9 +8394,9 @@ "license": "BSD-3-Clause" }, "node_modules/fast-xml-builder": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.0.tgz", - "integrity": "sha512-00aAWieqff+ZJhsXA4g1g7M8k+7AYoMUUHF+/zFb5U6Uv/P0Vl4QZo84/IcufzYalLuEj9928bXN9PbbFzMF0Q==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/fast-xml-builder/-/fast-xml-builder-1.2.1.tgz", + "integrity": "sha512-tPb5TTWfgfVx5BNSi2xV0eLr89POeXXn0dXIsCJ9m1narrWxeIyx6je9d7Rce/3NyXLbvuQmLkxq+RuxMWejvw==", "funding": [ { "type": "github", @@ -9237,9 +9237,9 @@ } }, "node_modules/hono": { - "version": "4.12.27", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.27.tgz", - "integrity": "sha512-1yrb/+w6HWQJrUCLkJ2IF5jNIPvvFkblV5RNOYl6bV+OA6p9GLcMpHFFGTosSvHvcAUibuUukRqhlYI4z32C7Q==", + "version": "4.12.28", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.28.tgz", + "integrity": "sha512-YwUvVpSF7m1yOblFPrU3Hbo8XhPheBoiyfGuII6z19LnOr6JpDnyyp7LFNrfV56wS8tpvtBFGRISHN02pDdLOA==", "dev": true, "license": "MIT", "engines": { @@ -9346,9 +9346,9 @@ } }, "node_modules/iconv-lite": { - "version": "0.7.2", - "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.2.tgz", - "integrity": "sha512-im9DjEDQ55s9fL4EYzOAv0yMqmMBSZp6G0VvFyTMPKWxiSBHUj9NW/qqLmXUwXrrM7AvqSlTCfvqRb0cM8yYqw==", + "version": "0.7.3", + "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.3.tgz", + "integrity": "sha512-IKXpvIzjnC9XTAUbVBcMfGS0EPaIXtW6v+zr+RRp+hqULEpo0owZax6wyRwPOJbWbzjYspQwusTsfVr0ifh4uQ==", "dev": true, "license": "MIT", "dependencies": { @@ -9407,9 +9407,9 @@ } }, "node_modules/immer": { - "version": "11.1.8", - "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.8.tgz", - "integrity": "sha512-/tbkHMW7y10Lx6i1crLjD4/OhNkRG+Fo7byZHtah0547nIeXYcpIXaUh0IAQY6gO5459qpGGYapcEOHtFXkIuA==", + "version": "11.1.11", + "resolved": "https://registry.npmjs.org/immer/-/immer-11.1.11.tgz", + "integrity": "sha512-qzXuyXAkPySAGYkfsAwodDPWT8Zm7/Uo5BNt4BjhMhG5WlWyZZ4wQqnWwdS8kjlQ1Cwu6gjw3A6+0gTQwlyYtw==", "license": "MIT", "funding": { "type": "opencollective", @@ -12075,9 +12075,9 @@ } }, "node_modules/lucide-react": { - "version": "1.22.0", - "resolved": "https://registry.npmjs.org/lucide-react/-/lucide-react-1.22.0.tgz", - "integrity": "sha512-c9o3l0PiNcgOQDW4F31BEYHudE7kgxVt3o30qMl36ZPwTxXlGB4QnLilhERvVM4uh/pl5MDyY1/gzZSYcHDtBg==", + "version": "1.23.0", + "resolved": "https://registry.npmjs.org/lucide-react/-/lucide-react-1.23.0.tgz", + "integrity": "sha512-38BpJcD0JhFosxHApP/BYsBetLpQFRoTRzEzstM/XCc3jsAG7wqaY1lgVwxiUe3xqYE+lNxo2PkCmYwXWrwwIw==", "dev": true, "license": "ISC", "peerDependencies": { @@ -13630,9 +13630,9 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "4.0.4", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", - "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", + "version": "4.0.5", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz", + "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", "engines": { @@ -15312,9 +15312,9 @@ "license": "ISC" }, "node_modules/shadcn": { - "version": "4.12.0", - "resolved": "https://registry.npmjs.org/shadcn/-/shadcn-4.12.0.tgz", - "integrity": "sha512-o781ieQziCnXH2FKsEqxp1fnbHdbgAPO9inTSPeZ59hQfsZXuMGp3ul8oFSV5KQS4nbUK9b+DrDE6C7OvfKKQQ==", + "version": "4.13.0", + "resolved": "https://registry.npmjs.org/shadcn/-/shadcn-4.13.0.tgz", + "integrity": "sha512-5fuJ4jI/GcPeA/iTL4cJivCZuYQGXz/N3bIzyd+Gd/FM6xUCy2MxGG+LaDQuw2cjNy9zGPSFPTEmI048UwPTZA==", "dev": true, "license": "MIT", "dependencies": { @@ -16454,9 +16454,9 @@ } }, "node_modules/systeminformation": { - "version": "5.31.11", - "resolved": "https://registry.npmjs.org/systeminformation/-/systeminformation-5.31.11.tgz", - "integrity": "sha512-I6O7iaUj23AXRgCPDDnvi3xHvdOLp4+1YMbF+X194lJwY1NeWojgHJPhslVKcmTtrLTguRk3QJK+xEdTiI3P0w==", + "version": "5.31.14", + "resolved": "https://registry.npmjs.org/systeminformation/-/systeminformation-5.31.14.tgz", + "integrity": "sha512-nefRpMCsAI4m71/6JHH//KPaP/d5nTuRVxEtQ7N7SlBrX18DAcC+5Z1JKZYeN9Iw49qMx95BTo/gBMk3Y2H6+g==", "dev": true, "license": "MIT", "os": [ @@ -17475,9 +17475,9 @@ } }, "node_modules/webpack": { - "version": "5.108.3", - "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.108.3.tgz", - "integrity": "sha512-hOpaCHmQVVY66IVTjofnH14IgSdmod2aquSGHGuYig/OIdWge01Hk2Wt988DZcwXumFUT4+FvJY5N+ikl8o/ww==", + "version": "5.108.4", + "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.108.4.tgz", + "integrity": "sha512-yur8LyJoeiWh47dErD+Ok7vlbmDsJ3UbbRPAoxbGJ54WpE2y5yVo5G/inUzujnYgw3tPmBRdn+G7PoxXaYC33w==", "dev": true, "license": "MIT", "dependencies": { @@ -17607,9 +17607,9 @@ } }, "node_modules/webpack-sources": { - "version": "3.5.0", - "resolved": "https://registry.npmjs.org/webpack-sources/-/webpack-sources-3.5.0.tgz", - "integrity": "sha512-HPuy+uuoTCaaoEoI1LQ3JN9+vrPBvEesnnX1jADHy728cHSMlq4wUc4afYqahq2B1mhQVZxCXOkNTnXltr+2vQ==", + "version": "3.5.1", + "resolved": "https://registry.npmjs.org/webpack-sources/-/webpack-sources-3.5.1.tgz", + "integrity": "sha512-jyuiGJdtvY434z5bUZrjz67v76/ePNvFZTp9Mdz29IlH4+GPsgyGjiv0fKI+M7BdkU6ADjulUcKAd3tUK3WlEw==", "dev": true, "license": "MIT", "engines": { @@ -17617,9 +17617,9 @@ } }, "node_modules/webpack/node_modules/enhanced-resolve": { - "version": "5.24.1", - "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.24.1.tgz", - "integrity": "sha512-7DdUaTjmNwMcH2gLr1qycesKII3BK4RLy/mdAb7x10Lq7bR4aNKHt1BR1ZALSv0rPM/hF5wYF0PhGop/rJm8vw==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.24.2.tgz", + "integrity": "sha512-rpsZEGT1jFuve6QlpyRp9ckQ+kN61hvF9BzCPyMdaKTm8UJce96KBn3sorXOFXlzjPrs3Vc4T1NsSroZ3PxlFw==", "dev": true, "license": "MIT", "dependencies": { @@ -18119,9 +18119,9 @@ } }, "node_modules/yocto-spinner": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/yocto-spinner/-/yocto-spinner-1.2.0.tgz", - "integrity": "sha512-Yw0hUB6UA3o4YUgKy3oSe9a4cxoaZ9sBfYDw+JSxo6Id0KoJGoxzPA24qqUXYKBWABs/zDSGTz9kww7t3F0XGw==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/yocto-spinner/-/yocto-spinner-1.2.1.tgz", + "integrity": "sha512-9cbFWLhbiZp+820O4pkHGNncI7+MrUGzBOjw8NMG+ewsY+aG0DdEXnr19Smxao32YOjLZRMdn1UtaxcrXOYOIg==", "dev": true, "license": "MIT", "dependencies": { From 00a9cb6c31a3e066596f369fbcb378b1a64a4b99 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 7 Jul 2026 11:17:22 -0600 Subject: [PATCH 03/24] Improve e2e helpers --- e2e-tests/fixtures/helpers.ts | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 49c7c4ba..417cc045 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -443,9 +443,11 @@ export async function openInterlinearizerFromScriptureEditor( projectName = 'WEB', ): Promise { // A Scripture Editor tab is titled by the project short name once a project is loaded (e.g. - // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. + // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. Escape projectName so + // a short name with regex metacharacters can't corrupt the pattern. + const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const editorTab = page - .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${projectName})\\b`) }) + .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${escapedProjectName})\\b`) }) .first(); const homeTab = page.locator('.dock-tab', { hasText: 'Home' }).first(); @@ -487,7 +489,6 @@ export async function openInterlinearizerFromScriptureEditor( .first(); await expect(selectProjectDialog.or(interlinearizerTab)).toBeVisible({ timeout: 15_000 }); if (await selectProjectDialog.isVisible()) { - const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); } @@ -505,7 +506,11 @@ export async function openInterlinearizerFromScriptureEditor( * @returns A `FrameLocator` scoped to the Interlinearizer WebView iframe. */ export function getInterlinearizerFrame(page: Page): FrameLocator { - return page.frameLocator('iframe[title*="Interlinearizer" i]'); + // Anchor on titles that START with "Interlinearizer" so this never matches the project-picker + // dialog ("Open Interlinearizer"), whose title also contains the word. The real WebView title is + // "Interlinearizer" (optionally suffixed with the unsaved-changes marker), so a prefix match keeps + // the dirty-state title while excluding the "Open …" picker. + return page.frameLocator('iframe[title^="Interlinearizer" i]'); } /** @@ -697,7 +702,14 @@ export async function ensureE2eProjectActive( } const selectTitle = frame.locator('#select-project-modal-title'); - const e2eEntry = dialog.getByRole('button', { name: new RegExp(E2E_PROJECT_NAME) }).first(); + // The entry button's accessible name is the project name followed by the analysis languages (and + // an "Active" badge when applicable), so an exact-string match won't work. Anchor the pattern to + // the start and require the name to be followed by whitespace or end-of-name, so it targets the + // E2E project exactly and never a different project whose label merely contains that text. + const escapedE2eName = E2E_PROJECT_NAME.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const e2eEntry = dialog + .getByRole('button', { name: new RegExp(`^${escapedE2eName}(\\s|$)`) }) + .first(); if ((await e2eEntry.count()) > 0) { await e2eEntry.click(); if (dirty) { From 36c1e5ed8319bf5e85164734a05b36c9b155567f Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 7 Jul 2026 16:04:34 -0600 Subject: [PATCH 04/24] Anchor Interlinearizer dock-tab locators in e2e helpers Exclude the "Open Interlinearizer" project-picker tab from the four broad .dock-tab matches (ensureInterlinearizerOpenOnWeb, isDraftDirty, closeInterlinearizerTab, rescueDraftToNewProject) so they target the real WebView tab. Also anchor the active-project check in ensureE2eProjectActive to match E2E_PROJECT_NAME exactly instead of via unanchored includes. Co-Authored-By: Claude Opus 4.8 (1M context) --- e2e-tests/fixtures/helpers.ts | 45 +++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 417cc045..3635679d 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -526,7 +526,11 @@ export function getInterlinearizerFrame(page: Page): FrameLocator { * timeouts. */ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise { - const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + // Match the Interlinearizer WebView tab but NOT the project-picker dialog's own dock tab ("Open + // Interlinearizer"), whose title also contains "Interlinearizer". + const interlinearizerTab = page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(); if (await interlinearizerTab.isVisible()) { await interlinearizerTab.click(); } else { @@ -597,8 +601,10 @@ const UNSAVED_TAB_MARKER = '●'; * @returns `true` when the tab title carries the unsaved-changes glyph. */ async function isDraftDirty(page: Page): Promise { + // Read from the Interlinearizer WebView tab, excluding the project-picker dialog's own dock tab + // ("Open Interlinearizer"), whose title also contains "Interlinearizer". const tabText = await page - .locator('.dock-tab', { hasText: 'Interlinearizer' }) + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) .first() .textContent(); return (tabText ?? '').includes(UNSAVED_TAB_MARKER); @@ -650,10 +656,12 @@ async function rescueDraftToNewProject(page: Page): Promise { await expect(saveAsTitle).not.toBeVisible({ timeout: 10_000 }); // The save clears the unsaved marker; wait for it so later dirty checks read the new state. - await expect(page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first()).not.toContainText( - UNSAVED_TAB_MARKER, - { timeout: 10_000 }, - ); + // Target the WebView tab, not the project-picker dialog's own "Open Interlinearizer" dock tab. + await expect( + page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(), + ).not.toContainText(UNSAVED_TAB_MARKER, { timeout: 10_000 }); } /** @@ -687,10 +695,17 @@ export async function ensureE2eProjectActive( let frame = await openSelectProjectModal(page); let dialog = frame.locator('dialog'); + // Anchor the pattern to the start and require the name to be followed by whitespace or + // end-of-name, so it targets the E2E project exactly and never a different project whose label + // merely contains that text (entry labels carry the analysis languages and an "Active" badge + // after the name). + const escapedE2eName = E2E_PROJECT_NAME.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const e2eNamePattern = new RegExp(`^${escapedE2eName}(\\s|$)`); + const activeEntry = dialog.locator('button[aria-current="true"]'); const activeIsE2e = (await activeEntry.count()) > 0 && - ((await activeEntry.first().textContent()) ?? '').includes(E2E_PROJECT_NAME); + e2eNamePattern.test((await activeEntry.first().textContent()) ?? ''); if (dirty && !activeIsE2e && rescueDirtyDraft) { await dialog.getByRole('button', { name: 'Cancel' }).click(); @@ -703,13 +718,9 @@ export async function ensureE2eProjectActive( const selectTitle = frame.locator('#select-project-modal-title'); // The entry button's accessible name is the project name followed by the analysis languages (and - // an "Active" badge when applicable), so an exact-string match won't work. Anchor the pattern to - // the start and require the name to be followed by whitespace or end-of-name, so it targets the - // E2E project exactly and never a different project whose label merely contains that text. - const escapedE2eName = E2E_PROJECT_NAME.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const e2eEntry = dialog - .getByRole('button', { name: new RegExp(`^${escapedE2eName}(\\s|$)`) }) - .first(); + // an "Active" badge when applicable), so an exact-string match won't work; reuse the anchored + // e2eNamePattern so it targets the E2E project exactly. + const e2eEntry = dialog.getByRole('button', { name: e2eNamePattern }).first(); if ((await e2eEntry.count()) > 0) { await e2eEntry.click(); if (dirty) { @@ -760,7 +771,11 @@ export async function wipeDraft(page: Page): Promise { * timeout. */ export async function closeInterlinearizerTab(page: Page): Promise { - const interlinearizerTab = page.locator('.dock-tab', { hasText: 'Interlinearizer' }).first(); + // Target the Interlinearizer WebView tab, excluding the project-picker dialog's own dock tab + // ("Open Interlinearizer"), whose title also contains "Interlinearizer". + const interlinearizerTab = page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(); await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); await interlinearizerTab.hover(); await interlinearizerTab.locator('.dock-tab-close-btn').click(); From c36beb28c4943822d2e7eba9b0fa580d3f43327d Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 7 Jul 2026 17:23:53 -0600 Subject: [PATCH 05/24] Fix e2e helper project-selection and unify draft dirty-state gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The e2e feature helpers couldn't reliably pass on the shared CDP instance: - ensureE2eProjectActive matched the project entry by the button's accessible name, but the modal renders name/badge/language spans with no separating whitespace, so the anchored regex never matched — every mutating test fell into the create branch and re-created an existing project. Match the name element by exact text instead (also fixes the "E2E Test Project 2" overmatch). - The create branch hung on a dirty draft (Create defers behind the discard confirm); dismiss it when dirty. - closeInterlinearizerTab used hover()+click(), which times out when the tab overflows off-viewport on CI; use dispatchEvent('click') like paranext-core. - ensureInterlinearizerOpenOnWeb settles the dock before its isVisible() branch. - Extract interlinearizerTabLocator/escapeRegExp helpers. The tab's unsaved marker used dirty||pendingEdits but ProjectModals gated its discard confirmation on committed dirty only, so opening/creating a project mid-typing silently discarded the uncommitted gloss. Rename the prop to hasUnsavedWork and feed it the combined state so the guard and marker agree. Document the broadened discard-on-swap behavior in user-questions.md. --- e2e-tests/fixtures/helpers.ts | 119 +++++++++++------- .../components/InterlinearizerLoader.test.tsx | 52 +++++++- .../components/modals/ProjectModals.test.tsx | 24 ++-- src/components/InterlinearizerLoader.tsx | 2 +- src/components/modals/ProjectModals.tsx | 30 ++--- user-questions.md | 11 ++ 6 files changed, 167 insertions(+), 71 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 3635679d..314ea402 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -4,6 +4,7 @@ import { ElectronApplication, expect, FrameLocator, + Locator, Page, } from '@playwright/test'; import fs from 'fs'; @@ -445,7 +446,7 @@ export async function openInterlinearizerFromScriptureEditor( // A Scripture Editor tab is titled by the project short name once a project is loaded (e.g. // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. Escape projectName so // a short name with regex metacharacters can't corrupt the pattern. - const escapedProjectName = projectName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escapedProjectName = escapeRegExp(projectName); const editorTab = page .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${escapedProjectName})\\b`) }) .first(); @@ -482,11 +483,7 @@ export async function openInterlinearizerFromScriptureEditor( // opens a floating "Open Interlinearizer" dock tab with the project list. When the editor // already has a project (a warm instance), the Interlinearizer tab opens directly instead. const selectProjectDialog = page.locator('.select-project-dialog'); - // Match the Interlinearizer WebView tab ("Interlinearizer") but NOT the project-picker dialog's - // own dock tab ("Open Interlinearizer"), whose title also contains "Interlinearizer". - const interlinearizerTab = page - .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) - .first(); + const interlinearizerTab = interlinearizerTabLocator(page); await expect(selectProjectDialog.or(interlinearizerTab)).toBeVisible({ timeout: 15_000 }); if (await selectProjectDialog.isVisible()) { const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); @@ -513,6 +510,32 @@ export function getInterlinearizerFrame(page: Page): FrameLocator { return page.frameLocator('iframe[title^="Interlinearizer" i]'); } +/** + * Locator for the Interlinearizer WebView's dock tab. Matches the tab whose title contains + * "Interlinearizer" while excluding the project-picker dialog's own dock tab ("Open + * Interlinearizer"), whose title also contains the word. Centralizes the exclusion so callers can't + * forget it (the `getInterlinearizerFrame` iframe uses a prefix match for the same purpose). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns A `Locator` for the Interlinearizer WebView dock tab. + */ +function interlinearizerTabLocator(page: Page): Locator { + return page + .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) + .first(); +} + +/** + * Escape a literal string so it can be embedded in a `RegExp` without its metacharacters being + * interpreted (e.g. a project short name containing `.` or `(`). + * + * @param literal The raw string to escape. + * @returns The string with all regex metacharacters backslash-escaped. + */ +function escapeRegExp(literal: string): string { + return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + /** * Ensure the Interlinearizer is open and focused, reusing an existing tab when one is present. * Standard precondition for feature tests running against the shared CDP instance: an existing @@ -526,11 +549,16 @@ export function getInterlinearizerFrame(page: Page): FrameLocator { * timeouts. */ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise { - // Match the Interlinearizer WebView tab but NOT the project-picker dialog's own dock tab ("Open - // Interlinearizer"), whose title also contains "Interlinearizer". - const interlinearizerTab = page - .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) - .first(); + const interlinearizerTab = interlinearizerTabLocator(page); + + // Settle the dock layout before the non-retrying isVisible() branch below. The readiness helpers + // only poll rpc.discover, not the DOM, so without this a not-yet-painted Interlinearizer tab (or + // one just closed by a prior test) would read as "absent" and send us needlessly down the full + // open-from-editor flow. Wait until either the Interlinearizer tab or some editor/Home anchor tab + // is mounted, so isVisible() reflects a settled layout. + const anchorTab = page.locator('.dock-tab', { hasText: /Scripture Editor|Home|WEB/ }).first(); + await expect(interlinearizerTab.or(anchorTab)).toBeVisible({ timeout: 30_000 }); + if (await interlinearizerTab.isVisible()) { await interlinearizerTab.click(); } else { @@ -588,8 +616,10 @@ export const E2E_PROJECT_NAME = 'E2E Test Project'; const RESCUE_PROJECT_PREFIX = 'e2e-rescued-work'; /** - * Glyph the Interlinearizer appends to its dock tab title while the draft has unsaved changes. Keep - * in sync with UNSAVED_TAB_MARKER in src/components/InterlinearizerLoader.tsx. + * Glyph the Interlinearizer appends to its dock tab title while the draft has unsaved changes. Only + * the glyph must match production's UNSAVED_TAB_MARKER in src/components/InterlinearizerLoader.tsx + * (which prefixes it with a space); this constant is used exclusively in substring checks, so the + * surrounding whitespace is deliberately not replicated. */ const UNSAVED_TAB_MARKER = '●'; @@ -601,12 +631,7 @@ const UNSAVED_TAB_MARKER = '●'; * @returns `true` when the tab title carries the unsaved-changes glyph. */ async function isDraftDirty(page: Page): Promise { - // Read from the Interlinearizer WebView tab, excluding the project-picker dialog's own dock tab - // ("Open Interlinearizer"), whose title also contains "Interlinearizer". - const tabText = await page - .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) - .first() - .textContent(); + const tabText = await interlinearizerTabLocator(page).textContent(); return (tabText ?? '').includes(UNSAVED_TAB_MARKER); } @@ -656,12 +681,9 @@ async function rescueDraftToNewProject(page: Page): Promise { await expect(saveAsTitle).not.toBeVisible({ timeout: 10_000 }); // The save clears the unsaved marker; wait for it so later dirty checks read the new state. - // Target the WebView tab, not the project-picker dialog's own "Open Interlinearizer" dock tab. - await expect( - page - .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) - .first(), - ).not.toContainText(UNSAVED_TAB_MARKER, { timeout: 10_000 }); + await expect(interlinearizerTabLocator(page)).not.toContainText(UNSAVED_TAB_MARKER, { + timeout: 10_000, + }); } /** @@ -695,17 +717,17 @@ export async function ensureE2eProjectActive( let frame = await openSelectProjectModal(page); let dialog = frame.locator('dialog'); - // Anchor the pattern to the start and require the name to be followed by whitespace or - // end-of-name, so it targets the E2E project exactly and never a different project whose label - // merely contains that text (entry labels carry the analysis languages and an "Active" badge - // after the name). - const escapedE2eName = E2E_PROJECT_NAME.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const e2eNamePattern = new RegExp(`^${escapedE2eName}(\\s|$)`); - + // Locate the E2E entry by its project-name element with an EXACT text match, then walk up to the + // enclosing entry button. Matching the whole button's accessible name doesn't work: the modal + // renders the name, an optional "Active" badge, and the analysis languages as adjacent inline + // s with no separating whitespace, so the accessible name reads "E2E Test Projecten" — + // there is no space after the name to anchor on. An exact-text match on the name element also + // avoids matching a different project whose name merely starts with E2E_PROJECT_NAME (e.g. "E2E + // Test Project 2"). Keep in sync with SelectInterlinearProjectModal's entry markup. const activeEntry = dialog.locator('button[aria-current="true"]'); const activeIsE2e = (await activeEntry.count()) > 0 && - e2eNamePattern.test((await activeEntry.first().textContent()) ?? ''); + (await activeEntry.first().getByText(E2E_PROJECT_NAME, { exact: true }).count()) > 0; if (dirty && !activeIsE2e && rescueDirtyDraft) { await dialog.getByRole('button', { name: 'Cancel' }).click(); @@ -717,10 +739,10 @@ export async function ensureE2eProjectActive( } const selectTitle = frame.locator('#select-project-modal-title'); - // The entry button's accessible name is the project name followed by the analysis languages (and - // an "Active" badge when applicable), so an exact-string match won't work; reuse the anchored - // e2eNamePattern so it targets the E2E project exactly. - const e2eEntry = dialog.getByRole('button', { name: e2eNamePattern }).first(); + // Rebuilt against the (possibly re-opened) dialog so it targets the current modal instance. + const e2eEntry = dialog + .locator('button', { has: frame.getByText(E2E_PROJECT_NAME, { exact: true }) }) + .first(); if ((await e2eEntry.count()) > 0) { await e2eEntry.click(); if (dirty) { @@ -735,6 +757,13 @@ export async function ensureE2eProjectActive( await expect(createTitle).toBeVisible({ timeout: 5_000 }); await frame.locator('#project-name').fill(E2E_PROJECT_NAME); await frame.locator('dialog').getByRole('button', { name: 'Create' }).click(); + // Creating a draft over a dirty one defers behind the discard confirmation instead of closing + // the create modal (handleCreateDraft in ProjectModals.tsx), so dismiss it when dirty. + if (dirty) { + const discardConfirm = frame.getByTestId('discard-draft-confirm'); + await expect(discardConfirm).toBeVisible({ timeout: 5_000 }); + await discardConfirm.click(); + } await expect(createTitle).not.toBeVisible({ timeout: 10_000 }); } await expect(selectTitle).not.toBeVisible({ timeout: 10_000 }); @@ -767,17 +796,15 @@ export async function wipeDraft(page: Page): Promise { * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @returns Resolves when the Interlinearizer tab is gone. - * @throws If the tab or its close button is not visible, or the tab does not close within the - * timeout. + * @throws If the tab is not visible, or the tab does not close within the timeout. */ export async function closeInterlinearizerTab(page: Page): Promise { - // Target the Interlinearizer WebView tab, excluding the project-picker dialog's own dock tab - // ("Open Interlinearizer"), whose title also contains "Interlinearizer". - const interlinearizerTab = page - .locator('.dock-tab', { hasText: 'Interlinearizer', hasNotText: 'Open Interlinearizer' }) - .first(); + const interlinearizerTab = interlinearizerTabLocator(page); await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); - await interlinearizerTab.hover(); - await interlinearizerTab.locator('.dock-tab-close-btn').click(); + // Dispatch the click rather than hover()+click(): the close button is only laid out on hover, and + // on small CI viewports the tab can overflow the tab strip and sit outside the viewport, where a + // real click (even with force) fails. dispatchEvent doesn't require the element to be in-viewport. + // Mirrors paranext-core's own dock-tab close helpers. + await interlinearizerTab.locator('.dock-tab-close-btn').dispatchEvent('click'); await expect(interlinearizerTab).not.toBeVisible({ timeout: 10_000 }); } diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index d0fd81be..1c158e7f 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -206,14 +206,18 @@ jest.mock('../../components/modals/ProjectModals', () => ({ /** * Minimal ProjectModals stand-in that drives modal state and active-project state through the * same `useWebViewState` hook the real component uses, so tests can assert on state transitions - * without mounting the full modal tree. Accepts (and ignores) the draft-related props the loader - * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`). + * without mounting the full modal tree. Accepts (and mostly ignores) the draft-related props the + * loader now passes (`hasUnsavedWork`, `getDraftSnapshot`, `loadFromProject`, `markSynced`); + * `hasUnsavedWork` is surfaced as a `data-*` attribute so tests can assert the loader feeds it + * the combined committed-and-pending unsaved state. * * @param modal - Current modal identifier controlling which stub panel is rendered. * @param setModal - Callback to transition to a different modal state. * @param activeProject - The currently active interlinear project, or undefined when none is * selected. * @param defaultAnalysisLanguage - BCP 47 tag forwarded as the create modal's default language. + * @param hasUnsavedWork - Whether the draft has committed-but-unsaved changes or uncommitted + * in-progress typing; gates the discard confirmation in the real component. * @param useWebViewState - Injected hook used to read and write persisted WebView state; must * support the `'activeProject'` key. * @returns A JSX element containing the stub modal panels keyed by `modal`. @@ -223,13 +227,14 @@ jest.mock('../../components/modals/ProjectModals', () => ({ setModal, activeProject, defaultAnalysisLanguage, + hasUnsavedWork, useWebViewState, }: { modal: string; setModal: (m: string) => void; activeProject: MockProject | undefined; defaultAnalysisLanguage?: string; - dirty: boolean; + hasUnsavedWork: boolean; getDraftSnapshot: () => DraftProject | undefined; loadFromProject: (project: unknown) => void; markSynced: () => void; @@ -245,6 +250,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ data-testid="project-modals" data-modal={modal} data-default-lang={defaultAnalysisLanguage} + data-has-unsaved-work={hasUnsavedWork} data-active-project-name={activeProject?.name} > {modal === 'select' && ( @@ -1200,6 +1206,46 @@ describe('InterlinearizerLoader', () => { expect(updateWebViewDefinition).toHaveBeenCalledWith({ title: 'Interlinearizer' }); }); + it('reports in-progress typing to ProjectModals as unsaved work so a swap is guarded', async () => { + await act(async () => { + renderLoader(); + }); + + // The persisted draft is clean, so the modal starts with no unsaved work to guard. + expect(screen.getByTestId('project-modals')).toHaveAttribute( + 'data-has-unsaved-work', + 'false', + ); + + // A gloss input begins holding uncommitted text. Even though nothing has committed (the draft + // stays clean), ProjectModals must now treat the draft as having unsaved work so opening or + // creating a project prompts before discarding the in-progress gloss. + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(true); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-has-unsaved-work', 'true'); + }); + + it('drops the unsaved-work guard once in-progress typing is abandoned', async () => { + await act(async () => { + renderLoader(); + }); + + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(true); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute('data-has-unsaved-work', 'true'); + + // The edit is reverted or the input unmounts with nothing committed: the guard clears. + act(() => { + capturedInterlinearizerProps?.onPendingEditsChange?.(false); + }); + expect(screen.getByTestId('project-modals')).toHaveAttribute( + 'data-has-unsaved-work', + 'false', + ); + }); + it('logs an error when the saveAnalysis command rejects during Save', async () => { await act(async () => renderLoader({ useWebViewState: makeWebViewState({ activeProject: STUB_ACTIVE_PROJECT }) }), diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index 2d8e60b0..e28df9e3 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -243,7 +243,7 @@ jest.mock('../../../components/modals/ProjectMetadataModal', () => ({ type ModalsOverrides = Partial<{ activeProject: InterlinearProjectSummary | undefined; defaultAnalysisLanguage: string; - dirty: boolean; + hasUnsavedWork: boolean; getDraftSnapshot: () => DraftProject | undefined; loadFromProject: jest.Mock; newDraft: jest.Mock; @@ -264,7 +264,7 @@ function buildProps(overrides: ModalsOverrides = {}) { return { activeProject: overrides.activeProject, defaultAnalysisLanguage: overrides.defaultAnalysisLanguage, - dirty: overrides.dirty ?? false, + hasUnsavedWork: overrides.hasUnsavedWork ?? false, getDraftSnapshot: overrides.getDraftSnapshot ?? (() => MOCK_DRAFT), loadFromProject: overrides.loadFromProject ?? jest.fn(), newDraft: overrides.newDraft ?? jest.fn(), @@ -584,7 +584,11 @@ describe('ProjectModals', () => { .mocked(papi.commands.sendCommand) .mockResolvedValueOnce(JSON.stringify(MOCK_FULL_PROJECT)); const loadFromProject = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('select-select')); // The discard confirm overlays the still-mounted select modal (so confirming Open does not @@ -599,7 +603,11 @@ describe('ProjectModals', () => { it('cancels the discard confirm and returns to the select modal', async () => { const loadFromProject = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('select-select')); await userEvent.click(screen.getByTestId('discard-cancel')); @@ -611,7 +619,9 @@ describe('ProjectModals', () => { it('confirms before creating a project when the draft is dirty', async () => { const newDraft = jest.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByTestId('create-submit')); expect(screen.getByTestId('discard-modal')).toBeInTheDocument(); @@ -652,7 +662,7 @@ describe('ProjectModals', () => { { resolveGet = resolve; }), ); - render(); + render(); await userEvent.click(screen.getByTestId('select-select')); expect(screen.getByTestId('discard-confirm')).toBeEnabled(); diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index a1b78792..5470a9e7 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -521,7 +521,7 @@ function InterlinearizerLoaderInner({ DraftProject | undefined; loadFromProject: (project: OpenableProject) => void; newDraft: (config: NewDraftConfig) => void; @@ -215,10 +217,10 @@ export default function ProjectModals({ * persisted immediately so it shows up in "Select Interlinear Project" right away. * * `newDraft` is called synchronously before the backend round-trip so the editor is ready - * immediately regardless of whether persistence succeeds. This is safe: when dirty is `false` (no - * discard confirmation shown), any data in the draft is either already committed to the active - * project or the draft was empty — nothing is lost. When dirty is `true` the - * {@link DiscardDraftConfirm} dialog has already obtained explicit user consent to discard. + * immediately regardless of whether persistence succeeds. This is safe: when `hasUnsavedWork` is + * `false` (no discard confirmation shown), any data in the draft is either already committed to + * the active project or the draft was empty — nothing is lost. When `hasUnsavedWork` is `true` + * the {@link DiscardDraftConfirm} dialog has already obtained explicit user consent to discard. * * The `interlinearizer.createProject` command sends its own error notification before rethrowing, * so the catch block only needs to log — callers do not need to send a second notification. This @@ -269,16 +271,16 @@ export default function ProjectModals({ /** * Called when the user selects a project in the select modal. Opens it immediately, or defers - * behind the unsaved-changes confirmation when the draft is dirty. + * behind the unsaved-changes confirmation when the draft has unsaved work. * * @param project - The project the user selected. */ const handleSelectProject = useCallback( (project: InterlinearProjectSummary) => { - if (dirty) setPendingReplace({ kind: 'open', project }); + if (hasUnsavedWork) setPendingReplace({ kind: 'open', project }); else openProject(project); }, - [dirty, openProject], + [hasUnsavedWork, openProject], ); /** @@ -306,19 +308,19 @@ export default function ProjectModals({ /** * Called when the New dialog is submitted. Creates and persists the project immediately, or - * defers behind the unsaved-changes confirmation when the draft is dirty. + * defers behind the unsaved-changes confirmation when the draft has unsaved work. * * @param config - The configuration collected by the New dialog. */ const handleCreateDraft = useCallback( async (config: CreateDraftConfig) => { - if (dirty) { + if (hasUnsavedWork) { setPendingReplace({ kind: 'new', config }); return; } await createDraftAndClose(config); }, - [createDraftAndClose, dirty], + [createDraftAndClose, hasUnsavedWork], ); /** diff --git a/user-questions.md b/user-questions.md index 0b9ee51d..46b34e55 100644 --- a/user-questions.md +++ b/user-questions.md @@ -87,6 +87,17 @@ Decisions made during development that we'd like reviewed: two separate menu items (each a single click, no scope step). Current choice: one menu item plus a scope-picker dialog. +11. **In-progress typing guards a project swap.** Switching projects (New / Open) shows the discard + confirm (item 2) whenever the draft has unsaved changes. This now also fires when a gloss field + holds **uncommitted** text — the same eager signal that lights the `●` marker (item 7) — not only + after that text has committed on blur. Previously the guard checked committed changes only, so + opening or creating a project while mid-typing a gloss discarded that in-progress text silently. + The two definitions of "unsaved" (the tab marker vs. the discard guard) are now unified, so the + prompt and the indicator always agree. Is prompting on uncommitted typing the right behavior, or + should a project swap only guard against changes that have actually committed (accepting that + mid-typing text is then lost without warning)? Note this is coupled to item 7: if the marker is + changed to wait until an edit commits, this guard should follow. + ## Suggestion engine: editing a shared analysis, and per-instance analyses The suggestion engine reuses an existing analysis on other matching surface forms by creating a new From f26c659e99898c4e8f0453296dce18b5c4dfbcfb Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 9 Jul 2026 16:01:07 -0600 Subject: [PATCH 06/24] Make CDP feature tier self-launch; fix strict-mode tab locators globalSetup now launches and tears down its own Platform.Bible instance (reusing a warm start:cdp when the CDP port is in use), and .or() tab waits get .first() so a docked Interlinearizer + WEB tab no longer trip Playwright strict mode. --- .github/workflows/test.yml | 14 +++- .gitignore | 5 ++ e2e-tests/README.md | 10 ++- e2e-tests/fixtures/cdp.fixture.ts | 8 +- e2e-tests/fixtures/helpers.ts | 15 +++- e2e-tests/global-setup-cdp.ts | 123 +++++++++++++++++++++++++++++ e2e-tests/global-setup.ts | 45 ++++++----- e2e-tests/global-teardown-cdp.ts | 72 +++++++++++++++++ e2e-tests/playwright-cdp.config.ts | 23 +++--- e2e-tests/playwright.config.ts | 7 +- package.json | 2 +- 11 files changed, 282 insertions(+), 42 deletions(-) create mode 100644 e2e-tests/global-setup-cdp.ts create mode 100644 e2e-tests/global-teardown-cdp.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 435d9058..e6f65682 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -52,9 +52,11 @@ jobs: working-directory: extension-repo run: npm run test:coverage - e2e-smoke: + e2e: runs-on: ${{ matrix.os }} strategy: + # Don't cancel the other OS's e2e run when one fails — we want both reports. + fail-fast: false matrix: os: [ubuntu-latest, windows-latest] steps: @@ -112,11 +114,17 @@ jobs: working-directory: paranext-core run: npm run build:dll - - name: Run e2e smoke tests (Linux) + # Linux runs the FULL suite: smoke plus the CDP feature tests. `test:e2e:cdp` self-launches + # its own Platform.Bible instance (globalSetup, --remote-debugging-port=9223), so the whole + # thing runs under one xvfb display with no separate app-start step. + - name: Run e2e tests (Linux) if: matrix.os == 'ubuntu-latest' working-directory: extension-repo - run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e:smoke + run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e + # Windows runs smoke only. The CDP self-launch spawns a detached Electron and kills it by + # POSIX process group (process.kill(-pid, ...)), which has no Windows equivalent, so the CDP + # tier is Linux-only until that teardown path is made Windows-safe. - name: Run e2e smoke tests (Windows) if: matrix.os == 'windows-latest' working-directory: extension-repo diff --git a/.gitignore b/.gitignore index a1dbba9e..9529ad4e 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,8 @@ temp-build # Playwright test output e2e-tests/playwright-report e2e-tests/test-results + +# Playwright runtime process markers (dev server / self-launched CDP app) +e2e-tests/.dev-server.pid +e2e-tests/.cdp-app.pid +e2e-tests/.cdp-app.user-data-dir diff --git a/e2e-tests/README.md b/e2e-tests/README.md index bb55ca6a..289a49ad 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -3,7 +3,13 @@ End-to-end tests for the interlinearizer extension using Playwright + Electron. The suite has two tiers: - **Smoke tests** (`tests/smoke/`, `app.fixture`) launch a fresh Platform.Bible instance with the extension loaded via `--extensions` and verify the extension starts up correctly. -- **Feature tests** (`tests/features/`, `cdp.fixture`) attach to an already-running `npm run start:cdp` instance and exercise interlinearizer UI flows (glossing, draft persistence, project modals). +- **Feature tests** (`tests/features/`, `cdp.fixture`) connect over CDP to a running Platform.Bible instance and exercise interlinearizer UI flows (glossing, draft persistence, project modals). + +Run everything with `npm run test:e2e` (smoke tier then CDP tier). Each tier can be run alone with `npm run test:e2e:smoke` and `npm run test:e2e:cdp`. + +Both tiers are self-launching: the CDP tier's `globalSetup` launches its own Platform.Bible instance (with `--remote-debugging-port=9223`) in an isolated user-data dir and tears it down afterward, so `npm run test:e2e:cdp` needs no manual `npm run start:cdp` first. To iterate against a warm instance instead, run `npm run start:cdp` in one terminal, then run the CDP config directly with `npx playwright test --config e2e-tests/playwright-cdp.config.ts`: the setup detects the in-use CDP port, reuses that instance, and leaves it running. + +In CI (`.github/workflows/test.yml`, `e2e` job) the full suite runs on Linux under `xvfb` via `npm run test:e2e`; Windows runs the smoke tier only, because the CDP self-launch tears its app down by POSIX process group, which has no Windows equivalent. Each tier writes its Playwright HTML report to its own subfolder (`playwright-report/smoke`, `playwright-report/cdp`) so a combined run keeps both. **Contents:** @@ -23,7 +29,7 @@ These tests are adapted from `paranext-core`'s e2e suite with changes to support ## Writing feature tests -Feature tests run with `npm run test:e2e:cdp` against a shared, long-lived `npm run start:cdp` instance, so they must assume nothing about its state and must leave nothing behind that could poison the next run. The protocol: +Feature tests run with `npm run test:e2e:cdp`. That command launches a fresh, isolated instance, but the tests are also run against a shared, long-lived `npm run start:cdp` instance during local iteration (see above), so they must assume nothing about the instance's state and must leave nothing behind that could poison the next run. The protocol: - **Import from `cdp.fixture`, never `app.fixture`.** The CDP config already serializes execution (`workers: 1`), so tests never race each other on the shared instance. - **The instance is only ever used with the WEB project.** This is an operating assumption, not something tests verify: `ensureInterlinearizerOpenOnWeb()` trusts an existing Interlinearizer tab and only picks WEB when opening fresh. Don't point `start:cdp` at other projects. diff --git a/e2e-tests/fixtures/cdp.fixture.ts b/e2e-tests/fixtures/cdp.fixture.ts index 381306a9..b5d5e921 100644 --- a/e2e-tests/fixtures/cdp.fixture.ts +++ b/e2e-tests/fixtures/cdp.fixture.ts @@ -7,10 +7,12 @@ * * - No app restart needed (no port 8876 conflict) * - Tests run against the same app instance used during development - * - No teardown/shutdown of the app on completion + * - No teardown/shutdown of the app by the fixture on completion * - * Prerequisite: Platform.Bible running with --remote-debugging-port=9223 and the interlinearizer - * extension loaded. + * The app is provided by the CDP config's `globalSetup` (which self-launches one for `npm run + * test:e2e:cdp`) or by a developer's own `npm run start:cdp`; either way it must be running with + * `--remote-debugging-port=9223` and the interlinearizer extension loaded by the time this fixture + * connects. */ import { test as base, chromium, Page } from '@playwright/test'; diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 314ea402..f1a8f5fe 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -454,7 +454,9 @@ export async function openInterlinearizerFromScriptureEditor( // Wait for the dock layout to actually mount before deciding which path to take — a fresh profile // briefly reports zero tabs, and a non-waiting `count()` would misread that as "no editor". - await expect(editorTab.or(homeTab)).toBeVisible({ timeout: 30_000 }); + // `.first()` on the whole `.or()`: when both the editor and Home tabs are present the union + // resolves to two elements, which would trip strict mode on this visibility assertion. + await expect(editorTab.or(homeTab).first()).toBeVisible({ timeout: 30_000 }); // If the layout came up without a Scripture Editor (single-tab Home layout), open the project // from Home so the editor (and its ≡ menu) exists before we try to focus it. @@ -484,7 +486,9 @@ export async function openInterlinearizerFromScriptureEditor( // already has a project (a warm instance), the Interlinearizer tab opens directly instead. const selectProjectDialog = page.locator('.select-project-dialog'); const interlinearizerTab = interlinearizerTabLocator(page); - await expect(selectProjectDialog.or(interlinearizerTab)).toBeVisible({ timeout: 15_000 }); + // `.first()` on the whole `.or()`: if the dialog and the tab are ever both present the union + // resolves to two elements, which would trip strict mode on this visibility assertion. + await expect(selectProjectDialog.or(interlinearizerTab).first()).toBeVisible({ timeout: 15_000 }); if (await selectProjectDialog.isVisible()) { const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); await selectProjectDialog.getByRole('button', { name: projectNameRegex }).click(); @@ -555,9 +559,12 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise // only poll rpc.discover, not the DOM, so without this a not-yet-painted Interlinearizer tab (or // one just closed by a prior test) would read as "absent" and send us needlessly down the full // open-from-editor flow. Wait until either the Interlinearizer tab or some editor/Home anchor tab - // is mounted, so isVisible() reflects a settled layout. + // is mounted, so isVisible() reflects a settled layout. When BOTH are present (the common case: + // an Interlinearizer tab alongside the WEB/editor tab), the union resolves to two elements, so + // `.first()` on the whole `.or()` keeps the visibility assertion out of strict-mode violation — + // per-operand `.first()` does not collapse the union to a single match. const anchorTab = page.locator('.dock-tab', { hasText: /Scripture Editor|Home|WEB/ }).first(); - await expect(interlinearizerTab.or(anchorTab)).toBeVisible({ timeout: 30_000 }); + await expect(interlinearizerTab.or(anchorTab).first()).toBeVisible({ timeout: 30_000 }); if (await interlinearizerTab.isVisible()) { await interlinearizerTab.click(); diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts new file mode 100644 index 00000000..95d7e7c7 --- /dev/null +++ b/e2e-tests/global-setup-cdp.ts @@ -0,0 +1,123 @@ +// Self-launching global setup for the CDP (feature-test) config. +import type { FullConfig } from '@playwright/test'; +import { spawn } from 'child_process'; +import { createRequire } from 'module'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + bootstrapRendererDevServer, + isPortInUse, + waitForPort, + WEBSOCKET_PORT, +} from './global-setup'; + +/** + * Chromium remote-debugging port the self-launched Electron instance exposes and the CDP fixture + * connects to. Kept in sync with the `CDP_URL` default in fixtures/cdp.fixture.ts and the + * `--remote-debugging-port` in the `start:cdp` npm script. + */ +export const CDP_PORT = 9223; + +/** File the launched Electron PID is written to, for {@link globalTeardownCdp} to kill it. */ +export const CDP_PID_FILE = path.join(__dirname, '.cdp-app.pid'); + +/** File the launched Electron's isolated user-data dir is written to, for teardown to remove it. */ +export const CDP_USER_DATA_FILE = path.join(__dirname, '.cdp-app.user-data-dir'); + +/** How long to wait for the launched app's WebSocket / CDP port before failing setup. */ +const APP_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; + +/** + * Playwright global setup for the CDP config. Unlike the smoke config — whose fixture launches + * Electron per worker — the CDP fixture connects over CDP to a separately-running app. This setup + * provides that app so `npm run test:e2e:cdp` is self-contained (no manual `npm run start:cdp`): + * + * 1. Bootstraps the renderer dev server via {@link bootstrapRendererDevServer}. + * 2. Launches Electron (paranext-core) detached, with the interlinearizer extension loaded via + * `--extensions` and Chromium remote debugging on {@link CDP_PORT}, in an isolated user-data + * dir. + * 3. Waits for the PAPI WebSocket and the CDP debug port to come up. + * 4. Records the PID and user-data dir for {@link globalTeardownCdp}. + * + * The isolated user-data dir means the run never touches a developer's real profile; the feature + * tests self-establish every precondition (they create the E2E project, navigate, and wipe the + * draft at the start of each test), so a fresh profile is sufficient. + * + * If the CDP port is already in use, a warm instance is assumed (a developer's own `npm run + * start:cdp`) and setup is a no-op: no app is launched and nothing is recorded for teardown, so the + * developer's instance is reused and left running. This keeps the manual + * iterate-against-a-warm-instance workflow working through the same config. + * + * @param _config Playwright config object — unused; required by Playwright's global-setup + * interface. + * @returns Resolves once a usable app is available (launched here, or an already-running one). + * @throws {Error} If the app's WebSocket or CDP port do not become ready in time. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export default async function globalSetupCdp(_config: FullConfig): Promise { + // A warm instance already owns the CDP port (a developer's `npm run start:cdp`). Reuse it: don't + // launch a second app (it would collide on the WebSocket singleton and exit) and don't record a + // PID, so teardown leaves the developer's instance running. + if (await isPortInUse(CDP_PORT)) { + console.log( + `CDP port ${CDP_PORT} already in use — reusing the already-running Platform.Bible instance ` + + '(not launching or tearing down an app).', + ); + return; + } + + await bootstrapRendererDevServer(); + + const coreDir = path.resolve(__dirname, '../../paranext-core'); + const extensionDist = path.resolve(__dirname, '../dist'); + + // Resolve the Electron binary from paranext-core's node_modules (its `electron` package's default + // export is the path to the platform binary). + const coreRequire = createRequire(path.resolve(coreDir, 'package.json')); + // eslint-disable-next-line no-type-assertion/no-type-assertion + const electronExecutable = coreRequire('electron') as string; + + // Isolated user-data dir so the singleton lock can't collide with a developer's own instance and + // the run leaves the real profile untouched. + const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'paranext-e2e-cdp-')); + fs.writeFileSync(CDP_USER_DATA_FILE, userDataDir); + + // VSCode/Claude Code set ELECTRON_RUN_AS_NODE=1, which forces the Electron binary to run as plain + // Node.js. Omit it so the Electron child launches a real GUI process. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { ELECTRON_RUN_AS_NODE, ...restEnv } = process.env; + + console.log(`Launching Platform.Bible (CDP) from: ${coreDir}`); + console.log(`Loading extension from: ${extensionDist}`); + console.log(`Remote debugging on port ${CDP_PORT}`); + + // Detached: unlike the smoke fixture's Playwright-owned `_electron.launch()`, the CDP fixture + // connects to this process over CDP, so Playwright must not own its lifecycle. Teardown kills the + // whole process group by the recorded PID. + const appProcess = spawn( + electronExecutable, + [ + `--user-data-dir=${userDataDir}`, + coreDir, + '--extensions', + extensionDist, + `--remote-debugging-port=${CDP_PORT}`, + ], + { + cwd: coreDir, + env: { ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false' }, + stdio: 'ignore', + detached: true, + }, + ); + appProcess.unref(); + + if (appProcess.pid) fs.writeFileSync(CDP_PID_FILE, String(appProcess.pid)); + + console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); + await waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT); + console.log(`Waiting for CDP debug port ${CDP_PORT}...`); + await waitForPort(CDP_PORT, APP_READY_TIMEOUT); + console.log('Platform.Bible (CDP) is ready.'); +} diff --git a/e2e-tests/global-setup.ts b/e2e-tests/global-setup.ts index 327a1bc7..c40ba3d7 100644 --- a/e2e-tests/global-setup.ts +++ b/e2e-tests/global-setup.ts @@ -6,8 +6,8 @@ import net from 'net'; import path from 'path'; import fs from 'fs'; -const WEBSOCKET_PORT = 8876; -const RENDERER_PORT = 1212; +export const WEBSOCKET_PORT = 8876; +export const RENDERER_PORT = 1212; /** * Check if a port is already in use. @@ -15,7 +15,7 @@ const RENDERER_PORT = 1212; * @param port Port number to probe. * @returns Resolves to `true` if the port is occupied, `false` if it is free. */ -function isPortInUse(port: number): Promise { +export function isPortInUse(port: number): Promise { return new Promise((resolve) => { const server = net.createServer(); server.once('error', () => { @@ -112,7 +112,7 @@ function waitForHttpOk(url: string, timeout: number): Promise { * @returns Resolves when a TCP connection to the port succeeds. * @throws {Error} If the port does not become available within `timeout` milliseconds. */ -function waitForPort(port: number, timeout: number): Promise { +export function waitForPort(port: number, timeout: number): Promise { const startTime = Date.now(); return new Promise((resolve, reject) => { /** Attempt one TCP connection; retries after 500 ms on failure within the overall timeout. */ @@ -136,24 +136,17 @@ function waitForPort(port: number, timeout: number): Promise { } /** - * Playwright global setup. Runs once before any test worker starts. + * Bootstrap everything an Electron launch needs, short of launching Electron itself: verify no + * conflicting instance is running, clear stale singleton locks, confirm the extension is built, + * ensure the paranext-core dev main bundle exists, and start the renderer dev server on port 1212 + * (recording its PID for teardown). Shared by both the smoke {@link globalSetup} (whose fixture then + * launches Electron) and the CDP setup (which launches Electron itself with remote debugging). * - * 1. Fails fast if port 8876 is already in use (a running Platform.Bible would conflict with the - * Electron instance launched by fixtures). - * 2. Removes stale Electron singleton lock files left behind by crashes. - * 3. Fails fast if the extension dist is missing (directs the developer to run `npm run build`). - * 4. Ensures the paranext-core dev main bundle exists, building it via `npm run prestart` if not. - * 5. Starts the paranext-core webpack renderer dev server on port 1212 if not already running, and - * stores its PID for {@link globalTeardown} to stop it. - * - * @param _config Playwright config object — unused; required by Playwright's global-setup - * interface. * @returns Resolves when the renderer dev server is ready. - * @throws {Error} If port 8876 is already in use. + * @throws {Error} If port 8876 is already in use (a running Platform.Bible would conflict). * @throws {Error} If the extension dist is missing. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export default async function globalSetup(_config: FullConfig): Promise { +export async function bootstrapRendererDevServer(): Promise { const extensionRoot = path.resolve(__dirname, '..'); const coreDir = path.resolve(__dirname, '../../paranext-core'); @@ -248,3 +241,19 @@ export default async function globalSetup(_config: FullConfig): Promise { console.log('Renderer dev server is ready.'); } } + +/** + * Playwright global setup for the smoke config. Runs once before any test worker starts. Bootstraps + * the renderer dev server via {@link bootstrapRendererDevServer}; the smoke fixture + * (`app.fixture.ts`) then launches its own Electron instance per worker. + * + * @param _config Playwright config object — unused; required by Playwright's global-setup + * interface. + * @returns Resolves when the renderer dev server is ready. + * @throws {Error} If port 8876 is already in use. + * @throws {Error} If the extension dist is missing. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export default async function globalSetup(_config: FullConfig): Promise { + await bootstrapRendererDevServer(); +} diff --git a/e2e-tests/global-teardown-cdp.ts b/e2e-tests/global-teardown-cdp.ts new file mode 100644 index 00000000..16224d0a --- /dev/null +++ b/e2e-tests/global-teardown-cdp.ts @@ -0,0 +1,72 @@ +// Teardown for the self-launching CDP (feature-test) config. +import type { FullConfig } from '@playwright/test'; +import fs from 'fs'; +import { CDP_PID_FILE, CDP_USER_DATA_FILE } from './global-setup-cdp'; +import globalTeardown from './global-teardown'; + +/** + * Playwright global teardown for the CDP config. Kills the Electron instance launched by + * {@link globalSetupCdp} (by the PID recorded in {@link CDP_PID_FILE}), removes its isolated + * user-data dir, then delegates to the shared {@link globalTeardown} to stop the renderer dev server + * and sweep any lingering core processes. + * + * @param config Playwright config object — forwarded to the shared teardown. + * @returns Resolves when the launched app is killed, its user-data dir removed, and shared teardown + * has completed. + */ +export default async function globalTeardownCdp(config: FullConfig): Promise { + // Kill the app we launched (whole process group) before the shared teardown's generic sweep. + // SIGKILL (not SIGTERM) to match the smoke teardown: Electron can ignore SIGTERM, and we need it + // fully dead before removing its user-data dir below. + let appKilled = false; + if (fs.existsSync(CDP_PID_FILE)) { + const pid = parseInt(fs.readFileSync(CDP_PID_FILE, 'utf-8').trim(), 10); + if (Number.isNaN(pid)) { + console.warn(`Invalid PID in ${CDP_PID_FILE}, skipping app kill`); + } else { + console.log(`Stopping self-launched Platform.Bible (CDP) app (PID: ${pid})...`); + try { + process.kill(-pid, 'SIGKILL'); + appKilled = true; + } catch { + try { + process.kill(pid, 'SIGKILL'); + appKilled = true; + } catch { + // Already stopped + } + } + } + fs.unlinkSync(CDP_PID_FILE); + } + + // Remove the isolated user-data dir created for this run. Give the just-killed Electron a moment + // to release the SingletonLock and flush files before removing, then retry once — the smoke + // teardown removes its dir the same defensive way. + if (fs.existsSync(CDP_USER_DATA_FILE)) { + const userDataDir = fs.readFileSync(CDP_USER_DATA_FILE, 'utf-8').trim(); + if (userDataDir) { + if (appKilled) { + await new Promise((resolve) => { + setTimeout(resolve, 1_000); + }); + } + try { + fs.rmSync(userDataDir, { recursive: true, force: true }); + } catch { + await new Promise((resolve) => { + setTimeout(resolve, 3_000); + }); + try { + fs.rmSync(userDataDir, { recursive: true, force: true }); + } catch (e) { + console.warn(`Could not remove CDP user-data dir ${userDataDir}: ${e}`); + } + } + } + fs.unlinkSync(CDP_USER_DATA_FILE); + } + + // Delegate to the shared teardown to stop the renderer dev server and sweep lingering processes. + await globalTeardown(config); +} diff --git a/e2e-tests/playwright-cdp.config.ts b/e2e-tests/playwright-cdp.config.ts index 0e1b8d52..7d373006 100644 --- a/e2e-tests/playwright-cdp.config.ts +++ b/e2e-tests/playwright-cdp.config.ts @@ -2,20 +2,24 @@ import { defineConfig } from '@playwright/test'; /** - * Playwright configuration for running E2E tests against an already-running Platform.Bible instance - * with CDP enabled (port 9223). + * Playwright configuration for the CDP (feature) test tier. These tests connect over CDP (port + * 9223) to a running Platform.Bible instance with the interlinearizer extension loaded. * - * Prerequisites: Platform.Bible running with --remote-debugging-port=9223 and the interlinearizer - * extension loaded. - * - * Use: npx playwright test --config=e2e-tests/playwright-cdp.config.ts + * `globalSetup` launches that instance automatically (with `--remote-debugging-port=9223`) and + * `globalTeardown` shuts it down, so `npm run test:e2e:cdp` is self-contained — no manual `npm run + * start:cdp` first. If the CDP port is already taken, the setup instead reuses the running instance + * and leaves it up, so a developer iterating against a warm `npm run start:cdp` instance can point + * Playwright at it directly (`npx playwright test --config e2e-tests/playwright-cdp.config.ts`). */ export default defineConfig({ testDir: './tests', testIgnore: ['**/smoke/**', '**/_example/**'], fullyParallel: false, workers: 1, - reporter: [['html', { outputFolder: 'playwright-report' }], ['list']], + // Tier-specific report/output folders so a combined `npm run test:e2e` run keeps both tiers' + // reports side by side instead of the cdp run overwriting the smoke run's. `open: 'never'` keeps + // a run from auto-launching a browser in CI. + reporter: [['html', { outputFolder: 'playwright-report/cdp', open: 'never' }], ['list']], timeout: 120_000, expect: { timeout: 10_000 }, use: { @@ -23,6 +27,7 @@ export default defineConfig({ screenshot: 'only-on-failure', video: 'retain-on-failure', }, - outputDir: './test-results', - // NO globalSetup/globalTeardown — app is already running + globalSetup: './global-setup-cdp.ts', + globalTeardown: './global-teardown-cdp.ts', + outputDir: './test-results/cdp', }); diff --git a/e2e-tests/playwright.config.ts b/e2e-tests/playwright.config.ts index e47a532f..a0066752 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -20,7 +20,10 @@ export default defineConfig({ forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 1, workers: 1, - reporter: [['html', { outputFolder: 'playwright-report' }], ['list']], + // Tier-specific report/output folders so `npm run test:e2e` (smoke then cdp, sequentially) doesn't + // have the second tier's report overwrite the first's. The `open: 'never'` keeps a run from + // auto-launching a browser in CI. + reporter: [['html', { outputFolder: 'playwright-report/smoke', open: 'never' }], ['list']], timeout: 120_000, expect: { timeout: 10_000, @@ -32,7 +35,7 @@ export default defineConfig({ }, globalSetup: './global-setup.ts', globalTeardown: './global-teardown.ts', - outputDir: './test-results', + outputDir: './test-results/smoke', projects: [ { name: 'smoke', diff --git a/package.json b/package.json index 96373c7c..ee1f3753 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "bump-versions": "ts-node ./lib/bump-versions.ts", "test": "jest", "test:coverage": "jest --coverage", - "test:e2e": "playwright test --config e2e-tests/playwright.config.ts", + "test:e2e": "npm run test:e2e:smoke && npm run test:e2e:cdp", "test:e2e:cdp": "playwright test --config e2e-tests/playwright-cdp.config.ts", "test:e2e:smoke": "playwright test --config e2e-tests/playwright.config.ts --project=smoke", "core:start": "npm --prefix ../paranext-core start", From 47905e553b7534d53ef1e4475238338df5a303af Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 9 Jul 2026 17:30:36 -0600 Subject: [PATCH 07/24] Run full e2e suite on Windows; make CDP teardown Windows-safe Kill the CDP app's whole process tree (taskkill /T /F on Windows, POSIX process-group SIGKILL elsewhere) so no orphans hold port 8876, and stream the launched app's output to a log instead of discarding it, so a startup crash shows its cause rather than an opaque WebSocket-port timeout. --- .github/workflows/test.yml | 15 ++++---- .gitignore | 1 + e2e-tests/global-setup-cdp.ts | 44 ++++++++++++++++++++++-- e2e-tests/global-teardown-cdp.ts | 59 ++++++++++++++++++++++++-------- 4 files changed, 94 insertions(+), 25 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e6f65682..65f20046 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,21 +114,19 @@ jobs: working-directory: paranext-core run: npm run build:dll - # Linux runs the FULL suite: smoke plus the CDP feature tests. `test:e2e:cdp` self-launches - # its own Platform.Bible instance (globalSetup, --remote-debugging-port=9223), so the whole - # thing runs under one xvfb display with no separate app-start step. + # Both OSes run the FULL suite: smoke plus the CDP feature tests. `test:e2e:cdp` self-launches + # its own Platform.Bible instance (globalSetup, --remote-debugging-port=9223) and tears it down + # cross-platform (POSIX process-group SIGKILL on Linux, `taskkill /T /F` on Windows), so there + # is no separate app-start step on either OS. Linux additionally needs an xvfb display. - name: Run e2e tests (Linux) if: matrix.os == 'ubuntu-latest' working-directory: extension-repo run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e - # Windows runs smoke only. The CDP self-launch spawns a detached Electron and kills it by - # POSIX process group (process.kill(-pid, ...)), which has no Windows equivalent, so the CDP - # tier is Linux-only until that teardown path is made Windows-safe. - - name: Run e2e smoke tests (Windows) + - name: Run e2e tests (Windows) if: matrix.os == 'windows-latest' working-directory: extension-repo - run: npm run test:e2e:smoke + run: npm run test:e2e - name: Upload Playwright test results uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -139,4 +137,5 @@ jobs: path: | extension-repo/e2e-tests/playwright-report/ extension-repo/e2e-tests/test-results/ + extension-repo/e2e-tests/.cdp-app-startup.log if-no-files-found: warn diff --git a/.gitignore b/.gitignore index 9529ad4e..3e9690d3 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ e2e-tests/test-results e2e-tests/.dev-server.pid e2e-tests/.cdp-app.pid e2e-tests/.cdp-app.user-data-dir +e2e-tests/.cdp-app-startup.log diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index 95d7e7c7..16ee597b 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -25,6 +25,14 @@ export const CDP_PID_FILE = path.join(__dirname, '.cdp-app.pid'); /** File the launched Electron's isolated user-data dir is written to, for teardown to remove it. */ export const CDP_USER_DATA_FILE = path.join(__dirname, '.cdp-app.user-data-dir'); +/** + * File the launched app's stdout/stderr is streamed to. Kept alongside the other `.cdp-*` marker + * files in `e2e-tests/` — a location Playwright does not clear (unlike `outputDir`) — and added to + * the CI artifact upload so it survives a failed run. Without this the app is spawned `stdio: + * 'ignore'` and a startup crash surfaces only as an opaque WebSocket-port timeout with no cause. + */ +export const CDP_APP_LOG_FILE = path.join(__dirname, '.cdp-app-startup.log'); + /** How long to wait for the launched app's WebSocket / CDP port before failing setup. */ const APP_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; @@ -92,9 +100,13 @@ export default async function globalSetupCdp(_config: FullConfig): Promise console.log(`Loading extension from: ${extensionDist}`); console.log(`Remote debugging on port ${CDP_PORT}`); + // Stream the app's output to a log file rather than discarding it (`stdio: 'ignore'`): when the + // app crashes on startup the only other symptom is an opaque WebSocket-port timeout below. + const appLogFd = fs.openSync(CDP_APP_LOG_FILE, 'w'); + // Detached: unlike the smoke fixture's Playwright-owned `_electron.launch()`, the CDP fixture // connects to this process over CDP, so Playwright must not own its lifecycle. Teardown kills the - // whole process group by the recorded PID. + // whole process tree by the recorded PID. const appProcess = spawn( electronExecutable, [ @@ -107,17 +119,43 @@ export default async function globalSetupCdp(_config: FullConfig): Promise { cwd: coreDir, env: { ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false' }, - stdio: 'ignore', + stdio: ['ignore', appLogFd, appLogFd], detached: true, }, ); appProcess.unref(); + // The child has inherited the fd; close our copy so the file is flushed and released on exit. + fs.closeSync(appLogFd); if (appProcess.pid) fs.writeFileSync(CDP_PID_FILE, String(appProcess.pid)); console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); - await waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT); + try { + await waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT); + } catch (error) { + // The app never came up. Echo its captured output so the failure cause is in the CI log itself, + // not just buried in the uploaded artifact, then re-throw the original timeout. + dumpAppLog(); + throw error; + } console.log(`Waiting for CDP debug port ${CDP_PORT}...`); await waitForPort(CDP_PORT, APP_READY_TIMEOUT); console.log('Platform.Bible (CDP) is ready.'); } + +/** + * Print the launched app's captured stdout/stderr to the console. Called when the app fails to open + * its ports so the startup failure's cause appears inline in the CI log. + * + * @returns Nothing; logging-only. + */ +function dumpAppLog(): void { + try { + const log = fs.readFileSync(CDP_APP_LOG_FILE, 'utf-8'); + console.error( + `--- Launched Platform.Bible (CDP) output (${CDP_APP_LOG_FILE}) ---\n${log || '(empty)'}\n--- end app output ---`, + ); + } catch { + console.error(`Could not read app log at ${CDP_APP_LOG_FILE}.`); + } +} diff --git a/e2e-tests/global-teardown-cdp.ts b/e2e-tests/global-teardown-cdp.ts index 16224d0a..8bac3ee5 100644 --- a/e2e-tests/global-teardown-cdp.ts +++ b/e2e-tests/global-teardown-cdp.ts @@ -1,9 +1,50 @@ // Teardown for the self-launching CDP (feature-test) config. import type { FullConfig } from '@playwright/test'; +import { execFileSync } from 'child_process'; import fs from 'fs'; import { CDP_PID_FILE, CDP_USER_DATA_FILE } from './global-setup-cdp'; import globalTeardown from './global-teardown'; +/** + * Forcibly kill the launched app and all of its children, cross-platform. + * + * Electron spawns a tree of processes (main, renderer, GPU, utility). Leaving any alive holds the + * PAPI WebSocket port and poisons the next run's fast-fail port check, so we must kill the whole + * tree, not just the top process. + * + * - On Windows there is no process group and `process.kill(-pid)` is meaningless, so shell out to + * `taskkill /T /F`, which terminates the process and its entire descendant tree. + * - Elsewhere the app was spawned `detached`, so it is its own process-group leader: signal the + * negative PID to SIGKILL the whole group in one call, falling back to the bare PID if the group + * is already gone. + * + * @param pid PID of the detached app process recorded by {@link globalSetupCdp}. + * @returns `true` if a kill was issued, `false` if the process was already gone. + */ +function killAppTree(pid: number): boolean { + if (process.platform === 'win32') { + try { + execFileSync('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + return true; + } catch { + // taskkill exits non-zero when the process is already gone — nothing left to kill. + return false; + } + } + try { + process.kill(-pid, 'SIGKILL'); + return true; + } catch { + try { + process.kill(pid, 'SIGKILL'); + return true; + } catch { + // Already stopped + return false; + } + } +} + /** * Playwright global teardown for the CDP config. Kills the Electron instance launched by * {@link globalSetupCdp} (by the PID recorded in {@link CDP_PID_FILE}), removes its isolated @@ -15,9 +56,9 @@ import globalTeardown from './global-teardown'; * has completed. */ export default async function globalTeardownCdp(config: FullConfig): Promise { - // Kill the app we launched (whole process group) before the shared teardown's generic sweep. - // SIGKILL (not SIGTERM) to match the smoke teardown: Electron can ignore SIGTERM, and we need it - // fully dead before removing its user-data dir below. + // Kill the app we launched (whole process tree) before the shared teardown's generic sweep. + // SIGKILL/`/F` (not SIGTERM) to match the smoke teardown: Electron can ignore SIGTERM, and we + // need it fully dead before removing its user-data dir below. let appKilled = false; if (fs.existsSync(CDP_PID_FILE)) { const pid = parseInt(fs.readFileSync(CDP_PID_FILE, 'utf-8').trim(), 10); @@ -25,17 +66,7 @@ export default async function globalTeardownCdp(config: FullConfig): Promise Date: Fri, 10 Jul 2026 09:51:20 -0600 Subject: [PATCH 08/24] Suggested changes on expand-e2e-testing (#160) * Suggested changes on expand-e2e-testing * Fix Linux CI --- .github/workflows/test.yml | 7 +-- .gitignore | 10 ++-- e2e-tests/README.md | 2 +- e2e-tests/fixtures/helpers.ts | 24 ++++++---- e2e-tests/global-setup-cdp.ts | 22 ++++++++- e2e-tests/global-teardown-cdp.ts | 44 +---------------- e2e-tests/global-teardown.ts | 11 +---- e2e-tests/process-utils.ts | 47 +++++++++++++++++++ .../example-interlinearizer-feature.spec.ts | 8 ++-- .../tests/features/draft-persistence.spec.ts | 6 +-- .../tests/features/gloss-roundtrip.spec.ts | 6 +-- .../tests/features/project-modals.spec.ts | 8 ++-- .../tests/smoke/extension-launch.spec.ts | 5 +- .../tests/smoke/open-interlinearizer.spec.ts | 6 +-- 14 files changed, 105 insertions(+), 101 deletions(-) create mode 100644 e2e-tests/process-utils.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 65f20046..f8e6a427 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,13 +114,10 @@ jobs: working-directory: paranext-core run: npm run build:dll - # Both OSes run the FULL suite: smoke plus the CDP feature tests. `test:e2e:cdp` self-launches - # its own Platform.Bible instance (globalSetup, --remote-debugging-port=9223) and tears it down - # cross-platform (POSIX process-group SIGKILL on Linux, `taskkill /T /F` on Windows), so there - # is no separate app-start step on either OS. Linux additionally needs an xvfb display. - name: Run e2e tests (Linux) if: matrix.os == 'ubuntu-latest' working-directory: extension-repo + # Linux needs an xvfb display run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e - name: Run e2e tests (Windows) @@ -135,7 +132,7 @@ jobs: name: playwright-results-${{ matrix.os }} retention-days: 7 path: | + extension-repo/e2e-tests/.cdp-app-startup.log extension-repo/e2e-tests/playwright-report/ extension-repo/e2e-tests/test-results/ - extension-repo/e2e-tests/.cdp-app-startup.log if-no-files-found: warn diff --git a/.gitignore b/.gitignore index 3e9690d3..13e0fd65 100644 --- a/.gitignore +++ b/.gitignore @@ -36,12 +36,8 @@ temp-build # AI files .claude -# Playwright test output +# Playwright output +e2e-tests/.cdp-* +e2e-tests/.dev-* e2e-tests/playwright-report e2e-tests/test-results - -# Playwright runtime process markers (dev server / self-launched CDP app) -e2e-tests/.dev-server.pid -e2e-tests/.cdp-app.pid -e2e-tests/.cdp-app.user-data-dir -e2e-tests/.cdp-app-startup.log diff --git a/e2e-tests/README.md b/e2e-tests/README.md index 289a49ad..94e2060c 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -9,7 +9,7 @@ Run everything with `npm run test:e2e` (smoke tier then CDP tier). Each tier can Both tiers are self-launching: the CDP tier's `globalSetup` launches its own Platform.Bible instance (with `--remote-debugging-port=9223`) in an isolated user-data dir and tears it down afterward, so `npm run test:e2e:cdp` needs no manual `npm run start:cdp` first. To iterate against a warm instance instead, run `npm run start:cdp` in one terminal, then run the CDP config directly with `npx playwright test --config e2e-tests/playwright-cdp.config.ts`: the setup detects the in-use CDP port, reuses that instance, and leaves it running. -In CI (`.github/workflows/test.yml`, `e2e` job) the full suite runs on Linux under `xvfb` via `npm run test:e2e`; Windows runs the smoke tier only, because the CDP self-launch tears its app down by POSIX process group, which has no Windows equivalent. Each tier writes its Playwright HTML report to its own subfolder (`playwright-report/smoke`, `playwright-report/cdp`) so a combined run keeps both. +In CI (`.github/workflows/test.yml`, `e2e` job) the full suite runs on both Linux and Windows. Each tier writes its Playwright HTML report to its own subfolder (`playwright-report/smoke`, `playwright-report/cdp`) so a combined run keeps both. **Contents:** diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index f1a8f5fe..538e24de 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -7,6 +7,7 @@ import { Locator, Page, } from '@playwright/test'; +import escapeStringRegexp from 'escape-string-regexp'; import fs from 'fs'; import { createRequire } from 'module'; import os from 'os'; @@ -446,7 +447,7 @@ export async function openInterlinearizerFromScriptureEditor( // A Scripture Editor tab is titled by the project short name once a project is loaded (e.g. // "WEB (Editable)"), and "Scripture Editor" only when no project is loaded. Escape projectName so // a short name with regex metacharacters can't corrupt the pattern. - const escapedProjectName = escapeRegExp(projectName); + const escapedProjectName = escapeStringRegexp(projectName); const editorTab = page .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${escapedProjectName})\\b`) }) .first(); @@ -456,7 +457,7 @@ export async function openInterlinearizerFromScriptureEditor( // briefly reports zero tabs, and a non-waiting `count()` would misread that as "no editor". // `.first()` on the whole `.or()`: when both the editor and Home tabs are present the union // resolves to two elements, which would trip strict mode on this visibility assertion. - await expect(editorTab.or(homeTab).first()).toBeVisible({ timeout: 30_000 }); + await expect(editorTab.or(homeTab).first()).toBeVisible({ timeout: 45_000 }); // If the layout came up without a Scripture Editor (single-tab Home layout), open the project // from Home so the editor (and its ≡ menu) exists before we try to focus it. @@ -471,8 +472,9 @@ export async function openInterlinearizerFromScriptureEditor( // The Scripture Editor renders its own toolbar inside its iframe. Click the ≡ ("Project") button. const editorFrame = page - .frameLocator(`iframe[title*="Scripture Editor" i], iframe[title^="${projectName}"]`) - .first(); + .locator(`iframe[title*="Scripture Editor" i], iframe[title^="${projectName}"]`) + .first() + .contentFrame(); await editorFrame.locator("button[aria-label='Project']").first().click(); // Click the "Open Interlinearizer for this Project" item contributed by this extension. @@ -530,14 +532,16 @@ function interlinearizerTabLocator(page: Page): Locator { } /** - * Escape a literal string so it can be embedded in a `RegExp` without its metacharacters being - * interpreted (e.g. a project short name containing `.` or `(`). + * Wait for Platform.Bible and the interlinearizer extension to finish starting up. Combines + * {@link waitForAppReady} and {@link waitForInterlinearizerReady}. * - * @param literal The raw string to escape. - * @returns The string with all regex metacharacters backslash-escaped. + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves when `interlinearizer.openForWebView` is listed in `rpc.discover`. + * @throws If the app or extension do not finish starting up within their default timeouts. */ -function escapeRegExp(literal: string): string { - return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +export async function waitForAppAndInterlinearizerReady(page: Page): Promise { + await waitForAppReady(page); + await waitForInterlinearizerReady(); } /** diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index 16ee597b..b05c2c0f 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -115,6 +115,9 @@ export default async function globalSetupCdp(_config: FullConfig): Promise '--extensions', extensionDist, `--remote-debugging-port=${CDP_PORT}`, + // GitHub-hosted Linux runners don't ship a root-owned setuid chrome-sandbox binary, so + // Electron's SUID sandbox helper aborts on launch. Skip the OS sandbox on Linux. + ...(process.platform === 'linux' ? ['--no-sandbox'] : []), ], { cwd: coreDir, @@ -130,11 +133,26 @@ export default async function globalSetupCdp(_config: FullConfig): Promise if (appProcess.pid) fs.writeFileSync(CDP_PID_FILE, String(appProcess.pid)); console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); + /** + * Rejects the moment {@link appProcess} exits, so a startup crash (e.g. a sandbox + * misconfiguration) fails setup immediately instead of only surfacing after the full + * {@link APP_READY_TIMEOUT} port-wait below elapses. + */ + const earlyExit = new Promise((_resolve, reject) => { + appProcess.once('exit', (code, signal) => { + reject( + new Error( + `Launched Platform.Bible (CDP) process exited early (code=${code}, signal=${signal}) ` + + 'before its WebSocket port came up.', + ), + ); + }); + }); try { - await waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT); + await Promise.race([waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT), earlyExit]); } catch (error) { // The app never came up. Echo its captured output so the failure cause is in the CI log itself, - // not just buried in the uploaded artifact, then re-throw the original timeout. + // not just buried in the uploaded artifact, then re-throw the original error. dumpAppLog(); throw error; } diff --git a/e2e-tests/global-teardown-cdp.ts b/e2e-tests/global-teardown-cdp.ts index 8bac3ee5..1b7fb7b7 100644 --- a/e2e-tests/global-teardown-cdp.ts +++ b/e2e-tests/global-teardown-cdp.ts @@ -1,49 +1,9 @@ // Teardown for the self-launching CDP (feature-test) config. import type { FullConfig } from '@playwright/test'; -import { execFileSync } from 'child_process'; import fs from 'fs'; import { CDP_PID_FILE, CDP_USER_DATA_FILE } from './global-setup-cdp'; import globalTeardown from './global-teardown'; - -/** - * Forcibly kill the launched app and all of its children, cross-platform. - * - * Electron spawns a tree of processes (main, renderer, GPU, utility). Leaving any alive holds the - * PAPI WebSocket port and poisons the next run's fast-fail port check, so we must kill the whole - * tree, not just the top process. - * - * - On Windows there is no process group and `process.kill(-pid)` is meaningless, so shell out to - * `taskkill /T /F`, which terminates the process and its entire descendant tree. - * - Elsewhere the app was spawned `detached`, so it is its own process-group leader: signal the - * negative PID to SIGKILL the whole group in one call, falling back to the bare PID if the group - * is already gone. - * - * @param pid PID of the detached app process recorded by {@link globalSetupCdp}. - * @returns `true` if a kill was issued, `false` if the process was already gone. - */ -function killAppTree(pid: number): boolean { - if (process.platform === 'win32') { - try { - execFileSync('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); - return true; - } catch { - // taskkill exits non-zero when the process is already gone — nothing left to kill. - return false; - } - } - try { - process.kill(-pid, 'SIGKILL'); - return true; - } catch { - try { - process.kill(pid, 'SIGKILL'); - return true; - } catch { - // Already stopped - return false; - } - } -} +import { killProcessTree } from './process-utils'; /** * Playwright global teardown for the CDP config. Kills the Electron instance launched by @@ -66,7 +26,7 @@ export default async function globalTeardownCdp(config: FullConfig): Promise fs.unlinkSync(pidFile); } else { console.log(`Stopping renderer dev server (PID: ${pid})...`); - try { - process.kill(-pid, 'SIGTERM'); - } catch { - try { - process.kill(pid, 'SIGTERM'); - } catch { - // Already stopped - } - } + killProcessTree(pid, 'SIGTERM'); fs.unlinkSync(pidFile); } } diff --git a/e2e-tests/process-utils.ts b/e2e-tests/process-utils.ts new file mode 100644 index 00000000..4d63b26c --- /dev/null +++ b/e2e-tests/process-utils.ts @@ -0,0 +1,47 @@ +// Cross-platform process-tree termination, shared by global-teardown.ts and global-teardown-cdp.ts. +import { execFileSync } from 'child_process'; + +/** + * Forcibly kill a process and all of its descendants, cross-platform. + * + * Both the renderer dev server (`npm run start:renderer`, spawned via a shell) and the + * self-launched Electron app spawn a tree of child processes (webpack workers; + * main/renderer/GPU/utility). Killing only the top PID leaves the children running, which for the + * dev server means orphaned webpack workers and for Electron means a held PAPI WebSocket port that + * poisons the next run's fast-fail port check. + * + * - On Windows there is no process group and `process.kill(-pid)` is meaningless, so shell out to + * `taskkill /T /F`, which terminates the process and its entire descendant tree. `taskkill` + * always force-kills (no graceful-signal equivalent), so `signal` is ignored on this branch. + * - Elsewhere the target was spawned `detached`, so it is its own process-group leader: signal the + * negative PID to signal the whole group in one call, falling back to the bare PID if the group + * is already gone. + * + * @param pid PID of the detached process to kill. + * @param signal POSIX signal to send when not on Windows (ignored on Windows, which always + * force-kills). Defaults to `'SIGTERM'`. + * @returns `true` if a kill was issued, `false` if the process was already gone. + */ +export function killProcessTree(pid: number, signal: NodeJS.Signals = 'SIGTERM'): boolean { + if (process.platform === 'win32') { + try { + execFileSync('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); + return true; + } catch { + // taskkill exits non-zero when the process is already gone — nothing left to kill. + return false; + } + } + try { + process.kill(-pid, signal); + return true; + } catch { + try { + process.kill(pid, signal); + return true; + } catch { + // Already stopped + return false; + } + } +} diff --git a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts index 0b583275..8ec02feb 100644 --- a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts +++ b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts @@ -14,7 +14,7 @@ * This file is excluded from test runs — it's documentation only. */ import { test, expect } from '../../fixtures/cdp.fixture'; -import { waitForAppReady, waitForInterlinearizerReady } from '../../fixtures/helpers'; +import { waitForAppAndInterlinearizerReady } from '../../fixtures/helpers'; /** * Filter out expected/benign console errors from a list of captured error messages. @@ -34,8 +34,7 @@ function filterConsoleErrors(errors: string[]): string[] { test.describe('Example: Open Interlinearizer via menu', () => { test('should open the interlinearizer WebView via menu', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); // Step 1: Click the top-level menu that contains the interlinearizer entry const menuTrigger = mainPage.getByRole('menuitem', { name: /Tools/i }); @@ -51,8 +50,7 @@ test.describe('Example: Open Interlinearizer via menu', () => { }); test('should render without critical console errors', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); const consoleErrors: string[] = []; mainPage.on('console', (msg) => { diff --git a/e2e-tests/tests/features/draft-persistence.spec.ts b/e2e-tests/tests/features/draft-persistence.spec.ts index c6f9a68a..5f08c882 100644 --- a/e2e-tests/tests/features/draft-persistence.spec.ts +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -5,8 +5,7 @@ import { ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, navigateToScriptureRef, - waitForAppReady, - waitForInterlinearizerReady, + waitForAppAndInterlinearizerReady, wipeDraft, } from '../../fixtures/helpers'; @@ -17,8 +16,7 @@ test.describe('Draft persistence', () => { // Two full open cycles (plus first-use project creation and book loading on a cold instance) // legitimately exceed the default 120 s budget. test.slow(); - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/gloss-roundtrip.spec.ts b/e2e-tests/tests/features/gloss-roundtrip.spec.ts index 2b073f10..93169946 100644 --- a/e2e-tests/tests/features/gloss-roundtrip.spec.ts +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -4,15 +4,13 @@ import { ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, navigateToScriptureRef, - waitForAppReady, - waitForInterlinearizerReady, + waitForAppAndInterlinearizerReady, wipeDraft, } from '../../fixtures/helpers'; test.describe('Gloss round-trip', () => { test('typing a gloss on a token renders it in the gloss field', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts index 03709137..e826b39f 100644 --- a/e2e-tests/tests/features/project-modals.spec.ts +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -3,8 +3,7 @@ import { ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, openInterlinearizerProjectMenu, - waitForAppReady, - waitForInterlinearizerReady, + waitForAppAndInterlinearizerReady, } from '../../fixtures/helpers'; /** @@ -36,15 +35,14 @@ test.describe('Project modals cancel tour', () => { test(`the ${modal.name} modal opens from the Project menu and cancels cleanly`, async ({ mainPage, }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); await ensureInterlinearizerOpenOnWeb(mainPage); const frame = await openInterlinearizerProjectMenu(mainPage); await frame.getByRole('menuitem', { name: modal.menuItem }).first().click(); const modalTitle = frame.locator(modal.titleSelector); - await expect(modalTitle).toBeVisible({ timeout: 10_000 }); + await expect(modalTitle).toBeVisible({ timeout: 5_000 }); await frame.locator('dialog').getByRole('button', { name: 'Cancel' }).click(); await expect(modalTitle).not.toBeVisible({ timeout: 5_000 }); diff --git a/e2e-tests/tests/smoke/extension-launch.spec.ts b/e2e-tests/tests/smoke/extension-launch.spec.ts index 077bce75..7e76c4ef 100644 --- a/e2e-tests/tests/smoke/extension-launch.spec.ts +++ b/e2e-tests/tests/smoke/extension-launch.spec.ts @@ -1,5 +1,5 @@ import { test, expect } from '../../fixtures/app.fixture'; -import { waitForAppReady, waitForInterlinearizerReady } from '../../fixtures/helpers'; +import { waitForAppAndInterlinearizerReady, waitForAppReady } from '../../fixtures/helpers'; test.describe('Launch app and register Interlinearizer', () => { test('should launch Platform.Bible and create at least one window', async ({ electronApp }) => { @@ -19,7 +19,6 @@ test.describe('Launch app and register Interlinearizer', () => { }); test('should register Interlinearizer PAPI commands', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); }); }); diff --git a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts index af440cc8..39b8fac8 100644 --- a/e2e-tests/tests/smoke/open-interlinearizer.spec.ts +++ b/e2e-tests/tests/smoke/open-interlinearizer.spec.ts @@ -1,14 +1,12 @@ import { expect, test } from '../../fixtures/app.fixture'; import { openInterlinearizerFromScriptureEditor, - waitForAppReady, - waitForInterlinearizerReady, + waitForAppAndInterlinearizerReady, } from '../../fixtures/helpers'; test.describe('Open Interlinearizer', () => { test('should open the interlinearizer and see its menus', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + await waitForAppAndInterlinearizerReady(mainPage); await openInterlinearizerFromScriptureEditor(mainPage); From 0c3705c2f927159281acfd81e83f74455638393b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 10:18:15 -0600 Subject: [PATCH 09/24] Harden CDP e2e tier against shared-instance overlay cascade Self-heal any modal a prior failed test left mounted (dismissLeftoverModals in ensureInterlinearizerOpenOnWeb) so its full-viewport overlay can't intercept every downstream test's clicks, force-click through the wipe modal's settle race, and enable CI retries now that each attempt starts clean. Also race the CDP-port wait against the early-exit sentinel so a late startup crash fails fast instead of timing out. --- e2e-tests/README.md | 2 +- e2e-tests/fixtures/helpers.ts | 80 +++++++++++++++++++++++++++++- e2e-tests/global-setup-cdp.ts | 15 ++++-- e2e-tests/playwright-cdp.config.ts | 6 +++ 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/e2e-tests/README.md b/e2e-tests/README.md index 94e2060c..9204e314 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -35,7 +35,7 @@ Feature tests run with `npm run test:e2e:cdp`. That command launches a fresh, is - **The instance is only ever used with the WEB project.** This is an operating assumption, not something tests verify: `ensureInterlinearizerOpenOnWeb()` trusts an existing Interlinearizer tab and only picks WEB when opening fresh. Don't point `start:cdp` at other projects. - **Mutating tests operate on the dedicated "E2E Test Project", never on a developer's own projects.** `ensureE2eProjectActive()` opens it (creating it on first use) at the start of each mutating test. Because the draft is the single per-source working buffer, replacing it could destroy unsaved developer work — so when the draft is dirty and the active project is _not_ the e2e project, the helper first saves the draft into a new `e2e-rescued-work-` project. Rescue projects are backups, not junk: delete them manually once recovered. Dirty state left while the e2e project is active is treated as leftover test data and discarded. - **Self-establish every precondition.** Each mutating test starts with `ensureInterlinearizerOpenOnWeb()` → `ensureE2eProjectActive()` → `navigateToScriptureRef()` (the scroll-group reference could be anywhere) → `wipeDraft()`. -- **Reset at the start, tidy at the end.** The start-of-test sequence is what guarantees correctness — it self-heals whatever a failed run left behind. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers; this is a courtesy so the next run doesn't misread test junk as rescuable developer work, not something correctness depends on. +- **Reset at the start, tidy at the end.** The start-of-test sequence is what guarantees correctness — it self-heals whatever a failed run left behind. Part of that self-heal is automatic: `ensureInterlinearizerOpenOnWeb()` calls `dismissLeftoverModals()`, which cancels any modal a timed-out prior test left mounted — a project modal's overlay is `fixed inset-0 z-50` and would otherwise intercept every click in the tests that follow, turning one real failure into a cascade. This is also why the CDP config can safely `retries` in CI: the retry lands on a self-healed instance, not on the overlay the failed attempt left up. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers; this is a courtesy so the next run doesn't misread test junk as rescuable developer work, not something correctness depends on. - **Use unique per-run values** (e.g. `` `e2e-gloss-${Date.now()}` ``) for anything written into the draft, so a stale leftover can never satisfy an assertion. - **Drive only the visible UI.** No JSON-RPC/WebSocket calls to set up or assert state (the rpc.discover readiness polls in the shared helpers are the one sanctioned exception). - **Prefer existing accessible selectors** (roles, aria-labels like `Gloss for {word}`, ModalShell title ids) over adding new `data-testid`s to production code. diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 538e24de..2eaee3be 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -544,6 +544,67 @@ export async function waitForAppAndInterlinearizerReady(page: Page): Promise` (not via `showModal()`), so native Escape does + * not fire their onCancel. Modals can chain (a discard-draft confirm can sit behind another), so + * cancel in a bounded loop until no overlay remains. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves once no modal overlay remains in the iframe (a no-op on the common clean path). + */ +export async function dismissLeftoverModals(page: Page): Promise { + const frame = getInterlinearizerFrame(page); + // Every project modal is a `` rendered by ModalShell inside a `tw:modal-overlay`, and + // that `` element is the only one in the iframe — so `frame.locator('dialog')` (the same + // handle every other modal helper uses) both detects an open modal and scopes the Cancel lookup, + // with no separate overlay selector needed. The overlay is the invisible click-blocker, but the + // dialog it wraps is visible exactly when the overlay is. + const dialog = frame.locator('dialog').first(); + + // Bounded so a modal that refuses to close can't spin forever; a couple of chained confirmations + // is the realistic worst case. + for (let attempt = 0; attempt < 3; attempt += 1) { + // Non-retrying visibility read: on the clean path (no leftover modal) this must fall through + // immediately rather than wait out a timeout on every single test. + // eslint-disable-next-line no-await-in-loop + if (!(await dialog.isVisible())) return; + + // Prefer an explicit Cancel; fall back to any secondary button (e.g. a confirm dialog whose + // back-out button is labeled differently) so an unexpected modal still gets dismissed. + const cancelButton = dialog.getByRole('button', { name: /Cancel|Keep|Close|No\b/i }).first(); + // eslint-disable-next-line no-await-in-loop + if (await cancelButton.isVisible()) { + // eslint-disable-next-line no-await-in-loop + await cancelButton.click(); + } else { + // No recognizable back-out control — press Escape as a last resort and stop retrying, since a + // further loop would just re-click the same unknown modal. + // eslint-disable-next-line no-await-in-loop + await page.keyboard.press('Escape'); + break; + } + // eslint-disable-next-line no-await-in-loop + await expect(dialog) + .not.toBeVisible({ timeout: 5_000 }) + .catch(() => { + /* Another modal may have taken its place; the next loop iteration handles it. */ + }); + } +} + /** * Ensure the Interlinearizer is open and focused, reusing an existing tab when one is present. * Standard precondition for feature tests running against the shared CDP instance: an existing @@ -551,6 +612,10 @@ export async function waitForAppAndInterlinearizerReady(page: Page): Promise await expect(frame.locator("button[aria-label='Project']").first()).toBeVisible({ timeout: 30_000, }); + + // Self-heal the shared instance: clear any modal a prior failed test left mounted before this + // test starts driving the UI, so its overlay can't intercept the clicks below. + await dismissLeftoverModals(page); } /** @@ -796,8 +865,15 @@ export async function wipeDraft(page: Page): Promise { const wipeDialogTitle = frame.locator('#wipe-modal-title'); await expect(wipeDialogTitle).toBeVisible({ timeout: 5_000 }); - await frame.getByTestId('wipe-scope-all').check(); - await frame.getByTestId('wipe-confirm').click(); + const scopeAll = frame.getByTestId('wipe-scope-all'); + // `force`: the radio reads visible+enabled+stable, but on a slow/software-rendered CI display the + // just-opened modal overlay hasn't won the hit-test yet, so a normal click is intercepted by the + // iframe's own `#root` for a few frames and then times out (the original gloss-roundtrip CI + // failure). We've already asserted this modal is open and are targeting an element inside it by a + // unique test id, so skipping the (spuriously-failing) hit-test check is safe here. + await expect(scopeAll).toBeEnabled({ timeout: 5_000 }); + await scopeAll.check({ force: true }); + await frame.getByTestId('wipe-confirm').click({ force: true }); await expect(wipeDialogTitle).not.toBeVisible({ timeout: 10_000 }); } diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index b05c2c0f..be0ed195 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -132,32 +132,37 @@ export default async function globalSetupCdp(_config: FullConfig): Promise if (appProcess.pid) fs.writeFileSync(CDP_PID_FILE, String(appProcess.pid)); - console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); /** * Rejects the moment {@link appProcess} exits, so a startup crash (e.g. a sandbox * misconfiguration) fails setup immediately instead of only surfacing after the full - * {@link APP_READY_TIMEOUT} port-wait below elapses. + * {@link APP_READY_TIMEOUT} port-wait below elapses. Raced against BOTH port waits: a crash + * between the WebSocket port coming up and the CDP port coming up must fail just as fast and with + * the same informative message, not silently swallow the exit and time out on the CDP wait. */ const earlyExit = new Promise((_resolve, reject) => { appProcess.once('exit', (code, signal) => { reject( new Error( `Launched Platform.Bible (CDP) process exited early (code=${code}, signal=${signal}) ` + - 'before its WebSocket port came up.', + 'before its WebSocket and CDP ports came up.', ), ); }); }); try { + console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); await Promise.race([waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT), earlyExit]); + console.log(`Waiting for CDP debug port ${CDP_PORT}...`); + // Race the same earlyExit sentinel here too: without it, a crash after the WebSocket port is up + // would be swallowed and this wait would run out the full APP_READY_TIMEOUT with a generic + // "port not available" error instead of the "process exited early" cause below. + await Promise.race([waitForPort(CDP_PORT, APP_READY_TIMEOUT), earlyExit]); } catch (error) { // The app never came up. Echo its captured output so the failure cause is in the CI log itself, // not just buried in the uploaded artifact, then re-throw the original error. dumpAppLog(); throw error; } - console.log(`Waiting for CDP debug port ${CDP_PORT}...`); - await waitForPort(CDP_PORT, APP_READY_TIMEOUT); console.log('Platform.Bible (CDP) is ready.'); } diff --git a/e2e-tests/playwright-cdp.config.ts b/e2e-tests/playwright-cdp.config.ts index 7d373006..a27413bf 100644 --- a/e2e-tests/playwright-cdp.config.ts +++ b/e2e-tests/playwright-cdp.config.ts @@ -15,6 +15,12 @@ export default defineConfig({ testDir: './tests', testIgnore: ['**/smoke/**', '**/_example/**'], fullyParallel: false, + forbidOnly: !!process.env.CI, + // Retry in CI like the smoke tier. On the shared, never-reset CDP instance a retry only helps + // because each test self-heals leftover modals at its start (ensureInterlinearizerOpenOnWeb → + // dismissLeftoverModals), so the retry lands on a clean instance instead of re-running against + // the overlay the timed-out attempt left mounted. + retries: process.env.CI ? 2 : 1, workers: 1, // Tier-specific report/output folders so a combined `npm run test:e2e` run keeps both tiers' // reports side by side instead of the cdp run overwriting the smoke run's. `open: 'never'` keeps From 631a08067387c0f5aaadc834391ba725d3cfd813 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 11:30:39 -0600 Subject: [PATCH 10/24] Fix e2e CI failures: DevTools viewport squeeze, Unknown-tab startup Disable electron-debug's auto-opened DevTools (ELECTRON_IS_DEV=0) and pin the window to 1280x960 so dock panels and modals are not clipped; gate readiness on dock-tab titles resolving (no "Unknown" tabs) in both waitForAppReady and CDP global setup so a broken instance fails fast with its startup log; upload the dotfile startup log (include-hidden-files) and force X11 on Linux so headless xvfb runs work on Wayland machines. --- .github/workflows/test.yml | 3 + e2e-tests/fixtures/helpers.ts | 74 +++++++++++++++++++++--- e2e-tests/global-setup-cdp.ts | 105 ++++++++++++++++++++++++++++++---- 3 files changed, 163 insertions(+), 19 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f8e6a427..f9ef1624 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -136,3 +136,6 @@ jobs: extension-repo/e2e-tests/playwright-report/ extension-repo/e2e-tests/test-results/ if-no-files-found: warn + # upload-artifact excludes hidden files by default (since v4.4), which silently dropped + # the dotfile app-startup log from every artifact. + include-hidden-files: true diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 2eaee3be..89314eff 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -125,6 +125,12 @@ export async function launchElectronWithExtension( ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false', + // With NODE_ENV=development, paranext-core's electron-debug auto-opens DevTools on every + // window. On CI Linux DevTools docks INSIDE the window, squeezing the app viewport to ~469px — + // dock panels collapse and modals get clipped at panel edges, so clicks land on neighboring + // iframes. electron-is-dev honors ELECTRON_IS_DEV=0, which disables electron-debug without + // affecting NODE_ENV-driven behavior (dev-server URL, etc.). + ELECTRON_IS_DEV: '0', ...opts.envOverrides, }; @@ -136,7 +142,22 @@ export async function launchElectronWithExtension( try { electronApp = await electron.launch({ executablePath: electronExecutable, - args: [`--user-data-dir=${userDataDir}`, coreDir, '--extensions', extensionDist], + args: [ + `--user-data-dir=${userDataDir}`, + coreDir, + '--extensions', + extensionDist, + // Deterministic window size instead of the 1024x728 electron-window-state default — + // paranext-core supports this argument for automation. Matches the CI xvfb screen + // (1280x960) so the dock panels have room and modals are not clipped. + '--window-size', + '1280x960', + // Force the X11 backend on Linux: in a Wayland session with DISPLAY redirected to xvfb + // (local headless runs), Electron otherwise picks the Wayland backend from the session + // environment and segfaults when the compositor socket is unreachable. On CI runners + // (X11-only) this is a no-op. + ...(process.platform === 'linux' ? ['--ozone-platform=x11'] : []), + ], cwd: coreDir, env, timeout: PROCESS_READY_TIMEOUT, @@ -371,22 +392,59 @@ export async function waitForPapiMethodRegistered( } /** - * Wait for the Platform.Bible UI to be fully ready: dock layout appears and `platform.about` - * command is registered (dialog service has finished initializing). + * Wait until the dock layout has at least one tab and no tab is still titled "Unknown". On a cold + * start the dock mounts with webview tabs titled "Unknown" (and blank panels) until project + * metadata resolves; every tab-title-based locator in this suite silently times out against that + * state. Waiting it out here turns those opaque per-test locator timeouts into either a pass (slow + * healthy startup) or one clear early failure (instance genuinely stuck — the Windows CI CDP-tier + * wipeout). * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param timeout Maximum time in milliseconds to wait before throwing. - * @returns Resolves when the dock layout is visible and `platform.about` is registered. - * @throws If the dock layout or `platform.about` command does not appear within `timeout` - * milliseconds. + * @returns Resolves when at least one dock tab exists and none contains "Unknown". + * @throws If tab titles have not resolved within `timeout` milliseconds. */ -export async function waitForAppReady(page: Page, timeout = 60_000): Promise { +export async function waitForDockTabTitlesResolved(page: Page, timeout: number): Promise { + try { + await page.waitForFunction( + () => { + const tabs = Array.from(document.querySelectorAll('.dock-tab')); + return tabs.length > 0 && tabs.every((tab) => !(tab.textContent ?? '').includes('Unknown')); + }, + undefined, + { timeout, polling: 500 }, + ); + } catch (error) { + throw new Error( + `Dock tab titles did not resolve within ${timeout}ms — the app came up but its webview tabs ` + + 'are still titled "Unknown" (project metadata never loaded). ' + + `Original error: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} + +/** + * Wait for the Platform.Bible UI to be fully ready: dock layout appears with resolved tab titles + * and `platform.about` command is registered (dialog service has finished initializing). + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param timeout Maximum time in milliseconds to wait before throwing. The default is generous + * because a cold CI instance has been observed taking over 45 s just to resolve tab titles; this + * is a wait-until, so healthy startups pay nothing for the headroom. + * @returns Resolves when the dock layout is visible with resolved tab titles and `platform.about` + * is registered. + * @throws If the dock layout, resolved tab titles, or the `platform.about` command do not appear + * within `timeout` milliseconds. + */ +export async function waitForAppReady(page: Page, timeout = 120_000): Promise { const start = Date.now(); await page.waitForSelector('div[class*="dock-layout"]', { state: 'attached', timeout, }); - const remaining = Math.max(0, timeout - (Date.now() - start)); + let remaining = Math.max(0, timeout - (Date.now() - start)); + await waitForDockTabTitlesResolved(page, remaining); + remaining = Math.max(0, timeout - (Date.now() - start)); await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, remaining); } diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index be0ed195..e64cc92a 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -1,10 +1,11 @@ // Self-launching global setup for the CDP (feature-test) config. -import type { FullConfig } from '@playwright/test'; +import { chromium, type FullConfig, type Page } from '@playwright/test'; import { spawn } from 'child_process'; import { createRequire } from 'module'; import fs from 'fs'; import os from 'os'; import path from 'path'; +import { waitForDockTabTitlesResolved } from './fixtures/helpers'; import { bootstrapRendererDevServer, isPortInUse, @@ -36,6 +37,15 @@ export const CDP_APP_LOG_FILE = path.join(__dirname, '.cdp-app-startup.log'); /** How long to wait for the launched app's WebSocket / CDP port before failing setup. */ const APP_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; +/** + * How long to wait after the ports are up for the renderer to actually settle (dock tabs present + * with resolved titles) before failing setup. Port readiness alone is not enough: a Windows CI + * instance has come up with PAPI responding but every dock tab stuck at "Unknown" and blank panels + * for the whole run, which made all five feature tests burn their own timeouts against one broken + * shared instance. Failing setup here instead surfaces one clear error plus the app's startup log. + */ +const RENDERER_SETTLE_TIMEOUT = process.env.CI ? 180_000 : 120_000; + /** * Playwright global setup for the CDP config. Unlike the smoke config — whose fixture launches * Electron per worker — the CDP fixture connects over CDP to a separately-running app. This setup @@ -45,7 +55,8 @@ const APP_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; * 2. Launches Electron (paranext-core) detached, with the interlinearizer extension loaded via * `--extensions` and Chromium remote debugging on {@link CDP_PORT}, in an isolated user-data * dir. - * 3. Waits for the PAPI WebSocket and the CDP debug port to come up. + * 3. Waits for the PAPI WebSocket and the CDP debug port to come up, then for the renderer to settle + * (dock tabs present with resolved titles — see {@link waitForRendererSettled}). * 4. Records the PID and user-data dir for {@link globalTeardownCdp}. * * The isolated user-data dir means the run never touches a developer's real profile; the feature @@ -115,13 +126,32 @@ export default async function globalSetupCdp(_config: FullConfig): Promise '--extensions', extensionDist, `--remote-debugging-port=${CDP_PORT}`, - // GitHub-hosted Linux runners don't ship a root-owned setuid chrome-sandbox binary, so - // Electron's SUID sandbox helper aborts on launch. Skip the OS sandbox on Linux. - ...(process.platform === 'linux' ? ['--no-sandbox'] : []), + // Deterministic window size instead of the 1024x728 electron-window-state default — + // paranext-core supports this argument for automation. Matches the CI xvfb screen (1280x960) + // so the dock panels have room and modals are not clipped. + '--window-size', + '1280x960', + // --no-sandbox: GitHub-hosted Linux runners don't ship a root-owned setuid chrome-sandbox + // binary, so Electron's SUID sandbox helper aborts on launch. --ozone-platform=x11: in a + // Wayland session with DISPLAY redirected to xvfb (local headless runs), Electron otherwise + // picks the Wayland backend from the session environment and segfaults when the compositor + // socket is unreachable; on CI runners (X11-only) it is a no-op. + ...(process.platform === 'linux' ? ['--no-sandbox', '--ozone-platform=x11'] : []), ], { cwd: coreDir, - env: { ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false' }, + env: { + ...restEnv, + NODE_ENV: 'development', + DEV_NOISY: process.env.DEV_NOISY ?? 'false', + // With NODE_ENV=development, paranext-core's electron-debug auto-opens DevTools on every + // window. On CI Linux DevTools docks INSIDE the window, squeezing the app viewport to + // ~469px — dock panels collapse and modals get clipped at panel edges, so clicks land on + // neighboring iframes (the gloss-roundtrip/Save-As CI failures). electron-is-dev honors + // ELECTRON_IS_DEV=0, which disables electron-debug without affecting NODE_ENV-driven + // behavior (dev-server URL, etc.). + ELECTRON_IS_DEV: '0', + }, stdio: ['ignore', appLogFd, appLogFd], detached: true, }, @@ -135,9 +165,9 @@ export default async function globalSetupCdp(_config: FullConfig): Promise /** * Rejects the moment {@link appProcess} exits, so a startup crash (e.g. a sandbox * misconfiguration) fails setup immediately instead of only surfacing after the full - * {@link APP_READY_TIMEOUT} port-wait below elapses. Raced against BOTH port waits: a crash - * between the WebSocket port coming up and the CDP port coming up must fail just as fast and with - * the same informative message, not silently swallow the exit and time out on the CDP wait. + * {@link APP_READY_TIMEOUT} port-wait below elapses. Raced against BOTH port waits and the + * renderer-settle wait: a crash after an earlier stage completes must fail just as fast and with + * the same informative message, not silently swallow the exit and time out on a later wait. */ const earlyExit = new Promise((_resolve, reject) => { appProcess.once('exit', (code, signal) => { @@ -149,6 +179,10 @@ export default async function globalSetupCdp(_config: FullConfig): Promise ); }); }); + // Setup returns with the app still running, so once the port/settle waits are done nothing + // awaits earlyExit anymore — mark it handled so the eventual teardown kill (which fires the same + // 'exit' event) can't surface as an unhandled rejection. + earlyExit.catch(() => {}); try { console.log(`Waiting for PAPI WebSocket on port ${WEBSOCKET_PORT}...`); await Promise.race([waitForPort(WEBSOCKET_PORT, APP_READY_TIMEOUT), earlyExit]); @@ -157,15 +191,64 @@ export default async function globalSetupCdp(_config: FullConfig): Promise // would be swallowed and this wait would run out the full APP_READY_TIMEOUT with a generic // "port not available" error instead of the "process exited early" cause below. await Promise.race([waitForPort(CDP_PORT, APP_READY_TIMEOUT), earlyExit]); + console.log('Ports are up. Waiting for the renderer to settle (dock tabs with real titles)...'); + await Promise.race([waitForRendererSettled(RENDERER_SETTLE_TIMEOUT), earlyExit]); } catch (error) { - // The app never came up. Echo its captured output so the failure cause is in the CI log itself, - // not just buried in the uploaded artifact, then re-throw the original error. + // The app never came up (or came up broken). Echo its captured output so the failure cause is + // in the CI log itself, not just buried in the uploaded artifact, then re-throw the original + // error. dumpAppLog(); throw error; } console.log('Platform.Bible (CDP) is ready.'); } +/** + * Connect to the launched app over CDP and wait for its renderer to settle: the renderer page + * exists and the dock tabs have real titles (none stuck at "Unknown"). This is the earliest point + * at which the tab-title-based locators the feature tests rely on can work, so gating setup on it + * converts a broken shared instance into one fast, diagnosable setup failure instead of a cascade + * of per-test timeouts. + * + * The Playwright connection is closed before returning either way — it only disconnects; the app + * keeps running for the test fixtures to connect to. + * + * @param timeout Maximum time in milliseconds to wait for the renderer page and settled tabs. + * @returns Resolves when the renderer page shows at least one dock tab and no "Unknown" titles. + * @throws {Error} If no renderer page appears or tab titles do not resolve within `timeout`. + */ +async function waitForRendererSettled(timeout: number): Promise { + const deadline = Date.now() + timeout; + const browser = await chromium.connectOverCDP(`http://localhost:${CDP_PORT}`, { + timeout: 30_000, + }); + try { + // The renderer page may not exist yet right after the CDP port opens — poll for it. Mirrors the + // page-finding logic in fixtures/cdp.fixture.ts. + let page: Page | undefined; + while (!page && Date.now() < deadline) { + const allPages = browser.contexts().flatMap((ctx) => ctx.pages()); + page = + allPages.find((p) => p.url().startsWith('http://localhost:1212/')) ?? + allPages.find((p) => !p.url().includes('devtools://')); + if (!page) { + // intentional poll delay + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => { + setTimeout(resolve, 500); + }); + } + } + if (!page) { + throw new Error(`No renderer page appeared over CDP within ${timeout}ms`); + } + await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now())); + } finally { + // Disconnect only — connectOverCDP close() does not terminate the app. + await browser.close(); + } +} + /** * Print the launched app's captured stdout/stderr to the console. Called when the app fails to open * its ports so the startup failure's cause appears inline in the CI log. From cd01eed6f2df06607c9ed7bbb439f9ebfbe4cf4f Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 11:47:22 -0600 Subject: [PATCH 11/24] Add test:e2e:headless to reproduce the Linux CI e2e run locally --- e2e-tests/README.md | 2 ++ package.json | 1 + 2 files changed, 3 insertions(+) diff --git a/e2e-tests/README.md b/e2e-tests/README.md index 9204e314..36964b6c 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -11,6 +11,8 @@ Both tiers are self-launching: the CDP tier's `globalSetup` launches its own Pla In CI (`.github/workflows/test.yml`, `e2e` job) the full suite runs on both Linux and Windows. Each tier writes its Playwright HTML report to its own subfolder (`playwright-report/smoke`, `playwright-report/cdp`) so a combined run keeps both. +To reproduce the Linux CI run locally before pushing, run `npm run test:e2e:headless` (requires `xvfb` — `sudo apt install xvfb`). It runs the full suite on the same virtual 1280x960 display CI uses, so nothing appears on screen and window geometry matches CI exactly. Ports 1212, 8876, and 9223 must be free (close any running Platform.Bible or renderer dev server first). A green local run does not cover the Windows leg, and CI's slower runners can still surface timing-dependent flakes, but it catches layout, selector, and logic failures before they reach CI. + **Contents:** - `*.json` — lint configs identical to those in `paranext-core/e2e-tests/` diff --git a/package.json b/package.json index ee1f3753..17b6d092 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "test:coverage": "jest --coverage", "test:e2e": "npm run test:e2e:smoke && npm run test:e2e:cdp", "test:e2e:cdp": "playwright test --config e2e-tests/playwright-cdp.config.ts", + "test:e2e:headless": "xvfb-run --auto-servernum --server-args=\"-screen 0 1280x960x24\" npm run test:e2e", "test:e2e:smoke": "playwright test --config e2e-tests/playwright.config.ts --project=smoke", "core:start": "npm --prefix ../paranext-core start", "core:stop": "npm --prefix ../paranext-core stop", From 536cb159846412729d0863a9ed627b095f9a6295 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 11:57:31 -0600 Subject: [PATCH 12/24] Reject non-positive PIDs in killProcessTree --- e2e-tests/process-utils.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/e2e-tests/process-utils.ts b/e2e-tests/process-utils.ts index 4d63b26c..bef683a6 100644 --- a/e2e-tests/process-utils.ts +++ b/e2e-tests/process-utils.ts @@ -17,12 +17,20 @@ import { execFileSync } from 'child_process'; * negative PID to signal the whole group in one call, falling back to the bare PID if the group * is already gone. * - * @param pid PID of the detached process to kill. + * @param pid PID of the detached process to kill. Non-positive values are rejected: `0` would + * target the caller's own process group (`process.kill(-0)` === `process.kill(0)`) and a negative + * value would signal an arbitrary unrelated PID, so both are treated as "nothing to kill". * @param signal POSIX signal to send when not on Windows (ignored on Windows, which always * force-kills). Defaults to `'SIGTERM'`. - * @returns `true` if a kill was issued, `false` if the process was already gone. + * @returns `true` if a kill was issued, `false` if the PID was invalid or the process was already + * gone. */ export function killProcessTree(pid: number, signal: NodeJS.Signals = 'SIGTERM'): boolean { + if (pid <= 0) { + // Guard against a malformed PID (e.g. parseInt yielding 0 or a negative from a corrupt PID + // file): signaling -0/0 would target our own process group and a negative PID an unrelated one. + return false; + } if (process.platform === 'win32') { try { execFileSync('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore' }); From 1616cd140d137707bbf8930d06fbb6f63f4e3664 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 12:09:50 -0600 Subject: [PATCH 13/24] Retry Windows paranext-core npm ci to survive cache-race flake The e2e job's full core install intermittently fails on windows-latest with EEXIST in the shared npm cache; retry up to 3x with cache verify between attempts. Linux keeps its single-shot install. --- .github/workflows/test.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f9ef1624..9761d15d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -88,12 +88,34 @@ jobs: # transpile TypeScript on Windows. run: npm ci --ignore-scripts - - name: Install core dependencies + - name: Install core dependencies (Linux) + if: matrix.os == 'ubuntu-latest' working-directory: paranext-core # Cannot --omit=optional: lightningcss native binaries are optional deps required by the # webpack build. run: npm ci --ignore-scripts + - name: Install core dependencies (Windows) + if: matrix.os == 'windows-latest' + working-directory: paranext-core + shell: pwsh + # Same install as Linux (no --omit=optional: lightningcss native binaries are optional deps + # required by the webpack build), but retried: this full install fetches the most packages + # through the shared C:\npm\cache and intermittently races on _cacache\tmp (EEXIST) or hits a + # locked node_modules file during npm's pre-install cleanup (EPERM rmdir). `npm cache verify` + # between attempts evicts the corrupt/partial cache entry so the retry starts clean. + run: | + $ErrorActionPreference = 'Continue' + for ($attempt = 1; $attempt -le 3; $attempt++) { + Write-Host "npm ci attempt $attempt of 3" + npm ci --ignore-scripts + if ($LASTEXITCODE -eq 0) { exit 0 } + Write-Host "npm ci failed (exit $LASTEXITCODE); verifying cache before retry" + npm cache verify + } + Write-Error 'npm ci failed after 3 attempts' + exit 1 + - name: Install Electron binary working-directory: paranext-core run: node node_modules/electron/install.js From dc77c3f98431da8cdd57f5cdbbbe5fb314b7913e Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 10 Jul 2026 13:52:38 -0600 Subject: [PATCH 14/24] chore: nudge CI to re-report CodeRabbit status From c744b1947c47ea8cc8ad8cbbaf2d862990bee760 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 11:13:21 -0600 Subject: [PATCH 15/24] Stop leaked project-picker tabs cascading CDP e2e failures The "Open Interlinearizer" picker is a floating dock tab that opening a project never disposes; on the shared CDP instance it accumulated until a bare .select-project-dialog read tripped strict mode and reddened every downstream test. Scope the locator to .first(), close the picker after selecting a project, and self-heal leftover pickers in the open precondition. Also log npm cache verify failures during the Windows ci retry. --- .github/workflows/test.yml | 1 + e2e-tests/fixtures/helpers.ts | 86 +++++++++++++++++++++++++++++++---- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9761d15d..3b78a0df 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -112,6 +112,7 @@ jobs: if ($LASTEXITCODE -eq 0) { exit 0 } Write-Host "npm ci failed (exit $LASTEXITCODE); verifying cache before retry" npm cache verify + if ($LASTEXITCODE -ne 0) { Write-Host "npm cache verify failed (exit $LASTEXITCODE)" } } Write-Error 'npm ci failed after 3 attempts' exit 1 diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 89314eff..cdf31c8c 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -544,10 +544,14 @@ export async function openInterlinearizerFromScriptureEditor( // When the editor has no project selected, the command calls papi.dialogs.selectProject, which // opens a floating "Open Interlinearizer" dock tab with the project list. When the editor // already has a project (a warm instance), the Interlinearizer tab opens directly instead. - const selectProjectDialog = page.locator('.select-project-dialog'); + // + // `.first()` throughout: on the shared CDP instance a prior test (or a prior call in this one) + // can leave the picker's floating dock tab mounted, so `.select-project-dialog` may match more + // than one element. Scoping to `.first()` keeps every read here — the union visibility wait AND + // the isVisible() branch — out of strict-mode violation, so a leaked picker can't crash this + // test before closeSelectProjectPickers has a chance to clean it up. See closeSelectProjectPickers. + const selectProjectDialog = page.locator('.select-project-dialog').first(); const interlinearizerTab = interlinearizerTabLocator(page); - // `.first()` on the whole `.or()`: if the dialog and the tab are ever both present the union - // resolves to two elements, which would trip strict mode on this visibility assertion. await expect(selectProjectDialog.or(interlinearizerTab).first()).toBeVisible({ timeout: 15_000 }); if (await selectProjectDialog.isVisible()) { const projectNameRegex = new RegExp(`^${escapedProjectName}$`, 'i'); @@ -557,6 +561,62 @@ export async function openInterlinearizerFromScriptureEditor( // Wait for the Interlinearizer tab to appear and focus it. await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); await interlinearizerTab.click(); + + // Close the "Open Interlinearizer" picker tab we just opened. Selecting a project opens the + // Interlinearizer but leaves the picker's floating dock tab mounted; on the shared CDP instance, + // where the DOM is never reset between tests, one leaks per open and they accumulate until a bare + // `.select-project-dialog` read trips strict mode across the whole suite. Closing it here stops + // the leak at its source. Bounded and best-effort: the picker only appears on the cold path, and + // a failure to close it is self-healed by closeSelectProjectPickers before the next test runs. + await closeSelectProjectPickers(page); +} + +/** + * Close every "Open Interlinearizer" project-picker dock tab currently mounted. The picker is the + * floating dock tab that `papi.dialogs.selectProject` opens (title "Open Interlinearizer", + * containing a `.select-project-dialog` panel); selecting a project from it opens the + * Interlinearizer but does not dispose the picker itself. + * + * On the shared CDP instance the renderer DOM is never reset between tests, so each leaked picker + * persists and a bare `.select-project-dialog` locator resolves to N elements — which trips + * Playwright strict mode on the very next `isVisible()`/`click()` and reddens every downstream + * test. This closes them via each tab's `.dock-tab-close-btn` (dispatched, mirroring + * {@link closeInterlinearizerTab}, so an off-viewport tab on small CI viewports still closes). + * + * Best-effort and non-throwing: a picker that refuses to close must not fail the caller, because + * the picker is incidental cleanup, not the behavior under test. It is bounded so a tab that won't + * close can't spin forever. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @returns Resolves once no picker tab remains, or after the bounded attempts are exhausted (a + * no-op on the common warm path where no picker was ever opened). + */ +export async function closeSelectProjectPickers(page: Page): Promise { + const pickerTab = page.locator('.dock-tab', { hasText: 'Open Interlinearizer' }); + + // Bounded so a picker that refuses to close can't spin forever. The realistic worst case is a + // handful of leaked pickers from earlier crashed tests plus the one this call opened. + for (let attempt = 0; attempt < 6; attempt += 1) { + // eslint-disable-next-line no-await-in-loop + const remaining = await pickerTab.count(); + if (remaining === 0) return; + // eslint-disable-next-line no-await-in-loop + await pickerTab + .first() + .locator('.dock-tab-close-btn') + .dispatchEvent('click') + .catch(() => { + /* The tab may have closed between the count() and the dispatch; the next loop re-checks. */ + }); + // Wait for this close to land (count drops) before the next iteration, so we don't race the + // dock's async tab removal and re-dispatch against the same, still-present tab. + // eslint-disable-next-line no-await-in-loop + await expect(pickerTab) + .toHaveCount(remaining - 1, { timeout: 5_000 }) + .catch(() => { + /* Slow removal or another picker took its place; the next iteration re-reads and retries. */ + }); + } } /** @@ -670,9 +730,14 @@ export async function dismissLeftoverModals(page: Page): Promise { * with WEB — see e2e-tests/README.md); otherwise the tab is opened fresh via the Scripture Editor * menu flow. Resolves only once the extension's toolbar has rendered inside the iframe. * - * As the first shared precondition every feature test runs, this also self-heals any modal a prior - * failed test left mounted (via {@link dismissLeftoverModals}) so a leftover overlay can't intercept - * this test's clicks — see that helper for why the shared CDP instance needs it. + * As the first shared precondition every feature test runs, this also self-heals two kinds of + * leftover state a prior failed test can leave on the shared CDP instance: any modal left mounted + * in the iframe (via {@link dismissLeftoverModals}, whose overlay would otherwise intercept this + * test's clicks) and any "Open Interlinearizer" project-picker dock tab left open (via + * {@link closeSelectProjectPickers}, whose accumulation would otherwise trip strict mode on the + * `.select-project-dialog` locator). A test that dies mid-open — e.g. the renderer is torn down + * before it can close its picker — is exactly how one picker leaks and then reddens every + * downstream test, so clearing it here is what keeps that single crash from cascading. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @returns Resolves when the Interlinearizer tab is focused and its toolbar is interactive. @@ -703,9 +768,14 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise timeout: 30_000, }); - // Self-heal the shared instance: clear any modal a prior failed test left mounted before this - // test starts driving the UI, so its overlay can't intercept the clicks below. + // Self-heal the shared instance before this test starts driving the UI: clear any modal a prior + // failed test left mounted (its overlay would intercept the clicks below) and any project-picker + // dock tab a prior test left open (its accumulation would trip strict mode on the + // `.select-project-dialog` locator). The picker cleanup covers the warm path too, where + // openInterlinearizerFromScriptureEditor never runs and so never gets a chance to close a picker + // leaked by an earlier crash. await dismissLeftoverModals(page); + await closeSelectProjectPickers(page); } /** From e2090f21662d542ad3e0a9b6b4c753429276fccf Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 12:04:45 -0600 Subject: [PATCH 16/24] Stop e2e nav hanging on the disabled platform toolbar control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit navigateToScriptureRef drove the platform toolbar's book-chapter control, which is permanently disabled in simple interface mode with no main-editor project (the CDP instance's state) — so trigger.click() retried for the full test timeout, hanging every navigating feature test for 6 minutes. Wait a bounded time for the control to enable and skip navigation if it never does; the view already shows its default reference and callers assert on tokens. --- e2e-tests/fixtures/helpers.ts | 38 +++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index cdf31c8c..050a401b 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -784,13 +784,47 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise * submits it. Requires a fully-qualified reference (book, chapter, and verse — e.g. `"GEN 1:1"`); * partial references are ambiguous and are not auto-submitted by the control. * + * The platform toolbar's book-chapter control only drives navigation when there is a resolved + * navigation-target web view — in simple (non-power) interface mode that is always the MAIN + * Scripture editor, never a focused secondary tab like the Interlinearizer (see paranext-core + * navigation-target.util.ts `resolveTargetWebView` / `pinToMainEditor`). The self-launched CDP + * instance comes up in simple mode with no project loaded into the main editor, so the control + * stays `disabled` and can navigate nothing. A plain `trigger.click()` there never fails — + * Playwright retries the click for the whole test timeout waiting for the button to become enabled, + * which is what hung every feature test for 6 minutes. So this waits a BOUNDED time for the control + * to become actionable and, if it never does, skips navigation instead of hanging: the + * Interlinearizer opens at its default reference (GEN 1:1) and the callers assert against specific + * verse tokens with their own visibility waits, so a skipped no-op navigation to a reference + * already on screen still lets the test proceed (and a genuinely-needed navigation that could not + * happen surfaces as a fast, clear assertion failure downstream rather than an opaque timeout + * here). + * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param reference Fully-qualified scripture reference to navigate to (e.g. `"GEN 1:1"`). - * @returns Resolves when the reference popover has closed after submitting. - * @throws If the reference control does not open, or the popover does not close after submitting. + * @returns Resolves once the reference has been submitted, or once navigation has been skipped + * because the toolbar control is not drivable in the current interface mode. + * @throws If the control is enabled but its popover does not open or close within the timeouts. */ export async function navigateToScriptureRef(page: Page, reference: string): Promise { const trigger = page.locator('button[aria-label="book-chapter-trigger"]').first(); + + // Bounded wait for the control to become enabled. In simple mode with no main-editor project it + // never enables, so cap the wait and skip rather than let the later click() burn the test timeout. + // `expect(...).toBeEnabled` polls with a real timeout (unlike `Locator.isEnabled`, which reads + // once); swallow its rejection into a boolean so the disabled case is a skip, not a throw. + const enabled = await expect(trigger) + .toBeEnabled({ timeout: 10_000 }) + .then(() => true) + .catch(() => false); + if (!enabled) { + console.log( + `navigateToScriptureRef: skipping navigation to "${reference}" — the platform book-chapter ` + + 'control is disabled (simple interface mode with no navigable target). The view stays at ' + + 'its current/default reference.', + ); + return; + } + await trigger.click(); const input = page.locator('input[cmdk-input]').first(); await expect(input).toBeVisible({ timeout: 5_000 }); From 718ac7098cafabecbe6d139dc212970d6ed1736e Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 13:01:12 -0600 Subject: [PATCH 17/24] Stop one stray "Unknown" tab cascading the shared CDP e2e tier The dock-readiness gate failed if any tab anywhere said "Unknown", so a single leftover panel poisoned every later feature test on the shared instance. Make it accept a lenient mode (dock mounted + >=1 tab resolved) for CDP feature tests while smoke/cold-start stay strict, report a closed-page teardown distinctly, and capture the smoke app's stdout. --- .github/workflows/test.yml | 1 + e2e-tests/fixtures/helpers.ts | 149 +++++++++++++++--- e2e-tests/global-setup-cdp.ts | 4 +- .../example-interlinearizer-feature.spec.ts | 10 +- .../tests/features/draft-persistence.spec.ts | 4 +- .../tests/features/gloss-roundtrip.spec.ts | 4 +- .../tests/features/project-modals.spec.ts | 4 +- 7 files changed, 151 insertions(+), 25 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3b78a0df..9bfdd398 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -156,6 +156,7 @@ jobs: retention-days: 7 path: | extension-repo/e2e-tests/.cdp-app-startup.log + extension-repo/e2e-tests/.smoke-app-startup.log extension-repo/e2e-tests/playwright-report/ extension-repo/e2e-tests/test-results/ if-no-files-found: warn diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 050a401b..6b565d1e 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -18,6 +18,15 @@ const DEFAULT_WEBSOCKET_PORT = 8876; const RPC_DISCOVER_POLL_INTERVAL_MS = 250; export const PROCESS_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; +/** + * File the smoke launcher streams the app's main-process stdout/stderr to. Named to mirror the CDP + * tier's `.cdp-app-startup.log` and kept in `e2e-tests/` (a directory Playwright does not clear, + * and one already covered by the CI artifact upload's `include-hidden-files: true`), so a + * cold-start stall's main-process log survives a failed run. Overwritten on each launch — one + * worker's app at a time — so it always holds the most recent launch's output. + */ +const SMOKE_APP_LOG_FILE = path.join(__dirname, '.smoke-app-startup.log'); + /** * Same serialized request type as `registerCommand('platform.about', ...)` in command.service * (`command` + `:` + `platform.about`). @@ -168,11 +177,26 @@ export async function launchElectronWithExtension( throw error; } + // Stream the launched app's main-process stdout/stderr to a log file. Unlike the CDP tier, the + // smoke launcher previously discarded this output, so a cold-start startup stall (the app comes up + // with dock tabs stuck at "Unknown" and never resolves — a silent platform-side race we can only + // tolerate, not fix) left NO main-process evidence to diagnose from; the renderer console was + // empty. Capturing it here means the next stall leaves a log to inspect. Best-effort: the log is + // not required, and any write error is swallowed so logging never fails a launch. + const appLog = fs.createWriteStream(SMOKE_APP_LOG_FILE, { flags: 'w' }); + appLog.on('error', () => { + /* Logging is best-effort; never let a log write failure break the launch. */ + }); + const appProcess = electronApp.process(); + appProcess.stdout?.pipe(appLog); + appProcess.stderr?.pipe(appLog); + console.log('Waiting for WebSocket server on port 8876...'); try { await waitForWebSocketReady(DEFAULT_WEBSOCKET_PORT, PROCESS_READY_TIMEOUT); } catch (error) { console.error('WebSocket readiness check failed after Electron launch:', error); + dumpSmokeAppLog(); const proc = electronApp.process(); if (proc?.pid) { try { @@ -199,6 +223,25 @@ export async function launchElectronWithExtension( return { electronApp, userDataDir, appClosed }; } +/** + * Echo the smoke launcher's captured main-process output ({@link SMOKE_APP_LOG_FILE}) to the + * console. Called when the app fails to open its WebSocket port so the startup failure's cause + * appears inline in the CI log, not only in the uploaded artifact. Best-effort: a missing or + * unreadable log is reported, never thrown. + * + * @returns Nothing; logging-only. + */ +function dumpSmokeAppLog(): void { + try { + const log = fs.readFileSync(SMOKE_APP_LOG_FILE, 'utf-8'); + console.error( + `--- Launched Platform.Bible (smoke) output (${SMOKE_APP_LOG_FILE}) ---\n${log || '(empty)'}\n--- end app output ---`, + ); + } catch { + console.error(`Could not read smoke app log at ${SMOKE_APP_LOG_FILE}.`); + } +} + /** * Tear down an Electron instance: kill the process group, wait for close, and clean up the isolated * user-data directory. @@ -391,59 +434,121 @@ export async function waitForPapiMethodRegistered( throw new Error(`PAPI method "${methodName}" not listed in rpc.discover within ${timeoutMs}ms`); } +/** Options for {@link waitForDockTabTitlesResolved}. */ +interface DockTabTitlesOptions { + /** + * How to judge the dock is ready: + * + * - `true` (cold-start): EVERY dock tab must have a resolved (non-"Unknown") title. Correct for a + * fresh per-worker instance (smoke tests, CDP global setup): a cold instance whose tabs are all + * still "Unknown" is genuinely broken, and there are no unrelated tabs to interfere. + * - `false` (shared/warm): the dock is mounted and AT LEAST ONE tab has a resolved title. Correct + * for the shared CDP feature instance, which global setup already settled before any test ran. + * Re-asserting the strict "no tab anywhere is Unknown" invariant per test is both redundant and + * fragile there: a single stray/leftover panel (e.g. one briefly re-titled by a close/reopen + * cycle) would fail the gate for EVERY subsequent test against that one shared instance, and no + * per-test retry can recover it — the Windows CDP-tier cascade this whole run showed. + */ + strict: boolean; +} + /** - * Wait until the dock layout has at least one tab and no tab is still titled "Unknown". On a cold - * start the dock mounts with webview tabs titled "Unknown" (and blank panels) until project + * Wait until the dock layout is mounted with resolved tab titles (none stuck at "Unknown"). On a + * cold start the dock mounts with webview tabs titled "Unknown" (and blank panels) until project * metadata resolves; every tab-title-based locator in this suite silently times out against that * state. Waiting it out here turns those opaque per-test locator timeouts into either a pass (slow - * healthy startup) or one clear early failure (instance genuinely stuck — the Windows CI CDP-tier - * wipeout). + * healthy startup) or one clear early failure. + * + * The strictness of "resolved" depends on {@link DockTabTitlesOptions.strict} — see that field for + * why the shared CDP instance must not use the strict all-tabs check. + * + * A torn-down renderer (page/context/browser closed out from under us — the actual symptom in the + * Windows CDP run, where `waitForFunction` reports "Target page … has been closed") is surfaced as + * its own error rather than mislabeled "tabs still Unknown", so the real cause is not buried. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param timeout Maximum time in milliseconds to wait before throwing. - * @returns Resolves when at least one dock tab exists and none contains "Unknown". - * @throws If tab titles have not resolved within `timeout` milliseconds. + * @param options Readiness options; see {@link DockTabTitlesOptions}. + * @returns Resolves once the dock is ready per the chosen `strict` mode. + * @throws If tab titles have not resolved within `timeout` milliseconds, or if the renderer page + * was closed while waiting. */ -export async function waitForDockTabTitlesResolved(page: Page, timeout: number): Promise { +export async function waitForDockTabTitlesResolved( + page: Page, + timeout: number, + options: DockTabTitlesOptions, +): Promise { + const { strict } = options; try { await page.waitForFunction( - () => { + (isStrict) => { const tabs = Array.from(document.querySelectorAll('.dock-tab')); - return tabs.length > 0 && tabs.every((tab) => !(tab.textContent ?? '').includes('Unknown')); + if (tabs.length === 0) return false; + const isResolved = (tab: Element) => !(tab.textContent ?? '').includes('Unknown'); + // Strict: no tab anywhere may be "Unknown". Lenient: at least one tab has resolved, which is + // enough to know the app is up and rendering real tabs on an already-settled shared instance. + return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); }, - undefined, + strict, { timeout, polling: 500 }, ); } catch (error) { + const message = error instanceof Error ? error.message : String(error); + // A closed page is a torn-down renderer, not a project-metadata stall — report it as such so it + // isn't chased as the "Unknown tabs" startup race (which it superficially resembles because the + // waitForFunction times out either way). + if (/target (page|context|browser) .*closed/i.test(message)) { + throw new Error( + `The renderer page was closed while waiting for the dock to become ready (after up to ` + + `${timeout}ms) — the app or its window went away mid-test, not a slow startup. ` + + `Original error: ${message}`, + ); + } throw new Error( `Dock tab titles did not resolve within ${timeout}ms — the app came up but its webview tabs ` + - 'are still titled "Unknown" (project metadata never loaded). ' + - `Original error: ${error instanceof Error ? error.message : String(error)}`, + `are still titled "Unknown" (project metadata never loaded). Original error: ${message}`, ); } } +/** Options for {@link waitForAppReady} and {@link waitForAppAndInterlinearizerReady}. */ +interface AppReadyOptions { + /** + * Timeout in milliseconds for the whole readiness wait. The default is generous because a cold CI + * instance has been observed taking over 45 s just to resolve tab titles; this is a wait-until, + * so healthy startups pay nothing for the headroom. + */ + timeout?: number; + /** + * Whether the dock-tab-title gate uses the strict cold-start check (every tab resolved) or the + * lenient shared-instance check (dock mounted, at least one tab resolved). Defaults to `true` + * (cold-start), correct for the fresh per-worker smoke instance. The CDP feature tests, which run + * against one shared, already-settled instance, pass `false` so one stray panel can't cascade + * across the whole tier — see {@link DockTabTitlesOptions.strict}. + */ + strict?: boolean; +} + /** * Wait for the Platform.Bible UI to be fully ready: dock layout appears with resolved tab titles * and `platform.about` command is registered (dialog service has finished initializing). * * @param page The Playwright `Page` for the Platform.Bible renderer window. - * @param timeout Maximum time in milliseconds to wait before throwing. The default is generous - * because a cold CI instance has been observed taking over 45 s just to resolve tab titles; this - * is a wait-until, so healthy startups pay nothing for the headroom. + * @param options Readiness options; see {@link AppReadyOptions}. * @returns Resolves when the dock layout is visible with resolved tab titles and `platform.about` * is registered. * @throws If the dock layout, resolved tab titles, or the `platform.about` command do not appear * within `timeout` milliseconds. */ -export async function waitForAppReady(page: Page, timeout = 120_000): Promise { +export async function waitForAppReady(page: Page, options: AppReadyOptions = {}): Promise { + const { timeout = 120_000, strict = true } = options; const start = Date.now(); await page.waitForSelector('div[class*="dock-layout"]', { state: 'attached', timeout, }); let remaining = Math.max(0, timeout - (Date.now() - start)); - await waitForDockTabTitlesResolved(page, remaining); + await waitForDockTabTitlesResolved(page, remaining, { strict }); remaining = Math.max(0, timeout - (Date.now() - start)); await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, remaining); } @@ -654,11 +759,17 @@ function interlinearizerTabLocator(page: Page): Locator { * {@link waitForAppReady} and {@link waitForInterlinearizerReady}. * * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param options Readiness options forwarded to {@link waitForAppReady}. Feature tests on the shared + * CDP instance pass `{ strict: false }` so one stray "Unknown" panel can't cascade across the + * tier; smoke tests (fresh per-worker instance) use the default strict cold-start gate. * @returns Resolves when `interlinearizer.openForWebView` is listed in `rpc.discover`. * @throws If the app or extension do not finish starting up within their default timeouts. */ -export async function waitForAppAndInterlinearizerReady(page: Page): Promise { - await waitForAppReady(page); +export async function waitForAppAndInterlinearizerReady( + page: Page, + options: AppReadyOptions = {}, +): Promise { + await waitForAppReady(page, options); await waitForInterlinearizerReady(); } diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index e64cc92a..f2974abc 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -242,7 +242,9 @@ async function waitForRendererSettled(timeout: number): Promise { if (!page) { throw new Error(`No renderer page appeared over CDP within ${timeout}ms`); } - await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now())); + // Strict cold-start gate: this is the freshly-launched instance's first settle, so every dock + // tab must resolve. The per-test feature gate is lenient (shared instance already settled here). + await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { strict: true }); } finally { // Disconnect only — connectOverCDP close() does not terminate the app. await browser.close(); diff --git a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts index 8ec02feb..9713fcf4 100644 --- a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts +++ b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts @@ -34,7 +34,10 @@ function filterConsoleErrors(errors: string[]): string[] { test.describe('Example: Open Interlinearizer via menu', () => { test('should open the interlinearizer WebView via menu', async ({ mainPage }) => { - await waitForAppAndInterlinearizerReady(mainPage); + // Feature tests run against the shared CDP instance, already settled by global setup. Pass + // `{ strict: false }` so one stray/leftover "Unknown" panel can't fail this (and every + // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. + await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); // Step 1: Click the top-level menu that contains the interlinearizer entry const menuTrigger = mainPage.getByRole('menuitem', { name: /Tools/i }); @@ -50,7 +53,10 @@ test.describe('Example: Open Interlinearizer via menu', () => { }); test('should render without critical console errors', async ({ mainPage }) => { - await waitForAppAndInterlinearizerReady(mainPage); + // Feature tests run against the shared CDP instance, already settled by global setup. Pass + // `{ strict: false }` so one stray/leftover "Unknown" panel can't fail this (and every + // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. + await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); const consoleErrors: string[] = []; mainPage.on('console', (msg) => { diff --git a/e2e-tests/tests/features/draft-persistence.spec.ts b/e2e-tests/tests/features/draft-persistence.spec.ts index 5f08c882..3c5bc954 100644 --- a/e2e-tests/tests/features/draft-persistence.spec.ts +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -16,7 +16,9 @@ test.describe('Draft persistence', () => { // Two full open cycles (plus first-use project creation and book loading on a cold instance) // legitimately exceed the default 120 s budget. test.slow(); - await waitForAppAndInterlinearizerReady(mainPage); + // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray + // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. + await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/gloss-roundtrip.spec.ts b/e2e-tests/tests/features/gloss-roundtrip.spec.ts index 93169946..1bdcee2f 100644 --- a/e2e-tests/tests/features/gloss-roundtrip.spec.ts +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -10,7 +10,9 @@ import { test.describe('Gloss round-trip', () => { test('typing a gloss on a token renders it in the gloss field', async ({ mainPage }) => { - await waitForAppAndInterlinearizerReady(mainPage); + // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray + // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. + await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts index e826b39f..520f352c 100644 --- a/e2e-tests/tests/features/project-modals.spec.ts +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -35,7 +35,9 @@ test.describe('Project modals cancel tour', () => { test(`the ${modal.name} modal opens from the Project menu and cancels cleanly`, async ({ mainPage, }) => { - await waitForAppAndInterlinearizerReady(mainPage); + // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray + // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. + await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); await ensureInterlinearizerOpenOnWeb(mainPage); const frame = await openInterlinearizerProjectMenu(mainPage); From 446ddcd66f1ae54f8566f2853c2d8b9cef5da3c6 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 13:31:21 -0600 Subject: [PATCH 18/24] Fail fast when a CDP feature test's shared instance has died MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The readiness wait used the 120s cold-start budget on every attempt, so a dead shared instance cost ~6 min per test (120s x 3 retries) before reporting. Add CDP_FEATURE_READY_TIMEOUT (30s), passed by the feature specs, and split waitForAppAndInterlinearizerReady's timeout across both the app-ready and extension-registration waits so the short budget caps the whole check — a dead instance now fails in ~90s. Smoke tests keep the generous cold-start default. --- e2e-tests/fixtures/helpers.ts | 41 ++++++++++++++++--- .../example-interlinearizer-feature.spec.ts | 21 +++++++--- .../tests/features/draft-persistence.spec.ts | 11 ++++- .../tests/features/gloss-roundtrip.spec.ts | 9 +++- .../tests/features/project-modals.spec.ts | 7 +++- 5 files changed, 73 insertions(+), 16 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 6b565d1e..da0f29b0 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -18,6 +18,16 @@ const DEFAULT_WEBSOCKET_PORT = 8876; const RPC_DISCOVER_POLL_INTERVAL_MS = 250; export const PROCESS_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; +/** + * Fail-fast readiness budget (ms) for a CDP feature test's per-test + * `waitForAppAndInterlinearizerReady`. The shared instance is proven-settled by global setup before + * any feature test runs, so a per-test readiness wait that runs long means the instance DIED + * mid-run — which no per-test retry can revive. Capping at 30 s (vs. the 120 s cold-start default) + * makes a dead shared instance fail in 30 s × the retry count instead of 120 s × it, the difference + * between a ~90 s and a 6-minute broken Windows run. + */ +export const CDP_FEATURE_READY_TIMEOUT = 30_000; + /** * File the smoke launcher streams the app's main-process stdout/stderr to. Named to mirror the CDP * tier's `.cdp-app-startup.log` and kept in `e2e-tests/` (a directory Playwright does not clear, @@ -557,15 +567,20 @@ export async function waitForAppReady(page: Page, options: AppReadyOptions = {}) * Wait for the interlinearizer extension to finish activating by polling `rpc.discover` until * `interlinearizer.openForWebView` is listed. * - * @param timeoutMs Maximum time in milliseconds to poll before throwing. + * @param timeoutMs Maximum time in milliseconds to poll before throwing. `undefined` selects the + * generous default (a cold instance can be slow to register the command); callers threading a + * shared budget pass the remaining time, clamped to a small floor so an already-exhausted budget + * still gets one real poll rather than throwing instantly. * @returns Resolves when `interlinearizer.openForWebView` is listed in `rpc.discover`. * @throws {Error} If the extension does not register within `timeoutMs` milliseconds. */ -export async function waitForInterlinearizerReady(timeoutMs = 90_000): Promise { +export async function waitForInterlinearizerReady( + timeoutMs: number | undefined = 90_000, +): Promise { await waitForPapiMethodRegistered( 'command:interlinearizer.openForWebView', DEFAULT_WEBSOCKET_PORT, - timeoutMs, + Math.max(1_000, timeoutMs ?? 90_000), ); } @@ -756,21 +771,35 @@ function interlinearizerTabLocator(page: Page): Locator { /** * Wait for Platform.Bible and the interlinearizer extension to finish starting up. Combines - * {@link waitForAppReady} and {@link waitForInterlinearizerReady}. + * {@link waitForAppReady} and {@link waitForInterlinearizerReady}, splitting the `timeout` budget + * across both so an explicit (shorter) budget caps the WHOLE wait, not just the first half. + * + * The CDP feature tier passes a short budget (`{ strict: false, timeout: ~30s }`): its shared + * instance is already proven-settled by global setup, so a per-test readiness wait that runs long + * means the instance DIED mid-run, not that startup is slow — and no per-test retry can revive a + * dead shared instance. Failing that in ~30s instead of the default 120s is what turns a broken + * Windows CDP run from a 6-minute-per-test crawl (120s × 3 attempts) into a ~90s one. Smoke tests + * (fresh per-worker instance, genuine cold start) omit `timeout` and keep the generous default. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param options Readiness options forwarded to {@link waitForAppReady}. Feature tests on the shared * CDP instance pass `{ strict: false }` so one stray "Unknown" panel can't cascade across the * tier; smoke tests (fresh per-worker instance) use the default strict cold-start gate. * @returns Resolves when `interlinearizer.openForWebView` is listed in `rpc.discover`. - * @throws If the app or extension do not finish starting up within their default timeouts. + * @throws If the app or extension do not finish starting up within the `timeout` budget. */ export async function waitForAppAndInterlinearizerReady( page: Page, options: AppReadyOptions = {}, ): Promise { + const start = Date.now(); await waitForAppReady(page, options); - await waitForInterlinearizerReady(); + // Cap the extension-registration wait by whatever budget remains, so an explicit short `timeout` + // bounds the combined wait. With no explicit budget (smoke), fall back to this helper's own + // generous default rather than starving it. + const remaining = + options.timeout === undefined ? undefined : options.timeout - (Date.now() - start); + await waitForInterlinearizerReady(remaining); } /** diff --git a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts index 9713fcf4..14f5c816 100644 --- a/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts +++ b/e2e-tests/tests/_example/example-interlinearizer-feature.spec.ts @@ -14,7 +14,10 @@ * This file is excluded from test runs — it's documentation only. */ import { test, expect } from '../../fixtures/cdp.fixture'; -import { waitForAppAndInterlinearizerReady } from '../../fixtures/helpers'; +import { + CDP_FEATURE_READY_TIMEOUT, + waitForAppAndInterlinearizerReady, +} from '../../fixtures/helpers'; /** * Filter out expected/benign console errors from a list of captured error messages. @@ -36,8 +39,12 @@ test.describe('Example: Open Interlinearizer via menu', () => { test('should open the interlinearizer WebView via menu', async ({ mainPage }) => { // Feature tests run against the shared CDP instance, already settled by global setup. Pass // `{ strict: false }` so one stray/leftover "Unknown" panel can't fail this (and every - // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. - await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); + // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. The short + // `timeout` fails fast when the shared instance has died mid-run (no retry can revive it). + await waitForAppAndInterlinearizerReady(mainPage, { + strict: false, + timeout: CDP_FEATURE_READY_TIMEOUT, + }); // Step 1: Click the top-level menu that contains the interlinearizer entry const menuTrigger = mainPage.getByRole('menuitem', { name: /Tools/i }); @@ -55,8 +62,12 @@ test.describe('Example: Open Interlinearizer via menu', () => { test('should render without critical console errors', async ({ mainPage }) => { // Feature tests run against the shared CDP instance, already settled by global setup. Pass // `{ strict: false }` so one stray/leftover "Unknown" panel can't fail this (and every - // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. - await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); + // downstream) test — see waitForDockTabTitlesResolved in fixtures/helpers.ts. The short + // `timeout` fails fast when the shared instance has died mid-run (no retry can revive it). + await waitForAppAndInterlinearizerReady(mainPage, { + strict: false, + timeout: CDP_FEATURE_READY_TIMEOUT, + }); const consoleErrors: string[] = []; mainPage.on('console', (msg) => { diff --git a/e2e-tests/tests/features/draft-persistence.spec.ts b/e2e-tests/tests/features/draft-persistence.spec.ts index 3c5bc954..ba80b729 100644 --- a/e2e-tests/tests/features/draft-persistence.spec.ts +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '../../fixtures/cdp.fixture'; import { + CDP_FEATURE_READY_TIMEOUT, closeInterlinearizerTab, ensureE2eProjectActive, ensureInterlinearizerOpenOnWeb, @@ -17,8 +18,14 @@ test.describe('Draft persistence', () => { // legitimately exceed the default 120 s budget. test.slow(); // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray - // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. - await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); + // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. Short + // budget: a long wait here means the shared instance died, so fail fast instead of burning 120s. + // (test.slow() above triples only the overall test timeout for the two open-cycles below, not + // this startup readiness check.) + await waitForAppAndInterlinearizerReady(mainPage, { + strict: false, + timeout: CDP_FEATURE_READY_TIMEOUT, + }); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/gloss-roundtrip.spec.ts b/e2e-tests/tests/features/gloss-roundtrip.spec.ts index 1bdcee2f..8efe5a8d 100644 --- a/e2e-tests/tests/features/gloss-roundtrip.spec.ts +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '../../fixtures/cdp.fixture'; import { + CDP_FEATURE_READY_TIMEOUT, ensureE2eProjectActive, ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, @@ -11,8 +12,12 @@ import { test.describe('Gloss round-trip', () => { test('typing a gloss on a token renders it in the gloss field', async ({ mainPage }) => { // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray - // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. - await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); + // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. Short + // budget: a long wait here means the shared instance died, so fail fast instead of burning 120s. + await waitForAppAndInterlinearizerReady(mainPage, { + strict: false, + timeout: CDP_FEATURE_READY_TIMEOUT, + }); await ensureInterlinearizerOpenOnWeb(mainPage); await ensureE2eProjectActive(mainPage); await navigateToScriptureRef(mainPage, 'GEN 1:1'); diff --git a/e2e-tests/tests/features/project-modals.spec.ts b/e2e-tests/tests/features/project-modals.spec.ts index 520f352c..2a0a16b6 100644 --- a/e2e-tests/tests/features/project-modals.spec.ts +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '../../fixtures/cdp.fixture'; import { + CDP_FEATURE_READY_TIMEOUT, ensureInterlinearizerOpenOnWeb, getInterlinearizerFrame, openInterlinearizerProjectMenu, @@ -37,7 +38,11 @@ test.describe('Project modals cancel tour', () => { }) => { // Lenient gate: the shared CDP instance was already settled by global setup, so a single stray // panel must not fail this (and every downstream) test — see waitForDockTabTitlesResolved. - await waitForAppAndInterlinearizerReady(mainPage, { strict: false }); + // Short budget: a long wait here means the shared instance died, so fail fast, not in 120s. + await waitForAppAndInterlinearizerReady(mainPage, { + strict: false, + timeout: CDP_FEATURE_READY_TIMEOUT, + }); await ensureInterlinearizerOpenOnWeb(mainPage); const frame = await openInterlinearizerProjectMenu(mainPage); From 6e137cc7bcdcc7b2e097a11c5b2ef74523120841 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 14:05:01 -0600 Subject: [PATCH 19/24] Gate e2e readiness on service-host registration Wait on the settings/menu-data/theme data-provider objects in rpc.discover before the dock-tab-title check, so a slow cold start resolves against its real cause instead of an opaque "Unknown"-tab timeout. Also fast-fail the cold-start wait on the fatal theme-settle page error, turning a 120s dead wait into a fast, labeled failure that retries onto a fresh launch. --- e2e-tests/fixtures/helpers.ts | 212 +++++++++++++++++++++++++++++----- e2e-tests/global-setup-cdp.ts | 42 ++++++- 2 files changed, 226 insertions(+), 28 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index da0f29b0..7a08049e 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -43,6 +43,48 @@ const SMOKE_APP_LOG_FILE = path.join(__dirname, '.smoke-app-startup.log'); */ const PLATFORM_ABOUT_COMMAND = 'command:platform.about'; +/** + * `rpc.discover` method names that flip present exactly when paranext-core's settings, menu-data, + * and theme service hosts finish registering their data-provider network objects — the upstream + * signal that actually gates a resolved dock (see {@link waitForServiceHostsRegistered}). + * + * Each service host exposes its provider as a data-provider network object, whose JSON-RPC handlers + * are enumerated by `rpc.discover` the instant it registers. Data providers append a `-data` suffix + * to the provider name and network-object request types are prefixed `object:`, so the existence + * handler for the settings provider `platform.settingsServiceDataProvider` serializes to + * `object:platform.settingsServiceDataProvider-data`. paranext-core's own smoke suite waits on the + * settings provider's `.set` method for the same "settings host is up" purpose, so the shape is + * load-bearing there too. + * + * These are the bare EXISTENCE handlers (`object:{id}`, no method suffix), not a named method like + * `.set`/`.getCurrentTheme`: `networkObjectService.set` registers the existence handler + * unconditionally as the first step of registering any network object — before fanning out the + * per-method handlers — so it is the canonical "this object is on the network" signal and can't be + * invalidated by a provider method being renamed. + */ +const SERVICE_HOST_OBJECT_METHODS = [ + 'object:platform.settingsServiceDataProvider-data', + 'object:platform.menuDataServiceDataProvider-data', + 'object:platform.themeServiceDataProvider-data', +]; + +/** + * Renderer page error that means this cold start is doomed, not merely slow. paranext-core's theme + * service host arms a 30 s `AsyncVariable` for `theme.service-host.allThemeFamiliesById`; if the + * extension host never pushes the initial theme data within that window it rejects with this + * message, and the dock is left with its webview tabs stuck at "Unknown" for the remainder of the + * run (the exact 120 s stall this suite kept eating on Windows/Linux cold starts). + * + * Crucially this fires AFTER the theme provider's network object has already registered — the host + * registers its data provider, then separately awaits the data to settle — so the positive + * {@link waitForServiceHostsRegistered} gate can pass while the renderer is still headed for this + * timeout. That is why the tab-title wait additionally fast-fails on this specific error rather + * than waiting out its full budget: once it fires, no amount of further waiting recovers the + * launch, and a fresh launch (a smoke retry, or a re-run of CDP setup) is the only path forward. + */ +const FATAL_STARTUP_PAGE_ERROR = + /Timeout reached when waiting for .*allThemeFamiliesById to settle/i; + /** * Keep in sync with GET_METHODS from @shared/data/rpc.model. Required to be 'rpc.discover' by the * OpenRPC specification. @@ -444,6 +486,39 @@ export async function waitForPapiMethodRegistered( throw new Error(`PAPI method "${methodName}" not listed in rpc.discover within ${timeoutMs}ms`); } +/** + * Wait for paranext-core's settings, menu-data, and theme service hosts to finish registering, by + * polling `rpc.discover` for each host's data-provider existence handler + * ({@link SERVICE_HOST_OBJECT_METHODS}). + * + * This is the UPSTREAM readiness signal for a resolved dock. On a cold start the renderer paints + * its webview tabs titled "Unknown" (and its panels blank) until the metadata these hosts serve + * arrives; the renderer even throws transient `Settings service undefined` / `Menu data service + * undefined` page errors while it retries against not-yet-registered providers. Gating here — + * before {@link waitForDockTabTitlesResolved} — absorbs that cold-start race into the readiness wait + * instead of letting it surface downstream as an opaque tab-title timeout: waiting on the hosts + * directly means the tab-title wait only ever runs once the data behind those titles actually + * exists. On a healthy startup the hosts are already up, so this resolves immediately and costs + * nothing; the poll uses the same `rpc.discover` mechanism as every other readiness check here. + * + * The three waits run concurrently and share the one `timeout` budget (the hosts register in + * parallel — settings and menu-data in the extension host, theme in the renderer — so serializing + * would triple the worst-case wait for no benefit). + * + * @param timeout Maximum time in milliseconds to wait for all three hosts. Floored to a small + * positive value so an already-thin remaining budget still gets one real poll. + * @returns Resolves once all three service-host providers are listed in `rpc.discover`. + * @throws {Error} If any of the three hosts is not registered within `timeout` milliseconds. + */ +export async function waitForServiceHostsRegistered(timeout: number): Promise { + const budget = Math.max(1_000, timeout); + await Promise.all( + SERVICE_HOST_OBJECT_METHODS.map((method) => + waitForPapiMethodRegistered(method, DEFAULT_WEBSOCKET_PORT, budget), + ), + ); +} + /** Options for {@link waitForDockTabTitlesResolved}. */ interface DockTabTitlesOptions { /** @@ -460,6 +535,16 @@ interface DockTabTitlesOptions { * per-test retry can recover it — the Windows CDP-tier cascade this whole run showed. */ strict: boolean; + /** + * When set, abort the wait the moment the renderer emits a {@link FATAL_STARTUP_PAGE_ERROR} — + * turning a doomed cold start (theme data never settled, tabs stuck "Unknown" for the full + * timeout) into a fast, correctly-labeled failure instead of a 120 s dead wait. Only meaningful + * on a cold start (a fresh smoke launch or CDP global setup, where the error signals _this_ + * launch failed and a fresh one is the only remedy), so it is opt-in; the warm shared-instance + * path leaves it off, where a stale error from a long-past cold start must not abort an + * otherwise-healthy wait. + */ + failFastOnFatalPageError?: boolean; } /** @@ -476,34 +561,84 @@ interface DockTabTitlesOptions { * Windows CDP run, where `waitForFunction` reports "Target page … has been closed") is surfaced as * its own error rather than mislabeled "tabs still Unknown", so the real cause is not buried. * + * With {@link DockTabTitlesOptions.failFastOnFatalPageError}, a doomed cold start (the renderer's + * theme data never settling — see {@link FATAL_STARTUP_PAGE_ERROR}) aborts the wait immediately + * instead of running out the full budget, since that failure never self-recovers. + * * @param page The Playwright `Page` for the Platform.Bible renderer window. - * @param timeout Maximum time in milliseconds to wait before throwing. + * @param timeout Maximum time in milliseconds to wait before throwing. Must be positive: a + * non-positive value means the caller's readiness budget is already exhausted, and is failed fast + * rather than forwarded — Playwright treats `waitForFunction({ timeout: 0 })` as "no timeout" (an + * unbounded wait), so a `0` here would silently turn an exhausted budget into a hang on the exact + * "Unknown"-tab stall this helper exists to bound. * @param options Readiness options; see {@link DockTabTitlesOptions}. * @returns Resolves once the dock is ready per the chosen `strict` mode. - * @throws If tab titles have not resolved within `timeout` milliseconds, or if the renderer page - * was closed while waiting. + * @throws If `timeout` is non-positive (budget exhausted before this wait began), if tab titles + * have not resolved within `timeout` milliseconds, if the renderer emitted a fatal startup error + * (with `failFastOnFatalPageError`), or if the renderer page was closed while waiting. */ export async function waitForDockTabTitlesResolved( page: Page, timeout: number, options: DockTabTitlesOptions, ): Promise { - const { strict } = options; - try { - await page.waitForFunction( - (isStrict) => { - const tabs = Array.from(document.querySelectorAll('.dock-tab')); - if (tabs.length === 0) return false; - const isResolved = (tab: Element) => !(tab.textContent ?? '').includes('Unknown'); - // Strict: no tab anywhere may be "Unknown". Lenient: at least one tab has resolved, which is - // enough to know the app is up and rendering real tabs on an already-settled shared instance. - return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); - }, - strict, - { timeout, polling: 500 }, + const { strict, failFastOnFatalPageError = false } = options; + // A non-positive budget must not reach page.waitForFunction: { timeout: 0 } disables Playwright's + // timeout entirely (an unbounded wait), so an already-exhausted budget would hang instead of + // failing. Fail fast with a clear message instead. + if (timeout <= 0) { + throw new Error( + `Dock tab titles could not be waited for: the readiness budget was exhausted before this ` + + `wait began (timeout=${timeout}ms). An earlier startup stage consumed the whole timeout.`, ); + } + + // Fatal-startup-error tripwire: reject the moment the renderer emits FATAL_STARTUP_PAGE_ERROR, so + // the wait below can lose the race to it and abort a doomed cold start early. The listener is + // registered only when opted in, and always removed in `finally` so it can't leak across tests on + // the shared CDP page. A sentinel value distinguishes "tripwire fired" from an ordinary reject. + const fatalError: { message?: string } = {}; + let onFatalPageError: ((err: Error) => void) | undefined; + const fatalErrorTripped = new Promise((_resolve, reject) => { + if (!failFastOnFatalPageError) return; + onFatalPageError = (err: Error) => { + if (!FATAL_STARTUP_PAGE_ERROR.test(err.message)) return; + fatalError.message = err.message; + reject(new Error(err.message)); + }; + page.on('pageerror', onFatalPageError); + }); + // Without an opted-in tripwire nothing ever rejects this promise; swallow the unused rejection + // path so a stray rejection (there is none) could never surface as an unhandled rejection. + fatalErrorTripped.catch(() => {}); + + try { + await Promise.race([ + page.waitForFunction( + (isStrict) => { + const tabs = Array.from(document.querySelectorAll('.dock-tab')); + if (tabs.length === 0) return false; + const isResolved = (tab: Element) => !(tab.textContent ?? '').includes('Unknown'); + // Strict: no tab anywhere may be "Unknown". Lenient: at least one tab has resolved, which + // is enough to know the app is up and rendering real tabs on an already-settled instance. + return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); + }, + strict, + { timeout, polling: 500 }, + ), + fatalErrorTripped, + ]); } catch (error) { const message = error instanceof Error ? error.message : String(error); + // The tripwire won the race: this cold start is doomed (theme data never settled), so report it + // as its own fast failure rather than the generic "tabs still Unknown" timeout. A smoke retry or + // a re-run of CDP setup relaunches the app, which is the only thing that recovers this. + if (fatalError.message !== undefined) { + throw new Error( + `Platform.Bible startup failed: the renderer reported a fatal startup error, so its dock ` + + `tabs will never resolve on this launch (a fresh launch is required). Error: ${fatalError.message}`, + ); + } // A closed page is a torn-down renderer, not a project-metadata stall — report it as such so it // isn't chased as the "Unknown tabs" startup race (which it superficially resembles because the // waitForFunction times out either way). @@ -518,6 +653,8 @@ export async function waitForDockTabTitlesResolved( `Dock tab titles did not resolve within ${timeout}ms — the app came up but its webview tabs ` + `are still titled "Unknown" (project metadata never loaded). Original error: ${message}`, ); + } finally { + if (onFatalPageError) page.off('pageerror', onFatalPageError); } } @@ -540,15 +677,23 @@ interface AppReadyOptions { } /** - * Wait for the Platform.Bible UI to be fully ready: dock layout appears with resolved tab titles - * and `platform.about` command is registered (dialog service has finished initializing). + * Wait for the Platform.Bible UI to be fully ready: dock layout attaches, the settings/menu-data/ + * theme service hosts register, the dock tab titles resolve, and `platform.about` is registered + * (dialog service has finished initializing). + * + * The service-host wait is what makes the tab-title wait meaningful rather than a downstream guess: + * the tabs stay titled "Unknown" precisely until those hosts serve their metadata, so waiting on + * the hosts first (see {@link waitForServiceHostsRegistered}) means the tab-title poll only runs + * once the data behind the titles exists — turning the cold-start "Unknown for the full timeout" + * stall from an opaque per-test tab-title timeout into an early, correctly-attributed wait on the + * actual cause. On a healthy startup the hosts are already up, so this stage resolves immediately. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param options Readiness options; see {@link AppReadyOptions}. - * @returns Resolves when the dock layout is visible with resolved tab titles and `platform.about` - * is registered. - * @throws If the dock layout, resolved tab titles, or the `platform.about` command do not appear - * within `timeout` milliseconds. + * @returns Resolves when the dock layout is visible with resolved tab titles, the service hosts are + * registered, and `platform.about` is registered. + * @throws If the dock layout, service hosts, resolved tab titles, or the `platform.about` command + * do not appear within `timeout` milliseconds. */ export async function waitForAppReady(page: Page, options: AppReadyOptions = {}): Promise { const { timeout = 120_000, strict = true } = options; @@ -557,10 +702,25 @@ export async function waitForAppReady(page: Page, options: AppReadyOptions = {}) state: 'attached', timeout, }); - let remaining = Math.max(0, timeout - (Date.now() - start)); - await waitForDockTabTitlesResolved(page, remaining, { strict }); - remaining = Math.max(0, timeout - (Date.now() - start)); - await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, remaining); + // Floor each leftover budget to a small positive value, never 0: a preceding stage can resolve at + // the very last millisecond (or wall-clock drift can push elapsed past `timeout`), leaving a + // non-positive remainder. Passing 0 to waitForDockTabTitlesResolved would trip its budget- + // exhausted guard on that benign near-miss; a small positive floor lets each stage still run one + // real poll. Mirrors the Math.max(1, …) clamp the CDP global setup uses for the same call. + const budgetLeft = () => Math.max(1_000, timeout - (Date.now() - start)); + // Gate on the upstream service hosts BEFORE the tab-title wait: the tabs can only resolve once + // these hosts serve their metadata, so waiting on them first means a slow cold start is spent on + // the real cause rather than surfacing as an opaque "tabs still Unknown" timeout downstream. + await waitForServiceHostsRegistered(budgetLeft()); + // failFastOnFatalPageError mirrors `strict`: a strict wait is a fresh cold-start instance (smoke), + // where a fatal theme-settle error means THIS launch is doomed and a retry's fresh launch is the + // fix. The lenient shared-instance path (CDP features) leaves it off — there a stale error from a + // long-past cold start must not abort an otherwise-healthy per-test wait. + await waitForDockTabTitlesResolved(page, budgetLeft(), { + strict, + failFastOnFatalPageError: strict, + }); + await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, budgetLeft()); } /** diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index f2974abc..0a8aa413 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -5,7 +5,7 @@ import { createRequire } from 'module'; import fs from 'fs'; import os from 'os'; import path from 'path'; -import { waitForDockTabTitlesResolved } from './fixtures/helpers'; +import { waitForDockTabTitlesResolved, waitForServiceHostsRegistered } from './fixtures/helpers'; import { bootstrapRendererDevServer, isPortInUse, @@ -83,6 +83,14 @@ export default async function globalSetupCdp(_config: FullConfig): Promise `CDP port ${CDP_PORT} already in use — reusing the already-running Platform.Bible instance ` + '(not launching or tearing down an app).', ); + // Clear any stale ownership markers left on disk by a PRIOR launched run whose teardown never + // completed (a crash or a `kill -9` of the test runner). This reuse path launches nothing and + // records nothing, but globalTeardownCdp infers "this run launched an app" purely from these + // files' existence — so a leftover .cdp-app.pid would make teardown SIGKILL a PID it never + // started (which the OS may have recycled onto an unrelated process) and rm a user-data dir it + // doesn't own. Removing them here keeps teardown a true no-op, honoring "leaves the developer's + // instance running." e2e-tests/ is never auto-cleared, so nothing else sweeps them. + clearStaleOwnershipMarkers(); return; } @@ -242,15 +250,45 @@ async function waitForRendererSettled(timeout: number): Promise { if (!page) { throw new Error(`No renderer page appeared over CDP within ${timeout}ms`); } + // Gate on the upstream service hosts before the dock-tab wait, mirroring waitForAppReady: the + // freshly-launched instance's tabs stay "Unknown" until the settings/menu-data/theme hosts serve + // their metadata, and this is the exact path a Windows CDP cold start stalled on. Polls the same + // rpc.discover WebSocket the tab-title wait's siblings use, so it needs no CDP page. + await waitForServiceHostsRegistered(Math.max(1, deadline - Date.now())); // Strict cold-start gate: this is the freshly-launched instance's first settle, so every dock // tab must resolve. The per-test feature gate is lenient (shared instance already settled here). - await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { strict: true }); + // failFastOnFatalPageError: this is that cold start, so a fatal theme-settle error means the + // launch is doomed — fail setup fast (and dump the app log) rather than wait out the full budget. + await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { + strict: true, + failFastOnFatalPageError: true, + }); } finally { // Disconnect only — connectOverCDP close() does not terminate the app. await browser.close(); } } +/** + * Remove the ownership marker files ({@link CDP_PID_FILE}, {@link CDP_USER_DATA_FILE}) if present. + * Called from the warm-instance reuse path, where this run launches no app and so owns none of the + * resources those markers describe. {@link globalTeardownCdp} treats a present marker as "this run + * launched an app it must kill/clean," so a stale marker from a prior launched run whose teardown + * never completed would make teardown act on foreign resources — clearing them here prevents that. + * Best-effort: each removal is guarded so a missing or unreadable marker never fails setup. + * + * @returns Nothing. + */ +function clearStaleOwnershipMarkers(): void { + [CDP_PID_FILE, CDP_USER_DATA_FILE].forEach((markerFile) => { + try { + fs.rmSync(markerFile, { force: true }); + } catch (error) { + console.warn(`Could not remove stale CDP marker ${markerFile}: ${error}`); + } + }); +} + /** * Print the launched app's captured stdout/stderr to the console. Called when the app fails to open * its ports so the startup failure's cause appears inline in the CI log. From 92fe203d975894de0a677a64b902e221dacd40d2 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 15:24:25 -0600 Subject: [PATCH 20/24] Trim version-pinned upstream detail from e2e helper comments Cut paranext-core internals (serialization shape, magic numbers, symbol names) that go stale silently; keep the observable contract and flag the few comments that still track an upstream detail. --- e2e-tests/fixtures/helpers.ts | 137 +++++++++++++++------------------- 1 file changed, 59 insertions(+), 78 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 7a08049e..dca1ecec 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -21,19 +21,16 @@ export const PROCESS_READY_TIMEOUT = process.env.CI ? 600_000 : 120_000; /** * Fail-fast readiness budget (ms) for a CDP feature test's per-test * `waitForAppAndInterlinearizerReady`. The shared instance is proven-settled by global setup before - * any feature test runs, so a per-test readiness wait that runs long means the instance DIED - * mid-run — which no per-test retry can revive. Capping at 30 s (vs. the 120 s cold-start default) - * makes a dead shared instance fail in 30 s × the retry count instead of 120 s × it, the difference - * between a ~90 s and a 6-minute broken Windows run. + * any feature test runs, so a per-test readiness wait that runs long means the instance died + * mid-run — which no per-test retry can revive. A short cap (vs. the 120 s cold-start default) + * fails a dead shared instance fast instead of burning the full cold-start budget on every retry. */ export const CDP_FEATURE_READY_TIMEOUT = 30_000; /** - * File the smoke launcher streams the app's main-process stdout/stderr to. Named to mirror the CDP - * tier's `.cdp-app-startup.log` and kept in `e2e-tests/` (a directory Playwright does not clear, - * and one already covered by the CI artifact upload's `include-hidden-files: true`), so a - * cold-start stall's main-process log survives a failed run. Overwritten on each launch — one - * worker's app at a time — so it always holds the most recent launch's output. + * File the smoke launcher streams the app's main-process stdout/stderr to. Kept in `e2e-tests/` (a + * directory Playwright does not clear, and one the CI artifact upload includes) so a cold-start + * stall's main-process log survives a failed run. Overwritten on each launch. */ const SMOKE_APP_LOG_FILE = path.join(__dirname, '.smoke-app-startup.log'); @@ -44,23 +41,15 @@ const SMOKE_APP_LOG_FILE = path.join(__dirname, '.smoke-app-startup.log'); const PLATFORM_ABOUT_COMMAND = 'command:platform.about'; /** - * `rpc.discover` method names that flip present exactly when paranext-core's settings, menu-data, - * and theme service hosts finish registering their data-provider network objects — the upstream - * signal that actually gates a resolved dock (see {@link waitForServiceHostsRegistered}). + * `rpc.discover` method names that flip present when paranext-core's settings, menu-data, and theme + * service hosts finish registering their provider network objects — the upstream signal that gates + * a resolved dock (see {@link waitForServiceHostsRegistered}). * - * Each service host exposes its provider as a data-provider network object, whose JSON-RPC handlers - * are enumerated by `rpc.discover` the instant it registers. Data providers append a `-data` suffix - * to the provider name and network-object request types are prefixed `object:`, so the existence - * handler for the settings provider `platform.settingsServiceDataProvider` serializes to - * `object:platform.settingsServiceDataProvider-data`. paranext-core's own smoke suite waits on the - * settings provider's `.set` method for the same "settings host is up" purpose, so the shape is - * load-bearing there too. - * - * These are the bare EXISTENCE handlers (`object:{id}`, no method suffix), not a named method like - * `.set`/`.getCurrentTheme`: `networkObjectService.set` registers the existence handler - * unconditionally as the first step of registering any network object — before fanning out the - * per-method handlers — so it is the canonical "this object is on the network" signal and can't be - * invalidated by a provider method being renamed. + * These are each object's bare EXISTENCE handler (`object:{id}`, no method suffix), not a named + * method like `.set`/`.getCurrentTheme`: the existence handler registers first when any network + * object comes up, so it is the "this object is on the network" signal and can't be invalidated by + * a provider method being renamed. These strings mirror upstream's serialization; if paranext-core + * changes how it names provider objects, update them to match. */ const SERVICE_HOST_OBJECT_METHODS = [ 'object:platform.settingsServiceDataProvider-data', @@ -69,18 +58,16 @@ const SERVICE_HOST_OBJECT_METHODS = [ ]; /** - * Renderer page error that means this cold start is doomed, not merely slow. paranext-core's theme - * service host arms a 30 s `AsyncVariable` for `theme.service-host.allThemeFamiliesById`; if the - * extension host never pushes the initial theme data within that window it rejects with this - * message, and the dock is left with its webview tabs stuck at "Unknown" for the remainder of the - * run (the exact 120 s stall this suite kept eating on Windows/Linux cold starts). + * Renderer page error that means this cold start is doomed, not merely slow: the theme host's + * initial theme data never settled, so the dock's webview tabs stay stuck at "Unknown" for the rest + * of the run. Once it fires no amount of further waiting recovers the launch — only a fresh launch + * (a smoke retry, or a re-run of CDP setup) does. * - * Crucially this fires AFTER the theme provider's network object has already registered — the host - * registers its data provider, then separately awaits the data to settle — so the positive - * {@link waitForServiceHostsRegistered} gate can pass while the renderer is still headed for this - * timeout. That is why the tab-title wait additionally fast-fails on this specific error rather - * than waiting out its full budget: once it fires, no amount of further waiting recovers the - * launch, and a fresh launch (a smoke retry, or a re-run of CDP setup) is the only path forward. + * This fires AFTER the theme provider's network object has registered (the host registers the + * provider, then separately awaits its data), so the positive {@link waitForServiceHostsRegistered} + * gate can pass while the renderer is still headed for this timeout — which is why the tab-title + * wait fast-fails on this error instead of waiting out its budget. The pattern matches an upstream + * paranext-core message; if that message changes, this stops catching the doomed start. */ const FATAL_STARTUP_PAGE_ERROR = /Timeout reached when waiting for .*allThemeFamiliesById to settle/i; @@ -186,11 +173,10 @@ export async function launchElectronWithExtension( ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false', - // With NODE_ENV=development, paranext-core's electron-debug auto-opens DevTools on every - // window. On CI Linux DevTools docks INSIDE the window, squeezing the app viewport to ~469px — - // dock panels collapse and modals get clipped at panel edges, so clicks land on neighboring - // iframes. electron-is-dev honors ELECTRON_IS_DEV=0, which disables electron-debug without - // affecting NODE_ENV-driven behavior (dev-server URL, etc.). + // With NODE_ENV=development, paranext-core auto-opens DevTools on every window; on CI Linux + // DevTools docks inside the window and squeezes the app viewport enough that dock panels + // collapse and modals get clipped, so clicks land on neighboring iframes. ELECTRON_IS_DEV=0 + // disables the auto-open without changing other NODE_ENV-driven behavior (dev-server URL, etc.). ELECTRON_IS_DEV: '0', ...opts.envOverrides, }; @@ -208,9 +194,8 @@ export async function launchElectronWithExtension( coreDir, '--extensions', extensionDist, - // Deterministic window size instead of the 1024x728 electron-window-state default — - // paranext-core supports this argument for automation. Matches the CI xvfb screen - // (1280x960) so the dock panels have room and modals are not clipped. + // Deterministic window size (paranext-core supports this arg for automation) matching the + // CI xvfb screen (1280x960), so dock panels have room and modals are not clipped. '--window-size', '1280x960', // Force the X11 backend on Linux: in a Wayland session with DISPLAY redirected to xvfb @@ -229,12 +214,10 @@ export async function launchElectronWithExtension( throw error; } - // Stream the launched app's main-process stdout/stderr to a log file. Unlike the CDP tier, the - // smoke launcher previously discarded this output, so a cold-start startup stall (the app comes up - // with dock tabs stuck at "Unknown" and never resolves — a silent platform-side race we can only - // tolerate, not fix) left NO main-process evidence to diagnose from; the renderer console was - // empty. Capturing it here means the next stall leaves a log to inspect. Best-effort: the log is - // not required, and any write error is swallowed so logging never fails a launch. + // Stream the launched app's main-process stdout/stderr to a log file, so a cold-start stall (dock + // tabs stuck at "Unknown" and never resolving — a platform-side race we can only tolerate, not + // fix) leaves main-process evidence to diagnose from; the renderer console is empty in that case. + // Best-effort: any write error is swallowed so logging never fails a launch. const appLog = fs.createWriteStream(SMOKE_APP_LOG_FILE, { flags: 'w' }); appLog.on('error', () => { /* Logging is best-effort; never let a log write failure break the launch. */ @@ -491,15 +474,14 @@ export async function waitForPapiMethodRegistered( * polling `rpc.discover` for each host's data-provider existence handler * ({@link SERVICE_HOST_OBJECT_METHODS}). * - * This is the UPSTREAM readiness signal for a resolved dock. On a cold start the renderer paints + * This is the upstream readiness signal for a resolved dock. On a cold start the renderer paints * its webview tabs titled "Unknown" (and its panels blank) until the metadata these hosts serve - * arrives; the renderer even throws transient `Settings service undefined` / `Menu data service - * undefined` page errors while it retries against not-yet-registered providers. Gating here — - * before {@link waitForDockTabTitlesResolved} — absorbs that cold-start race into the readiness wait - * instead of letting it surface downstream as an opaque tab-title timeout: waiting on the hosts - * directly means the tab-title wait only ever runs once the data behind those titles actually - * exists. On a healthy startup the hosts are already up, so this resolves immediately and costs - * nothing; the poll uses the same `rpc.discover` mechanism as every other readiness check here. + * arrives. Gating here — before {@link waitForDockTabTitlesResolved} — absorbs that cold-start race + * into the readiness wait instead of letting it surface downstream as an opaque tab-title timeout: + * waiting on the hosts directly means the tab-title wait only ever runs once the data behind those + * titles actually exists. On a healthy startup the hosts are already up, so this resolves + * immediately and costs nothing; the poll uses the same `rpc.discover` mechanism as every other + * readiness check here. * * The three waits run concurrently and share the one `timeout` budget (the hosts register in * parallel — settings and menu-data in the extension host, theme in the renderer — so serializing @@ -532,7 +514,7 @@ interface DockTabTitlesOptions { * Re-asserting the strict "no tab anywhere is Unknown" invariant per test is both redundant and * fragile there: a single stray/leftover panel (e.g. one briefly re-titled by a close/reopen * cycle) would fail the gate for EVERY subsequent test against that one shared instance, and no - * per-test retry can recover it — the Windows CDP-tier cascade this whole run showed. + * per-test retry can recover it (a cascade the Windows CDP tier is prone to). */ strict: boolean; /** @@ -557,9 +539,9 @@ interface DockTabTitlesOptions { * The strictness of "resolved" depends on {@link DockTabTitlesOptions.strict} — see that field for * why the shared CDP instance must not use the strict all-tabs check. * - * A torn-down renderer (page/context/browser closed out from under us — the actual symptom in the - * Windows CDP run, where `waitForFunction` reports "Target page … has been closed") is surfaced as - * its own error rather than mislabeled "tabs still Unknown", so the real cause is not buried. + * A torn-down renderer (page/context/browser closed out from under us, where `waitForFunction` + * reports "Target page … has been closed") is surfaced as its own error rather than mislabeled + * "tabs still Unknown", so the real cause is not buried. * * With {@link DockTabTitlesOptions.failFastOnFatalPageError}, a doomed cold start (the renderer's * theme data never settling — see {@link FATAL_STARTUP_PAGE_ERROR}) aborts the wait immediately @@ -936,10 +918,10 @@ function interlinearizerTabLocator(page: Page): Locator { * * The CDP feature tier passes a short budget (`{ strict: false, timeout: ~30s }`): its shared * instance is already proven-settled by global setup, so a per-test readiness wait that runs long - * means the instance DIED mid-run, not that startup is slow — and no per-test retry can revive a - * dead shared instance. Failing that in ~30s instead of the default 120s is what turns a broken - * Windows CDP run from a 6-minute-per-test crawl (120s × 3 attempts) into a ~90s one. Smoke tests - * (fresh per-worker instance, genuine cold start) omit `timeout` and keep the generous default. + * means the instance died mid-run, not that startup is slow — and no per-test retry can revive a + * dead shared instance, so failing fast beats burning the full cold-start budget on every retry. + * Smoke tests (fresh per-worker instance, genuine cold start) omit `timeout` and keep the generous + * default. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param options Readiness options forwarded to {@link waitForAppReady}. Feature tests on the shared @@ -1086,18 +1068,17 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise * * The platform toolbar's book-chapter control only drives navigation when there is a resolved * navigation-target web view — in simple (non-power) interface mode that is always the MAIN - * Scripture editor, never a focused secondary tab like the Interlinearizer (see paranext-core - * navigation-target.util.ts `resolveTargetWebView` / `pinToMainEditor`). The self-launched CDP - * instance comes up in simple mode with no project loaded into the main editor, so the control - * stays `disabled` and can navigate nothing. A plain `trigger.click()` there never fails — - * Playwright retries the click for the whole test timeout waiting for the button to become enabled, - * which is what hung every feature test for 6 minutes. So this waits a BOUNDED time for the control - * to become actionable and, if it never does, skips navigation instead of hanging: the - * Interlinearizer opens at its default reference (GEN 1:1) and the callers assert against specific - * verse tokens with their own visibility waits, so a skipped no-op navigation to a reference - * already on screen still lets the test proceed (and a genuinely-needed navigation that could not - * happen surfaces as a fast, clear assertion failure downstream rather than an opaque timeout - * here). + * Scripture editor, never a focused secondary tab like the Interlinearizer (this is paranext-core's + * navigation-target logic). The self-launched CDP instance comes up in simple mode with no project + * loaded into the main editor, so the control stays `disabled` and can navigate nothing. A plain + * `trigger.click()` there never fails — Playwright retries the click for the whole test timeout + * waiting for the button to become enabled, which is what hung every feature test for 6 minutes. So + * this waits a BOUNDED time for the control to become actionable and, if it never does, skips + * navigation instead of hanging: the Interlinearizer opens at its default reference (GEN 1:1) and + * the callers assert against specific verse tokens with their own visibility waits, so a skipped + * no-op navigation to a reference already on screen still lets the test proceed (and a + * genuinely-needed navigation that could not happen surfaces as a fast, clear assertion failure + * downstream rather than an opaque timeout here). * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param reference Fully-qualified scripture reference to navigate to (e.g. `"GEN 1:1"`). From 4553ee5329039b12d1c13c746ed5c97218d14dcd Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 15:41:05 -0600 Subject: [PATCH 21/24] Arm fatal-startup tripwire across both e2e readiness stages Also fix the smoke launcher's dual stdout/stderr pipe: use { end: false } so neither stream ends the shared log early, and flush it before dumping. --- e2e-tests/fixtures/helpers.ts | 208 ++++++++++++++++++++++------------ e2e-tests/global-setup-cdp.ts | 33 +++--- 2 files changed, 154 insertions(+), 87 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index dca1ecec..3bb5e85e 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -223,14 +223,28 @@ export async function launchElectronWithExtension( /* Logging is best-effort; never let a log write failure break the launch. */ }); const appProcess = electronApp.process(); - appProcess.stdout?.pipe(appLog); - appProcess.stderr?.pipe(appLog); + // { end: false } on BOTH pipes: two sources share one destination, so the default end-on-source-end + // would have whichever stream (stdout/stderr) closes first call appLog.end(), dropping the other + // stream's later output and throwing "write after end". We own appLog's lifecycle instead — closing + // it when the app exits (below) or when we flush it before dumping on a failed launch. + appProcess.stdout?.pipe(appLog, { end: false }); + appProcess.stderr?.pipe(appLog, { end: false }); + // Close the log once the app process is gone: with { end: false } the pipes never close it, so tie + // its lifetime to the app to avoid leaking the descriptor on a healthy launch. + electronApp.once('close', () => { + appLog.end(); + }); console.log('Waiting for WebSocket server on port 8876...'); try { await waitForWebSocketReady(DEFAULT_WEBSOCKET_PORT, PROCESS_READY_TIMEOUT); } catch (error) { console.error('WebSocket readiness check failed after Electron launch:', error); + // Flush buffered pipe output to disk before the synchronous read in dumpSmokeAppLog: the app is + // still alive here (killed just below), so we cannot wait for the source streams to end — ending + // appLog ourselves flushes what has been written so the dump captures the failure's evidence + // instead of an empty file. + await flushAppLog(appLog); dumpSmokeAppLog(); const proc = electronApp.process(); if (proc?.pid) { @@ -258,6 +272,30 @@ export async function launchElectronWithExtension( return { electronApp, userDataDir, appClosed }; } +/** + * Flush and close the smoke app-log write stream, resolving once its buffered data has reached + * disk. The launcher pipes the app's stdout/stderr into this stream with `{ end: false }` (so + * neither source closes it), which means those writes may still be buffered when a failed launch + * wants to read the file back; ending the stream here forces the flush and this awaits the + * resulting `finish` (or `error`) so a following synchronous read sees the captured output rather + * than an empty file. Best-effort: a stream error resolves rather than rejects, so a logging + * failure never breaks launch. + * + * @param appLog The write stream created for {@link SMOKE_APP_LOG_FILE}. + * @returns Resolves once the stream has flushed and closed (or errored). + */ +function flushAppLog(appLog: fs.WriteStream): Promise { + return new Promise((resolve) => { + // Already-closed stream: end() would never emit 'finish', so resolve immediately. + if (appLog.writableEnded) { + resolve(); + return; + } + appLog.once('error', () => resolve()); + appLog.end(() => resolve()); + }); +} + /** * Echo the smoke launcher's captured main-process output ({@link SMOKE_APP_LOG_FILE}) to the * console. Called when the app fails to open its WebSocket port so the startup failure's cause @@ -517,16 +555,69 @@ interface DockTabTitlesOptions { * per-test retry can recover it (a cascade the Windows CDP tier is prone to). */ strict: boolean; - /** - * When set, abort the wait the moment the renderer emits a {@link FATAL_STARTUP_PAGE_ERROR} — - * turning a doomed cold start (theme data never settled, tabs stuck "Unknown" for the full - * timeout) into a fast, correctly-labeled failure instead of a 120 s dead wait. Only meaningful - * on a cold start (a fresh smoke launch or CDP global setup, where the error signals _this_ - * launch failed and a fresh one is the only remedy), so it is opt-in; the warm shared-instance - * path leaves it off, where a stale error from a long-past cold start must not abort an - * otherwise-healthy wait. - */ - failFastOnFatalPageError?: boolean; +} + +/** + * Run `fn` with a fatal-startup-error tripwire armed on `page`: if the renderer emits a + * {@link FATAL_STARTUP_PAGE_ERROR} while `fn` is in flight, the returned promise rejects immediately + * with a fast, correctly-labeled failure instead of `fn` running out its full readiness budget. + * + * This wraps the WHOLE readiness sequence (service-host wait AND dock-tab wait), not just one + * stage: the fatal theme-settle error can surface during either, and it never self-recovers, so a + * doomed cold start must abort the moment the error fires no matter which stage is running. Keeping + * the listener armed across both stages is why this is a wrapper rather than logic inside a single + * wait. + * + * The listener is registered only when `enabled` (a warm shared instance leaves it off — a stale + * error from a long-past cold start must not abort an otherwise-healthy wait), and always removed + * in `finally` so it can't leak across tests on the shared CDP page. A sentinel distinguishes + * "tripwire fired" from an ordinary `fn` rejection, so only a genuine fatal error is remapped to + * the fast failure; any other rejection from `fn` propagates unchanged. + * + * @param page The Playwright `Page` for the Platform.Bible renderer window. + * @param enabled Whether to arm the tripwire. When `false`, `fn` runs with no listener attached. + * @param fn The readiness work to run under the tripwire. + * @returns Resolves with `fn`'s result once it completes without the tripwire firing. + * @throws If the renderer emitted a fatal startup error while `fn` was in flight (with `enabled`), + * or whatever `fn` itself throws. + */ +export async function withFatalStartupTripwire( + page: Page, + enabled: boolean, + fn: () => Promise, +): Promise { + // A sentinel value distinguishes "tripwire fired" from an ordinary reject of `fn`. + const fatalError: { message?: string } = {}; + let onFatalPageError: ((err: Error) => void) | undefined; + const fatalErrorTripped = new Promise((_resolve, reject) => { + if (!enabled) return; + onFatalPageError = (err: Error) => { + if (!FATAL_STARTUP_PAGE_ERROR.test(err.message)) return; + fatalError.message = err.message; + reject(new Error(err.message)); + }; + page.on('pageerror', onFatalPageError); + }); + // Without an opted-in tripwire nothing ever rejects this promise; swallow the unused rejection + // path so a stray rejection (there is none) could never surface as an unhandled rejection. + fatalErrorTripped.catch(() => {}); + + try { + return await Promise.race([fn(), fatalErrorTripped]); + } catch (error) { + // The tripwire won the race: this cold start is doomed (theme data never settled), so report it + // as its own fast failure. A smoke retry or a re-run of CDP setup relaunches the app, which is + // the only thing that recovers this. + if (fatalError.message !== undefined) { + throw new Error( + `Platform.Bible startup failed: the renderer reported a fatal startup error, so its dock ` + + `tabs will never resolve on this launch (a fresh launch is required). Error: ${fatalError.message}`, + ); + } + throw error; + } finally { + if (onFatalPageError) page.off('pageerror', onFatalPageError); + } } /** @@ -543,9 +634,9 @@ interface DockTabTitlesOptions { * reports "Target page … has been closed") is surfaced as its own error rather than mislabeled * "tabs still Unknown", so the real cause is not buried. * - * With {@link DockTabTitlesOptions.failFastOnFatalPageError}, a doomed cold start (the renderer's - * theme data never settling — see {@link FATAL_STARTUP_PAGE_ERROR}) aborts the wait immediately - * instead of running out the full budget, since that failure never self-recovers. + * A doomed cold start (the renderer's theme data never settling — see + * {@link FATAL_STARTUP_PAGE_ERROR}) is aborted early by {@link withFatalStartupTripwire}, which + * callers wrap around the whole readiness sequence; this wait does not arm that tripwire itself. * * @param page The Playwright `Page` for the Platform.Bible renderer window. * @param timeout Maximum time in milliseconds to wait before throwing. Must be positive: a @@ -556,15 +647,15 @@ interface DockTabTitlesOptions { * @param options Readiness options; see {@link DockTabTitlesOptions}. * @returns Resolves once the dock is ready per the chosen `strict` mode. * @throws If `timeout` is non-positive (budget exhausted before this wait began), if tab titles - * have not resolved within `timeout` milliseconds, if the renderer emitted a fatal startup error - * (with `failFastOnFatalPageError`), or if the renderer page was closed while waiting. + * have not resolved within `timeout` milliseconds, or if the renderer page was closed while + * waiting. */ export async function waitForDockTabTitlesResolved( page: Page, timeout: number, options: DockTabTitlesOptions, ): Promise { - const { strict, failFastOnFatalPageError = false } = options; + const { strict } = options; // A non-positive budget must not reach page.waitForFunction: { timeout: 0 } disables Playwright's // timeout entirely (an unbounded wait), so an already-exhausted budget would hang instead of // failing. Fail fast with a clear message instead. @@ -575,52 +666,21 @@ export async function waitForDockTabTitlesResolved( ); } - // Fatal-startup-error tripwire: reject the moment the renderer emits FATAL_STARTUP_PAGE_ERROR, so - // the wait below can lose the race to it and abort a doomed cold start early. The listener is - // registered only when opted in, and always removed in `finally` so it can't leak across tests on - // the shared CDP page. A sentinel value distinguishes "tripwire fired" from an ordinary reject. - const fatalError: { message?: string } = {}; - let onFatalPageError: ((err: Error) => void) | undefined; - const fatalErrorTripped = new Promise((_resolve, reject) => { - if (!failFastOnFatalPageError) return; - onFatalPageError = (err: Error) => { - if (!FATAL_STARTUP_PAGE_ERROR.test(err.message)) return; - fatalError.message = err.message; - reject(new Error(err.message)); - }; - page.on('pageerror', onFatalPageError); - }); - // Without an opted-in tripwire nothing ever rejects this promise; swallow the unused rejection - // path so a stray rejection (there is none) could never surface as an unhandled rejection. - fatalErrorTripped.catch(() => {}); - try { - await Promise.race([ - page.waitForFunction( - (isStrict) => { - const tabs = Array.from(document.querySelectorAll('.dock-tab')); - if (tabs.length === 0) return false; - const isResolved = (tab: Element) => !(tab.textContent ?? '').includes('Unknown'); - // Strict: no tab anywhere may be "Unknown". Lenient: at least one tab has resolved, which - // is enough to know the app is up and rendering real tabs on an already-settled instance. - return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); - }, - strict, - { timeout, polling: 500 }, - ), - fatalErrorTripped, - ]); + await page.waitForFunction( + (isStrict) => { + const tabs = Array.from(document.querySelectorAll('.dock-tab')); + if (tabs.length === 0) return false; + const isResolved = (tab: Element) => !(tab.textContent ?? '').includes('Unknown'); + // Strict: no tab anywhere may be "Unknown". Lenient: at least one tab has resolved, which + // is enough to know the app is up and rendering real tabs on an already-settled instance. + return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); + }, + strict, + { timeout, polling: 500 }, + ); } catch (error) { const message = error instanceof Error ? error.message : String(error); - // The tripwire won the race: this cold start is doomed (theme data never settled), so report it - // as its own fast failure rather than the generic "tabs still Unknown" timeout. A smoke retry or - // a re-run of CDP setup relaunches the app, which is the only thing that recovers this. - if (fatalError.message !== undefined) { - throw new Error( - `Platform.Bible startup failed: the renderer reported a fatal startup error, so its dock ` + - `tabs will never resolve on this launch (a fresh launch is required). Error: ${fatalError.message}`, - ); - } // A closed page is a torn-down renderer, not a project-metadata stall — report it as such so it // isn't chased as the "Unknown tabs" startup race (which it superficially resembles because the // waitForFunction times out either way). @@ -635,8 +695,6 @@ export async function waitForDockTabTitlesResolved( `Dock tab titles did not resolve within ${timeout}ms — the app came up but its webview tabs ` + `are still titled "Unknown" (project metadata never loaded). Original error: ${message}`, ); - } finally { - if (onFatalPageError) page.off('pageerror', onFatalPageError); } } @@ -690,17 +748,19 @@ export async function waitForAppReady(page: Page, options: AppReadyOptions = {}) // exhausted guard on that benign near-miss; a small positive floor lets each stage still run one // real poll. Mirrors the Math.max(1, …) clamp the CDP global setup uses for the same call. const budgetLeft = () => Math.max(1_000, timeout - (Date.now() - start)); - // Gate on the upstream service hosts BEFORE the tab-title wait: the tabs can only resolve once - // these hosts serve their metadata, so waiting on them first means a slow cold start is spent on - // the real cause rather than surfacing as an opaque "tabs still Unknown" timeout downstream. - await waitForServiceHostsRegistered(budgetLeft()); - // failFastOnFatalPageError mirrors `strict`: a strict wait is a fresh cold-start instance (smoke), - // where a fatal theme-settle error means THIS launch is doomed and a retry's fresh launch is the - // fix. The lenient shared-instance path (CDP features) leaves it off — there a stale error from a - // long-past cold start must not abort an otherwise-healthy per-test wait. - await waitForDockTabTitlesResolved(page, budgetLeft(), { - strict, - failFastOnFatalPageError: strict, + // Arm the fatal-startup tripwire around BOTH the service-host wait and the tab-title wait: the + // fatal theme-settle error can surface during either stage (the hosts and the theme settle + // concurrently), so a doomed cold start must abort the moment it fires no matter which stage is + // running. The tripwire mirrors `strict`: a strict wait is a fresh cold-start instance (smoke), + // where a fatal error means THIS launch is doomed and a retry's fresh launch is the fix. The + // lenient shared-instance path (CDP features) leaves it off — there a stale error from a long-past + // cold start must not abort an otherwise-healthy per-test wait. + await withFatalStartupTripwire(page, strict, async () => { + // Gate on the upstream service hosts BEFORE the tab-title wait: the tabs can only resolve once + // these hosts serve their metadata, so waiting on them first means a slow cold start is spent on + // the real cause rather than surfacing as an opaque "tabs still Unknown" timeout downstream. + await waitForServiceHostsRegistered(budgetLeft()); + await waitForDockTabTitlesResolved(page, budgetLeft(), { strict }); }); await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, budgetLeft()); } diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index 0a8aa413..a23155ff 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -5,7 +5,11 @@ import { createRequire } from 'module'; import fs from 'fs'; import os from 'os'; import path from 'path'; -import { waitForDockTabTitlesResolved, waitForServiceHostsRegistered } from './fixtures/helpers'; +import { + waitForDockTabTitlesResolved, + waitForServiceHostsRegistered, + withFatalStartupTripwire, +} from './fixtures/helpers'; import { bootstrapRendererDevServer, isPortInUse, @@ -250,18 +254,21 @@ async function waitForRendererSettled(timeout: number): Promise { if (!page) { throw new Error(`No renderer page appeared over CDP within ${timeout}ms`); } - // Gate on the upstream service hosts before the dock-tab wait, mirroring waitForAppReady: the - // freshly-launched instance's tabs stay "Unknown" until the settings/menu-data/theme hosts serve - // their metadata, and this is the exact path a Windows CDP cold start stalled on. Polls the same - // rpc.discover WebSocket the tab-title wait's siblings use, so it needs no CDP page. - await waitForServiceHostsRegistered(Math.max(1, deadline - Date.now())); - // Strict cold-start gate: this is the freshly-launched instance's first settle, so every dock - // tab must resolve. The per-test feature gate is lenient (shared instance already settled here). - // failFastOnFatalPageError: this is that cold start, so a fatal theme-settle error means the - // launch is doomed — fail setup fast (and dump the app log) rather than wait out the full budget. - await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { - strict: true, - failFastOnFatalPageError: true, + // Arm the fatal-startup tripwire around BOTH readiness stages, mirroring waitForAppReady: this + // is the freshly-launched cold-start instance, so a fatal theme-settle error means the launch is + // doomed — fail setup fast (and dump the app log) rather than wait out the full budget. The error + // can surface during either stage, so the tripwire must stay armed across both. + await withFatalStartupTripwire(page, true, async () => { + // Gate on the upstream service hosts before the dock-tab wait, mirroring waitForAppReady: the + // freshly-launched instance's tabs stay "Unknown" until the settings/menu-data/theme hosts + // serve their metadata, and this is the exact path a Windows CDP cold start stalled on. Polls + // the same rpc.discover WebSocket the tab-title wait's siblings use, so it needs no CDP page. + await waitForServiceHostsRegistered(Math.max(1, deadline - Date.now())); + // Strict cold-start gate: this is the freshly-launched instance's first settle, so every dock + // tab must resolve. The per-test feature gate is lenient (shared instance already settled). + await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { + strict: true, + }); }); } finally { // Disconnect only — connectOverCDP close() does not terminate the app. From 51f3171dccfc617b804e392139f2ac307ee252a6 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 16:04:16 -0600 Subject: [PATCH 22/24] Write smoke e2e startup log to e2e-tests/, not fixtures/ --- e2e-tests/fixtures/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 3bb5e85e..2a1432a2 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -32,7 +32,7 @@ export const CDP_FEATURE_READY_TIMEOUT = 30_000; * directory Playwright does not clear, and one the CI artifact upload includes) so a cold-start * stall's main-process log survives a failed run. Overwritten on each launch. */ -const SMOKE_APP_LOG_FILE = path.join(__dirname, '.smoke-app-startup.log'); +const SMOKE_APP_LOG_FILE = path.join(__dirname, '..', '.smoke-app-startup.log'); /** * Same serialized request type as `registerCommand('platform.about', ...)` in command.service From ff77a3ad9e38982bf8bc22d3cbe2f9303b7504eb Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 16:43:09 -0600 Subject: [PATCH 23/24] Route smoke e2e kill paths through killProcessTree Replaces the manual process-group kill in launchElectronWithExtension and teardownElectronApp with the shared killProcessTree utility, matching both global-teardown files and closing the Windows tree-kill gap. --- e2e-tests/fixtures/helpers.ts | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index 2a1432a2..ed83a221 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -13,6 +13,7 @@ import { createRequire } from 'module'; import os from 'os'; import path from 'path'; import WebSocket from 'ws'; +import { killProcessTree } from '../process-utils'; const DEFAULT_WEBSOCKET_PORT = 8876; const RPC_DISCOVER_POLL_INTERVAL_MS = 250; @@ -247,17 +248,7 @@ export async function launchElectronWithExtension( await flushAppLog(appLog); dumpSmokeAppLog(); const proc = electronApp.process(); - if (proc?.pid) { - try { - process.kill(-proc.pid, 'SIGKILL'); - } catch { - try { - proc.kill('SIGKILL'); - } catch { - /* already dead */ - } - } - } + if (proc?.pid) killProcessTree(proc.pid, 'SIGKILL'); fs.rmSync(userDataDir, { recursive: true, force: true }); throw error; } @@ -330,30 +321,11 @@ export async function teardownElectronApp(ctx: ElectronAppContext): Promise { - if (!electronProcess?.pid) return; - try { - process.kill(-electronProcess.pid, sig); - } catch { - try { - electronProcess.kill(sig); - } catch { - /* already dead */ - } - } - }; - // Node.js ChildProcess.exitCode/signalCode are null until the process exits // eslint-disable-next-line no-null/no-null if (electronProcess && electronProcess.exitCode === null && electronProcess.signalCode === null) { console.log('[teardown] Sending SIGKILL to process group...'); - killGroup('SIGKILL'); + if (electronProcess.pid) killProcessTree(electronProcess.pid, 'SIGKILL'); console.log('[teardown] Waiting for appClosed after SIGKILL (up to 3s)...'); await Promise.race([ appClosed, From fd77dfd98e4957574eac68e0ce4cb941b57a6f16 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 13 Jul 2026 16:50:51 -0600 Subject: [PATCH 24/24] Raise CDP e2e readiness-budget floor to survive one poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dock-tab and service-host waits floored their leftover budget to 1ms (Math.max(1, ...)), which clears waitForDockTabTitlesResolved's timeout<=0 guard but expires during the CDP round-trip before the first predicate evaluation — failing "tabs still Unknown" on a healthy app. Floor to 1000ms to match waitForAppReady, and correct its stale "mirrors Math.max(1, ...)" comment. --- e2e-tests/fixtures/helpers.ts | 10 ++++++---- e2e-tests/global-setup-cdp.ts | 12 ++++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/e2e-tests/fixtures/helpers.ts b/e2e-tests/fixtures/helpers.ts index ed83a221..46816119 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -714,11 +714,13 @@ export async function waitForAppReady(page: Page, options: AppReadyOptions = {}) state: 'attached', timeout, }); - // Floor each leftover budget to a small positive value, never 0: a preceding stage can resolve at - // the very last millisecond (or wall-clock drift can push elapsed past `timeout`), leaving a + // Floor each leftover budget to 1000ms, never 0 or a hair above it: a preceding stage can resolve + // at the very last millisecond (or wall-clock drift can push elapsed past `timeout`), leaving a // non-positive remainder. Passing 0 to waitForDockTabTitlesResolved would trip its budget- - // exhausted guard on that benign near-miss; a small positive floor lets each stage still run one - // real poll. Mirrors the Math.max(1, …) clamp the CDP global setup uses for the same call. + // exhausted guard on that benign near-miss; a single-digit remainder would clear that guard but + // then expire during the CDP round-trip its forwarded page.waitForFunction needs to evaluate the + // predicate even once, failing "tabs still Unknown" on a healthy app. 1000ms lets each stage still + // run one real poll. The CDP global setup applies the same Math.max(1_000, …) floor for this call. const budgetLeft = () => Math.max(1_000, timeout - (Date.now() - start)); // Arm the fatal-startup tripwire around BOTH the service-host wait and the tab-title wait: the // fatal theme-settle error can surface during either stage (the hosts and the theme settle diff --git a/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts index a23155ff..2ee0eda0 100644 --- a/e2e-tests/global-setup-cdp.ts +++ b/e2e-tests/global-setup-cdp.ts @@ -254,6 +254,14 @@ async function waitForRendererSettled(timeout: number): Promise { if (!page) { throw new Error(`No renderer page appeared over CDP within ${timeout}ms`); } + // Floor each leftover budget to 1000ms, never a hair above 0: the preceding stage can finish at + // (or, once waitForServiceHostsRegistered applies its own Math.max(1_000, …) floor, past) the + // deadline, leaving a non-positive or single-digit remainder. waitForDockTabTitlesResolved + // forwards its budget straight to page.waitForFunction, whose predicate is evaluated over a CDP + // round-trip — a ~1ms budget expires during that round-trip and fails with a misleading "tabs + // still Unknown" error even on a healthy app, whereas 1000ms leaves room for one real poll. + // Matches the budgetLeft floor in waitForAppReady, which forwards to the same call. + const budgetLeft = () => Math.max(1_000, deadline - Date.now()); // Arm the fatal-startup tripwire around BOTH readiness stages, mirroring waitForAppReady: this // is the freshly-launched cold-start instance, so a fatal theme-settle error means the launch is // doomed — fail setup fast (and dump the app log) rather than wait out the full budget. The error @@ -263,10 +271,10 @@ async function waitForRendererSettled(timeout: number): Promise { // freshly-launched instance's tabs stay "Unknown" until the settings/menu-data/theme hosts // serve their metadata, and this is the exact path a Windows CDP cold start stalled on. Polls // the same rpc.discover WebSocket the tab-title wait's siblings use, so it needs no CDP page. - await waitForServiceHostsRegistered(Math.max(1, deadline - Date.now())); + await waitForServiceHostsRegistered(budgetLeft()); // Strict cold-start gate: this is the freshly-launched instance's first settle, so every dock // tab must resolve. The per-test feature gate is lenient (shared instance already settled). - await waitForDockTabTitlesResolved(page, Math.max(1, deadline - Date.now()), { + await waitForDockTabTitlesResolved(page, budgetLeft(), { strict: true, }); });