From f6edcb3d762139f5ae579190c12d8acbd06f424e Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 16 Jul 2026 21:50:04 -0400 Subject: [PATCH] test(desktop): cover the review store (refresh, selection, mutations, ship flow) --- apps/desktop/src/store/review.test.ts | 449 ++++++++++++++++++++++++++ 1 file changed, 449 insertions(+) create mode 100644 apps/desktop/src/store/review.test.ts diff --git a/apps/desktop/src/store/review.test.ts b/apps/desktop/src/store/review.test.ts new file mode 100644 index 000000000000..d0f68419fc49 --- /dev/null +++ b/apps/desktop/src/store/review.test.ts @@ -0,0 +1,449 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { HermesReviewFile, HermesReviewShipInfo } from '@/global' + +import { + $reviewCommitDefault, + $reviewCommitMsgBusy, + $reviewDiff, + $reviewDiffLoading, + $reviewFiles, + $reviewIsRepo, + $reviewLoading, + $reviewMaxChurn, + $reviewOpen, + $reviewRevertTarget, + $reviewSelectedPath, + $reviewShipBusy, + $reviewShipInfo, + $reviewTreeMode, + cancelRevert, + clearReviewSelection, + closeReview, + commitChanges, + confirmRevert, + createOrOpenPr, + generateCommitMessage, + openReview, + pushChanges, + refreshReview, + refreshShipInfo, + requestRevert, + revertReviewFile, + selectReviewFile, + stageReviewFile, + toggleReviewTreeMode, + unstageReviewFile +} from './review' +import { $currentCwd } from './session' + +// requestOneShot is the only cross-module dependency that must be faked (it +// reaches the gateway); everything else routes through window.hermesDesktop.git, +// which we stub per-test like the sibling coding-status.test.ts does. +const requestOneShot = vi.fn(async (_args: unknown) => 'generated message') +vi.mock('@/lib/oneshot', () => ({ requestOneShot: (args: unknown) => requestOneShot(args) })) +// refreshRepoStatus is a fire-and-forget side effect of mutations; stub it so it +// doesn't try to hit the (absent) probe and log. +vi.mock('./coding-status', () => ({ refreshRepoStatus: vi.fn() })) + +function file(path: string, over: Partial = {}): HermesReviewFile { + return { path, status: 'modified', staged: false, added: 1, removed: 0, ...over } as HermesReviewFile +} + +type ReviewStub = Record> + +// Install a review bridge on window.hermesDesktop. Any op not supplied defaults +// to a resolved no-op so a test only declares what it exercises. +function stubReview(over: ReviewStub = {}) { + const review: ReviewStub = { + list: vi.fn(async () => ({ files: [] })), + diff: vi.fn(async () => ''), + stage: vi.fn(async () => undefined), + unstage: vi.fn(async () => undefined), + revert: vi.fn(async () => undefined), + commit: vi.fn(async () => undefined), + commitContext: vi.fn(async () => ({ diff: 'd', recent: 'r' })), + push: vi.fn(async () => undefined), + shipInfo: vi.fn(async () => ({ ghReady: false, pr: null })), + createPr: vi.fn(async () => ({ url: 'https://example.com/pr/1' })), + ...over + } + + ;(window as unknown as { hermesDesktop?: unknown }).hermesDesktop = { + git: { review }, + openExternal: vi.fn() + } + + return review +} + +beforeEach(() => { + requestOneShot.mockClear() + requestOneShot.mockResolvedValue('generated message') + // Reset stores touched across tests. + $reviewOpen.set(false) + $reviewFiles.set([]) + $reviewLoading.set(false) + $reviewIsRepo.set(true) + $reviewDiff.set(null) + $reviewDiffLoading.set(false) + $reviewSelectedPath.set(null) + $reviewShipInfo.set({ ghReady: false, pr: null }) + $reviewShipBusy.set(false) + $reviewCommitMsgBusy.set(false) + $reviewRevertTarget.set(undefined) + $currentCwd.set('/repo') +}) + +afterEach(() => { + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop +}) + +describe('refreshReview', () => { + it('is a no-op that clears state when the pane is closed', async () => { + const review = stubReview() + $reviewOpen.set(false) + $reviewFiles.set([file('a.ts')]) + + await refreshReview() + + expect(review.list).not.toHaveBeenCalled() + expect($reviewFiles.get()).toEqual([]) + expect($reviewLoading.get()).toBe(false) + }) + + it('flags not-a-repo (and clears loading) when there is no bridge/cwd', async () => { + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + $reviewOpen.set(true) + $reviewLoading.set(true) + + await refreshReview() + + expect($reviewIsRepo.get()).toBe(false) + expect($reviewLoading.get()).toBe(false) + }) + + it('populates the changed-file list from the bridge', async () => { + stubReview({ list: vi.fn(async () => ({ files: [file('a.ts'), file('b.ts')] })) }) + $reviewOpen.set(true) + + await refreshReview() + + expect($reviewFiles.get().map(f => f.path)).toEqual(['a.ts', 'b.ts']) + expect($reviewIsRepo.get()).toBe(true) + expect($reviewLoading.get()).toBe(false) + }) + + it('filters excluded paths (node_modules et al.) out of the list', async () => { + stubReview({ list: vi.fn(async () => ({ files: [file('src/a.ts'), file('node_modules/x/index.js')] })) }) + $reviewOpen.set(true) + + await refreshReview() + + expect($reviewFiles.get().map(f => f.path)).toEqual(['src/a.ts']) + }) + + it('drops a selection whose file vanished from the new list', async () => { + stubReview({ list: vi.fn(async () => ({ files: [file('kept.ts')] })) }) + $reviewOpen.set(true) + $reviewSelectedPath.set('gone.ts') + $reviewDiff.set('old diff') + + await refreshReview() + + expect($reviewSelectedPath.get()).toBeNull() + expect($reviewDiff.get()).toBeNull() + }) + + it('clears the list but keeps isRepo true when the bridge throws', async () => { + stubReview({ + list: vi.fn(async () => { + throw new Error('git failed') + }) + }) + $reviewOpen.set(true) + $reviewFiles.set([file('stale.ts')]) + + await refreshReview() + + expect($reviewFiles.get()).toEqual([]) + expect($reviewIsRepo.get()).toBe(true) + expect($reviewLoading.get()).toBe(false) + }) +}) + +describe('$reviewMaxChurn', () => { + it('is the largest added+removed across files', () => { + $reviewFiles.set([file('a', { added: 3, removed: 2 }), file('b', { added: 10, removed: 1 }), file('c')]) + expect($reviewMaxChurn.get()).toBe(11) + }) + + it('is 0 for an empty list', () => { + $reviewFiles.set([]) + expect($reviewMaxChurn.get()).toBe(0) + }) +}) + +describe('selectReviewFile / clearReviewSelection', () => { + it('sets the selected path and fetches its diff', async () => { + const review = stubReview({ diff: vi.fn(async () => 'the diff') }) + + await selectReviewFile(file('a.ts')) + + expect($reviewSelectedPath.get()).toBe('a.ts') + expect($reviewDiff.get()).toBe('the diff') + expect($reviewDiffLoading.get()).toBe(false) + expect(review.diff).toHaveBeenCalledWith('/repo', 'a.ts', 'uncommitted', null, false) + }) + + it('coerces a falsy diff to empty string (not null)', async () => { + stubReview({ diff: vi.fn(async () => '') }) + + await selectReviewFile(file('a.ts')) + + expect($reviewDiff.get()).toBe('') + }) + + it('sets diff null when there is no bridge', async () => { + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + + await selectReviewFile(file('a.ts')) + + expect($reviewSelectedPath.get()).toBe('a.ts') + expect($reviewDiff.get()).toBeNull() + }) + + it('clears path, diff and loading', () => { + $reviewSelectedPath.set('a.ts') + $reviewDiff.set('x') + $reviewDiffLoading.set(true) + + clearReviewSelection() + + expect($reviewSelectedPath.get()).toBeNull() + expect($reviewDiff.get()).toBeNull() + expect($reviewDiffLoading.get()).toBe(false) + }) +}) + +describe('view state', () => { + it('toggleReviewTreeMode flips list <-> tree', () => { + $reviewTreeMode.set('tree') + toggleReviewTreeMode() + expect($reviewTreeMode.get()).toBe('list') + toggleReviewTreeMode() + expect($reviewTreeMode.get()).toBe('tree') + }) + + it('openReview opens the pane and kicks off a refresh', async () => { + const review = stubReview() + openReview() + expect($reviewOpen.get()).toBe(true) + // openReview fires refreshReview + refreshShipInfo without awaiting. + await Promise.resolve() + await Promise.resolve() + expect(review.list).toHaveBeenCalled() + }) + + it('closeReview closes the pane and clears the selection', () => { + stubReview() + $reviewOpen.set(true) + $reviewSelectedPath.set('a.ts') + $reviewDiff.set('x') + + closeReview() + + expect($reviewOpen.get()).toBe(false) + expect($reviewSelectedPath.get()).toBeNull() + expect($reviewDiff.get()).toBeNull() + }) +}) + +describe('mutations', () => { + it('stageReviewFile forwards the path and re-syncs', async () => { + const review = stubReview() + $reviewOpen.set(true) // afterMutation's refreshReview only lists when the pane is open + await stageReviewFile('a.ts') + expect(review.stage).toHaveBeenCalledWith('/repo', 'a.ts') + expect(review.list).toHaveBeenCalled() + }) + + it('unstageReviewFile forwards the path', async () => { + const review = stubReview() + await unstageReviewFile('a.ts') + expect(review.unstage).toHaveBeenCalledWith('/repo', 'a.ts') + }) + + it('revertReviewFile forwards the path', async () => { + const review = stubReview() + await revertReviewFile('a.ts') + expect(review.revert).toHaveBeenCalledWith('/repo', 'a.ts') + }) + + it('stage with null path means "all"', async () => { + const review = stubReview() + await stageReviewFile(null) + expect(review.stage).toHaveBeenCalledWith('/repo', null) + }) +}) + +describe('revert confirm dialog', () => { + it('requestRevert opens a target, cancelRevert closes it', () => { + requestRevert('a.ts') + expect($reviewRevertTarget.get()).toEqual({ path: 'a.ts' }) + cancelRevert() + expect($reviewRevertTarget.get()).toBeUndefined() + }) + + it('requestRevert(null) encodes the "revert all" target distinctly from closed', () => { + requestRevert(null) + expect($reviewRevertTarget.get()).toEqual({ path: null }) + }) + + it('confirmRevert closes the dialog then performs the revert', async () => { + const review = stubReview() + requestRevert('a.ts') + + await confirmRevert() + + expect($reviewRevertTarget.get()).toBeUndefined() + expect(review.revert).toHaveBeenCalledWith('/repo', 'a.ts') + }) + + it('confirmRevert is a no-op when nothing is pending', async () => { + const review = stubReview() + $reviewRevertTarget.set(undefined) + + await confirmRevert() + + expect(review.revert).not.toHaveBeenCalled() + }) +}) + +describe('ship flow', () => { + it('commitChanges commits the trimmed message and toggles the busy flag', async () => { + const review = stubReview() + const seen: boolean[] = [] + const unsub = $reviewShipBusy.subscribe(v => seen.push(v)) + + await commitChanges(' a message ', { push: true }) + + expect(review.commit).toHaveBeenCalledWith('/repo', 'a message', true) + expect(seen).toContain(true) + expect($reviewShipBusy.get()).toBe(false) + unsub() + }) + + it('commitChanges bails on a blank message', async () => { + const review = stubReview() + await commitChanges(' ') + expect(review.commit).not.toHaveBeenCalled() + }) + + it('pushChanges pushes and refreshes ship info', async () => { + const review = stubReview() + await pushChanges() + expect(review.push).toHaveBeenCalledWith('/repo') + }) + + it('createOrOpenPr opens the existing PR without creating a new one', async () => { + const review = stubReview() + $reviewShipInfo.set({ ghReady: true, pr: { url: 'https://example.com/pr/9' } } as HermesReviewShipInfo) + + await createOrOpenPr() + + expect(review.createPr).not.toHaveBeenCalled() + expect((window.hermesDesktop as unknown as { openExternal: ReturnType }).openExternal).toHaveBeenCalledWith( + 'https://example.com/pr/9' + ) + }) + + it('createOrOpenPr creates a PR when none exists, then opens it', async () => { + const review = stubReview({ createPr: vi.fn(async () => ({ url: 'https://example.com/pr/new' })) }) + $reviewShipInfo.set({ ghReady: true, pr: null }) + + await createOrOpenPr() + + expect(review.createPr).toHaveBeenCalledWith('/repo') + expect((window.hermesDesktop as unknown as { openExternal: ReturnType }).openExternal).toHaveBeenCalledWith( + 'https://example.com/pr/new' + ) + }) +}) + +describe('refreshShipInfo', () => { + it('populates ship info from the bridge', async () => { + const info: HermesReviewShipInfo = { ghReady: true, pr: { url: 'https://example.com/pr/3' } } as HermesReviewShipInfo + stubReview({ shipInfo: vi.fn(async () => info) }) + + await refreshShipInfo() + + expect($reviewShipInfo.get()).toEqual(info) + }) + + it('resets ship info when there is no bridge', async () => { + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + $reviewShipInfo.set({ ghReady: true, pr: { url: 'x' } } as HermesReviewShipInfo) + + await refreshShipInfo() + + expect($reviewShipInfo.get()).toEqual({ ghReady: false, pr: null }) + }) + + it('resets ship info when the bridge throws', async () => { + stubReview({ + shipInfo: vi.fn(async () => { + throw new Error('gh missing') + }) + }) + $reviewShipInfo.set({ ghReady: true, pr: { url: 'x' } } as HermesReviewShipInfo) + + await refreshShipInfo() + + expect($reviewShipInfo.get()).toEqual({ ghReady: false, pr: null }) + }) +}) + +describe('generateCommitMessage', () => { + it('returns a one-shot message from the working-tree diff', async () => { + stubReview() + + const msg = await generateCommitMessage('avoid this') + + expect(msg).toBe('generated message') + expect(requestOneShot).toHaveBeenCalledWith( + expect.objectContaining({ + template: 'commit_message', + variables: expect.objectContaining({ avoid: 'avoid this', diff: 'd', recent_commits: 'r' }) + }) + ) + expect($reviewCommitMsgBusy.get()).toBe(false) + }) + + it('returns empty (no model call) when the diff is blank', async () => { + stubReview({ commitContext: vi.fn(async () => ({ diff: ' ', recent: '' })) }) + + const msg = await generateCommitMessage() + + expect(msg).toBe('') + expect(requestOneShot).not.toHaveBeenCalled() + }) + + it('returns empty when the bridge lacks commitContext', async () => { + const review = stubReview() + delete review.commitContext + + const msg = await generateCommitMessage() + + expect(msg).toBe('') + }) +}) + +describe('$reviewCommitDefault', () => { + it('remembers the split-button default action', () => { + $reviewCommitDefault.set('commitPush') + expect($reviewCommitDefault.get()).toBe('commitPush') + $reviewCommitDefault.set('commit') + expect($reviewCommitDefault.get()).toBe('commit') + }) +})