diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 758b9cfc..83c273c7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -112,7 +112,7 @@ Rules: no synchronous disk I/O on hot paths after boot, no full-state broadcasts ## 9. Testing and CI - **Unit** (`vitest`): shared logic (tree moves, URL parsing, accelerators, theme math, persistence normalization), main-process models with fakes. Fast; run on every commit. -- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server. The app runs behind a dead local proxy that loopback bypasses, and `launchOrbit` waits until every profile's session uses it, so a test that searches or follows a link to a real site gets the URL but loads nothing. Playwright reaches main through the inspector, which can interrupt main-process code part-way, even inside a store update. So `launchOrbit` makes each `app.evaluate` start in a task of its own, and the hooks' `run` and `action` defer the same way (D25). Main logs an exception nothing caught as `[orbit] uncaught exception:` rather than letting Electron show its modal box, which blocks quitting. `app.close()` fails a test whose main process logged one (`takeUncaught()` claims one thrown on purpose), and kills an app that hasn't quit 15 s after being asked (D28). **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`. +- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server. The app runs behind a dead local proxy that loopback bypasses, and `launchOrbit` waits until every profile's session uses it, so a test that searches or follows a link to a real site gets the URL but loads nothing. Playwright reaches main through the inspector, which can interrupt main-process code part-way, even inside a store update. So `launchOrbit` makes each `app.evaluate` start in a task of its own, and the hooks' `run` and `action` defer the same way (D25). Main logs an exception nothing caught as `[orbit] uncaught exception:` rather than letting Electron show its modal box, which blocks quitting. `app.close()` fails a test whose main process logged one (`takeUncaught()` claims one thrown on purpose). It also kills an app that hasn't quit 15 s after the quit reached main (main logs `[orbit] quit requested` in e2e mode), or that the quit hasn't reached 35 s after `app.close()` (D28). An app that quit on its own, when its last window closed, is checked by the next `launchOrbit()`. On Linux the failure names the main thread's kernel wait; on macOS, the deepest frames of a one-second `sample`. **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`. - **CI** (`.github/workflows/ci.yml`): typecheck + lint + unit (Linux), e2e under Xvfb (Linux), and build + unit + e2e + unsigned `.dmg`/`.zip` packaging (macOS). Green CI is required before merge. - **Feel checks and clips**: `scripts/capture/record.mjs ` drives the built app through `scripts/capture/scenarios/.mjs` while ffmpeg (`ORBIT_FFMPEG`, needs x11grab) records the X display, so native tab views are captured exactly as composited. Clips are named by feature ID and stored in `clips/` in the Orca context directory, screenshots in `screenshots/`. "Feels right" in `FEATURES.md` is a lead sign-off against `feel.md`. diff --git a/src/main/index.ts b/src/main/index.ts index a70b9e20..ea75f7c8 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -20,6 +20,11 @@ perf.mark('main:start') // An exception nothing caught is logged, and Orbit carries on. Electron's default shows it in a modal // box that blocks the main thread until someone dismisses it: until then Orbit can't even quit (D28). process.on('uncaughtException', (err) => console.error('[orbit] uncaught exception:', err)) +// A promise rejection nothing handled is logged with the same marker, so the e2e quit check fails +// the test that caused it. Electron's default only prints a warning (PR #11 review F3). +process.on('unhandledRejection', (reason) => + console.error('[orbit] uncaught exception: (a promise rejection nothing handled)', reason), +) // Tests and perf runs isolate their profile data. if (process.env.ORBIT_USER_DATA) app.setPath('userData', process.env.ORBIT_USER_DATA) @@ -90,6 +95,8 @@ function start(): void { if (windows.ids().length === 0) windows.open() }) app.on('before-quit', () => { + // The e2e harness times a quit from here: Playwright does work of its own before it asks (D28). + if (e2e) console.error('[orbit] quit requested') windows.quitting = true persistence.flush() }) diff --git a/src/main/tabs/runtime.ts b/src/main/tabs/runtime.ts index 82c8da19..ab89f064 100644 --- a/src/main/tabs/runtime.ts +++ b/src/main/tabs/runtime.ts @@ -418,7 +418,9 @@ export class TabRuntime { } private load(contents: WebContents, url: string): void { - if (!url || url === 'about:blank') return + // A tab closed while its history was restoring lands here from restore's catch, after its + // contents are gone: loadURL would throw, and the rejection would go unhandled. + if (!url || url === 'about:blank' || contents.isDestroyed()) return contents.loadURL(url).catch(() => { // Failures surface through did-fail-load; ERR_ABORTED is normal on redirects. }) diff --git a/tests/e2e/core.spec.ts b/tests/e2e/core.spec.ts index 180f5339..604412a1 100644 --- a/tests/e2e/core.spec.ts +++ b/tests/e2e/core.spec.ts @@ -1,4 +1,5 @@ import { mkdtempSync, writeFileSync } from 'node:fs' +import { createServer } from 'node:http' import { tmpdir } from 'node:os' import { join } from 'node:path' import { pathToFileURL } from 'node:url' @@ -10,6 +11,7 @@ import { action, type Fixtures, launchOrbit, + listenInRange, MOD, type Orbit, openViaCommandBar, @@ -330,6 +332,44 @@ test('[TB-22] the quit check also catches an exception whose log is long (D28, P await expect.poll(() => orbit.takeUncaught()).toContain('long zzz') }) +test('[TB-22] the quit check also catches a promise rejection nothing handled (D28, PR #11 review F3)', async () => { + // Electron only prints a warning for these, so the check used to miss them. + await orbit.app.evaluate(() => { + void Promise.reject(new Error('a rejection nothing handled')) + }) + await expect.poll(() => orbit.takeUncaught()).toContain('a rejection nothing handled') +}) + +test('[TB-22] a tab closed while its history is restoring leaves no rejection behind (D28, archive review F8)', async () => { + const { app } = orbit + // A page that stops answering once `hang` is set, so restoring a history that ends on it never finishes. + let hang = false + const slow = createServer((_req, res) => { + if (!hang) res.end('Slow') + }) + const port = await listenInRange(slow) + try { + const tabId = (await run(app, 'tabs.create', { url: fx.url('/page/One') })) as string + await tabPage(app, '/page/One') + await run(app, 'tabs.navigate', { tabId, url: `http://127.0.0.1:${port}/slow` }) + await tabPage(app, '/slow') + await run(app, 'tabs.create', { url: fx.url('/page/Other') }) + await tabPage(app, '/page/Other') + // Asleep, the tab keeps its history; waking it restores that history, ending on the slow page. + await app.evaluate((_e, id) => (globalThis as unknown as OrbitGlobal).__orbit.ctx.tabs.sleep(id), tabId) + hang = true + await run(app, 'tabs.activate', { tabId }) + await expect.poll(async () => (await state(app)).tabs[tabId]?.loading).toBe(true) + // Closing it mid-restore made restore's catch load the page into contents already destroyed. + await run(app, 'tabs.close', { tabId }) + await new Promise((r) => setTimeout(r, 1500)) + expect(orbit.takeUncaught()).toBe('') + } finally { + slow.closeAllConnections() + slow.close() + } +}) + test('[TB-02] opening tabs keeps every UI mirror in step without resyncs (review M2, F4)', async () => { const errors: string[] = [] orbit.shell.on('console', (m) => m.type() === 'error' && errors.push(m.text())) diff --git a/tests/e2e/helpers.ts b/tests/e2e/helpers.ts index d3fb939e..b9730be4 100644 --- a/tests/e2e/helpers.ts +++ b/tests/e2e/helpers.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'node:child_process' import { mkdtempSync, readFileSync } from 'node:fs' import { createServer, type Server } from 'node:http' import type { AddressInfo } from 'node:net' @@ -42,6 +43,8 @@ const OFFLINE = `--proxy-server=http://${DEAD_PROXY}` export async function launchOrbit( opts: { userData?: string; env?: Record; reducedMotion?: boolean; args?: string[] } = {}, ): Promise { + // An app that quit on its own (its last window closed) was never asked to close: check it now. + for (const old of quitAlone) await old.close() const userData = opts.userData ?? mkdtempSync(join(tmpdir(), 'orbit-e2e-')) const motion = opts.reducedMotion ? '--force-prefers-reduced-motion' : '--force-prefers-no-reduced-motion' const args = [OFFLINE, motion, ...(opts.args ?? []), resolve('out/main/index.js')] @@ -99,48 +102,99 @@ function evaluateBetweenTasks(app: ElectronApplication): void { }) as ElectronApplication['evaluate'] } -/** How Orbit's main process logs an exception nothing caught (src/main/index.ts). */ +/** + * How Orbit's main process logs an exception nothing caught, or a promise rejection nothing handled + * (src/main/index.ts). + */ const UNCAUGHT = '[orbit] uncaught exception:' +/** What Orbit's main process logs in e2e mode when a quit reaches it (src/main/index.ts). */ +const QUITTING = '[orbit] quit requested' +/** How long a quit may take once it has reached Orbit's main process. Healthy ones take under 0.5 s. */ +const QUIT_BUDGET = 15_000 +/** + * How long `app.close()` waits for the quit to reach main at all. Playwright does work of its own + * before it asks: about 5 s after a failed test here, and longer on a slower runner. + */ +const ASK_BUDGET = 35_000 + +/** Apps that quit on their own, with no `app.close()` to check them yet. */ +const quitAlone = new Set() /** - * `app.close()` checks the quit too. The test fails if Orbit's main process logged an exception - * nothing caught, even one thrown while quitting. If Orbit still hasn't quit 15 s after being asked, - * the test fails and Orbit is killed, so one stuck quit can't hang the rest of the run (D28). + * `app.close()` checks the quit too (D28): + * - the test fails if Orbit's main process logged an exception nothing caught or a promise + * rejection nothing handled, even while quitting; + * - if Orbit hasn't quit 15 s after the quit reached its main process, or the quit hasn't reached it + * 35 s after `app.close()`, the test fails and Orbit is killed, so one stuck quit can't hang the + * rest of the run. + * + * An app that quits on its own is checked by the next `launchOrbit()`, or by its own `app.close()`. */ function checkQuit(app: ElectronApplication): () => string { const proc = app.process() let recent = '' let thrown = '' + let quitting = false + let onQuitting: (() => void) | undefined proc.stderr?.on('data', (chunk) => { + const piece = String(chunk) + // Look for the markers before trimming: one write can be longer than what is kept. The tail + // kept is just long enough to find a marker split across two writes. + const text = recent + piece + recent = text.slice(-Math.max(UNCAUGHT.length, QUITTING.length)) + if (!quitting && text.includes(QUITTING)) { + quitting = true + onQuitting?.() + } if (thrown) { - if (thrown.length < 4000) thrown += String(chunk) + if (thrown.length < 4000) thrown += piece return } - // Look for the marker before trimming: one write can be longer than what is kept. The tail - // kept is just long enough to find a marker split across two writes. - const text = recent + String(chunk) const at = text.indexOf(UNCAUGHT) if (at >= 0) thrown = text.slice(at, at + 4000) - else recent = text.slice(-UNCAUGHT.length) }) const outputDone = new Promise((resolve) => (proc.stderr ? proc.stderr.once('close', resolve) : resolve(null))) const close = app.close.bind(app) + let asked = false let checked = false + app.once('close', () => { + if (!asked) quitAlone.add(app) + }) app.close = async () => { - let timer: NodeJS.Timeout | undefined - const late = new Promise((resolve) => { - timer = setTimeout(() => resolve(true), 15_000) - }) + asked = true + quitAlone.delete(app) const closed = close() - const stuck = await Promise.race([closed.then(() => false), late]) - clearTimeout(timer) + const timers: NodeJS.Timeout[] = [] + const stuck = await new Promise((resolve, reject) => { + const settle = () => { + for (const t of timers) clearTimeout(t) + onQuitting = undefined + } + const done = (why: string) => { + settle() + resolve(why) + } + closed.then( + () => done(''), + (err) => { + settle() + reject(err) + }, + ) + const quitClock = () => timers.push(setTimeout(() => done('it had the quit for 15 s'), QUIT_BUDGET)) + if (quitting) quitClock() + else onQuitting = quitClock + timers.push(setTimeout(() => done('the quit never reached its main process'), ASK_BUDGET)) + }) if (checked) return checked = true if (stuck) { const waiting = mainThreadWait(proc.pid) proc.kill('SIGKILL') await closed - expect.soft(`still running (main thread: ${waiting})`, 'Orbit 15 s after app.close(); it was killed').toBe('quit') + expect + .soft(`still running: ${stuck} (main thread: ${waiting})`, 'Orbit after app.close(); it was killed') + .toBe('quit') } await Promise.race([outputDone, new Promise((resolve) => setTimeout(resolve, 1000))]) expect.soft(thrown, "Orbit's main process threw an exception nothing caught").toBe('') @@ -153,15 +207,32 @@ function checkQuit(app: ElectronApplication): () => string { } } -/** Where a process's main thread is waiting in the kernel (Linux only). */ +/** + * Where a stuck process's main thread is: its wait in the kernel on Linux, the deepest frames of a + * one-second `sample` on macOS (no `/proc` there). + */ function mainThreadWait(pid: number | undefined): string { try { + if (process.platform === 'darwin') return sampleMainThread(pid) return readFileSync(`/proc/${pid}/wchan`, 'utf8') } catch { return 'unknown' } } +/** The deepest frames of the main thread in a one-second `sample` of the process (macOS). */ +function sampleMainThread(pid: number | undefined): string { + const file = join(mkdtempSync(join(tmpdir(), 'orbit-sample-')), 'sample.txt') + execFileSync('/usr/bin/sample', [String(pid), '1', '-file', file], { stdio: 'ignore', timeout: 20_000 }) + const lines = readFileSync(file, 'utf8').split('\n') + const start = lines.findIndex((line) => line.includes('com.apple.main-thread')) + if (start < 0) return `unknown (no main thread in ${file})` + // The main thread's call tree runs until the next thread's header or the blank line after it. + const end = lines.findIndex((line, i) => i > start && (/^\s*\d+ Thread_/.test(line) || line.trim() === '')) + const frames = lines.slice(start + 1, end < 0 ? undefined : end).map((line) => line.replace(/^[\s+!:|]+/, '')) + return `${frames.slice(-40).join('\n')}\n(full sample: ${file})` +} + /** Finds an Orbit UI page (shell/overlay) by its ?view= query. */ export async function viewPage(app: ElectronApplication, view: 'shell' | 'overlay'): Promise { const match = (p: Page) => p.url().includes(`view=${view}`)