diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 435d9058..480ad9ce 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: @@ -86,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, 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 + if ($LASTEXITCODE -ne 0) { Write-Host "npm cache verify failed (exit $LASTEXITCODE)" } + } + 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 @@ -112,15 +136,16 @@ jobs: working-directory: paranext-core run: npm run build:dll - - name: Run e2e smoke tests (Linux) + - 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 + # Linux needs an xvfb display + run: xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test:e2e - - 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 @@ -129,6 +154,11 @@ jobs: name: playwright-results-${{ matrix.os }} 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 + # 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/.gitignore b/.gitignore index a1dbba9e..13e0fd65 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +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 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..24bee04c 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -1,6 +1,17 @@ # 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`) 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 both Linux and Windows. + +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:** @@ -17,3 +28,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`. 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. +- **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.** A mutating test's setup composes these steps as needed: `ensureInterlinearizerOpenOnWeb()`, `ensureE2eProjectActive()`, `navigateToScriptureRef()` (the scroll-group reference could be anywhere), and `wipeDraft()`. They stay as separate calls rather than one wrapper on purpose — tests pick the subset they need (e.g. the draft-persistence reopen deliberately runs only open + navigate, skipping the project reset and wipe that would destroy the state it is verifying). +- **Reset at the start, tidy at the end.** Correctness rests on the start-of-test sequence, which self-heals whatever a failed run left behind (leftover modals, stray project pickers, another test's dirty draft); see the JSDocs on `ensureInterlinearizerOpenOnWeb()` and `ensureE2eProjectActive()` for the mechanism. This self-healing is also what lets the CDP config safely `retries` in CI. Mutating tests additionally end with `ensureE2eProjectActive(page, { rescueDirtyDraft: false })` to discard their own leftovers — a courtesy to the next run, 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/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 a8c604a0..46816119 100644 --- a/e2e-tests/fixtures/helpers.ts +++ b/e2e-tests/fixtures/helpers.ts @@ -1,21 +1,78 @@ // 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, + Locator, + Page, +} from '@playwright/test'; +import escapeStringRegexp from 'escape-string-regexp'; import fs from 'fs'; 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; 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. 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. 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'); + /** * Same serialized request type as `registerCommand('platform.about', ...)` in command.service * (`command` + `:` + `platform.about`). */ const PLATFORM_ABOUT_COMMAND = 'command:platform.about'; +/** + * `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}). + * + * 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', + 'object:platform.menuDataServiceDataProvider-data', + 'object:platform.themeServiceDataProvider-data', +]; + +/** + * 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. + * + * 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; + /** * Keep in sync with GET_METHODS from @shared/data/rpc.model. Required to be 'rpc.discover' by the * OpenRPC specification. @@ -117,6 +174,11 @@ export async function launchElectronWithExtension( ...restEnv, NODE_ENV: 'development', DEV_NOISY: process.env.DEV_NOISY ?? 'false', + // 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, }; @@ -128,7 +190,21 @@ 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 (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 + // (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, @@ -139,23 +215,40 @@ export async function launchElectronWithExtension( throw error; } + // 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. */ + }); + const appProcess = electronApp.process(); + // { 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) { - 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; } @@ -170,6 +263,49 @@ 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 + * 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. @@ -185,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, @@ -363,56 +480,317 @@ 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 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. 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 { + /** + * 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 (a cascade the Windows CDP tier is prone to). + */ + strict: 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 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. + * @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 waitForAppReady(page: Page, timeout = 60_000): Promise { +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); + } +} + +/** + * 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. + * + * 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, 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 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 + * 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 `timeout` is non-positive (budget exhausted before this wait began), 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, + options: DockTabTitlesOptions, +): Promise { + 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. + 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.`, + ); + } + + 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 instance. + return isStrict ? tabs.every(isResolved) : tabs.some(isResolved); + }, + 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: ${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 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, 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; const start = Date.now(); await page.waitForSelector('div[class*="dock-layout"]', { state: 'attached', timeout, }); - const remaining = Math.max(0, timeout - (Date.now() - start)); - await waitForPapiMethodRegistered(PLATFORM_ABOUT_COMMAND, DEFAULT_WEBSOCKET_PORT, remaining); + // 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 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 + // 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()); } /** * 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), ); } /** * Open the Interlinearizer WebView from the Scripture Editor's top (≡) menu. Prerequisite stage - * shared by all e2e tests that require the Interlinearizer to be open. Assumes no project has been - * loaded yet. + * shared by all e2e tests that require the Interlinearizer to be open. + * + * The startup dock layout varies, so the editor is located resiliently rather than assumed: + * + * - A truly-fresh core profile opens the default multi-tab layout, which already includes a + * "Scripture Editor" tab (no project loaded). + * - A profile whose layout collapsed to a single tab opens only the "Home" tab (no editor). + * - A warm CDP instance already has the editor open on a project (tab titled e.g. "WEB (Editable)"). * * Steps: * - * 1. Click the "Scripture Editor" dock tab to bring it to the front. - * 2. Enter the Scripture Editor iframe and click the ≡ ("Project") menu button. - * 3. Click "Open Interlinearizer for this Project". - * 4. In the "Open Interlinearizer" project-picker dialog, click the named project. - * 5. Wait for the "Interlinearizer" dock tab and click it to focus it. + * 1. Wait for the layout to settle to a known state: either a Scripture Editor tab or the Home tab is + * present. (A bare `count()` races the async dock render — a fresh profile reports zero tabs for + * a beat before the layout mounts.) + * 2. If no editor tab is present, open `projectName` from Home (Home tab → project row → "Open"), + * mirroring paranext-core's own `openFromEditorHamburger` helper. + * 3. Focus the Scripture Editor tab. Its title (and its iframe's title) is the project short name with + * an editability suffix once a project is loaded (e.g. "WEB (Editable)"), and "Scripture Editor" + * only when no project is loaded — both are accepted. + * 4. Enter the Scripture Editor iframe and click the ≡ ("Project") menu button. + * 5. Click "Open Interlinearizer for this Project". + * 6. If the "Open Interlinearizer" project-picker dialog appears (it only does when the editor has no + * project loaded), click the named project. When the editor already has a project, the command + * opens the Interlinearizer for it directly with no dialog. + * 7. Wait for the "Interlinearizer" dock tab and click it to focus it. * * @param page The Playwright `Page` for the Platform.Bible renderer window. - * @param projectName Name of the project to select in the project-picker (default: `"WEB"`). + * @param projectName Name of the project to open and select in the project-picker (default: + * `"WEB"`). * @returns Resolves when the Interlinearizer tab is focused and visible. * @throws If any step does not complete within its timeout. */ @@ -420,13 +798,37 @@ export async function openInterlinearizerFromScriptureEditor( page: Page, projectName = 'WEB', ): Promise { - // Focus the Scripture Editor tab (opens without a project at fresh start). - const editorTab = page.locator('.dock-tab', { hasText: 'Scripture Editor' }).first(); + // 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 = escapeStringRegexp(projectName); + const editorTab = page + .locator('.dock-tab', { hasText: new RegExp(`^(Scripture Editor|${escapedProjectName})\\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". + // `.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: 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. + 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(); // 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 + .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. @@ -435,16 +837,538 @@ 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. - 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(); + // 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. + // + // `.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); + 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(); + } // 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(); + + // 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. */ + }); + } +} + +/** + * 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 { + // 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]'); +} + +/** + * 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(); +} + +/** + * Wait for Platform.Bible and the interlinearizer extension to finish starting up. Combines + * {@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, 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 + * 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 the `timeout` budget. + */ +export async function waitForAppAndInterlinearizerReady( + page: Page, + options: AppReadyOptions = {}, +): Promise { + const start = Date.now(); + await waitForAppReady(page, options); + // 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); +} + +/** + * Dismiss any modal left mounted inside the Interlinearizer iframe by a prior failed test, so its + * full-viewport `tw:modal-overlay` (fixed inset-0 z-50, see src/components/modals/ModalShell.tsx) + * can't intercept every click in the run that follows. + * + * This is the shared-instance recovery step. The CDP fixture connects to one long-lived + * Platform.Bible instance and never resets its DOM between tests, so a test that dies with a modal + * open (e.g. a `wipeDraft` whose click timed out) leaves that overlay covering the iframe — which + * then blocks the NEXT test before it can even open a menu. Running this at the start of the + * open-Interlinearizer precondition converts that cascade (one real failure reddening every + * downstream test) into a single self-healed hiccup, and is what makes a CDP retry actually land on + * a clean instance instead of re-running against the poisoned overlay. + * + * Each project modal's only reliable dismiss affordance is its Cancel/secondary button — the + * dialogs are rendered as a plain `` (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 + * 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. + * + * 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. + * @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 = 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. 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).first()).toBeVisible({ timeout: 30_000 }); + + 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, + }); + + // 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); +} + +/** + * 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. + * + * 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 (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"`). + * @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 }); + 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. 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 = '●'; + +/** + * 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 interlinearizerTabLocator(page).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(interlinearizerTabLocator(page)).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'); + + // 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 && + (await activeEntry.first().getByText(E2E_PROJECT_NAME, { exact: true }).count()) > 0; + + 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'); + // 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) { + // 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(); + // 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 }); +} + +/** + * 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 }); + 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 }); +} + +/** + * 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 is not visible, or the tab does not close within the timeout. + */ +export async function closeInterlinearizerTab(page: Page): Promise { + const interlinearizerTab = interlinearizerTabLocator(page); + await expect(interlinearizerTab).toBeVisible({ timeout: 15_000 }); + // 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/e2e-tests/global-setup-cdp.ts b/e2e-tests/global-setup-cdp.ts new file mode 100644 index 00000000..2ee0eda0 --- /dev/null +++ b/e2e-tests/global-setup-cdp.ts @@ -0,0 +1,322 @@ +// Self-launching global setup for the CDP (feature-test) config. +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, + waitForServiceHostsRegistered, + withFatalStartupTripwire, +} from './fixtures/helpers'; +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'); + +/** + * 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; + +/** + * 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 + * 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, 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 + * 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).', + ); + // 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; + } + + 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}`); + + // 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 tree by the recorded PID. + const appProcess = spawn( + electronExecutable, + [ + `--user-data-dir=${userDataDir}`, + coreDir, + '--extensions', + extensionDist, + `--remote-debugging-port=${CDP_PORT}`, + // 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', + // 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, + }, + ); + 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)); + + /** + * 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 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) => { + reject( + new Error( + `Launched Platform.Bible (CDP) process exited early (code=${code}, signal=${signal}) ` + + 'before its WebSocket and CDP ports came up.', + ), + ); + }); + }); + // 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]); + 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]); + 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 (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`); + } + // 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 + // 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(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, budgetLeft(), { + strict: 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. + * + * @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-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..1b7fb7b7 --- /dev/null +++ b/e2e-tests/global-teardown-cdp.ts @@ -0,0 +1,63 @@ +// 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'; +import { killProcessTree } from './process-utils'; + +/** + * 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 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); + 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})...`); + appKilled = killProcessTree(pid, 'SIGKILL'); + } + 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/global-teardown.ts b/e2e-tests/global-teardown.ts index 8739e466..4e5902b4 100644 --- a/e2e-tests/global-teardown.ts +++ b/e2e-tests/global-teardown.ts @@ -3,6 +3,7 @@ import type { FullConfig } from '@playwright/test'; import { execSync } from 'child_process'; import path from 'path'; import fs from 'fs'; +import { killProcessTree } from './process-utils'; /** * Playwright global teardown. Runs once after all test workers have finished. @@ -28,15 +29,7 @@ export default async function globalTeardown(_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/playwright-cdp.config.ts b/e2e-tests/playwright-cdp.config.ts index 0e1b8d52..a27413bf 100644 --- a/e2e-tests/playwright-cdp.config.ts +++ b/e2e-tests/playwright-cdp.config.ts @@ -2,20 +2,30 @@ 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, + 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, - 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 +33,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..a6908e61 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -20,7 +20,9 @@ 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 the second tier's report doesn't 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 +34,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/e2e-tests/process-utils.ts b/e2e-tests/process-utils.ts new file mode 100644 index 00000000..bef683a6 --- /dev/null +++ b/e2e-tests/process-utils.ts @@ -0,0 +1,55 @@ +// 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. 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 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' }); + 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..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 { waitForAppReady, waitForInterlinearizerReady } 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. @@ -34,8 +37,14 @@ 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(); + // 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. 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 }); @@ -51,8 +60,14 @@ test.describe('Example: Open Interlinearizer via menu', () => { }); test('should render without critical console errors', async ({ mainPage }) => { - await waitForAppReady(mainPage); - await waitForInterlinearizerReady(); + // 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. 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 new file mode 100644 index 00000000..ba80b729 --- /dev/null +++ b/e2e-tests/tests/features/draft-persistence.spec.ts @@ -0,0 +1,70 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + CDP_FEATURE_READY_TIMEOUT, + closeInterlinearizerTab, + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + waitForAppAndInterlinearizerReady, + 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(); + // 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. 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'); + 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..8efe5a8d --- /dev/null +++ b/e2e-tests/tests/features/gloss-roundtrip.spec.ts @@ -0,0 +1,46 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + CDP_FEATURE_READY_TIMEOUT, + ensureE2eProjectActive, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + navigateToScriptureRef, + 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 }) => { + // 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. 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'); + 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..2a0a16b6 --- /dev/null +++ b/e2e-tests/tests/features/project-modals.spec.ts @@ -0,0 +1,66 @@ +import { expect, test } from '../../fixtures/cdp.fixture'; +import { + CDP_FEATURE_READY_TIMEOUT, + ensureInterlinearizerOpenOnWeb, + getInterlinearizerFrame, + openInterlinearizerProjectMenu, + waitForAppAndInterlinearizerReady, +} 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, + }) => { + // 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. + // 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); + await frame.getByRole('menuitem', { name: modal.menuItem }).first().click(); + + const modalTitle = frame.locator(modal.titleSelector); + await expect(modalTitle).toBeVisible({ timeout: 5_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(); + }); + }); +}); 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); 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": { diff --git a/package.json b/package.json index 96373c7c..17b6d092 100644 --- a/package.json +++ b/package.json @@ -30,8 +30,9 @@ "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: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", 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