Fix failing e2e test

[jolierabideau]

Fix flaky / failing smoke E2E tests (Windows CI and local)

Summary

Stabilizes the Playwright smoke project (npm run test:e2e:smoke) by aligning tests with real DOM and startup ordering, waiting for real PAPI registration via rpc.discover (no renderer globalThis flag), hardening the theme toggle test, fixing Electron launch env typing for Playwright, tightening theme-toggle assertions so slow updates fail fast, and typing browser globals used inside page.evaluate.

Problem

• CI (Windows): ui-interaction.spec.ts failed on “About via PAPI” (wrong locator) and “toggle theme” (theme id unchanged / timing). Help-menu About could fail on first run and pass on retry.
• Local: Occasional first-run failure on “About from Help menu” with command:platform.about not found, while the dock was already visible.

Root causes

1. Dock vs. dialog service timing — The React tree (including the dock layout) can paint before startDialogService() finishes registering commands such as platform.about. waitForAppReady only waited for the dock, so UI tests could open the Help menu before the handler existed.

2. About PAPI assertion — getByLabel('%mainMenu_about%') does not match how tabs expose accessibility: PlatformTabTitle uses aria-label from %tab_aria_tab% (“Tab”), not the about menu key. The reliable signal is the same as the Help-menu test: a .dock-tab whose text matches /About/i.

3. Theme toggle — Icon-only CSS selectors (lucide-sun / lucide-moon) and asserting before the theme stylesheet node existed were brittle on CI. The stylesheet node must exist before asserting on data-theme-id; stable selection uses data-testid.

4. Electron env — ELECTRON_RUN_AS_NODE: undefined is invalid for Playwright’s env type (Record<string, string>). The variable must be omitted (destructure it out of process.env), not set to undefined.

5. E2E TypeScript — e2e-tests used lib: ["ES2022"] only, so document inside page.evaluate failed type-checking; adding DOM fixes that without hacks in each test.

Changes

E2E readiness — rpc.discover instead of a renderer global

launchElectronApp already waits until the PAPI WebSocket accepts connections on port 8876, so the test process can open a short-lived client from Node anytime after that.

waitForAppReady (in e2e-tests/fixtures/helpers.ts):

1. Wait for the dock layout selector (unchanged).
2. Poll rpc.discover (GET_METHODS / OpenRPC) until result.methods includes command:platform.about, with the same overall timeout as the dock wait.

That uses the same method registry as the live app (renderer registrations included) and avoids test-only globalThis hooks or extra typings in papi.d.ts.

Supporting helpers: sendPapiRequestOnce (JSON-RPC where method is a PAPI request type), waitForPapiMethodRegistered (exported for reuse).

Renderer / typings — removed E2E-only global

• dialog.service-host.ts — No longer sets globalThis.paranextDialogServiceCommandsReady after registration.
• src/shared/global-this.model.ts / lib/papi-dts/papi.d.ts — Removed paranextDialogServiceCommandsReady declarations.

E2E helpers (e2e-tests/fixtures/helpers.ts)

• waitForAppReady — Dock wait + rpc.discover polling for command:platform.about (see above).
• launchElectronApp — Build env by destructuring ELECTRON_RUN_AS_NODE out of process.env, then set NODE_ENV: 'development', so the variable is removed without passing undefined.

E2E TypeScript (e2e-tests/tsconfig.json)

• Add DOM to compilerOptions.lib so document and other browser APIs in page.evaluate callbacks type-check.

Smoke UI tests (e2e-tests/tests/smoke/ui-interaction.spec.ts)

• About via PAPI — Assert the About dock tab (.dock-tab + /About/i) instead of getByLabel('%mainMenu_about%').
• Theme toggle — Wait for #theme-styles to exist; locate the button with getByTestId('theme-toggle'); scrollIntoViewIfNeeded before click; use 5s toPass timeouts for toggle and restore so slow theme updates surface as failures instead of being masked by a long window.

Toolbar (platform-bible-toolbar.tsx)

• Theme Button: aria-label={themeButtonTooltip} (matches existing tooltip copy) and data-testid="theme-toggle" for stable E2E selection and clearer accessibility.
• Fix wrong localization key for the theme error tooltip: %tooltip_theme_loading_error% → %toolbar_theme_loading_error%.

Files touched

File	Role
src/renderer/services/dialog.service-host.ts	Removed E2E-only globalThis readiness flag
src/shared/global-this.model.ts	Removed paranextDialogServiceCommandsReady
lib/papi-dts/papi.d.ts	Removed matching declaration
e2e-tests/fixtures/helpers.ts	waitForAppReady via rpc.discover + Electron env fix
e2e-tests/tsconfig.json	DOM lib for page.evaluate / document
e2e-tests/tests/smoke/ui-interaction.spec.ts	About + theme smoke assertions
src/renderer/components/platform-bible-toolbar.tsx	Theme button aria-label, data-testid, localization fix

How to verify

npm run test:e2e:smoke

Confirm smoke passes on first run without retries (especially Help → About and theme toggle).

Notes

• Windows CI logs may still show EBUSY / temp user-data cleanup warnings during teardown; those are separate from the assertion failures addressed here.
• rpc.discover polling uses small delays between attempts so we do not hammer the WebSocket while the renderer is still registering handlers.
• Theme toPass stays at 5s on purpose: if the theme data provider regresses and updates take many seconds, the test should fail loudly.

---

---

This change is 

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

[tjcouch-sil]

Thanks for looking into this! Just one piece of feedback

@tjcouch-sil reviewed 7 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolierabideau).

---

src/renderer/services/dialog.service-host.ts line 415 at r1 (raw file):

  // The dock layout can paint before this async service finishes; menu items that invoke
  // `platform.about` and similar commands must not run until registration completes (E2E reads this).
  globalThis.paranextDialogServiceCommandsReady = true;

I don't really like the idea of adding globalThis stuff like this just for E2E testing; feels jank. What if, in waitForAppReady, instead you called the command rpc.discover to check if the dialog's request handlers were registered? I dunno if you are already connected to the WebSocket at that point in the E2E process; I'm not familiar enough with them. Just an idea.

Another option we could consider is running a network event when all the services are done loading in index.tsx (and I guess making a request handler that tells you whether the renderer is done loading). That would also require being able to connect to the WebSocket in the E2E process before waitForAppReady. I would naturally think we would want to do it in the app service. I guess you could run the event from the renderer and add an event handler in the app service.

[jolierabideau]

@jolierabideau made 1 comment.
Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on tjcouch-sil).

---

src/renderer/services/dialog.service-host.ts line 415 at r1 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I don't really like the idea of adding globalThis stuff like this just for E2E testing; feels jank. What if, in waitForAppReady, instead you called the command rpc.discover to check if the dialog's request handlers were registered? I dunno if you are already connected to the WebSocket at that point in the E2E process; I'm not familiar enough with them. Just an idea.

Another option we could consider is running a network event when all the services are done loading in index.tsx (and I guess making a request handler that tells you whether the renderer is done loading). That would also require being able to connect to the WebSocket in the E2E process before waitForAppReady. I would naturally think we would want to do it in the app service. I guess you could run the event from the renderer and add an event handler in the app service.

Done. I went with the first option!

[tjcouch-sil]

Nice! I have some suggestions, but it looks good overall!

@tjcouch-sil reviewed 4 files and all commit messages, made 5 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on jolierabideau).

---

src/renderer/services/dialog.service-host.ts line 415 at r1 (raw file):

Previously, jolierabideau wrote…

Done. I went with the first option!

Thanks!

---

e2e-tests/fixtures/helpers.ts line 10 at r2 (raw file):

/** Keep in sync with `GET_METHODS` in `src/shared/data/rpc.model.ts`. */
const GET_METHODS = 'rpc.discover';

Did you try importing like import { GET_METHODS } from '../../src/shared/data/rpc.model';?

---

e2e-tests/fixtures/helpers.ts line 268 at r2 (raw file):

}

type OpenRpcDiscoverResult = {

Can you import this from here?

---

e2e-tests/fixtures/helpers.ts line 276 at r2 (raw file):

 * a connection, sends one request, waits for the matching response id, then closes.
 */
async function sendPapiRequestOnce<T>(

Can this be refactored to have less duplication with sendPapiCommand?

[jolierabideau]

@jolierabideau resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved.

[tjcouch-sil]

Nice improvement! Sadly there is another thing to look at

@tjcouch-sil reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on jolierabideau).

---

e2e-tests/tsconfig.json line 13 at r4 (raw file):

    "noEmit": true,
    "resolveJsonModule": true,
    "baseUrl": "..",

Are these baseUrl and paths required anymore? Unfortunate timing; I just put up a PR to remove baseUrl because it is apparently going away.