From f0ef250b125e3537b079348cfb5c745e73e60751 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 24 Oct 2026 09:49:07 -0700 Subject: [PATCH] test: remove duplicate capability coverage and strengthen boundary assertions (#33197) * test: remove duplicate capability coverage and strengthen boundary assertions * test: preserve background task name assertion across mock resets --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../desktop-notification-channel.test.ts | 13 ++- .../mobile-push-lease-renewal.test.ts | 5 +- .../notification-opt-in-gate.test.ts | 2 +- .../push-background-dismissal.test.ts | 12 ++- .../push-dismissal-watermarks.test.ts | 3 +- .../push-host-fingerprint.test.ts | 24 ++--- .../push-notification-identity.test.ts | 33 ++++--- .../lib/typing-latency/sample-window.test.ts | 23 +++-- .../agent-process-presence-platform.test.ts | 2 + .../agent-status-run-capability.test.ts | 6 -- src/shared/agent-tui-input-clear.test.ts | 4 +- src/shared/automation-cron-dialect.test.ts | 2 +- .../external-automation-jobs-file.test.ts | 2 +- .../twin-repo-host-routing.test-fixture.ts | 97 ++++++++++++------- 14 files changed, 133 insertions(+), 95 deletions(-) diff --git a/mobile/src/notifications/desktop-notification-channel.test.ts b/mobile/src/notifications/desktop-notification-channel.test.ts index 01ae373b34c..1627f8f65ed 100644 --- a/mobile/src/notifications/desktop-notification-channel.test.ts +++ b/mobile/src/notifications/desktop-notification-channel.test.ts @@ -20,13 +20,16 @@ beforeEach(() => { }) describe('ensureDesktopNotificationChannel', () => { - it('creates the channel the gateway payload names', async () => { + it('creates the audible and silent channels the gateway payload names', async () => { await ensureDesktopNotificationChannel() - expect(Notifications.setNotificationChannelAsync).toHaveBeenCalledWith( - 'orca-desktop', - expect.objectContaining({ importance: 'high' }) - ) + expect(vi.mocked(Notifications.setNotificationChannelAsync).mock.calls).toEqual([ + [ + 'orca-desktop-silent', + expect.objectContaining({ importance: 'high', sound: null, enableVibrate: false }) + ], + ['orca-desktop', expect.objectContaining({ importance: 'high' })] + ]) }) it('does nothing on iOS, which has no notification channels', () => { diff --git a/mobile/src/notifications/mobile-push-lease-renewal.test.ts b/mobile/src/notifications/mobile-push-lease-renewal.test.ts index 1f440e93abc..658c9160c84 100644 --- a/mobile/src/notifications/mobile-push-lease-renewal.test.ts +++ b/mobile/src/notifications/mobile-push-lease-renewal.test.ts @@ -7,10 +7,10 @@ const remove = vi.fn() vi.mock('react-native', () => ({ AppState: { currentState: 'active', - addEventListener: (_: string, callback: typeof onChange) => { + addEventListener: vi.fn((_: string, callback: typeof onChange) => { onChange = callback return { remove } - } + }) } })) afterEach(() => { @@ -23,6 +23,7 @@ it('renews only while mobile is foregrounded, resumes on return, and tears down' AppState.currentState = 'active' const renew = vi.fn(async () => {}) const stop = startMobilePushLeaseRenewal(renew) + expect(AppState.addEventListener).toHaveBeenCalledExactlyOnceWith('change', expect.any(Function)) await vi.advanceTimersByTimeAsync(15 * 60_000) expect(renew).toHaveBeenCalledTimes(1) AppState.currentState = 'background' diff --git a/mobile/src/notifications/notification-opt-in-gate.test.ts b/mobile/src/notifications/notification-opt-in-gate.test.ts index 8b99423dfb4..e2cd53aab62 100644 --- a/mobile/src/notifications/notification-opt-in-gate.test.ts +++ b/mobile/src/notifications/notification-opt-in-gate.test.ts @@ -7,7 +7,7 @@ vi.mock('../storage/preferences', () => ({ })) describe('notification opt-in gate', () => { - it('asks for push-service consent when no choice is saved, regardless of OS permission', async () => { + it('asks for push-service consent when no choice is saved', async () => { vi.mocked(readPushNotificationsPreference).mockResolvedValue({ value: null, loaded: true }) await expect(shouldPresentNotificationOptIn()).resolves.toBe(true) }) diff --git a/mobile/src/notifications/push-background-dismissal.test.ts b/mobile/src/notifications/push-background-dismissal.test.ts index 40e535e2fcf..179904c6396 100644 --- a/mobile/src/notifications/push-background-dismissal.test.ts +++ b/mobile/src/notifications/push-background-dismissal.test.ts @@ -1,7 +1,13 @@ import { expect, it, vi } from 'vitest' -const state = vi.hoisted(() => ({ task: null as null | ((input: unknown) => Promise) })) +const state = vi.hoisted( + (): { taskName: string; task: null | ((input: unknown) => Promise) } => ({ + taskName: '', + task: null + }) +) vi.mock('expo-task-manager', () => ({ - defineTask: (_name: string, task: typeof state.task) => { + defineTask: (name: string, task: typeof state.task) => { + state.taskName = name state.task = task }, isAvailableAsync: async () => true @@ -24,6 +30,8 @@ import { registerPushDismissalTask } from './push-background-dismissal' it('handles native background JSON and scopes dismissal to the originating host', async () => { await registerPushDismissalTask() + expect(state.taskName).toBe('orca-push-dismissal') + expect(Notifications.registerTaskAsync).toHaveBeenCalledExactlyOnceWith('orca-push-dismissal') await state.task!({ data: { data: { diff --git a/mobile/src/notifications/push-dismissal-watermarks.test.ts b/mobile/src/notifications/push-dismissal-watermarks.test.ts index cef0cf3404e..228e624c426 100644 --- a/mobile/src/notifications/push-dismissal-watermarks.test.ts +++ b/mobile/src/notifications/push-dismissal-watermarks.test.ts @@ -63,7 +63,8 @@ it('joins an overtaking JavaScript write before retrying a delayed negative snap return snapshot }) const pending = wasPushDismissed(payload) - await vi.waitFor(() => expect(finish).toBeDefined()) + await Promise.resolve() + expect(finish).toBeDefined() await rememberPushDismissal(payload) finish() expect(await pending).toBe(true) diff --git a/mobile/src/notifications/push-host-fingerprint.test.ts b/mobile/src/notifications/push-host-fingerprint.test.ts index 2fc5b44dba1..3205c39d184 100644 --- a/mobile/src/notifications/push-host-fingerprint.test.ts +++ b/mobile/src/notifications/push-host-fingerprint.test.ts @@ -1,14 +1,6 @@ import { describe, expect, it } from 'vitest' -import { sha256 } from '@noble/hashes/sha256' import { deriveHostFingerprint, resolveHostIdForFingerprint } from './push-host-fingerprint' -// Why Buffer here: it computes the same value through a completely different -// base64 path than the module's btoa/replace, so the vector is a real cross-check -// of the derivation the desktop and gateway independently perform. -function expectedFingerprint(publicKey: Uint8Array): string { - return Buffer.from(sha256(publicKey)).toString('base64url').slice(0, 16) -} - const publicKey = Uint8Array.from({ length: 32 }, (_, index) => index) const publicKeyB64 = Buffer.from(publicKey).toString('base64') @@ -16,16 +8,16 @@ describe('deriveHostFingerprint', () => { it('matches base64url(sha256(publicKey)) truncated to 16 chars', () => { const fingerprint = deriveHostFingerprint(publicKeyB64) - expect(fingerprint).toBe(expectedFingerprint(publicKey)) + expect(fingerprint).toBe('Yw3NKWbEM2aRElRI') expect(fingerprint).toHaveLength(16) }) it('produces url-safe characters only, so a fingerprint survives a JSON payload', () => { - // 0xff bytes are what push '+' and '/' into a standard base64 digest. - const dense = new Uint8Array(32).fill(0xff) + // This digest prefix contains both '/' and '+' before URL normalization. + const dense = new Uint8Array(32).fill(87) const fingerprint = deriveHostFingerprint(Buffer.from(dense).toString('base64')) - expect(fingerprint).toBe(expectedFingerprint(dense)) + expect(fingerprint).toBe('3G2G1n4fGgJbM_r-') expect(fingerprint).toMatch(/^[A-Za-z0-9_-]{16}$/) }) @@ -47,16 +39,14 @@ describe('resolveHostIdForFingerprint', () => { ] it('maps a push fingerprint back to the paired host id', () => { - expect(resolveHostIdForFingerprint(expectedFingerprint(publicKey), hosts)).toBe('host-1') + expect(resolveHostIdForFingerprint('Yw3NKWbEM2aRElRI', hosts)).toBe('host-1') }) it('returns null for a fingerprint no paired host derives', () => { expect(resolveHostIdForFingerprint('0123456789abcdef', hosts)).toBeNull() }) - it('rejects a fingerprint of the wrong length before hashing anything', () => { - expect( - resolveHostIdForFingerprint(expectedFingerprint(publicKey).slice(0, 8), hosts) - ).toBeNull() + it('rejects a fingerprint of the wrong length', () => { + expect(resolveHostIdForFingerprint('Yw3NKWbEM2aRElRI'.slice(0, 8), hosts)).toBeNull() }) }) diff --git a/mobile/src/notifications/push-notification-identity.test.ts b/mobile/src/notifications/push-notification-identity.test.ts index df3dfd9d152..96a65b13722 100644 --- a/mobile/src/notifications/push-notification-identity.test.ts +++ b/mobile/src/notifications/push-notification-identity.test.ts @@ -6,20 +6,31 @@ import { it('reads a bounded individual notification identity', () => { const identity: PushNotificationIdentity = { - notificationId: 'agent:one', - notificationEpoch: 'epoch-1', - notificationSeq: 7 + notificationId: 'n'.repeat(2048), + notificationEpoch: 'e'.repeat(128), + notificationSeq: Number.MAX_SAFE_INTEGER } expect(readPushNotificationIdentity(identity)).toEqual(identity) }) -it('rejects incomplete or non-integral notification identities', () => { +it('rejects incomplete or out-of-bounds notification identities', () => { + const identity = { + notificationId: 'agent:one', + notificationEpoch: 'epoch-1', + notificationSeq: 7 + } expect(readPushNotificationIdentity({ notificationId: 'agent:one' })).toBeNull() - expect( - readPushNotificationIdentity({ - notificationId: 'agent:one', - notificationEpoch: 'epoch-1', - notificationSeq: 1.5 - }) - ).toBeNull() + for (const invalid of [ + { notificationId: '' }, + { notificationId: 'n'.repeat(2049) }, + { notificationEpoch: '' }, + { notificationEpoch: 'e'.repeat(129) }, + { notificationEpoch: undefined }, + { notificationSeq: undefined }, + { notificationSeq: -1 }, + { notificationSeq: 1.5 }, + { notificationSeq: Number.MAX_SAFE_INTEGER + 1 } + ]) { + expect(readPushNotificationIdentity({ ...identity, ...invalid })).toBeNull() + } }) diff --git a/src/renderer/src/lib/typing-latency/sample-window.test.ts b/src/renderer/src/lib/typing-latency/sample-window.test.ts index 909e1bd430b..9ab8addea6d 100644 --- a/src/renderer/src/lib/typing-latency/sample-window.test.ts +++ b/src/renderer/src/lib/typing-latency/sample-window.test.ts @@ -11,18 +11,25 @@ describe('typing latency sample window', () => { source: 'direct', text: 'a', inputToDispatchMs: value, - dispatchToParseMs: value, - parseToPaintMs: value, - inputToPaintMs: value, - outputBytes: value, - outputWrites: value + dispatchToParseMs: value + 10000, + parseToPaintMs: value + 20000, + inputToPaintMs: 3 * value + 30000, + outputBytes: value + 40000, + outputWrites: value + 5000 }) } - for (const values of Object.values(samples)) { + for (const [values, first, last] of [ + [samples.inputToDispatchMs, 1, 2000], + [samples.dispatchToParseMs, 10_001, 12_000], + [samples.parseToPaintMs, 20_001, 22_000], + [samples.inputToPaintMs, 30_003, 36_000], + [samples.outputBytes, 40_001, 42_000], + [samples.outputWrites, 5001, 7000] + ] as const) { expect(values).toHaveLength(2000) - expect(values[0]).toBe(1) - expect(values.at(-1)).toBe(2000) + expect(values[0]).toBe(first) + expect(values.at(-1)).toBe(last) } }) }) diff --git a/src/shared/agent-process-presence-platform.test.ts b/src/shared/agent-process-presence-platform.test.ts index 3bcfad9f4bf..98d3064bb77 100644 --- a/src/shared/agent-process-presence-platform.test.ts +++ b/src/shared/agent-process-presence-platform.test.ts @@ -32,6 +32,7 @@ describe('platform process evidence', () => { platform('linux') read.mockRejectedValueOnce(Object.assign(new Error('gone'), { code: 'ENOENT' })) expect(await readAgentProcess(4242)).toEqual({ verdict: 'exited' }) + expect(read).toHaveBeenLastCalledWith('/proc/4242/stat', 'utf8') const fields = ['S', ...Array(18).fill('0'), '100'] read .mockResolvedValueOnce(`4242 (agent (name)) ${fields.join(' ')}`) @@ -46,6 +47,7 @@ describe('platform process evidence', () => { zombie: false, stopped: false }) + expect(read).toHaveBeenLastCalledWith('/proc/sys/kernel/random/boot_id', 'utf8') read .mockResolvedValueOnce(`4242 (agent) ${['T', ...fields.slice(1)].join(' ')}`) .mockResolvedValueOnce('boot-id\n') diff --git a/src/shared/agent-status-run-capability.test.ts b/src/shared/agent-status-run-capability.test.ts index b5848dd49ac..3533b8d347f 100644 --- a/src/shared/agent-status-run-capability.test.ts +++ b/src/shared/agent-status-run-capability.test.ts @@ -28,12 +28,6 @@ describe('agent status run capability codec', () => { ).toEqual(new Set([AGENT_STATUS_RUNS_RUNTIME_CAPABILITY])) }) - it('finds the run capability in the current host capability inventory', () => { - expect( - hasAgentStatusRunCapability([...RUNTIME_CAPABILITIES, AGENT_STATUS_RUNS_RUNTIME_CAPABILITY]) - ).toBe(true) - }) - it.each([null, {}, 'agent-status.runs.v1', [null], ['']])( 'rejects malformed capability envelope %#', (value) => { diff --git a/src/shared/agent-tui-input-clear.test.ts b/src/shared/agent-tui-input-clear.test.ts index 67715ce2df2..7092bf0decb 100644 --- a/src/shared/agent-tui-input-clear.test.ts +++ b/src/shared/agent-tui-input-clear.test.ts @@ -32,9 +32,7 @@ describe('buildAgentTuiClearInput', () => { }) it('clears before the cursor before clearing the suffix after it', () => { - expect(buildAgentTuiClearInput(4)).toBe( - AGENT_TUI_CLEAR_INPUT_LINE.repeat(7) + AGENT_TUI_CLEAR_INPUT_FORWARD.repeat(7) - ) + expect(buildAgentTuiClearInput(4)).toBe('\x15'.repeat(7) + '\x0b'.repeat(7)) }) it('still clears one line for a zero or negative count', () => { diff --git a/src/shared/automation-cron-dialect.test.ts b/src/shared/automation-cron-dialect.test.ts index dd6500b1549..88a3b226397 100644 --- a/src/shared/automation-cron-dialect.test.ts +++ b/src/shared/automation-cron-dialect.test.ts @@ -107,7 +107,7 @@ describe('cron day restriction (#15896)', () => { // The star test is per comma term, so a list that reaches a star anywhere is unrestricted. it('treats a day list containing a star term as a star', () => { - expect(matchingDaysOfMay2026('0 9 */3 * 1,5')).toEqual([1, 4, 22, 25]) + expect(matchingDaysOfMay2026('0 9 2,*/3 * 1,5')).toEqual([1, 4, 22, 25]) }) it('normalizes Sunday from 0, from 7 and from the name', () => { diff --git a/src/shared/external-automation-jobs-file.test.ts b/src/shared/external-automation-jobs-file.test.ts index 13b8a7078bb..3c9ed7fbd75 100644 --- a/src/shared/external-automation-jobs-file.test.ts +++ b/src/shared/external-automation-jobs-file.test.ts @@ -35,7 +35,7 @@ describe('readExternalAutomationJobsFile', () => { ).resolves.toEqual([{ id: 'two' }]) }) - it('rejects oversized sparse files before reading them wholesale', async () => { + it('rejects oversized sparse files with the explicit memory-limit error', async () => { const path = await tempFile('oversized.json') await writeFile(path, '') await truncate(path, EXTERNAL_AUTOMATION_JOBS_FILE_MAX_BYTES + 64 * 1024 * 1024) diff --git a/src/shared/twin-repo-host-routing.test-fixture.ts b/src/shared/twin-repo-host-routing.test-fixture.ts index 1d0cf2ab2b0..255ef013027 100644 --- a/src/shared/twin-repo-host-routing.test-fixture.ts +++ b/src/shared/twin-repo-host-routing.test-fixture.ts @@ -78,46 +78,69 @@ export type TwinRepoHostCase = { expected: ExecutionHostId } -const PAIRS: [string, TwinRepoRow, ExecutionHostId, TwinRepoRow, ExecutionHostId][] = [ - ['local+ssh', LOCAL_ROW, 'local', SSH_ROW, 'ssh:box'], - ['local+runtime', LOCAL_ROW, 'local', RUNTIME_ROW, 'runtime:env-1'], - ['ssh+runtime', SSH_ROW, 'ssh:box', RUNTIME_ROW, 'runtime:env-1'] +const PAIRS: [ + string, + TwinRepoRow, + ExecutionHostId, + ExecutionHostId, + TwinRepoRow, + ExecutionHostId, + ExecutionHostId, + ExecutionHostId +][] = [ + ['local+ssh', LOCAL_ROW, 'local', 'local', SSH_ROW, 'ssh:box', 'local', 'local'], + [ + 'local+runtime', + LOCAL_ROW, + 'local', + 'local', + RUNTIME_ROW, + 'runtime:env-1', + 'runtime:env-1', + 'local' + ], + [ + 'ssh+runtime', + SSH_ROW, + 'ssh:box', + 'local', + RUNTIME_ROW, + 'runtime:env-1', + 'runtime:env-1', + 'ssh:box' + ] ] -// The window: a contested id goes to the shared primary, a runtime-owned one to its runtime, and -// any other (SSH-only, local-only, unknown) to local, because the twin repo names no single host. -function windowFresh(hosts: ExecutionHostId[]): ExecutionHostId { - if (hosts.length > 1) { - return hosts.includes('local') - ? 'local' - : (hosts.find((h) => h.startsWith('ssh:')) ?? hosts[0]!) +const GENERATED_CASES: TwinRepoHostCase[] = PAIRS.flatMap( + ([pair, rowA, hostA, expectedA, rowB, hostB, expectedB, contested]) => { + // Both catalog orders, so a case cannot pass by taking the first row. + const orders = [ + [rowB, rowA], + [rowA, rowB] + ] + const holdings: [string, ExecutionHostId[], ExecutionHostId][] = [ + ['contested', [hostA, hostB], contested], + [`${hostA} only`, [hostA], expectedA], + [`${hostB} only`, [hostB], expectedB], + ['unknown', [], 'local'] + ] + return orders.flatMap((repos, order) => + holdings.flatMap(([holding, hosts, expected]) => { + const catalog = hosts.map(listedOn) + return [ + { name: `${pair} #${order} ${holding}, fresh`, repos, catalog, saved: [], expected }, + { + name: `${pair} #${order} ${holding}, saved`, + repos, + catalog, + saved: [expected], + expected + } + ] + }) + ) } - return hosts[0]?.startsWith('runtime:') ? hosts[0] : 'local' -} - -const GENERATED_CASES: TwinRepoHostCase[] = PAIRS.flatMap(([pair, rowA, hostA, rowB, hostB]) => { - // Both catalog orders, so a case cannot pass by taking the first row. - const orders = [ - [rowB, rowA], - [rowA, rowB] - ] - const holdings: [string, ExecutionHostId[]][] = [ - ['contested', [hostA, hostB]], - [`${hostA} only`, [hostA]], - [`${hostB} only`, [hostB]], - ['unknown', []] - ] - return orders.flatMap((repos, order) => - holdings.flatMap(([holding, hosts]) => { - const expected = windowFresh(hosts) - const catalog = hosts.map(listedOn) - return [ - { name: `${pair} #${order} ${holding}, fresh`, repos, catalog, saved: [], expected }, - { name: `${pair} #${order} ${holding}, saved`, repos, catalog, saved: [expected], expected } - ] - }) - ) -}) +) export const TWIN_REPO_HOST_CASES: TwinRepoHostCase[] = [ ...GENERATED_CASES,