Skip to content

Commit 0dfdcb2

Browse files
committed
E2E: stop relying on OS focus for the Escape-binding tests
The two `Escape closes the X window (production binding)` tests in settings.spec.ts and viewer.spec.ts went through `keyboard.press('Escape')`, which delivers the keystroke via the OS. Under Xvfb on Linux CI the keystroke routinely lands on the main window (the previous test left focus there, or the open-animation hasn't transferred it yet), the scoped webview never sees it, and the test sits waiting for a window-close that doesn't come. The existing two-attempt focus poll helped but didn't eliminate the race — both tests flaked on the most recent PR run. Switch both to a synthetic `document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))` via `evaluate`. That bubbles through the `<svelte:window on:keydown>` listener regardless of OS focus, which is what the test is actually meant to exercise (the handler's close logic). The Tauri/webkit2gtk OS → webview event pipeline isn't cmdr's responsibility to test. Pattern matches `network-toggle.spec.ts` and `file-operations.spec.ts`, which already use synthetic Escape dispatch for the same reason. Drops the helper `tryEscape` / focus-poll machinery; the test is now linear.
1 parent e8259ee commit 0dfdcb2

2 files changed

Lines changed: 34 additions & 69 deletions

File tree

apps/desktop/test/e2e-playwright/settings.spec.ts

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -314,42 +314,25 @@ test.describe('Settings keyboard binding', () => {
314314
const settings = await openSettingsWindowViaProd(main)
315315
await settings.waitForSelector('.settings-window', 3000)
316316

317-
// Verify focus is actually inside the settings webview before pressing
318-
// Escape. Without this the keystroke can land on the main window (or
319-
// wherever focus drifted to during async onMount in the settings UI),
320-
// the settings window never receives it, and the test sits waiting for
321-
// a window-close that won't come. Two attempts max as cheap insurance.
322-
const tryEscape = async (): Promise<boolean> => {
323-
const focused = await pollUntil(
324-
settings,
325-
async () =>
326-
settings.evaluate<boolean>(`(function(){
327-
if (!document.hasFocus()) return false;
328-
var root = document.querySelector('.settings-window');
329-
return !!(root && document.activeElement && root.contains(document.activeElement));
330-
})()`),
331-
1000,
332-
)
333-
if (!focused) {
334-
// Re-focus inside the settings webview and let the caller retry.
335-
await settings.evaluate(`(function(){
336-
var root = document.querySelector('.settings-window');
337-
if (root && 'focus' in root) root.focus();
338-
})()`)
339-
return false
340-
}
341-
// Fire-and-forget: the dispatched Escape triggers getCurrentWindow().close()
342-
// synchronously inside the handler, so the window may die before pw_result
343-
// fires back. The poll on listWindows() below is the assertion.
344-
settings.keyboard.press('Escape').catch(() => {
317+
// Dispatch a synthetic Escape keydown into the settings webview rather
318+
// than going through Playwright's OS-level `keyboard.press`. The handler
319+
// we want to exercise is the JS `<svelte:window on:keydown>` in
320+
// `routes/settings/+page.svelte` — bubbling from `document` fires that
321+
// listener regardless of OS focus. Going through the OS path adds a
322+
// focus dependency that flakes under Xvfb on Linux CI (the keystroke
323+
// lands on the main window and the settings webview never sees it).
324+
// The Tauri/webkit2gtk OS → webview event pipeline isn't cmdr's
325+
// responsibility to test; this binding is.
326+
//
327+
// Fire-and-forget: the dispatched Escape triggers
328+
// `getCurrentWindow().close()` (after a 2-rAF defer) inside the handler,
329+
// so the window may die before the evaluate promise's `pw_result` fires
330+
// back. The poll on `listWindows()` below is the assertion that matters.
331+
settings
332+
.evaluate(`document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))`)
333+
.catch(() => {
345334
/* window died mid-script before pw_result; expected */
346335
})
347-
return true
348-
}
349-
350-
if (!(await tryEscape())) {
351-
await tryEscape()
352-
}
353336

354337
const gone = await pollUntil(
355338
main,

apps/desktop/test/e2e-playwright/viewer.spec.ts

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -189,43 +189,25 @@ test.describe('File viewer keyboard binding', () => {
189189
const label = viewer.targetWindow
190190
if (!label) throw new Error('Scoped viewer page has no targetWindow label')
191191

192-
// Verify focus is inside the viewer webview before pressing Escape. If the
193-
// keystroke lands on the main window (focus race with the open animation,
194-
// or a late-mounting modal stealing focus), the viewer never receives it
195-
// and the test sits waiting for the window-close. Two attempts max.
196-
const tryEscape = async (): Promise<boolean> => {
197-
const focused = await pollUntil(
198-
viewer,
199-
async () =>
200-
viewer.evaluate<boolean>(`(function(){
201-
if (!document.hasFocus()) return false;
202-
var root = document.querySelector('.viewer-container');
203-
return !!(root && document.activeElement && root.contains(document.activeElement));
204-
})()`),
205-
1000,
206-
)
207-
if (!focused) {
208-
await viewer.evaluate(`(function(){
209-
var root = document.querySelector('.viewer-container');
210-
if (root && 'focus' in root) root.focus();
211-
})()`)
212-
return false
213-
}
214-
// Fire-and-forget: the eval that dispatches Escape may not resolve if the
215-
// window dies before pw_result fires back to the test runner. The
216-
// closeWindow() path uses two rAFs before calling .close(), so in practice
217-
// the eval usually resolves first, but defending against either ordering
218-
// keeps the test deterministic. We assert on the windowDisappeared, not
219-
// on the press itself.
220-
viewer.keyboard.press('Escape').catch(() => {
192+
// Dispatch a synthetic Escape keydown into the viewer webview rather
193+
// than going through Playwright's OS-level `keyboard.press`. The handler
194+
// we want to exercise is the JS `<svelte:window on:keydown>` in
195+
// `routes/viewer/+page.svelte` — bubbling from `document` fires that
196+
// listener regardless of OS focus. Going through the OS path adds a
197+
// focus dependency that flakes under Xvfb on Linux CI (the keystroke
198+
// lands on the main window and the viewer webview never sees it).
199+
// The Tauri/webkit2gtk OS → webview event pipeline isn't cmdr's
200+
// responsibility to test; this binding is.
201+
//
202+
// Fire-and-forget: the dispatched Escape triggers `closeWindow()`
203+
// (which itself defers via two rAFs before calling `.close()`), so the
204+
// window may die before the evaluate promise's `pw_result` fires back.
205+
// The poll on `listWindows()` below is the assertion that matters.
206+
viewer
207+
.evaluate(`document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))`)
208+
.catch(() => {
221209
/* window died mid-script before pw_result; expected */
222210
})
223-
return true
224-
}
225-
226-
if (!(await tryEscape())) {
227-
await tryEscape()
228-
}
229211

230212
const gone = await pollUntil(
231213
main,

0 commit comments

Comments
 (0)