From 42886779e249196fb1ed2e732fa9470f81b5c672 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:41:29 -0700 Subject: [PATCH] fix(win-e2e): scope modal dismissal and bound retries --- config/vitest.config.ts | 1 + tools/win-update-e2e/app-driver.mjs | 111 +++++++++++++---------- tools/win-update-e2e/app-driver.test.mjs | 74 +++++++++++++++ 3 files changed, 137 insertions(+), 49 deletions(-) create mode 100644 tools/win-update-e2e/app-driver.test.mjs diff --git a/config/vitest.config.ts b/config/vitest.config.ts index 9e8da4051bf..7f6fde0f0ca 100644 --- a/config/vitest.config.ts +++ b/config/vitest.config.ts @@ -20,6 +20,7 @@ export default defineConfig({ 'src/**/*.test.tsx', 'config/scripts/**/*.test.ts', 'config/scripts/**/*.test.mjs', + 'tools/**/*.test.mjs', 'tests/e2e/**/*.unit.test.ts' ], // Why: the full suite runs heavy TS transforms plus real git/http fixtures; diff --git a/tools/win-update-e2e/app-driver.mjs b/tools/win-update-e2e/app-driver.mjs index 3a0b4d9efb9..71492fa804b 100644 --- a/tools/win-update-e2e/app-driver.mjs +++ b/tools/win-update-e2e/app-driver.mjs @@ -267,55 +267,80 @@ export async function ensureTerminal(page, { allowCreate = true, timeoutMs = 60_ * plain terminal (not an agent), then submit "Create worktree". */ async function createWorkspaceFromSeededRepo(page, timeoutMs) { - // Why: an overlay (e.g. the agent-CLI feature wall) can appear between the - // dismissal rounds and this first click; retry with a dismissal pass instead - // of force-clicking through whatever is on top. const newWorkspace = page .getByRole(NEW_WORKSPACE_BUTTON.role, { name: NEW_WORKSPACE_BUTTON.name }) .first() - let clicked = false - for (let attempt = 0; attempt < 3 && !clicked; attempt++) { - clicked = await newWorkspace - .click({ timeout: 15_000 }) - .then(() => true) - .catch(() => false) - if (!clicked) { - await dismissOverlays(page) - } - } - if (!clicked) { - await newWorkspace.click({ timeout: timeoutMs }) + if (!(await tryClickWithKnownOverlayRetry(page, newWorkspace, timeoutMs))) { + // Preserve Playwright's locator diagnostics without exceeding the caller's + // timeout by another full click attempt. + await newWorkspace.click({ timeout: 1 }) } // Choose the plain-terminal mode (best-effort — if it is already the default // or the label differs, the create below still produces a worktree). - await page - .getByRole('button', { name: 'Blank Terminal' }) - .first() - .click({ timeout: 15_000 }) - .catch(() => {}) + await tryClickWithKnownOverlayRetry( + page, + page.getByRole('button', { name: 'Blank Terminal' }).first(), + 15_000 + ) // Submit. The create button's accessible name carries the shortcut hint // ("Create worktreeCtrl"), so match by prefix; fall back to the documented // Ctrl+Enter shortcut if the button is not directly clickable. - const created = await page - .getByRole('button', { name: /^Create worktree/ }) - .last() - .click({ timeout: 15_000 }) - .then(() => true) - .catch(() => false) + const created = await tryClickWithKnownOverlayRetry( + page, + page.getByRole('button', { name: /^Create worktree/ }).last(), + 15_000 + ) if (!created) { await page.keyboard.press('Control+Enter') } } -// Why 'Close': the agent-CLI feature-wall modal ("Let agents drive Orca with the Orca CLI") -// can render after an async capability check, mid-run, and swallows all clicks until closed. -const OVERLAY_DISMISS_LABELS = [ - 'Got it', - 'Dismiss setup scripts', - 'Dismiss tip', - 'Dismiss update', - 'Close' -] +const OVERLAY_DISMISS_LABELS = ['Got it', 'Dismiss setup scripts', 'Dismiss tip', 'Dismiss update'] +const DIALOG_CLOSE_BUTTON = '[data-slot="dialog-content"]:visible [data-slot="dialog-close"]' + +async function dismissKnownOverlays(page) { + let acted = false + // Why: a global "Close" role also matches the Windows/Linux title-bar button. + const dialogClose = page.locator(DIALOG_CLOSE_BUTTON).first() + if (await dialogClose.isVisible().catch(() => false)) { + const clicked = await dialogClose + .click({ timeout: 3_000 }) + .then(() => true) + .catch(() => false) + acted ||= clicked + } + for (const name of OVERLAY_DISMISS_LABELS) { + const btn = page.getByRole('button', { name }).first() + if (await btn.isVisible().catch(() => false)) { + const clicked = await btn + .click({ timeout: 3_000 }) + .then(() => true) + .catch(() => false) + acted ||= clicked + } + } + return acted +} + +async function tryClickWithKnownOverlayRetry(page, locator, timeoutMs) { + const deadline = Date.now() + timeoutMs + do { + const remainingMs = deadline - Date.now() + if (remainingMs <= 0) { + return false + } + try { + await locator.click({ timeout: Math.min(5_000, remainingMs) }) + return true + } catch (error) { + if (page.isClosed()) { + throw error + } + await dismissKnownOverlays(page) + } + } while (Date.now() < deadline) + return false +} /** * Best-effort dismissal of the modals/banners that appear after creating a @@ -325,14 +350,7 @@ const OVERLAY_DISMISS_LABELS = [ */ export async function dismissOverlays(page, rounds = 3) { for (let i = 0; i < rounds; i++) { - let acted = false - for (const name of OVERLAY_DISMISS_LABELS) { - const btn = page.getByRole('button', { name }).first() - if (await btn.isVisible().catch(() => false)) { - await btn.click({ timeout: 3_000 }).catch(() => {}) - acted = true - } - } + const acted = await dismissKnownOverlays(page) await page.keyboard.press('Escape').catch(() => {}) if (!acted) { return @@ -377,12 +395,7 @@ export async function listTabIds(page) { export async function focusActiveTerminal(page, terminalTabId = null) { // A feature-tip modal can appear late and swallow keystrokes; clear any before // focusing so typed commands actually reach the shell. - for (const name of OVERLAY_DISMISS_LABELS) { - const btn = page.getByRole('button', { name }).first() - if (await btn.isVisible().catch(() => false)) { - await btn.click({ timeout: 2_000 }).catch(() => {}) - } - } + await dismissKnownOverlays(page) const selector = terminalTabId ? `[data-terminal-tab-id="${terminalTabId}"]:visible` : TERMINAL_SURFACE_VISIBLE diff --git a/tools/win-update-e2e/app-driver.test.mjs b/tools/win-update-e2e/app-driver.test.mjs new file mode 100644 index 00000000000..2048537dbcb --- /dev/null +++ b/tools/win-update-e2e/app-driver.test.mjs @@ -0,0 +1,74 @@ +import { describe, expect, it, vi } from 'vitest' +import { dismissOverlays, ensureTerminal } from './app-driver.mjs' + +function hiddenButton() { + return { + first: () => hiddenButton(), + isVisible: vi.fn().mockResolvedValue(false), + click: vi.fn() + } +} + +describe('dismissOverlays', () => { + it('dismisses a dialog without clicking the desktop window Close button', async () => { + const dialogClose = { + first: vi.fn(), + isVisible: vi.fn().mockResolvedValue(true), + click: vi.fn().mockResolvedValue(undefined) + } + dialogClose.first.mockReturnValue(dialogClose) + const windowClose = { + first: vi.fn(), + isVisible: vi.fn().mockResolvedValue(true), + click: vi.fn().mockRejectedValue(new Error('window closed')) + } + windowClose.first.mockReturnValue(windowClose) + + const page = { + locator: vi.fn().mockReturnValue(dialogClose), + getByRole: vi.fn((_role, { name }) => (name === 'Close' ? windowClose : hiddenButton())), + keyboard: { press: vi.fn().mockResolvedValue(undefined) }, + waitForTimeout: vi.fn().mockResolvedValue(undefined) + } + + await dismissOverlays(page, 1) + + expect(dialogClose.click).toHaveBeenCalledOnce() + expect(windowClose.click).not.toHaveBeenCalled() + expect(page.locator).toHaveBeenCalledWith( + '[data-slot="dialog-content"]:visible [data-slot="dialog-close"]' + ) + }) + + it('keeps overlay retries within the caller timeout budget', async () => { + vi.useFakeTimers() + try { + const newWorkspace = { + first: vi.fn(), + click: vi.fn(async ({ timeout }) => { + await vi.advanceTimersByTimeAsync(timeout) + throw new Error('button remained blocked') + }) + } + newWorkspace.first.mockReturnValue(newWorkspace) + const page = { + locator: vi.fn().mockImplementation(() => hiddenButton()), + getByRole: vi.fn((_role, { name }) => + name === 'New workspace' ? newWorkspace : hiddenButton() + ), + keyboard: { press: vi.fn().mockResolvedValue(undefined) }, + isClosed: vi.fn().mockReturnValue(false) + } + + await expect(ensureTerminal(page, { timeoutMs: 12_000 })).rejects.toThrow( + 'button remained blocked' + ) + + expect(newWorkspace.click.mock.calls.map(([options]) => options.timeout)).toEqual([ + 5_000, 5_000, 2_000, 1 + ]) + } finally { + vi.useRealTimers() + } + }) +})