From b15a2ca6b61aca3119537e131aab9cf796509d97 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 20 Aug 2026 03:57:32 -0400 Subject: [PATCH] test(browser): close the round-five coverage seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test-adequacy findings from the fifth review round — no production changes. The real backend's listOpenPageIds order across park/wake was only exercised through a test fake; the e2e index spec never asserted the creation order end to end; nothing pinned that a live active tab suppresses parked-active promotion on close; blank env knobs relied on an untested guard (Number('') is 0); and stale ids in a creation order had no case. Each new test was mutation-checked where a named mutant existed: listOpenPageIds returning [] and the promotion condition dropped both go red against exactly their test. --- .../headless-browser-tab-listing.test.ts | 8 ++++ .../offscreen-browser-backend-reclaim.test.ts | 45 +++++++++++++++++++ .../offscreen-browser-page-reclaim.test.ts | 7 +++ ...headless-serve-browser-tab-reclaim.spec.ts | 6 ++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/main/browser/headless-browser-tab-listing.test.ts b/src/main/browser/headless-browser-tab-listing.test.ts index 0b3e42c2152..8000e12e67e 100644 --- a/src/main/browser/headless-browser-tab-listing.test.ts +++ b/src/main/browser/headless-browser-tab-listing.test.ts @@ -87,6 +87,14 @@ describe('mergeParkedBrowserTabs', () => { ]) }) + it('ignores stale ids in the creation order', () => { + const merged = mergeParkedBrowserTabs([live('b', 0, true)], [parked('a')], ['closed', 'a', 'b']) + expect(merged.map((tab) => [tab.browserPageId, tab.index])).toEqual([ + ['a', 0], + ['b', 1] + ]) + }) + it('keeps ids missing from the creation order at their merged position', () => { const merged = mergeParkedBrowserTabs([live('x', 0, true)], [parked('a')], ['a']) expect(merged.map((tab) => [tab.browserPageId, tab.index])).toEqual([ diff --git a/src/main/browser/offscreen-browser-backend-reclaim.test.ts b/src/main/browser/offscreen-browser-backend-reclaim.test.ts index d36f4ff2983..b08c2a043be 100644 --- a/src/main/browser/offscreen-browser-backend-reclaim.test.ts +++ b/src/main/browser/offscreen-browser-backend-reclaim.test.ts @@ -730,6 +730,51 @@ describe('OffscreenBrowserBackend reclamation', () => { expect(claimed.map((page) => page.browserPageId)).toEqual(['b']) }) + it('lists open page ids in creation order across parks and wakes', async () => { + // Why: the merged tab listing sorts by this order; if it drifted with + // residency, a background park would renumber indices callers already read. + const h = createHarness() + await h.backend.createTab({ url: 'https://a', browserPageId: 'a', worktreeId: 'wt-1' }) + h.clock.value += 1_000 + await h.backend.createTab({ url: 'https://b', browserPageId: 'b', worktreeId: 'wt-1' }) + h.clock.value += 120_000 + await h.backend.reclaimIdlePages() + await h.backend.wakeTab('a') + + expect(h.backend.listOpenPageIds('wt-1')).toEqual(['a', 'b']) + expect(h.backend.listOpenPageIds()).toEqual(['a', 'b']) + }) + + it('does not promote a parked page while a live tab holds active', async () => { + // Why: promotion exists for the no-live-active gap only; a live active tab + // already gives the listing its selection, and a parked page claiming it + // too would put two stars on the worktree. + const h = createHarness({ activePageId: 'live' }) + await h.backend.createTab({ + url: 'https://parked', + browserPageId: 'parked', + worktreeId: 'wt-1' + }) + h.clock.value += 1_000 + await h.backend.createTab({ url: 'https://live', browserPageId: 'live', worktreeId: 'wt-1' }) + h.clock.value += 1_000 + await h.backend.createTab({ + url: 'https://doomed', + browserPageId: 'doomed', + worktreeId: 'wt-1' + }) + h.clock.value += 120_000 + await h.backend.wakeTab('live') + await h.backend.wakeTab('doomed') + await h.backend.reclaimIdlePages() + + await h.backend.closeTab('doomed') + + expect( + h.backend.listParkedPages('wt-1').map((page) => [page.browserPageId, page.active === true]) + ).toEqual([['parked', false]]) + }) + it('hands the active flag to a survivor when the parked holder closes', async () => { // Why: closing a parked page has no bridge teardown to promote a successor, // so without this the worktree's every remaining tab reports inactive and a diff --git a/src/main/browser/offscreen-browser-page-reclaim.test.ts b/src/main/browser/offscreen-browser-page-reclaim.test.ts index 39c168416ec..22c363700ad 100644 --- a/src/main/browser/offscreen-browser-page-reclaim.test.ts +++ b/src/main/browser/offscreen-browser-page-reclaim.test.ts @@ -85,6 +85,13 @@ describe('readOffscreenBrowserRetentionBudget', () => { process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = '1234abc' expect(readOffscreenBrowserRetentionBudget().idleMs).toBe(OFFSCREEN_BROWSER_IDLE_PARK_MS) }) + + it('treats blank as unset, not as a zero window', () => { + // Why: Number('') and Number(' ') are 0 — a blank knob must not silently + // configure instant parking. + process.env.ORCA_HEADLESS_BROWSER_PARK_IDLE_MS = ' ' + expect(readOffscreenBrowserRetentionBudget().idleMs).toBe(OFFSCREEN_BROWSER_IDLE_PARK_MS) + }) }) describe('nextOffscreenBrowserReclaimCheckAt', () => { diff --git a/tests/e2e/headless-serve-browser-tab-reclaim.spec.ts b/tests/e2e/headless-serve-browser-tab-reclaim.spec.ts index 0888c45b1f9..efc0ffddb2f 100644 --- a/tests/e2e/headless-serve-browser-tab-reclaim.spec.ts +++ b/tests/e2e/headless-serve-browser-tab-reclaim.spec.ts @@ -233,11 +233,13 @@ test('resolves --index against the listing that includes parked tabs', async ({ const pageServer = await startAnimatedPageServer() try { const worktreeId = await resolveSeededWorktreeId(host, testRepoPath) - await createTabs(host, worktreeId, pageServer.url, 5) + const pageIds = await createTabs(host, worktreeId, pageServer.url, 5) await expect.poll(() => countOffscreenRenderers(host), { timeout: 30_000 }).toBe(2) const before = await listTabs(host, worktreeId) - expect(before).toHaveLength(5) + // Why: the listing is creation-ordered end to end — parking must not move + // a tab's position, or the index a caller read goes stale on a timer. + expect(before.map((tab) => tab.browserPageId)).toEqual(pageIds) // Why: the listing is creation-ordered, so parked and resident tabs are // interleaved. Target a parked one — an index the bridge's live-only index // handling would either miss or resolve to a different (live) tab.