From 0c06f1f082327c290ea94d7dfafcfb5bee451a3d Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 24 Oct 2026 01:22:18 -0700 Subject: [PATCH] test: consolidate duplicates and strengthen behavioral boundaries (#33088) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../scripts/check-test-file-runners.test.mjs | 14 +++- .../session/mobile-diff-comment-edit.test.ts | 1 + .../src/session/mobile-diff-comments.test.ts | 13 ---- .../mobile-diff-review-positioning.test.ts | 4 +- .../session/mobile-diff-review-queue.test.ts | 17 +++-- .../session/mobile-diff-review-state.test.ts | 1 + ...tration-structured-sender-identity.test.ts | 65 ++++++++++++++----- ...ion-structured-session-no-identity.test.ts | 4 ++ .../server-program-status-hook-yield.test.ts | 1 + .../program-status/hook-precedence.test.ts | 2 + .../filesystem-watcher-event-batch.test.ts | 12 +++- .../ipc/parcel-watcher-event-delivery.test.ts | 8 ++- .../methods/file-watch-event-batcher.test.ts | 1 + .../methods/files-watch-batch-cadence.test.ts | 10 ++- .../lib/markdown-document-templates.test.ts | 4 +- .../src/lib/pr-comment-action-state.test.ts | 1 + 16 files changed, 114 insertions(+), 44 deletions(-) diff --git a/config/scripts/check-test-file-runners.test.mjs b/config/scripts/check-test-file-runners.test.mjs index 66cc945b5c9..c9a8da41011 100644 --- a/config/scripts/check-test-file-runners.test.mjs +++ b/config/scripts/check-test-file-runners.test.mjs @@ -29,7 +29,19 @@ describe('findUncollectedTestFiles', () => { it('counts a suite module as collected only while a collected entry runtime-imports it', () => { const sources = new Map([ - ['src/page.test.ts', "import './page.case.spec'\nimport './twin.spec'\n"], + [ + 'src/page.test.ts', + [ + "import './page.case.spec'", + "import './twin.spec'", + "import '@renderer/alias-one.spec'", + "import '@/alias-two.spec.js'", + "import './nested-suite'" + ].join('\n') + ], + ['src/renderer/src/alias-one.spec.ts', SUITE], + ['src/renderer/src/alias-two.spec.ts', SUITE], + ['src/nested-suite/index.ts', SUITE], ['src/twin.spec.ts', SUITE], ['src/twin.spec.tsx', SUITE], ['src/page.case.spec.ts', `import './page-harness.spec'\n${SUITE}`], diff --git a/mobile/src/session/mobile-diff-comment-edit.test.ts b/mobile/src/session/mobile-diff-comment-edit.test.ts index 49328064cd5..6081e97d9a3 100644 --- a/mobile/src/session/mobile-diff-comment-edit.test.ts +++ b/mobile/src/session/mobile-diff-comment-edit.test.ts @@ -33,6 +33,7 @@ describe('mobile diff comment editing', () => { expect(result.comment).toMatchObject({ id: 'a', body: 'updated', updatedAt: 200 }) expect(result.comment?.sentAt).toBeUndefined() + expect(result.comments).toEqual([result.comment]) }) it('marks notes sent and excludes them from unsent counts', () => { diff --git a/mobile/src/session/mobile-diff-comments.test.ts b/mobile/src/session/mobile-diff-comments.test.ts index 8897b5e32e4..0a959b33f13 100644 --- a/mobile/src/session/mobile-diff-comments.test.ts +++ b/mobile/src/session/mobile-diff-comments.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest' import type { DiffComment } from '../../../src/shared/diff-comment-types' import { addMobileDiffComment, - formatDiffComments, formatMobileDiffReviewPrompt, normalizeMobileDiffComments, removeDeliveredMobileDiffComments, @@ -117,18 +116,6 @@ describe('mobile diff comments', () => { ).toEqual([comment({ id: 'a', body: 'new note' }), comment({ id: 'b' })]) }) - it('uses the desktop-compatible prompt format', () => { - expect(formatDiffComments([comment({ id: 'a', body: 'quote "this"' })])).toBe( - ['File: src/app.ts', 'Line: 4', 'User comment: "quote \\"this\\""'].join('\n') - ) - }) - - it('formats file-level notes with file scope', () => { - expect(formatDiffComments([comment({ id: 'a', lineNumber: 0 })])).toBe( - ['File: src/app.ts', 'Scope: file', 'User comment: "check this"'].join('\n') - ) - }) - it('wraps sent review notes in the mobile agent prompt', () => { expect(formatMobileDiffReviewPrompt([comment({ id: 'a' })])).toBe( [ diff --git a/mobile/src/session/mobile-diff-review-positioning.test.ts b/mobile/src/session/mobile-diff-review-positioning.test.ts index 32fe37a82a3..fd23dfd4ece 100644 --- a/mobile/src/session/mobile-diff-review-positioning.test.ts +++ b/mobile/src/session/mobile-diff-review-positioning.test.ts @@ -53,14 +53,14 @@ describe('mobile diff review positioning', () => { }) it('matches renamed files by oldPath through the queue key', () => { - const queue = [item('new.ts', 'branch', 'old.ts'), item('other.ts', 'branch')] + const queue = [item('other.ts', 'branch'), item('new.ts', 'branch', 'old.ts')] expect( findMobileDiffReviewInitialIndex(queue, { filePath: 'old.ts', area: 'branch' }) - ).toBe(0) + ).toBe(1) }) it('falls back to the first file when the target is missing', () => { diff --git a/mobile/src/session/mobile-diff-review-queue.test.ts b/mobile/src/session/mobile-diff-review-queue.test.ts index 653bc6ae2ab..8ddd67e8faf 100644 --- a/mobile/src/session/mobile-diff-review-queue.test.ts +++ b/mobile/src/session/mobile-diff-review-queue.test.ts @@ -192,11 +192,18 @@ describe('mobile diff review queue', () => { reviewState }) - expect(filterMobileDiffReviewQueue(queue, 'unreviewed').map((item) => item.filePath)).toEqual([ - 'a.ts', - 'b.ts' - ]) - expect(filterMobileDiffReviewQueue(queue, 'notes').map((item) => item.filePath)).toEqual([ + const reviewedItem: MobileDiffReviewQueueItem = { + ...queue[0], + key: createMobileDiffReviewFileKey('unstaged', 'unstaged', 'c.ts'), + filePath: 'c.ts', + isReviewed: true + } + const filterQueue = [...queue, reviewedItem] + + expect( + filterMobileDiffReviewQueue(filterQueue, 'unreviewed').map((item) => item.filePath) + ).toEqual(['a.ts', 'b.ts']) + expect(filterMobileDiffReviewQueue(filterQueue, 'notes').map((item) => item.filePath)).toEqual([ 'b.ts' ]) }) diff --git a/mobile/src/session/mobile-diff-review-state.test.ts b/mobile/src/session/mobile-diff-review-state.test.ts index cd76f8f44f1..9bebb6ac133 100644 --- a/mobile/src/session/mobile-diff-review-state.test.ts +++ b/mobile/src/session/mobile-diff-review-state.test.ts @@ -101,5 +101,6 @@ describe('mobile diff review state', () => { it('builds stable content identities from ordered parts', () => { expect(buildMobileDiffIdentity(['a', 'b'])).toBe(buildMobileDiffIdentity(['a', 'b'])) expect(buildMobileDiffIdentity(['a', 'b'])).not.toBe(buildMobileDiffIdentity(['ab'])) + expect(buildMobileDiffIdentity(['a', 'b'])).not.toBe(buildMobileDiffIdentity(['b', 'a'])) }) }) diff --git a/src/cli/orchestration-structured-sender-identity.test.ts b/src/cli/orchestration-structured-sender-identity.test.ts index 7d44ed8514f..ae0db16f645 100644 --- a/src/cli/orchestration-structured-sender-identity.test.ts +++ b/src/cli/orchestration-structured-sender-identity.test.ts @@ -46,12 +46,32 @@ import { useWorktreeAwarenessEnvironment } from './index-test-harness' const STRUCTURED_HANDLE = 'structworker_a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d' // Cover distinct handler shapes that share sender resolution and liveness checks. -const SENDER_VERBS: { argv: string[]; method: string }[] = [ - { argv: ['orchestration', 'run-current'], method: 'orchestration.runCurrent' }, - { argv: ['orchestration', 'run-create', '--objective', 'x'], method: 'orchestration.runCreate' }, - { argv: ['orchestration', 'task-list'], method: 'orchestration.taskList' }, - { argv: ['orchestration', 'gate-list'], method: 'orchestration.gateList' }, - { argv: ['orchestration', 'dispatch-show', '--task', 't1'], method: 'orchestration.dispatchShow' } +const SENDER_VERBS: { + argv: string[] + method: string + senderField: 'from' | 'callerTerminalHandle' +}[] = [ + { + argv: ['orchestration', 'run-current'], + method: 'orchestration.runCurrent', + senderField: 'from' + }, + { + argv: ['orchestration', 'run-create', '--objective', 'x'], + method: 'orchestration.runCreate', + senderField: 'from' + }, + { + argv: ['orchestration', 'task-list'], + method: 'orchestration.taskList', + senderField: 'callerTerminalHandle' + }, + { argv: ['orchestration', 'gate-list'], method: 'orchestration.gateList', senderField: 'from' }, + { + argv: ['orchestration', 'dispatch-show', '--task', 't1', '--preamble'], + method: 'orchestration.dispatchShow', + senderField: 'from' + } ] describe('a structured worker running orchestration commands as itself', () => { @@ -74,33 +94,42 @@ describe('a structured worker running orchestration commands as itself', () => { _meta: { runtimeId: 'runtime-1' } } } - return { id: 'req', ok: true, result: {}, _meta: { runtimeId: 'runtime-1' } } + const result = + method === 'orchestration.runCreate' + ? { run: { id: 'run-1', objective: 'x' } } + : method === 'orchestration.runCurrent' + ? { run: null } + : method === 'orchestration.taskList' + ? { tasks: [], count: 0 } + : method === 'orchestration.gateList' + ? { gates: [], count: 0 } + : { dispatch: null, preamble: 'preview' } + return { id: 'req', ok: true, result, _meta: { runtimeId: 'runtime-1' } } }) } it.each(SENDER_VERBS)( 'resolves its own identity for $method with no --from', - async ({ argv, method }) => { + async ({ argv, method, senderField }) => { process.env.ORCA_TERMINAL_HANDLE = STRUCTURED_HANDLE answerCalls() vi.spyOn(console, 'log').mockImplementation(() => {}) + const priorExitCode = process.exitCode await expect(main(argv)).resolves.not.toThrow() + expect(process.exitCode).toBe(priorExitCode) const called = callMock.mock.calls.map((call) => call[0] as string) - expect(called).toContain(method) + expect(callMock).toHaveBeenCalledWith('terminal.resolveIdentity', { + terminal: STRUCTURED_HANDLE + }) + expect(callMock).toHaveBeenCalledWith( + method, + expect.objectContaining({ [senderField]: STRUCTURED_HANDLE }) + ) // Structured workers lack writable PTYs; terminal.show must not validate them. expect(called).not.toContain('terminal.show') } ) - it('sends the structured handle as the sender, not a guessed sibling', async () => { - process.env.ORCA_TERMINAL_HANDLE = STRUCTURED_HANDLE - answerCalls() - vi.spyOn(console, 'log').mockImplementation(() => {}) - await main(['orchestration', 'run-create', '--objective', 'x']) - const create = callMock.mock.calls.find((call) => call[0] === 'orchestration.runCreate') - expect((create?.[1] as { from?: string } | undefined)?.from).toBe(STRUCTURED_HANDLE) - }) - it('still refuses a handle the runtime reports dead, with no pane key to remint from', async () => { process.env.ORCA_TERMINAL_HANDLE = 'term_stale' callMock.mockImplementation(async (method: string) => { diff --git a/src/cli/orchestration-structured-session-no-identity.test.ts b/src/cli/orchestration-structured-session-no-identity.test.ts index c5a46872187..2a268aeec50 100644 --- a/src/cli/orchestration-structured-session-no-identity.test.ts +++ b/src/cli/orchestration-structured-session-no-identity.test.ts @@ -81,5 +81,9 @@ describe('a structured chat session with no orchestration identity', () => { callMock.mockResolvedValue({ result: { messages: [], count: 0 } }) await invoke('orchestration check') expect(getTerminalHandleMock).toHaveBeenCalled() + expect(callMock).toHaveBeenCalledWith( + 'orchestration.check', + expect.objectContaining({ terminal: 'term_sibling' }) + ) }) }) diff --git a/src/main/agent-hooks/server-program-status-hook-yield.test.ts b/src/main/agent-hooks/server-program-status-hook-yield.test.ts index 959f1e5bc58..e1ba6bfc888 100644 --- a/src/main/agent-hooks/server-program-status-hook-yield.test.ts +++ b/src/main/agent-hooks/server-program-status-hook-yield.test.ts @@ -239,6 +239,7 @@ describe('OSC 7501 status beside hooks', () => { programStatus({ state: 'done', agentType: 'pi' }) vi.advanceTimersByTime(1_000) programStatus({ state: 'working', agentType: 'pi' }) + expect(paneRow()?.state).toBe('waiting') vi.advanceTimersByTime(HOOK_SILENCE_BEFORE_PROGRAM_STATUS_FALLBACK_MS) expect(paneRow()?.state).toBe('working') }) diff --git a/src/main/agent-hooks/server/program-status/hook-precedence.test.ts b/src/main/agent-hooks/server/program-status/hook-precedence.test.ts index d3142efb7b8..cb3c34fa9f6 100644 --- a/src/main/agent-hooks/server/program-status/hook-precedence.test.ts +++ b/src/main/agent-hooks/server/program-status/hook-precedence.test.ts @@ -61,6 +61,7 @@ describe('programStatusHookYieldEndsAt', () => { const block = report({ state: 'waiting', agentType: 'antigravity' }) expect(programStatusHookYieldEndsAt(hook, block)).toBe(Infinity) expect(programStatusHookYieldEndsAt(hook, block, NOW - 1)).toBe(Infinity) + expect(programStatusHookYieldEndsAt(hook, block, NOW)).toBe(Infinity) expect(programStatusHookYieldEndsAt(hook, block, NOW + 1)).toBeUndefined() for (const state of ['working', 'blocked'] as const) { expect(programStatusHookYieldEndsAt(hook, report({ state, agentType: 'antigravity' }))).toBe( @@ -74,6 +75,7 @@ describe('programStatusHookYieldEndsAt', () => { const hook = row({ state, agentType: 'pi' }) const working = report({ state: 'working', agentType: 'pi' }) expect(programStatusHookYieldEndsAt(hook, working, NOW - 1)).toBe(Infinity) + expect(programStatusHookYieldEndsAt(hook, working, NOW)).toBe(Infinity) expect(programStatusHookYieldEndsAt(hook, working, NOW + 1)).toBe(NOW + SILENCE) expect(programStatusHookYieldEndsAt(hook, working)).toBe(NOW + SILENCE) expect(programStatusHookYieldEndsAt(hook, report({ state: 'done', agentType: 'pi' }))).toBe( diff --git a/src/main/ipc/filesystem-watcher-event-batch.test.ts b/src/main/ipc/filesystem-watcher-event-batch.test.ts index 31f983b96ce..bcc4d4aa6e8 100644 --- a/src/main/ipc/filesystem-watcher-event-batch.test.ts +++ b/src/main/ipc/filesystem-watcher-event-batch.test.ts @@ -7,10 +7,10 @@ import { } from './filesystem-watcher-event-batch' describe('queueWatcherEvents', () => { - function events(count: number): WatcherEvent[] { + function events(count: number, start = 0): WatcherEvent[] { return Array.from({ length: count }, (_, index): WatcherEvent => ({ type: 'update', - path: `/repo/file-${index}.ts` + path: `/repo/file-${start + index}.ts` })) } @@ -21,6 +21,14 @@ describe('queueWatcherEvents', () => { expect(batch.overflowed).toBe(false) expect(batch.events.map((event) => event.path)).toEqual(['/repo/file-0.ts', '/repo/file-1.ts']) + + const retained = [...batch.events] + const remaining = events(MAX_BATCHED_WATCHER_EVENTS - retained.length, retained.length) + queueWatcherEvents(batch, remaining) + + expect(batch.overflowed).toBe(false) + expect(batch.events).toEqual([...retained, ...remaining]) + expect(batch.events).toHaveLength(MAX_BATCHED_WATCHER_EVENTS) }) it('marks overflow without retaining oversized event lists', () => { diff --git a/src/main/ipc/parcel-watcher-event-delivery.test.ts b/src/main/ipc/parcel-watcher-event-delivery.test.ts index a7efb5b299c..24422ad1635 100644 --- a/src/main/ipc/parcel-watcher-event-delivery.test.ts +++ b/src/main/ipc/parcel-watcher-event-delivery.test.ts @@ -78,12 +78,18 @@ describe('watcher event queue flush', () => { it('settles all flush callers when closed while delivery is blocked', async () => { const blocked = Promise.withResolvers() - const queue = createWatcherProcessEventDeliveryQueue({}, () => blocked.promise, vi.fn()) + const deliver = vi.fn(() => blocked.promise) + const queue = createWatcherProcessEventDeliveryQueue({}, deliver, vi.fn()) queue.enqueue([{ type: 'update', path: '/repo/file' }]) + await vi.waitFor(() => expect(deliver).toHaveBeenCalledTimes(1)) const first = queue.flush() const second = queue.flush() expect(second).toBe(first) + const settled = vi.fn() + void first.then(settled) queue.close() + await Promise.resolve() + expect(settled).toHaveBeenCalledTimes(1) await expect(first).resolves.toBeUndefined() await expect(queue.flush()).resolves.toBeUndefined() blocked.resolve() diff --git a/src/main/runtime/rpc/methods/file-watch-event-batcher.test.ts b/src/main/runtime/rpc/methods/file-watch-event-batcher.test.ts index 77739dd4f9b..538cbac6f04 100644 --- a/src/main/runtime/rpc/methods/file-watch-event-batcher.test.ts +++ b/src/main/runtime/rpc/methods/file-watch-event-batcher.test.ts @@ -65,6 +65,7 @@ describe('createFileWatchEventBatcher', () => { vi.advanceTimersByTime(10) } expect(emit.mock.calls.length).toBeGreaterThanOrEqual(1_000 / WATCH_BATCH_INTERVAL_MS) + expect(emit.mock.calls.length).toBeLessThanOrEqual(1 + 1_000 / WATCH_BATCH_INTERVAL_MS) } finally { vi.useRealTimers() } diff --git a/src/main/runtime/rpc/methods/files-watch-batch-cadence.test.ts b/src/main/runtime/rpc/methods/files-watch-batch-cadence.test.ts index 2650e47334a..9083e44faf7 100644 --- a/src/main/runtime/rpc/methods/files-watch-batch-cadence.test.ts +++ b/src/main/runtime/rpc/methods/files-watch-batch-cadence.test.ts @@ -68,8 +68,16 @@ describe('files.watch batch cadence negotiation', () => { it('treats an unknown cadence from a newer client as legacy instead of rejecting', async () => { vi.useFakeTimers() try { - const { replies } = await startWatch({ worktree: 'id:wt-1', batchCadence: 'future' }) + const { replies, emitChange } = await startWatch({ + worktree: 'id:wt-1', + batchCadence: 'future' + }) expect(replies[1]?.result?.type).toBe('ready') + emitChange([{ kind: 'update', absolutePath: '/repo/a.ts' }]) + await vi.advanceTimersByTimeAsync(149) + expect(changedCount(replies)).toBe(0) + await vi.advanceTimersByTimeAsync(1) + expect(changedCount(replies)).toBe(1) } finally { vi.useRealTimers() } diff --git a/src/renderer/src/lib/markdown-document-templates.test.ts b/src/renderer/src/lib/markdown-document-templates.test.ts index 107ffde0f30..01e791a36b4 100644 --- a/src/renderer/src/lib/markdown-document-templates.test.ts +++ b/src/renderer/src/lib/markdown-document-templates.test.ts @@ -69,7 +69,9 @@ describe('markdown document templates', () => { const nonBreakingSpace = String.fromCharCode(160) const fileName = ` launch___plan\t${nonBreakingSpace}${' pasted '.repeat(300)}.md` - expect(getMarkdownTemplateTitleForFileName(fileName)).toContain('Launch plan pasted pasted') + expect(getMarkdownTemplateTitleForFileName(fileName)).toBe( + `Launch plan ${'pasted '.repeat(299)}pasted` + ) expect( replace.mock.calls.filter( ([pattern]) => pattern instanceof RegExp && pattern.source === '\\s+' diff --git a/src/renderer/src/lib/pr-comment-action-state.test.ts b/src/renderer/src/lib/pr-comment-action-state.test.ts index 62f7e3979d1..9e048b9dac3 100644 --- a/src/renderer/src/lib/pr-comment-action-state.test.ts +++ b/src/renderer/src/lib/pr-comment-action-state.test.ts @@ -34,6 +34,7 @@ describe('pr-comment-action-state', () => { expect(getPRCommentGroupActionState(groups[0]!)).toBe('open') expect(getPRCommentGroupActionState(groups[1]!)).toBe('resolved') expect(getPRCommentGroupActionState(groups[2]!)).toBe('conversation') + expect(groups.map(isPRCommentGroupQueueableForAI)).toEqual([true, false, true]) }) it('partitions groups for triage sections', () => {