mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
test(e2e): stop a spec's parking-delay override from leaking into the rest of its worker (#21571)
* test(e2e): scope the parking-delay override to the spec that needs it
A Playwright worker imports many spec files into one Node process, and the app
fixtures launch Electron with a spread of that process's env. The split-
orientation spec set ORCA_E2E_TERMINAL_PARKING_DELAY_MS at module scope, so the
2s override outlived the file and reconfigured every app launched by every spec
that followed it in the same worker — measured directly: a probe spec sees a
30000ms cold-park delay on its own and 2000ms when that spec runs first.
Module scope is the part that cannot be undone. A write inside a test body can
save and restore, as four other specs here do; a write at import time runs
before any hook exists to restore it. test.use({ orcaAppExtraEnv }) reaches the
app launch without touching the worker every other spec shares.
The ratchet holds the module-scope writer count at zero.
* test(e2e): re-apply screen-reader mode while reading the accessibility tree
Separate from the env leak above, and unproven against the CI failure it
resembles: this is robustness, not a diagnosed fix.
screenReaderMode is an option on the xterm instance and the accessibility tree
belongs to that instance's DOM. The SSH cold-activation spec set it once,
imperatively, then waited on the node. A pane that parks and remounts, or
rebinds after a reconnect, comes back as a new instance with the option off, so
the one-shot mutation stops producing the node the wait is waiting for and the
wait reports "element(s) not found" rather than a content mismatch.
The helper re-applies the option inside the poll and returns null when the node
is absent, so a replaced instance is retried instead of being fatal. Five other
specs still use the one-shot pattern and are left alone.
This commit is contained in:
@@ -34,12 +34,14 @@ import {
|
||||
/** Legacy pane id shape: not a stable pane UUID, so it never reaches the published leaf set. */
|
||||
const LEGACY_LEAF_ID = 'pane:9'
|
||||
|
||||
/**
|
||||
* Shrinks both the cold-park delay and the hot-retain window. Set at module scope because the
|
||||
* `orcaPage` fixture launches the app before any test body runs.
|
||||
*/
|
||||
/** Shrinks both the cold-park delay and the hot-retain window. */
|
||||
const PARK_DELAY_MS = 2_000
|
||||
process.env.ORCA_E2E_TERMINAL_PARKING_DELAY_MS ??= String(PARK_DELAY_MS)
|
||||
const PARK_DELAY_ENV = { ORCA_E2E_TERMINAL_PARKING_DELAY_MS: String(PARK_DELAY_MS) }
|
||||
|
||||
// Why a fixture option and not `process.env`: a Playwright worker runs many spec files in one
|
||||
// process, so a module-scope env write outlives this file and shrinks parking for every spec
|
||||
// that follows it in the same worker.
|
||||
test.use({ orcaAppExtraEnv: PARK_DELAY_ENV })
|
||||
|
||||
function collectLeafIds(node: TerminalPaneLayoutNode | null | undefined): string[] {
|
||||
if (!node) {
|
||||
@@ -174,7 +176,11 @@ test('publishes an unmounted split with its real orientation when a legacy leaf
|
||||
)
|
||||
|
||||
const offer = await createRuntimeDesktopPairingOffer(orcaPage)
|
||||
client = await launchPairedElectronClient(offer, testInfo, 'legacy-leaf-orientation-observer')
|
||||
// The observer inherited the same override back when it came from `process.env`; keep it so
|
||||
// scoping the write to this file does not also change what the client does.
|
||||
client = await launchPairedElectronClient(offer, testInfo, 'legacy-leaf-orientation-observer', {
|
||||
extraEnv: PARK_DELAY_ENV
|
||||
})
|
||||
await waitForPairedClientWorktree(client.page, worktreeId)
|
||||
|
||||
await expect
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { readFileSync, readdirSync, statSync } from 'node:fs'
|
||||
import { join, relative, resolve } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
/**
|
||||
* One Playwright worker imports many spec files into one Node process, and the app fixtures
|
||||
* launch Electron with a spread of that process's `process.env`. A module-scope write therefore
|
||||
* reconfigures every app launched by every spec that follows in the same worker — and it runs at
|
||||
* import time, before any hook or `finally` exists that could undo it, so unlike a write inside a
|
||||
* test body it cannot be restored at all.
|
||||
*
|
||||
* That is not a hypothetical: a parking-delay override written this way shrank the terminal
|
||||
* cold-park delay from 30s to 2s for later specs, which unmounted panes those specs still needed.
|
||||
* Use `test.use({ orcaAppExtraEnv })`, which Playwright scopes to the file.
|
||||
*/
|
||||
const E2E_ROOT = resolve(__dirname)
|
||||
|
||||
/**
|
||||
* Module scope is read off column 0. The tree is prettier-formatted, so every statement nested in
|
||||
* a function, hook, block, or `app.evaluate` callback is indented — including the in-body writes
|
||||
* that legitimately save and restore around a relaunch. A write that starts a line is top-level.
|
||||
*/
|
||||
const MODULE_SCOPE_ENV_WRITE =
|
||||
/^(?:process\.env\.[A-Za-z_][A-Za-z0-9_]*\s*(?:\??\|\||\?\?|)=[^=]|process\.env\[|delete\s+process\.env[.[]|Object\.assign\(\s*process\.env)/
|
||||
|
||||
/**
|
||||
* The count of files writing `process.env` at module scope.
|
||||
*
|
||||
* May only ever be DECREASED. Raising it is never the fix: the replacement is a fixture option,
|
||||
* which is strictly more capable here because it reaches the app launch without touching the
|
||||
* worker every other spec shares.
|
||||
*/
|
||||
const MODULE_SCOPE_ENV_WRITER_PIN = 0
|
||||
|
||||
const SCANNED_EXTENSIONS = ['.ts', '.tsx']
|
||||
const IGNORED_DIRECTORIES = new Set(['node_modules', 'dist', 'out', 'build', '__fixtures__'])
|
||||
|
||||
function collectE2eFiles(root: string): string[] {
|
||||
const found: string[] = []
|
||||
for (const entry of readdirSync(root)) {
|
||||
if (IGNORED_DIRECTORIES.has(entry)) {
|
||||
continue
|
||||
}
|
||||
const path = join(root, entry)
|
||||
if (statSync(path).isDirectory()) {
|
||||
found.push(...collectE2eFiles(path))
|
||||
} else if (SCANNED_EXTENSIONS.some((extension) => path.endsWith(extension))) {
|
||||
found.push(path)
|
||||
}
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
function findModuleScopeEnvWrites(path: string): string[] {
|
||||
return readFileSync(path, 'utf8')
|
||||
.split('\n')
|
||||
.flatMap((line, index) =>
|
||||
MODULE_SCOPE_ENV_WRITE.test(line)
|
||||
? [`${relative(E2E_ROOT, path)}:${index + 1}: ${line.trim()}`]
|
||||
: []
|
||||
)
|
||||
}
|
||||
|
||||
describe('e2e worker env isolation', () => {
|
||||
const offenders = collectE2eFiles(E2E_ROOT).flatMap(findModuleScopeEnvWrites)
|
||||
|
||||
it('no e2e file writes process.env at module scope', () => {
|
||||
expect(offenders).toEqual([])
|
||||
})
|
||||
|
||||
it('holds the module-scope env writer count at its ratchet', () => {
|
||||
const files = new Set(offenders.map((offender) => offender.split(':')[0]))
|
||||
expect(files.size).toBeLessThanOrEqual(MODULE_SCOPE_ENV_WRITER_PIN)
|
||||
})
|
||||
|
||||
it('detects the shape it is meant to catch', () => {
|
||||
// Guards the regex itself: a green that cannot go red would pass this whole file forever.
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test("process.env.ORCA_E2E_X ??= '1'")).toBe(true)
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test("process.env.ORCA_E2E_X = '1'")).toBe(true)
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test('delete process.env.ORCA_E2E_X')).toBe(true)
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test("Object.assign(process.env, { ORCA_E2E_X: '1' })")).toBe(
|
||||
true
|
||||
)
|
||||
// Reads, and writes nested in any body, stay legal.
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test('const x = Number(process.env.ORCA_E2E_X) || 500')).toBe(
|
||||
false
|
||||
)
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test(" process.env.ORCA_E2E_X = '1'")).toBe(false)
|
||||
expect(MODULE_SCOPE_ENV_WRITE.test("if (process.env.ORCA_E2E_X === '1') {")).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,46 @@
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { expect } from '@stablyai/playwright-test'
|
||||
|
||||
/**
|
||||
* Reads a tab's `.xterm-accessibility-tree` text, enabling screen-reader mode on the pane it
|
||||
* finds each time it is called.
|
||||
*
|
||||
* Why re-apply rather than set it once up front: `screenReaderMode` is an option on the xterm
|
||||
* instance, and the node it renders belongs to that instance's DOM. A pane that parks and
|
||||
* remounts, or rebinds after a reconnect, comes back as a new instance with the option off and
|
||||
* no tree — so a one-shot mutation before the read stops producing the very node the read is
|
||||
* waiting for, and the wait fails as "element not found" rather than as a content mismatch.
|
||||
*
|
||||
* Returns null when the pane or the node is not there yet, so a poll keeps retrying.
|
||||
*/
|
||||
export function readTerminalAccessibilityText(page: Page, tabId: string): Promise<string | null> {
|
||||
return page.evaluate((id) => {
|
||||
const manager = window.__paneManagers?.get(id)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0]
|
||||
if (!pane) {
|
||||
return null
|
||||
}
|
||||
if (!pane.terminal.options.screenReaderMode) {
|
||||
pane.terminal.options.screenReaderMode = true
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
}
|
||||
const node = document.querySelector(
|
||||
`[data-terminal-tab-id="${CSS.escape(id)}"] .xterm-accessibility-tree`
|
||||
)
|
||||
return node?.textContent ?? null
|
||||
}, tabId)
|
||||
}
|
||||
|
||||
export async function expectTerminalAccessibilityText(
|
||||
page: Page,
|
||||
tabId: string,
|
||||
expected: string,
|
||||
timeoutMs = 30_000
|
||||
): Promise<void> {
|
||||
await expect
|
||||
.poll(() => readTerminalAccessibilityText(page, tabId), {
|
||||
timeout: timeoutMs,
|
||||
message: `terminal tab ${tabId} never rendered ${expected} in its accessibility tree`
|
||||
})
|
||||
.toContain(expected)
|
||||
}
|
||||
@@ -25,6 +25,11 @@ import {
|
||||
waitForTerminalOutput as waitForTerminalOutputImpl
|
||||
} from './terminal-pane-operations'
|
||||
|
||||
export {
|
||||
expectTerminalAccessibilityText,
|
||||
readTerminalAccessibilityText
|
||||
} from './terminal-accessibility-tree'
|
||||
|
||||
export {
|
||||
getTerminalContent,
|
||||
readPaneIdentitySnapshot,
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { ElectronApplication } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
import {
|
||||
expectTerminalAccessibilityText,
|
||||
focusActiveTerminalInput,
|
||||
waitForActivePanePtyId,
|
||||
waitForActiveTerminalManager
|
||||
@@ -193,26 +194,12 @@ test.describe('SSH cold activation restore', () => {
|
||||
}
|
||||
)
|
||||
.toBe(firstTabId)
|
||||
await orcaPage.evaluate((tabId) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0]
|
||||
if (!pane) {
|
||||
throw new Error('Restored SSH pane unavailable')
|
||||
}
|
||||
pane.terminal.options.screenReaderMode = true
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
}, firstTabId)
|
||||
|
||||
const marker = `SSH_RESTORE_OK_${Date.now()}`
|
||||
const proofFile = '/tmp/orca-ssh-restore-proof'
|
||||
await focusActiveTerminalInput(orcaPage)
|
||||
await orcaPage.keyboard.type(`printf '${marker}' > ${proofFile} && printf '${marker}\\n'`)
|
||||
await orcaPage.keyboard.press('Enter')
|
||||
await expect(
|
||||
orcaPage.locator(
|
||||
`[data-terminal-tab-id=${JSON.stringify(firstTabId)}] .xterm-accessibility-tree`
|
||||
)
|
||||
).toContainText(marker, { timeout: 30_000 })
|
||||
await expectTerminalAccessibilityText(orcaPage, firstTabId, marker)
|
||||
expect(execDockerSshRelayTargetCommand(target, `cat ${proofFile}`)).toBe(marker)
|
||||
} finally {
|
||||
cleanupDockerSshRelayTarget(target)
|
||||
@@ -292,27 +279,13 @@ test.describe('SSH cold activation restore', () => {
|
||||
.toBe(remote.worktreeId)
|
||||
await waitForActiveTerminalManager(secondLaunch.page, 60_000)
|
||||
expect(await waitForActivePanePtyId(secondLaunch.page, 60_000)).toBe(firstPtyId)
|
||||
await secondLaunch.page.evaluate((tabId) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0]
|
||||
if (!pane) {
|
||||
throw new Error('Restored SSH pane unavailable')
|
||||
}
|
||||
pane.terminal.options.screenReaderMode = true
|
||||
pane.terminal.refresh(0, pane.terminal.rows - 1)
|
||||
}, restoredTabId)
|
||||
|
||||
const restoredMarker = `SSH_OWNER_RESTORED_${Date.now()}`
|
||||
await focusActiveTerminalInput(secondLaunch.page)
|
||||
await secondLaunch.page.keyboard.type(
|
||||
`printf '%s|%s|%s|%s\\n' "$$" "$ORCA_BG_PID" "$ORCA_RESTART_TOKEN" "$PWD" > ${afterProofPath}; printf '${restoredMarker}\\n'`
|
||||
)
|
||||
await secondLaunch.page.keyboard.press('Enter')
|
||||
await expect(
|
||||
secondLaunch.page.locator(
|
||||
`[data-terminal-tab-id=${JSON.stringify(restoredTabId)}] .xterm-accessibility-tree`
|
||||
)
|
||||
).toContainText(restoredMarker, { timeout: 30_000 })
|
||||
await expectTerminalAccessibilityText(secondLaunch.page, restoredTabId, restoredMarker)
|
||||
await expect.poll(() => readRemoteProof(target!, afterProofPath)).toBe(beforeProof)
|
||||
} finally {
|
||||
if (secondApp) {
|
||||
|
||||
Reference in New Issue
Block a user