diff --git a/commands_test.go b/commands_test.go index f73fbcb7..7acbff39 100644 --- a/commands_test.go +++ b/commands_test.go @@ -209,9 +209,7 @@ func TestDispatchUnexpectedPositionals(t *testing.T) { } func TestDispatchVersion(t *testing.T) { - oldV, oldC := version.Version, version.Commit - version.Version, version.Commit = "v9.9.9", "abcdef1" - defer func() { version.Version, version.Commit = oldV, oldC }() + setVersion(t, "v9.9.9", "abcdef1") for _, argv := range [][]string{ {"lightcode", "version"}, @@ -233,9 +231,7 @@ func TestDispatchVersion(t *testing.T) { } func TestDispatchVersionJSON(t *testing.T) { - oldV, oldC := version.Version, version.Commit - version.Version, version.Commit = "v9.9.9", "abcdef1" - defer func() { version.Version, version.Commit = oldV, oldC }() + setVersion(t, "v9.9.9", "abcdef1") _, code, stdout, stderr := capture(t, []string{"lightcode", "version", "--json"}) if code != 0 || stderr != "" { diff --git a/frontend/src/client/app.test.ts b/frontend/src/client/app.test.ts index a4e0f69c..056a49d3 100644 --- a/frontend/src/client/app.test.ts +++ b/frontend/src/client/app.test.ts @@ -397,12 +397,12 @@ describe('initial composition', () => { vi.mocked(sdk.GetWarnings).mockResolvedValue(ok({ warnings_revision: { instance_id: INSTANCE, revision: '1' }, warnings: [] })); vi.mocked(sdk.GetSessionHydration).mockResolvedValue(ok(makeHydration())); // The boot list is held; a repair then completes while it is parked. - let releaseList: (value: unknown) => void = () => {}; + const { promise: listGate, resolve: releaseList } = Promise.withResolvers(); let listCalls = 0; vi.mocked(sdk.ListSessions).mockImplementation(async () => { listCalls += 1; if (listCalls === 1) { - return new Promise((resolve) => { releaseList = resolve; }); + return listGate; } return ok([rootSession('root1', NEWER)]); }); @@ -645,8 +645,7 @@ describe('composer', () => { }); it('forms independent work from unchanged text while a response is held', async () => { - let releaseFirst: (value: unknown) => void = () => {}; - const firstGate = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: firstGate, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.SubmitSession) .mockImplementationOnce(() => firstGate as never) .mockResolvedValue(ok({ disposition: 'queued' })); @@ -723,8 +722,8 @@ describe('composer', () => { }); it('never clears a newer draft on a late success', async () => { - let resolveSubmit: (value: unknown) => void = () => {}; - vi.mocked(sdk.SubmitSession).mockImplementationOnce(() => new Promise((resolve) => { resolveSubmit = resolve; })); + const { promise: submitGate, resolve: resolveSubmit } = Promise.withResolvers(); + vi.mocked(sdk.SubmitSession).mockImplementationOnce(() => submitGate); const { target } = await mountBasic(); typeInto(target, 'first'); await settle(); @@ -748,8 +747,8 @@ describe('composer', () => { }); it('keeps an unknown attempt against its own session when the root switches and does not resend', async () => { - let resolveSubmit: (value: unknown) => void = () => {}; - vi.mocked(sdk.SubmitSession).mockImplementationOnce(() => new Promise((_resolve, reject) => { resolveSubmit = reject; })); + const { promise: submitGate, reject: resolveSubmit } = Promise.withResolvers(); + vi.mocked(sdk.SubmitSession).mockImplementationOnce(() => submitGate); const { target } = await mountBasic(); typeInto(target, 'delayed'); await settle(); @@ -815,8 +814,8 @@ describe('controls', () => { vi.mocked(sdk.GetSessionHydration).mockResolvedValue(ok(activeHydration())); const { target } = await mountApp(connection); await settle(); - let resolveInterrupt: (value: unknown) => void = () => {}; - vi.mocked(sdk.InterruptSession).mockImplementationOnce(() => new Promise((resolve) => { resolveInterrupt = resolve; })); + const { promise: interruptGate, resolve: resolveInterrupt } = Promise.withResolvers(); + vi.mocked(sdk.InterruptSession).mockImplementationOnce(() => interruptGate); (target.querySelector('.interrupt') as HTMLElement).click(); // Move the selection to root2 while the root1 control is in flight. await selectRoot2(target); @@ -832,8 +831,8 @@ describe('controls', () => { // ...with no view adoption into root2: the displayed session stays root2. expect(target.querySelector('.toolbar')?.textContent).toContain('root2'); // An unknown transport outcome is likewise reported for its address. - let resolveStop: (value: unknown) => void = () => {}; - vi.mocked(sdk.StopSession).mockImplementationOnce(() => new Promise((_resolve, reject) => { resolveStop = reject; })); + const { promise: stopGate, reject: resolveStop } = Promise.withResolvers(); + vi.mocked(sdk.StopSession).mockImplementationOnce(() => stopGate); (target.querySelector('.stop') as HTMLElement).click(); resolveStop(new Error('stop lost')); await settle(); @@ -980,8 +979,8 @@ describe('keyboard pickers', () => { configuration_revision: { instance_id: INSTANCE, generation: '1' }, models: [{ ref: 'prov/a', provider: 'prov', provider_name: 'Provider', model: 'a', display_name: 'Model A', context_window: 1000, max_output_tokens: 10, hidden: false, provider_hidden: false, incomplete: false, source: 'user' }], })); - let resolveModel: (value: unknown) => void = () => {}; - vi.mocked(sdk.SetAgentTypeModel).mockImplementation(() => new Promise((resolve) => { resolveModel = resolve; })); + const { promise: modelGate, resolve: resolveModel } = Promise.withResolvers(); + vi.mocked(sdk.SetAgentTypeModel).mockImplementation(() => modelGate); const { target } = await mountBasic(); const opensBefore = target ? 0 : 0; void opensBefore; @@ -1428,9 +1427,9 @@ describe('repair', () => { connection.openMock.mockResolvedValue({ instance_id: INSTANCE, generation: '2', workspace: '/ws' }); // Gate the repair's fresh hydration so the second loss lands strictly // before the read completes. - let settleRead: (value: unknown) => void = () => {}; + const { promise: readGate, resolve: settleRead } = Promise.withResolvers(); vi.mocked(sdk.GetSessionHydration).mockImplementationOnce( - () => new Promise((resolve) => { settleRead = resolve; }), + () => readGate, ); // A genuinely current (installed generation '1') loss. connection.failureHandler?.(new Error('first loss'), '1'); @@ -1489,9 +1488,9 @@ describe('repair', () => { const opensBefore = connection.openMock.mock.calls.length; const readsAtBoot = vi.mocked(sdk.GetSessionHydration).mock.calls.length; connection.openMock.mockResolvedValue({ instance_id: INSTANCE, generation: '2', workspace: '/ws' }); - let settleRead: (value: unknown) => void = () => {}; + const { promise: readGate, resolve: settleRead } = Promise.withResolvers(); vi.mocked(sdk.GetSessionHydration).mockImplementationOnce( - () => new Promise((resolve) => { settleRead = resolve; }), + () => readGate, ); connection.failureHandler?.(new Error('stream lost'), '1'); await vi.waitFor(() => expect(vi.mocked(sdk.GetSessionHydration).mock.calls.length).toBe(readsAtBoot + 1)); @@ -1521,8 +1520,7 @@ describe('repair', () => { await vi.waitFor(() => expect(target.textContent).toContain('root1')); // A configuration update arrives and its own read is held while the user // navigates to another root. - let releaseConfiguration: (value: unknown) => void = () => {}; - const held = new Promise((resolve) => { releaseConfiguration = resolve; }); + const { promise: held, resolve: releaseConfiguration } = Promise.withResolvers(); let reads = 0; vi.mocked(sdk.GetConfiguration).mockImplementation(async () => { reads += 1; @@ -1658,6 +1656,16 @@ function bodyOf(call: string): string { return call.slice(at + marker.length, call.length - 1); } +// The owned-input Fork action shared by the replacement and restore scenarios: +// open the dialog for i-owned and, when asked, enable the rewind toggle before +// submitting. +async function openOwnedFork(target: HTMLElement, rewind: boolean) { + (target.querySelector('[aria-label="Fork from input i-owned"]') as HTMLElement).click(); + await settle(); + if (rewind) await toggleCheckbox(target, 'Rewind code before Fork', true); + (target.querySelector('[aria-label="Submit fork"]') as HTMLElement).click(); +} + describe('repair projection ownership', () => { function ownedHydration() { return makeHydration({ @@ -1772,10 +1780,8 @@ describe('repair projection ownership', () => { const connection = fakeConnection(); basicMocks(connection); vi.mocked(sdk.GetSessionHydration).mockResolvedValue(ok(ownedHydration())); - let releaseSubmit: (value: unknown) => void = () => {}; - const submitGate = new Promise((_resolve, reject) => { releaseSubmit = reject; }); - let releaseFork: (value: unknown) => void = () => {}; - const forkGate = new Promise((_resolve, reject) => { releaseFork = reject; }); + const { promise: submitGate, reject: releaseSubmit } = Promise.withResolvers(); + const { promise: forkGate, reject: releaseFork } = Promise.withResolvers(); vi.mocked(sdk.SubmitSession).mockImplementationOnce(() => submitGate as never); vi.mocked(sdk.ForkSession).mockImplementationOnce(() => forkGate as never); const { target } = await mountApp(connection); @@ -1812,15 +1818,13 @@ describe('repair projection ownership', () => { const prov1 = providerFixture({ connected: false, key_source: 'external' }); const prov2 = providerFixture({ id: 'prov2', name: 'Prov Two' }); vi.mocked(sdk.ListProviders).mockResolvedValue(ok({ configuration_revision: { instance_id: INSTANCE, generation: '1' }, providers: [prov1, prov2] })); - let releaseRead: (value: unknown) => void = () => {}; - const readGate = new Promise((resolve) => { releaseRead = resolve; }); + const { promise: readGate, resolve: releaseRead } = Promise.withResolvers(); vi.mocked(sdk.GetProviderDetail).mockImplementation(async (params: string) => { const id = (JSON.parse(params) as { query: { provider_id: string } }).query.provider_id; if (id === 'prov2') return readGate as never; return ok({ configuration_revision: { instance_id: INSTANCE, generation: '1' }, provider: prov1 }); }); - let releaseMutation: (value: unknown) => void = () => {}; - const mutationGate = new Promise((resolve) => { releaseMutation = resolve; }); + const { promise: mutationGate, resolve: releaseMutation } = Promise.withResolvers(); vi.mocked(sdk.ConnectProvider).mockImplementationOnce(() => mutationGate as never); const { target } = await mountApp(connection); await openSettings(target); @@ -1901,8 +1905,7 @@ describe('workspace navigation', () => { { root: '/ws2', display_name: 'ws2' }, { root: '/ws3', display_name: 'ws3' }, ])); - let releaseStale: (value: unknown) => void = () => {}; - const staleGate = new Promise((resolve) => { releaseStale = resolve; }); + const { promise: staleGate, resolve: releaseStale } = Promise.withResolvers(); vi.mocked(sdk.ListSessions).mockImplementation(async (options) => { const workspace = (JSON.parse(options) as { query: { workspace: string } }).query.workspace; if (workspace === '/ws2') return staleGate as never; @@ -1935,8 +1938,7 @@ describe('workspace navigation', () => { { root: '/ws', display_name: 'ws' }, { root: '/ws2', display_name: 'ws2' }, ])); - let releasePick: (value: string) => void = () => {}; - const pickGate = new Promise((resolve) => { releasePick = resolve; }); + const { promise: pickGate, resolve: releasePick } = Promise.withResolvers(); vi.mocked(PickDirectory).mockImplementationOnce(() => pickGate as never); const { target } = await mountApp(connection); (target.querySelector('[aria-label="Select workspace"]') as HTMLElement).click(); @@ -1961,8 +1963,7 @@ describe('workspace navigation', () => { const connection = fakeConnection(); basicMocks(connection); vi.mocked(sdk.ListWorkspaces).mockResolvedValue(ok([{ root: '/ws', display_name: 'ws' }])); - let rejectPick: (error: Error) => void = () => {}; - const pickGate = new Promise((_resolve, reject) => { rejectPick = reject; }); + const { promise: pickGate, reject: rejectPick } = Promise.withResolvers(); vi.mocked(PickDirectory).mockImplementationOnce(() => pickGate as never); const { target } = await mountApp(connection); (target.querySelector('[aria-label="Select workspace"]') as HTMLElement).click(); @@ -2026,27 +2027,13 @@ describe('session picker tasks', () => { expect(target.textContent).not.toContain('Delete outcome unknown'); }); - it('reports a lost Delete as outcome unknown and never as success', async () => { - const connection = fakeConnection(); - basicMocks(connection); - vi.mocked(sdk.ArchiveSession).mockResolvedValue(ok(rootSession('root2', OLDER))); - vi.mocked(sdk.DeleteSession).mockRejectedValue(new Error('delete lost')); - const { target } = await mountApp(connection); - await openPicker(target); - (target.querySelector('[aria-label="Delete session root2"]') as HTMLElement).click(); - await settle(); - await settle(); - expect(vi.mocked(sdk.ArchiveSession)).toHaveBeenCalledWith('{"session_id":"root2"}'); - expect(target.textContent).toContain('Delete outcome unknown for session root2; nothing was retransmitted.'); - expect(vi.mocked(sdk.DeleteSession)).toHaveBeenCalledTimes(1); - }); - - it('reports an invalid-delivery Delete as outcome unknown and never as success', async () => { - const connection = fakeConnection(); - basicMocks(connection); + it.each([ + ['reports a lost Delete as outcome unknown and never as success', () => Promise.reject(new Error('delete lost'))], + ['reports an invalid-delivery Delete as outcome unknown and never as success', () => Promise.resolve(receivedInvalid('delete body undecodable'))], + ])('%s', async (_title, deleteOutcome) => { vi.mocked(sdk.ArchiveSession).mockResolvedValue(ok(rootSession('root2', OLDER))); - vi.mocked(sdk.DeleteSession).mockResolvedValue(receivedInvalid('delete body undecodable')); - const { target } = await mountApp(connection); + vi.mocked(sdk.DeleteSession).mockImplementation(deleteOutcome); + const { target } = await mountBasic(); await openPicker(target); (target.querySelector('[aria-label="Delete session root2"]') as HTMLElement).click(); await settle(); @@ -2095,8 +2082,7 @@ describe('session picker tasks', () => { it('invalidates an older list after a deletion, including a nonselected one', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseFirstDeleteList: (value: unknown) => void = () => {}; - const firstDeleteList = new Promise((resolve) => { releaseFirstDeleteList = resolve; }); + const { promise: firstDeleteList, resolve: releaseFirstDeleteList } = Promise.withResolvers(); let openCalls = 0; vi.mocked(sdk.ListSessions).mockImplementation(async (options) => { const lifecycle = (JSON.parse(options) as { query: { lifecycle: string } }).query.lifecycle; @@ -2143,8 +2129,7 @@ describe('session picker tasks', () => { } return ok([]); }); - let releaseDelete: (value: unknown) => void = () => {}; - const deleteGate = new Promise((resolve) => { releaseDelete = resolve; }); + const { promise: deleteGate, resolve: releaseDelete } = Promise.withResolvers(); vi.mocked(sdk.DeleteSession).mockImplementationOnce(() => deleteGate as never); const { target } = await mountApp(connection); window.dispatchEvent(new KeyboardEvent('keydown', { key: 's', ctrlKey: true })); @@ -2170,8 +2155,7 @@ describe('session picker tasks', () => { const connection = fakeConnection(); basicMocks(connection); let openListCalls = 0; - let releaseStage: (value: unknown) => void = () => {}; - const stageGate = new Promise((resolve) => { releaseStage = resolve; }); + const { promise: stageGate, resolve: releaseStage } = Promise.withResolvers(); vi.mocked(sdk.ListSessions).mockImplementation(async (options) => { const lifecycle = (JSON.parse(options) as { query: { lifecycle: string } }).query.lifecycle; if (lifecycle === 'archived') { @@ -2327,8 +2311,7 @@ describe('resume selection intent', () => { function archivedSession(id: string it('adopts only the newest resume when the later Reopen settles first', async () => { const connection = archivedConnection(); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(ok(rootSession('arch9', OLDER))); @@ -2350,10 +2333,8 @@ describe('resume selection intent', () => { function archivedSession(id: string it('keeps the newest resume intent when the earlier Reopen settles first', async () => { const connection = archivedConnection(); - let releaseFirst: (value: unknown) => void = () => {}; - let releaseSecond: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession) .mockImplementationOnce(() => first as never) .mockImplementationOnce(() => second as never); @@ -2377,8 +2358,7 @@ describe('resume selection intent', () => { function archivedSession(id: string it('a rejected or lost newest resume keeps the prior root and never adopts a stale success', async () => { const connection = archivedConnection(); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(httpError(404, { code: 'not_found', message: 'no longer archived' })); @@ -2397,8 +2377,7 @@ describe('resume selection intent', () => { function archivedSession(id: string expect(hydratedIDs()).not.toContain('arch8'); expect(target.querySelector('.toolbar')?.textContent).toContain('root1'); // A lost newest resume behaves the same. - let releaseSecond: (value: unknown) => void = () => {}; - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession) .mockImplementationOnce(() => second as never) .mockRejectedValueOnce(new Error('reopen lost')); @@ -2422,8 +2401,7 @@ describe('resume selection intent', () => { function archivedSession(id: string { root: '/ws2', display_name: 'ws2' }, ])); vi.mocked(sdk.CreateSession).mockResolvedValue(ok(rootSession('root-new', NEWER))); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession).mockImplementationOnce(() => first as never); const { target } = await mountApp(connection); await openArchived(target); @@ -2440,8 +2418,7 @@ describe('resume selection intent', () => { function archivedSession(id: string expect(hydratedIDs()).not.toContain('arch8'); expect(target.querySelector('.toolbar')?.textContent).toContain('root2'); // A Workspace composition supersedes a parked resume too. - let releaseSecond: (value: unknown) => void = () => {}; - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession).mockImplementationOnce(() => second as never); await reopenArchived(target); (target.querySelector('[aria-label="Resume session arch8"]') as HTMLElement).click(); @@ -2457,8 +2434,7 @@ describe('resume selection intent', () => { function archivedSession(id: string expect(hydratedIDs()).not.toContain('arch8'); expect(target.textContent).toContain('/ws2'); // A picker New that succeeds supersedes a parked resume as well. - let releaseThird: (value: unknown) => void = () => {}; - const third = new Promise((resolve) => { releaseThird = resolve; }); + const { promise: third, resolve: releaseThird } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession).mockImplementationOnce(() => third as never); await reopenArchived(target); (target.querySelector('[aria-label="Resume session arch8"]') as HTMLElement).click(); @@ -2493,8 +2469,7 @@ describe('resume selection intent', () => { function archivedSession(id: string conversation: { items: [inputItem('i-old', 'user', 'op-old'), inputItem('i-new', 'user', 'op-new')] }, operations: [runningOperation('op-old')], }); - let releaseParked: (value: unknown) => void = () => {}; - const parked = new Promise((resolve) => { releaseParked = resolve; }); + const { promise: parked, resolve: releaseParked } = Promise.withResolvers(); vi.mocked(sdk.GetSessionHydration) .mockResolvedValueOnce(ok(initial)) .mockImplementationOnce(() => parked as never) @@ -2537,8 +2512,7 @@ describe('resume selection intent', () => { function archivedSession(id: string ? ok([rootSession('root1', NEWER), rootSession('root2', OLDER)]) : ok([{ ...rootSession('arch8', OLDER), lifecycle: 'archived' as const }]); }); - let releaseResume: (value: unknown) => void = () => {}; - const resumeGate = new Promise((resolve) => { releaseResume = resolve; }); + const { promise: resumeGate, resolve: releaseResume } = Promise.withResolvers(); vi.mocked(sdk.ReopenSession).mockImplementationOnce(() => resumeGate as never); const { target } = await mountApp(connection); await openArchived(target); @@ -2651,8 +2625,7 @@ describe('settings editor', () => { it('gates every authored control while a save is pending and sends the complete body with no precondition', async () => { const connection = settingsRawConnection(); - let releaseSave: (value: unknown) => void = () => {}; - const gate = new Promise((resolve) => { releaseSave = resolve; }); + const { promise: gate, resolve: releaseSave } = Promise.withResolvers(); vi.mocked(sdk.UpdateConfigurationSettings).mockImplementationOnce(() => gate as never); const { target } = await mountApp(connection); await openSettings(target); @@ -2737,8 +2710,7 @@ describe('host-local appearance and version', () => { it('keeps the host-local rows live while the Runtime settings read is pending', async () => { const connection = settingsRawConnection(); - let release: (value: unknown) => void = () => {}; - const gate = new Promise((resolve) => { release = resolve; }); + const { promise: gate, resolve: release } = Promise.withResolvers(); let reads = 0; // The boot global read resolves; the editor's own exact read stays parked. vi.mocked(sdk.GetConfiguration).mockImplementation(async () => { @@ -3279,8 +3251,7 @@ describe('provider discovery supersession and busy controls', () => { it('publishes only the newest model-discovery response for the same provider', async () => { const connection = editorConnection([providerFixture()]); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.DiscoverProviderModelCandidates) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(ok([candidate('cand-new')])); @@ -3303,10 +3274,8 @@ describe('provider discovery supersession and busy controls', () => { it('discards an earlier provider discovery success after the selection moves', async () => { const connection = editorConnection([providerFixture(), providerFixture({ id: 'prov2', name: 'Prov Two' })]); - let releaseFirst: (value: unknown) => void = () => {}; - let releaseSecond: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.DiscoverProviderModelCandidates) .mockImplementationOnce(() => first as never) .mockImplementationOnce(() => second as never); @@ -3335,8 +3304,7 @@ describe('provider discovery supersession and busy controls', () => { it('discards an earlier provider discovery error after the selection moves', async () => { const connection = editorConnection([providerFixture(), providerFixture({ id: 'prov2', name: 'Prov Two' })]); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.DiscoverProviderModelCandidates).mockImplementationOnce(() => first as never); const { target } = await mountApp(connection); await openProviderEditor(target); @@ -3354,8 +3322,7 @@ describe('provider discovery supersession and busy controls', () => { it('invalidates an earlier discovery request across an A-to-B-to-A selection move', async () => { const connection = editorConnection([providerFixture(), providerFixture({ id: 'prov2', name: 'Prov Two' })]); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.DiscoverProviderModelCandidates) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(ok([candidate('cand-new')])); @@ -3411,10 +3378,8 @@ describe('provider discovery supersession and busy controls', () => { it('keeps provider and model controls disabled through a parked mutation and its parked refresh', async () => { const connection = editorConnection([providerFixture()]); - let releaseWrite: (value: unknown) => void = () => {}; - const writeGate = new Promise((resolve) => { releaseWrite = resolve; }); - let releaseRefresh: (value: unknown) => void = () => {}; - const refreshGate = new Promise((resolve) => { releaseRefresh = resolve; }); + const { promise: writeGate, resolve: releaseWrite } = Promise.withResolvers(); + const { promise: refreshGate, resolve: releaseRefresh } = Promise.withResolvers(); vi.mocked(sdk.UpdateProviderDetail).mockImplementationOnce(() => writeGate as never); vi.mocked(sdk.GetProviderDetail) .mockResolvedValueOnce(ok({ configuration_revision: { instance_id: INSTANCE, generation: '1' }, provider: providerFixture() })) @@ -3448,10 +3413,8 @@ describe('provider discovery supersession and busy controls', () => { it('keeps the connect form disabled through its mutation and the provider-list refresh', async () => { const disconnected = providerFixture({ connected: false, key_source: 'external' }); const connection = editorConnection([disconnected]); - let releaseConnect: (value: unknown) => void = () => {}; - const connectGate = new Promise((resolve) => { releaseConnect = resolve; }); - let releaseList: (value: unknown) => void = () => {}; - const listGate = new Promise((resolve) => { releaseList = resolve; }); + const { promise: connectGate, resolve: releaseConnect } = Promise.withResolvers(); + const { promise: listGate, resolve: releaseList } = Promise.withResolvers(); vi.mocked(sdk.ConnectProvider).mockImplementationOnce(() => connectGate as never); vi.mocked(sdk.ListProviders) .mockImplementationOnce(async () => ok({ configuration_revision: { instance_id: INSTANCE, generation: '1' }, providers: [disconnected] })) @@ -3483,8 +3446,7 @@ describe('provider discovery supersession and busy controls', () => { it('keeps the create form and candidate selection disabled while the create is parked', async () => { const connection = editorConnection([]); vi.mocked(sdk.DiscoverProviderCandidates).mockResolvedValue(ok([candidate('cand-1')])); - let releaseCreate: (value: unknown) => void = () => {}; - const createGate = new Promise((resolve) => { releaseCreate = resolve; }); + const { promise: createGate, resolve: releaseCreate } = Promise.withResolvers(); vi.mocked(sdk.CreateProvider).mockImplementationOnce(() => createGate as never); const { target } = await mountApp(connection); await openSettings(target); @@ -3514,10 +3476,8 @@ describe('provider discovery supersession and busy controls', () => { it('keeps the upsert form and candidate selection disabled while the upsert is parked', async () => { const connection = editorConnection([providerFixture()]); vi.mocked(sdk.DiscoverProviderModelCandidates).mockResolvedValue(ok([candidate('cand-9')])); - let releaseUpsert: (value: unknown) => void = () => {}; - const upsertGate = new Promise((resolve) => { releaseUpsert = resolve; }); - let releaseDetail: (value: unknown) => void = () => {}; - const detailGate = new Promise((resolve) => { releaseDetail = resolve; }); + const { promise: upsertGate, resolve: releaseUpsert } = Promise.withResolvers(); + const { promise: detailGate, resolve: releaseDetail } = Promise.withResolvers(); vi.mocked(sdk.UpdateProviderModel).mockImplementationOnce(() => upsertGate as never); vi.mocked(sdk.GetProviderDetail) .mockResolvedValueOnce(ok({ configuration_revision: { instance_id: INSTANCE, generation: '1' }, provider: providerFixture() })) @@ -3570,8 +3530,7 @@ describe('file viewer', () => { it('ignores a superseded viewer reply and keeps the newest read', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.ReadWorkspaceFile) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(ok({ content: 'second body', canonical_path: '/ws/b.ts', truncated: false })); @@ -3745,8 +3704,7 @@ describe('code snapshots', () => { vi.mocked(sdk.GetSessionCodeSnapshots).mockResolvedValue(ok([ { operation_id: 'op1', group_item_id: 'group1', files: [{ path: 'a.ts', existed: true }] }, ])); - let releaseRevert: (value: unknown) => void = () => {}; - const revertGate = new Promise((_resolve, reject) => { releaseRevert = reject; }); + const { promise: revertGate, reject: releaseRevert } = Promise.withResolvers(); vi.mocked(sdk.RevertSessionCode).mockImplementationOnce(() => revertGate as never); const { target } = await mountApp(connection); await openSettings(target); @@ -3845,13 +3803,6 @@ describe('replacement Fork and ordered rewind', () => { return connection; } - async function openOwnedFork(target: HTMLElement, rewind: boolean) { - (target.querySelector('[aria-label="Fork from input i-owned"]') as HTMLElement).click(); - await settle(); - if (rewind) await toggleCheckbox(target, 'Rewind code before Fork', true); - (target.querySelector('[aria-label="Submit fork"]') as HTMLElement).click(); - } - it('exposes Fork only on root user-origin inputs, with rewind only for an owned input', async () => { const connection = forkConnection(); const { target } = await mountApp(connection); @@ -3990,8 +3941,7 @@ describe('replacement Fork and ordered rewind', () => { it('shows a known partial restore failure before still issuing Fork', async () => { const connection = forkConnection(); - let releaseFork: (value: unknown) => void = () => {}; - const forkGate = new Promise((resolve) => { releaseFork = resolve; }); + const { promise: forkGate, resolve: releaseFork } = Promise.withResolvers(); vi.mocked(sdk.RevertSessionCode).mockResolvedValue(httpError(409, { restored: ['a.ts'], skipped: [{ path: 'b.ts', reason: 'io' }], @@ -4035,8 +3985,7 @@ describe('replacement Fork and ordered rewind', () => { it('a late non-adopted destination still reports the earlier restore effects', async () => { const connection = forkConnection(); - let releaseFork: (value: unknown) => void = () => {}; - const forkGate = new Promise((resolve) => { releaseFork = resolve; }); + const { promise: forkGate, resolve: releaseFork } = Promise.withResolvers(); vi.mocked(sdk.RevertSessionCode).mockResolvedValue(httpError(409, { restored: ['a.ts'], skipped: [], @@ -4113,10 +4062,8 @@ describe('replacement Fork and ordered rewind', () => { it('a held Retry of an unknown Fork transitions to pending and permits no second retry while its response is held', async () => { const connection = forkConnection(); vi.mocked(sdk.RevertSessionCode).mockResolvedValue(ok({ restored: [], skipped: [] })); - let releaseFork: (value: unknown) => void = () => {}; - let releaseRetry: (value: unknown) => void = () => {}; - const firstGate = new Promise((resolve) => { releaseFork = resolve; }); - const retryGate = new Promise((resolve) => { releaseRetry = resolve; }); + const { promise: firstGate, resolve: releaseFork } = Promise.withResolvers(); + const { promise: retryGate, resolve: releaseRetry } = Promise.withResolvers(); vi.mocked(sdk.ForkSession) .mockImplementationOnce(() => firstGate as never) .mockImplementationOnce(() => retryGate as never); @@ -4160,10 +4107,8 @@ describe('replacement Fork and ordered rewind', () => { it('a held Retry of a rejected Fork transitions to pending and its release reports the final outcome beside the newer draft', async () => { const connection = forkConnection(); - let releaseFork: (value: unknown) => void = () => {}; - let releaseRetry: (value: unknown) => void = () => {}; - const firstGate = new Promise((resolve) => { releaseFork = resolve; }); - const retryGate = new Promise((resolve) => { releaseRetry = resolve; }); + const { promise: firstGate, resolve: releaseFork } = Promise.withResolvers(); + const { promise: retryGate, resolve: releaseRetry } = Promise.withResolvers(); vi.mocked(sdk.ForkSession) .mockImplementationOnce(() => firstGate as never) .mockImplementationOnce(() => retryGate as never); @@ -4202,8 +4147,7 @@ describe('replacement Fork and ordered rewind', () => { it('never adopts a late destination over a moved selection and keeps a newer replacement draft', async () => { const connection = forkConnection(); - let releaseFork: (value: unknown) => void = () => {}; - const forkGate = new Promise((resolve) => { releaseFork = resolve; }); + const { promise: forkGate, resolve: releaseFork } = Promise.withResolvers(); vi.mocked(sdk.ForkSession).mockImplementationOnce(() => forkGate as never); const { target } = await mountApp(connection); await openOwnedFork(target, false); @@ -4262,44 +4206,29 @@ describe('restore outcomes through the fork path', () => { return connection; } - async function submitRewindFork(target: HTMLElement) { - (target.querySelector('[aria-label="Fork from input i-owned"]') as HTMLElement).click(); - await settle(); - await toggleCheckbox(target, 'Rewind code before Fork', true); - (target.querySelector('[aria-label="Submit fork"]') as HTMLElement).click(); - } - it('warns on a plain pre-traversal restore rejection and still issues Fork', async () => { const connection = forkConnection(); vi.mocked(sdk.RevertSessionCode).mockResolvedValue(httpError(404, { code: 'not_found', message: 'no snapshot group' })); const { target } = await mountApp(connection); - await submitRewindFork(target); + await openOwnedFork(target, true); await settle(); await settle(); await settle(); expect(vi.mocked(sdk.ForkSession)).toHaveBeenCalledTimes(1); }); - it('treats a malformed 2xx restore body as outcome unknown and never sends Fork', async () => { - const connection = forkConnection(); - vi.mocked(sdk.RevertSessionCode).mockResolvedValue(ok({})); - const { target } = await mountApp(connection); - await submitRewindFork(target); - await settle(); - await settle(); - expect(vi.mocked(sdk.ForkSession)).not.toHaveBeenCalled(); - expect(target.textContent).toContain('Rewind outcome unknown'); - }); - - it('treats a 2xx restore body carrying an error member as outcome unknown and never sends Fork', async () => { - const connection = forkConnection(); - vi.mocked(sdk.RevertSessionCode).mockResolvedValue(ok({ + it.each([ + ['treats a malformed 2xx restore body as outcome unknown and never sends Fork', ok({})], + ['treats a 2xx restore body carrying an error member as outcome unknown and never sends Fork', ok({ restored: ['a.ts'], skipped: [], error: { code: 'storage', message: 'partial failure' }, - })); + })], + ])('%s', async (_title, restoreBody) => { + const connection = forkConnection(); + vi.mocked(sdk.RevertSessionCode).mockResolvedValue(restoreBody); const { target } = await mountApp(connection); - await submitRewindFork(target); + await openOwnedFork(target, true); await settle(); await settle(); expect(vi.mocked(sdk.ForkSession)).not.toHaveBeenCalled(); @@ -4310,8 +4239,7 @@ describe('restore outcomes through the fork path', () => { describe('viewer supersession with an error', () => { it('ignores a superseded viewer error and keeps the newest read', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.ReadWorkspaceFile) .mockImplementationOnce(() => first as never) .mockResolvedValueOnce(ok({ content: 'fresh body', canonical_path: '/ws/b.ts', truncated: false })); @@ -4344,8 +4272,7 @@ describe('editor read supersession', () => { configuration_revision: { instance_id: INSTANCE, generation: '1' }, providers: [prov1, prov2], })); - let releaseFirst: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); vi.mocked(sdk.GetProviderDetail).mockImplementation(async (params: string) => { const id = (JSON.parse(params) as { query: { provider_id: string } }).query.provider_id; if (id === 'prov1') return first as never; @@ -4382,8 +4309,7 @@ describe('read lifetime boundaries', () => { it('releases its own viewer loading when a superseded read settles without a newer read', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseRead: (value: unknown) => void = () => {}; - const read = new Promise((resolve) => { releaseRead = resolve; }); + const { promise: read, resolve: releaseRead } = Promise.withResolvers(); vi.mocked(sdk.ReadWorkspaceFile).mockImplementationOnce(() => read as never); const { target } = await mountApp(connection); await openSettings(target); @@ -4405,10 +4331,8 @@ describe('read lifetime boundaries', () => { it('lets no older same-stamp viewer reply publish over or clear a newer read', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseFirst: (value: unknown) => void = () => {}; - let releaseSecond: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.ReadWorkspaceFile) .mockImplementationOnce(() => first as never) .mockImplementationOnce(() => second as never); @@ -4439,8 +4363,7 @@ describe('read lifetime boundaries', () => { it('a superseded settings read publishes nothing and a reopen reads fresh', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseText: (value: unknown) => void = () => {}; - const textRead = new Promise((resolve) => { releaseText = resolve; }); + const { promise: textRead, resolve: releaseText } = Promise.withResolvers(); let configCalls = 0; vi.mocked(sdk.GetConfiguration).mockImplementation(async () => { configCalls += 1; @@ -4475,10 +4398,8 @@ describe('read lifetime boundaries', () => { it('a reopened model dialog discards an older same-stamp read and keeps the newer loading', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseFirst: (value: unknown) => void = () => {}; - let releaseSecond: (value: unknown) => void = () => {}; - const first = new Promise((resolve) => { releaseFirst = resolve; }); - const second = new Promise((resolve) => { releaseSecond = resolve; }); + const { promise: first, resolve: releaseFirst } = Promise.withResolvers(); + const { promise: second, resolve: releaseSecond } = Promise.withResolvers(); vi.mocked(sdk.ListModels) .mockImplementationOnce(() => first as never) .mockImplementationOnce(() => second as never); @@ -4649,8 +4570,7 @@ describe('loaded snapshot identity', () => { it('invalidates a pending retained read when its draft target changes', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseRead: (value: unknown) => void = () => {}; - const pending = new Promise((resolve) => { releaseRead = resolve; }); + const { promise: pending, resolve: releaseRead } = Promise.withResolvers(); vi.mocked(sdk.GetRetainedCodeSnapshots).mockImplementationOnce(() => pending as never); const { target } = await mountApp(connection); await openSettings(target); @@ -4737,8 +4657,7 @@ describe('session mutation commits', () => { it('a stale archive completion cannot clear a newer selection', async () => { const connection = fakeConnection(); basicMocks(connection); - let releaseArchive: (value: unknown) => void = () => {}; - const pending = new Promise((resolve) => { releaseArchive = resolve; }); + const { promise: pending, resolve: releaseArchive } = Promise.withResolvers(); vi.mocked(sdk.ArchiveSession).mockImplementationOnce(() => pending as never); vi.mocked(sdk.DeleteSession).mockResolvedValue(noContent()); const { target } = await mountApp(connection); @@ -4906,8 +4825,7 @@ describe('selection-bound Fork source', () => { conversation: { items: [inputItem('i-owned', 'user', 'op-owned')] }, operations: [runningOperation('op-owned')], }))); - let releaseFork: (value: unknown) => void = () => {}; - const forkGate = new Promise((resolve) => { releaseFork = resolve; }); + const { promise: forkGate, resolve: releaseFork } = Promise.withResolvers(); vi.mocked(sdk.ForkSession).mockImplementationOnce(() => forkGate as never); const { target } = await mountApp(connection); (target.querySelector('[aria-label="Fork from input i-owned"]') as HTMLElement).click(); @@ -5007,11 +4925,11 @@ describe('composition failure outcomes', () => { vi.mocked(sdk.ListWorkspaces).mockResolvedValue(ok([ { root: '/wsB', display_name: 'B' }, ])); - let releaseInitial: (value: unknown) => void = () => {}; - let releasePicker: (reason?: unknown) => void = () => {}; + const { promise: initialGate, resolve: releaseInitial } = Promise.withResolvers(); + const { promise: pickerGate, reject: releasePicker } = Promise.withResolvers(); vi.mocked(sdk.CreateSession) - .mockImplementationOnce(() => new Promise((resolve) => { releaseInitial = resolve; }) as never) - .mockImplementationOnce(() => new Promise((_resolve, reject) => { releasePicker = reject; }) as never) + .mockImplementationOnce(() => initialGate as never) + .mockImplementationOnce(() => pickerGate as never) .mockResolvedValue(ok(rootSession('b-1', NEWER))); vi.mocked(sdk.GetSessionHydration).mockImplementation(async (options) => { const id = (JSON.parse(options) as { session_id: string }).session_id; diff --git a/internal/atomicfs/atomicfs_test.go b/internal/atomicfs/atomicfs_test.go index 1ccd29a0..670fea45 100644 --- a/internal/atomicfs/atomicfs_test.go +++ b/internal/atomicfs/atomicfs_test.go @@ -103,103 +103,75 @@ func TestCreateExclusiveRefusesOverwrite(t *testing.T) { } } -// TestWriteSyncFailureBeforePublicationAborts proves a pre-publication temp -// sync failure is an ordinary failed write: Write returns the error, leaves -// no temp behind, and does not publish the file. -func TestWriteSyncFailureBeforePublicationAborts(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "rec.json") - - injected := errors.New("injected temp sync failure") - SyncFileFunc = func(*os.File) error { return injected } - defer func() { SyncFileFunc = nil }() - - err := Write(path, []byte("data"), 0o600) - if err == nil { - t.Fatal("Write succeeded; want the injected temp sync failure") - } - if !strings.Contains(err.Error(), "sync temp") { - t.Fatalf("error = %v, want a temp sync error", err) - } - if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { - t.Fatalf("path exists after the aborted write: %v", statErr) - } - entries, _ := os.ReadDir(dir) - if len(entries) != 0 { - t.Fatalf("leftover entries after the aborted write: %v", entries) - } -} - -// TestWriteSyncFailureBeforePublicationPreservesExistingContent proves a -// pre-publication temp sync failure on a present target is an ordinary -// failed write that leaves the old complete contents intact: Write returns -// the error, removes the temp, and the destination still holds the bytes it -// held before the call. The sibling -// TestWriteSyncFailureBeforePublicationAborts starts from an absent path, so -// its assertions cannot distinguish removing the temp from removing the -// target — only a present target can see the difference. -func TestWriteSyncFailureBeforePublicationPreservesExistingContent(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "rec.json") - if err := Write(path, []byte("original"), 0o600); err != nil { - t.Fatalf("seed write: %v", err) - } - - injected := errors.New("injected temp sync failure") - SyncFileFunc = func(*os.File) error { return injected } - defer func() { SyncFileFunc = nil }() - - err := Write(path, []byte("replacement"), 0o600) - if err == nil { - t.Fatal("Write succeeded; want the injected temp sync failure") - } - got, readErr := os.ReadFile(path) - if readErr != nil { - t.Fatalf("read destination: %v", readErr) - } - if string(got) != "original" { - t.Fatalf("destination = %q after the failed write, want original", got) - } - entries, _ := os.ReadDir(dir) - if len(entries) != 1 || entries[0].Name() != "rec.json" { - t.Fatalf("leftover entries after the aborted write: %v", entries) - } -} - -// TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent -// proves the same present-target rule for the link-based publisher: -// CreateExclusive returns the error, leaves no temp behind (its defer removes -// it), and the existing file keeps its original contents. -func TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "rec.json") +// TestPrePublicationSyncFailureRows covers both publishers with absent and +// present destinations. A pre-publication temp sync failure is an ordinary +// failed write: the publisher returns the error, leaves no temp behind, and +// publishes nothing. A present destination also distinguishes removing the +// temp from removing the target: only it can see the old complete contents +// survive the failed replacement. +func TestPrePublicationSyncFailureRows(t *testing.T) { + writePublisher := func(path string, data []byte, mode os.FileMode) (bool, error) { + err := Write(path, data, mode) + return err == nil, err + } + cases := []struct { + name string + publish func(string, []byte, os.FileMode) (bool, error) + present bool + wantErrPart string + }{ + {"TestWriteSyncFailureBeforePublicationAborts", writePublisher, false, "sync temp"}, + {"TestWriteSyncFailureBeforePublicationPreservesExistingContent", writePublisher, true, ""}, + {"TestCreateExclusiveSyncFailureBeforePublicationPreservesExistingContent", CreateExclusive, true, ""}, + {"TestCreateExclusiveSyncFailureBeforePublicationAborts", CreateExclusive, false, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "rec.json") + + if tc.present { + created, err := tc.publish(path, []byte("original"), 0o600) + if err != nil || !created { + t.Fatalf("seed publish: (%v, %v)", created, err) + } + } - created, err := CreateExclusive(path, []byte("original"), 0o600) - if err != nil || !created { - t.Fatalf("seed create: (%v, %v)", created, err) - } + injected := errors.New("injected temp sync failure") + SyncFileFunc = func(*os.File) error { return injected } + defer func() { SyncFileFunc = nil }() - injected := errors.New("injected temp sync failure") - SyncFileFunc = func(*os.File) error { return injected } - defer func() { SyncFileFunc = nil }() + created, err := tc.publish(path, []byte("replacement"), 0o600) + if err == nil { + t.Fatal("publish succeeded; want the injected temp sync failure") + } + if created { + t.Fatal("created = true on a failed write") + } + if tc.wantErrPart != "" && !strings.Contains(err.Error(), tc.wantErrPart) { + t.Fatalf("error = %v, want a %s error", err, tc.wantErrPart) + } - created, err = CreateExclusive(path, []byte("replacement"), 0o600) - if err == nil { - t.Fatal("CreateExclusive succeeded; want the injected temp sync failure") - } - if created { - t.Fatal("created = true on a failed write") - } - got, readErr := os.ReadFile(path) - if readErr != nil { - t.Fatalf("read destination: %v", readErr) - } - if string(got) != "original" { - t.Fatalf("destination = %q after the failed write, want original", got) - } - entries, _ := os.ReadDir(dir) - if len(entries) != 1 || entries[0].Name() != "rec.json" { - t.Fatalf("leftover entries after the aborted write: %v", entries) + if tc.present { + got, readErr := os.ReadFile(path) + if readErr != nil { + t.Fatalf("read destination: %v", readErr) + } + if string(got) != "original" { + t.Fatalf("destination = %q after the failed write, want original", got) + } + } else if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { + t.Fatalf("path exists after the aborted write: %v", statErr) + } + entries, _ := os.ReadDir(dir) + if tc.present { + if len(entries) != 1 || entries[0].Name() != "rec.json" { + t.Fatalf("leftover entries after the aborted write: %v", entries) + } + } else if len(entries) != 0 { + t.Fatalf("leftover entries after the aborted write: %v", entries) + } + }) } } @@ -235,34 +207,6 @@ func TestWriteDirSyncFailureAfterPublicationReturnsSuccess(t *testing.T) { } } -// TestCreateExclusiveSyncFailureBeforePublicationAborts proves the same -// pre-publication rule for the link-based publisher: CreateExclusive -// returns the error, leaves no temp behind (its defer removes it), and -// does not publish the file. -func TestCreateExclusiveSyncFailureBeforePublicationAborts(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, "rec.json") - - injected := errors.New("injected temp sync failure") - SyncFileFunc = func(*os.File) error { return injected } - defer func() { SyncFileFunc = nil }() - - created, err := CreateExclusive(path, []byte("data"), 0o600) - if err == nil { - t.Fatal("CreateExclusive succeeded; want the injected temp sync failure") - } - if created { - t.Fatal("created = true on a failed write") - } - if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { - t.Fatalf("path exists after the aborted write: %v", statErr) - } - entries, _ := os.ReadDir(dir) - if len(entries) != 0 { - t.Fatalf("leftover entries after the aborted write: %v", entries) - } -} - // TestCreateExclusiveDirSyncFailureAfterPublicationReturnsSuccess proves // the post-publication rule for the link-based publisher: CreateExclusive // returns (true, nil), the file stays published, and the diagnostic is diff --git a/internal/config/managedenv_cross_test.go b/internal/config/managedenv_cross_test.go index 21a3ebe6..e8b6e0c5 100644 --- a/internal/config/managedenv_cross_test.go +++ b/internal/config/managedenv_cross_test.go @@ -16,7 +16,7 @@ func TestManagedEnvCrossInstanceOperations(t *testing.T) { envPath := filepath.Join(dir, ".env") newMgr := func() *ManagedEnv { - return &ManagedEnv{path: envPath, managed: map[string]struct{}{}} + return NewManagedEnvForTest(envPath) } keys := []string{"LIGHTCODE_TEST_A", "LIGHTCODE_TEST_B", "LIGHTCODE_TEST_C"} diff --git a/internal/lsp/instance_ready_test.go b/internal/lsp/instance_ready_test.go index 01e1ffea..bf372454 100644 --- a/internal/lsp/instance_ready_test.go +++ b/internal/lsp/instance_ready_test.go @@ -147,6 +147,37 @@ func newFakeServerInstanceWithLifetime(t *testing.T, log, failMarker string, ini return inst } +// newPIDRecordingFakeServer builds the fake server behind a shell wrapper +// that records the launched process's pid before exec'ing the real script, so +// a test can observe whether a process was launched and whether it was +// reaped. The pidfile and the launch log use separate temporary directories. +func newPIDRecordingFakeServer(t *testing.T, initDelay string) (home, pidfile, log string, def *server.Definition) { + t.Helper() + home = t.TempDir() + cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake") + if err := os.MkdirAll(cacheDir, 0o755); err != nil { + t.Fatal(err) + } + real := filepath.Join(cacheDir, "fake-lsp.py") + if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil { + t.Fatal(err) + } + wrapper := filepath.Join(cacheDir, "fake-lsp") + wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n" + if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil { + t.Fatal(err) + } + pidfile = filepath.Join(t.TempDir(), "lsp.pid") + t.Setenv("FAKE_PIDFILE", pidfile) + log = filepath.Join(t.TempDir(), "launches.log") + def = &server.Definition{ + Name: "fake", + Command: "fake-lsp", + Args: []string{log, "", initDelay, "0", "", "1"}, + } + return home, pidfile, log, def +} + func countLaunches(log string) int { b, err := os.ReadFile(log) if err != nil { @@ -435,31 +466,8 @@ func TestStartRefusedWhileReady(t *testing.T) { // between cmd.Start and the post-launch attempt-identity recheck, and the // waitCommand seam counts exactly one wait on this side of the handoff. func TestStartAfterPermanentCloseSelfReaps(t *testing.T) { - home := t.TempDir() - cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake") - if err := os.MkdirAll(cacheDir, 0o755); err != nil { - t.Fatal(err) - } - real := filepath.Join(cacheDir, "fake-lsp.py") - if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil { - t.Fatal(err) - } - // The wrapper records the launched process's pid before exec'ing the real - // script, so the test can assert the close loser was actually reaped. - wrapper := filepath.Join(cacheDir, "fake-lsp") - wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n" - if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil { - t.Fatal(err) - } - pidfile := filepath.Join(t.TempDir(), "lsp.pid") - t.Setenv("FAKE_PIDFILE", pidfile) - - log := filepath.Join(t.TempDir(), "launches.log") - inst := newInstance(&server.Definition{ - Name: "fake", - Command: "fake-lsp", - Args: []string{log, "", "0", "0", "", "1"}, - }, t.TempDir(), home, context.Background(), nil) + home, pidfile, log, def := newPIDRecordingFakeServer(t, "0") + inst := newInstance(def, t.TempDir(), home, context.Background(), nil) t.Cleanup(func() { inst.shutdown() }) // Count every instance-owned process wait through the seam. diff --git a/internal/lsp/manager_test.go b/internal/lsp/manager_test.go index 6f0a6fff..24995ccd 100644 --- a/internal/lsp/manager_test.go +++ b/internal/lsp/manager_test.go @@ -148,31 +148,7 @@ func TestStartServerAfterClosedRejectsBeforeStart(t *testing.T) { // rejected before instance.start: permanent close refuses before OS start, // so no process is ever launched after close. The old launched-then-killed // path is gone. - home := t.TempDir() - cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake") - if err := os.MkdirAll(cacheDir, 0o755); err != nil { - t.Fatal(err) - } - real := filepath.Join(cacheDir, "fake-lsp.py") - if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil { - t.Fatal(err) - } - // A wrapper records the launched process's pid before exec'ing the real - // script, so the test can assert no process was ever launched. - wrapper := filepath.Join(cacheDir, "fake-lsp") - wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n" - if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil { - t.Fatal(err) - } - pidfile := filepath.Join(t.TempDir(), "lsp.pid") - t.Setenv("FAKE_PIDFILE", pidfile) - - log := filepath.Join(t.TempDir(), "launches.log") - def := &server.Definition{ - Name: "fake", - Command: "fake-lsp", - Args: []string{log, "", "0", "0", "", "1"}, - } + home, pidfile, _, def := newPIDRecordingFakeServer(t, "0") m := NewManager(t.TempDir(), home) m.mu.Lock() @@ -255,31 +231,7 @@ func TestStartServerAfterClosedSkipsInstall(t *testing.T) { // stateFailed. Removing the provisional map insertion makes close miss the // instance and the released start launch it — the regression this test pins. func TestStartServerCloseRaceProvisionalMappingHoldsTerminalAdmission(t *testing.T) { - home := t.TempDir() - cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake") - if err := os.MkdirAll(cacheDir, 0o755); err != nil { - t.Fatal(err) - } - real := filepath.Join(cacheDir, "fake-lsp.py") - if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil { - t.Fatal(err) - } - // The wrapper records the launched process's pid before exec'ing the real - // script, so the test can assert no process was ever launched. - wrapper := filepath.Join(cacheDir, "fake-lsp") - wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n" - if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil { - t.Fatal(err) - } - pidfile := filepath.Join(t.TempDir(), "lsp.pid") - t.Setenv("FAKE_PIDFILE", pidfile) - - log := filepath.Join(t.TempDir(), "launches.log") - def := &server.Definition{ - Name: "fake", - Command: "fake-lsp", - Args: []string{log, "", "0", "0", "", "1"}, - } + home, pidfile, log, def := newPIDRecordingFakeServer(t, "0") m := NewManager(t.TempDir(), home) t.Cleanup(func() { m.ShutdownAll() }) @@ -350,33 +302,11 @@ func TestStartServerCloseRaceProvisionalMappingHoldsTerminalAdmission(t *testing // On the old branch the instance was mapped only after start completed, so // ShutdownAll's snapshot missed it and the process survived the call. func TestStartServerCloseRaceMappedInstanceIsTornDown(t *testing.T) { - home := t.TempDir() requestsLog := filepath.Join(t.TempDir(), "requests.log") t.Setenv("FAKE_LSP_REQUESTS_LOG", requestsLog) - cacheDir := filepath.Join(home, ".cache", "lightcode", "lsp", "fake") - if err := os.MkdirAll(cacheDir, 0o755); err != nil { - t.Fatal(err) - } - real := filepath.Join(cacheDir, "fake-lsp.py") - if err := os.WriteFile(real, []byte(fakeServerScript), 0o755); err != nil { - t.Fatal(err) - } - wrapper := filepath.Join(cacheDir, "fake-lsp") - wrapperScript := "#!/bin/sh\necho $$ > \"${FAKE_PIDFILE:?}\"\nexec python3 \"$(dirname \"$0\")/fake-lsp.py\" \"$@\"\n" - if err := os.WriteFile(wrapper, []byte(wrapperScript), 0o755); err != nil { - t.Fatal(err) - } - pidfile := filepath.Join(t.TempDir(), "lsp.pid") - t.Setenv("FAKE_PIDFILE", pidfile) - // init_delay holds the start in the initialize call after the process has // launched and the instance has been provisionally mapped. - log := filepath.Join(t.TempDir(), "launches.log") - def := &server.Definition{ - Name: "fake", - Command: "fake-lsp", - Args: []string{log, "", "2", "0", "", "1"}, - } + home, pidfile, _, def := newPIDRecordingFakeServer(t, "2") m := NewManager(t.TempDir(), home) started := make(chan struct{}) diff --git a/internal/tool/apply_patch_match_test.go b/internal/tool/apply_patch_match_test.go index 540d5137..a7e5ab62 100644 --- a/internal/tool/apply_patch_match_test.go +++ b/internal/tool/apply_patch_match_test.go @@ -5,157 +5,199 @@ import ( "testing" ) -func TestLocateExactMatch(t *testing.T) { - lines := []string{"foo", "bar", "baz"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "bar"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 1 { - t.Fatalf("start = %d, want 1", start) - } -} - -func TestLocateForwardScan(t *testing.T) { - lines := []string{"foo", "bar", "foo", "bar"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}, {Kind: lineContext, Text: "bar"}}} - - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0 (first hit)", start) - } - - start, err = locate(lines, h, "p", 2) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 2 { - t.Fatalf("start = %d, want 2 (hit at/after cursor=2)", start) - } -} - -func TestLocateLevel2TrailingWhitespace(t *testing.T) { - lines := []string{"foo ", "bar", "baz"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateLevel3LeadingAndTrailingWhitespace(t *testing.T) { - lines := []string{" foo ", "bar", "baz"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateLevel4UnicodeDashes(t *testing.T) { - lines := []string{"foo\u2014bar", "baz"} // em-dash - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo-bar"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateLevel4UnicodeQuotes(t *testing.T) { - lines := []string{"say \u201Chello\u201D", "baz"} // curly double quotes - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "say \"hello\""}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateLevel4ExoticSpaces(t *testing.T) { - // non-breaking space (U+00A0) on each side - lines := []string{"\u00A0foo\u00A0", "bar"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateLowerLevelBeatsHigher(t *testing.T) { - // Level 1 matches at position 0 ("foo"); level-2 fuzzy match at position 2. - // First hit wins → start = 0. - lines := []string{"foo", "bar", "foo "} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0 (first exact match wins, not the trailing-ws match)", start) - } -} - -func TestLocateAnchorAdvancesCursor(t *testing.T) { - lines := []string{"foo", "anchor", "bar", "baz"} - h := hunk{ - anchor: "anchor", - lines: []hunkLine{{Kind: lineContext, Text: "bar"}}, - } - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 2 { - t.Fatalf("start = %d, want 2 (cursor advanced past anchor at index 1)", start) - } -} - -func TestLocateAnchorDoesNotNarrowRegion(t *testing.T) { - // Pattern appears many lines after the anchor; should still match. - lines := []string{"anchor", "skip1", "skip2", "skip3", "match"} - h := hunk{ - anchor: "anchor", - lines: []hunkLine{{Kind: lineContext, Text: "match"}}, - } - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 4 { - t.Fatalf("start = %d, want 4", start) - } -} - -func TestLocateAnchorFoundViaFuzzyLadder(t *testing.T) { - lines := []string{"anchor ", "match"} - h := hunk{ - anchor: "anchor", - lines: []hunkLine{{Kind: lineContext, Text: "match"}}, - } - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 1 { - t.Fatalf("start = %d, want 1", start) +func TestLocateScenarios(t *testing.T) { + type probe struct { + cursor int + start int + wantErr bool + errPrefix string + } + cases := []struct { + name string + lines []string + h hunk + probes []probe + }{ + { + name: "TestLocateExactMatch", + lines: []string{"foo", "bar", "baz"}, + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "bar"}}}, + probes: []probe{ + {cursor: 0, start: 1}, + }, + }, + { + name: "TestLocateForwardScan", + lines: []string{"foo", "bar", "foo", "bar"}, + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}, {Kind: lineContext, Text: "bar"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + {cursor: 2, start: 2}, + }, + }, + { + name: "TestLocateLevel2TrailingWhitespace", + lines: []string{"foo ", "bar", "baz"}, + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateLevel3LeadingAndTrailingWhitespace", + lines: []string{" foo ", "bar", "baz"}, + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateLevel4UnicodeDashes", + lines: []string{"foo\u2014bar", "baz"}, // em-dash + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo-bar"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateLevel4UnicodeQuotes", + lines: []string{"say \u201Chello\u201D", "baz"}, // curly double quotes + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "say \"hello\""}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateLevel4ExoticSpaces", + lines: []string{"\u00A0foo\u00A0", "bar"}, // non-breaking space (U+00A0) on each side + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateLowerLevelBeatsHigher", + lines: []string{"foo", "bar", "foo "}, // Level 1 matches at 0; level-2 fuzzy match at 2. + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateAnchorAdvancesCursor", + lines: []string{"foo", "anchor", "bar", "baz"}, + h: hunk{ + anchor: "anchor", + lines: []hunkLine{{Kind: lineContext, Text: "bar"}}, + }, + probes: []probe{ + {cursor: 0, start: 2}, + }, + }, + { + name: "TestLocateAnchorDoesNotNarrowRegion", + lines: []string{"anchor", "skip1", "skip2", "skip3", "match"}, // Pattern appears many lines after the anchor. + h: hunk{ + anchor: "anchor", + lines: []hunkLine{{Kind: lineContext, Text: "match"}}, + }, + probes: []probe{ + {cursor: 0, start: 4}, + }, + }, + { + name: "TestLocateAnchorFoundViaFuzzyLadder", + lines: []string{"anchor ", "match"}, + h: hunk{ + anchor: "anchor", + lines: []hunkLine{{Kind: lineContext, Text: "match"}}, + }, + probes: []probe{ + {cursor: 0, start: 1}, + }, + }, + { + name: "TestLocateEOFTrailingEmptyRetryDoesNotMatchBeforeEOF", + lines: []string{"alpha", "beta", "charlie"}, + h: hunk{lines: []hunkLine{ + {Kind: lineContext, Text: "alpha"}, + {Kind: lineRemove, Text: ""}, + }}, + probes: []probe{ + {cursor: 0, wantErr: true, errPrefix: "Failed to find expected lines in p:"}, + }, + }, + { + name: "TestLocatePatternNotFound", + lines: []string{"foo", "bar"}, + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "missing"}}}, + probes: []probe{ + {cursor: 0, wantErr: true, errPrefix: "Failed to find expected lines in p:"}, + }, + }, + { + name: "TestLocateAnchorNotFound", + lines: []string{"foo", "bar"}, + h: hunk{ + anchor: "missing", + lines: []hunkLine{{Kind: lineContext, Text: "foo"}}, + }, + probes: []probe{ + {cursor: 0, wantErr: true, errPrefix: "Failed to find context 'missing' in p"}, + }, + }, + { + name: "TestLocateMixedContextAndRemove", + lines: []string{"alpha", "BEFORE", "beta"}, // File carries the lines the patch will remove. + h: hunk{lines: []hunkLine{ + {Kind: lineContext, Text: "alpha"}, + {Kind: lineRemove, Text: "BEFORE"}, + {Kind: lineAdd, Text: "REPLACEMENT"}, // not in pattern + {Kind: lineContext, Text: "beta"}, + }}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateNoContextOrRemove", + lines: []string{"foo", "bar"}, // Hunk with only add lines: no pattern to match; return the cursor. + h: hunk{lines: []hunkLine{{Kind: lineAdd, Text: "anything"}}}, + probes: []probe{ + {cursor: 0, start: 0}, + }, + }, + { + name: "TestLocateCursorClamped", + lines: []string{"foo", "bar"}, // Negative cursor clamps to 0; cursor past end clamps to len. + h: hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}}, + probes: []probe{ + {cursor: -5, start: 0}, + {cursor: 100, wantErr: true}, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, p := range tc.probes { + start, err := locate(tc.lines, tc.h, "p", p.cursor) + if p.wantErr { + if err == nil { + t.Fatalf("locate(cursor=%d) err = nil, want non-nil", p.cursor) + } + if p.errPrefix != "" && !strings.HasPrefix(err.Error(), p.errPrefix) { + t.Fatalf("locate(cursor=%d) err = %q, want prefix %q", p.cursor, err.Error(), p.errPrefix) + } + continue + } + if err != nil { + t.Fatalf("locate(cursor=%d) err = %v", p.cursor, err) + } + if start != p.start { + t.Fatalf("locate(cursor=%d) start = %d, want %d", p.cursor, start, p.start) + } + } + }) } } @@ -185,21 +227,6 @@ func TestLocateEOFTrailingEmptyRetry(t *testing.T) { } } -func TestLocateEOFTrailingEmptyRetryDoesNotMatchBeforeEOF(t *testing.T) { - lines := []string{"alpha", "beta", "charlie"} - h := hunk{lines: []hunkLine{ - {Kind: lineContext, Text: "alpha"}, - {Kind: lineRemove, Text: ""}, - }} - _, err := locate(lines, h, "p", 0) - if err == nil { - t.Fatal("locate err = nil, want non-EOF trailing-empty retry to fail") - } - if !strings.HasPrefix(err.Error(), "Failed to find expected lines in p:") { - t.Fatalf("err = %q, want expected-lines failure", err.Error()) - } -} - func TestLocateNoEOFTrailingEmptyWhenFileHasIt(t *testing.T) { // File has a trailing newline; pattern's last context line is empty; // the empty matches lines[2]="". The full pattern matches without retry. @@ -224,81 +251,3 @@ func TestLocateNoEOFTrailingEmptyWhenFileHasIt(t *testing.T) { t.Fatalf("locateHunk = %+v, want start 0 len 3 no synthetic line", match) } } - -func TestLocatePatternNotFound(t *testing.T) { - lines := []string{"foo", "bar"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "missing"}}} - _, err := locate(lines, h, "p", 0) - if err == nil { - t.Fatal("locate err = nil, want non-nil") - } - if !strings.HasPrefix(err.Error(), "Failed to find expected lines in p:") { - t.Fatalf("err = %q, want prefix %q", err.Error(), "Failed to find expected lines in p:") - } -} - -func TestLocateAnchorNotFound(t *testing.T) { - lines := []string{"foo", "bar"} - h := hunk{ - anchor: "missing", - lines: []hunkLine{{Kind: lineContext, Text: "foo"}}, - } - _, err := locate(lines, h, "p", 0) - if err == nil { - t.Fatal("locate err = nil, want non-nil") - } - if !strings.HasPrefix(err.Error(), "Failed to find context 'missing' in p") { - t.Fatalf("err = %q, want prefix %q", err.Error(), "Failed to find context 'missing' in p") - } -} - -func TestLocateMixedContextAndRemove(t *testing.T) { - // Pattern is built from context + remove lines only (add lines excluded). - // The file's current state carries the lines the patch will remove. - lines := []string{"alpha", "BEFORE", "beta"} - h := hunk{lines: []hunkLine{ - {Kind: lineContext, Text: "alpha"}, - {Kind: lineRemove, Text: "BEFORE"}, - {Kind: lineAdd, Text: "REPLACEMENT"}, // not in pattern - {Kind: lineContext, Text: "beta"}, - }} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateNoContextOrRemove(t *testing.T) { - // Hunk with only add lines: no pattern to match; return the cursor. - lines := []string{"foo", "bar"} - h := hunk{lines: []hunkLine{{Kind: lineAdd, Text: "anything"}}} - start, err := locate(lines, h, "p", 0) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0", start) - } -} - -func TestLocateCursorClamped(t *testing.T) { - // Negative cursor clamps to 0; cursor past end clamps to len. - lines := []string{"foo", "bar"} - h := hunk{lines: []hunkLine{{Kind: lineContext, Text: "foo"}}} - - start, err := locate(lines, h, "p", -5) - if err != nil { - t.Fatalf("locate err = %v", err) - } - if start != 0 { - t.Fatalf("start = %d, want 0 (cursor clamped)", start) - } - - _, err = locate(lines, h, "p", 100) - if err == nil { - t.Fatal("locate err = nil, want non-nil (pattern cannot be located past end)") - } -} diff --git a/internal/tool/apply_patch_parse_test.go b/internal/tool/apply_patch_parse_test.go index 6f3c8765..ceeb8d00 100644 --- a/internal/tool/apply_patch_parse_test.go +++ b/internal/tool/apply_patch_parse_test.go @@ -263,78 +263,73 @@ func TestParsePatchPreservesCRInUpdateHunk(t *testing.T) { } } -func TestParsePatchDuplicatePath(t *testing.T) { - input := "*** Begin Patch\n*** Add File: a\n+x\n*** Add File: a\n+y\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchDupPath) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchDupPath) - } - if !strings.Contains(err.Error(), "a") { - t.Fatalf("err = %q, want it to mention the duplicated path", err) - } -} - -func TestParsePatchMoveDestDuplicatesExistingPath(t *testing.T) { - input := "*** Begin Patch\n*** Add File: b\n+z\n*** Update File: a\n*** Move to: b\n@@\n-x\n+y\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchDupPath) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchDupPath) - } -} - -func TestParsePatchAddFileBadBodyLine(t *testing.T) { - input := "*** Begin Patch\n*** Add File: a\nbad\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchAddBody) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchAddBody) - } -} - -func TestParsePatchAddFileEmptyBody(t *testing.T) { - input := "*** Begin Patch\n*** Add File: a\n*** End Patch" - _, err := parsePatch(input) - if err == nil || !strings.Contains(err.Error(), "no content") { - t.Fatalf("parsePatch err = %v, want a no-content error", err) - } -} - -func TestParsePatchUpdateNoHunks(t *testing.T) { - input := "*** Begin Patch\n*** Update File: a\n*** End Patch" - _, err := parsePatch(input) - if err == nil || !strings.Contains(err.Error(), "no hunks") { - t.Fatalf("parsePatch err = %v, want a no-hunks error", err) - } -} - -func TestParsePatchHunkBadLine(t *testing.T) { - input := "*** Begin Patch\n*** Update File: a\n@@\nctx\nbad\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchHunkLine) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchHunkLine) - } -} - -func TestParsePatchHunkEmptyLine(t *testing.T) { - input := "*** Begin Patch\n*** Update File: a\n@@\nctx\n\nmore\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchHunkLine) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchHunkLine) - } -} - -func TestParsePatchMissingHeaderBeforeHunk(t *testing.T) { - input := "*** Begin Patch\n@@\nctx\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchHunkExpected) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchHunkExpected) +func TestParsePatchRefusals(t *testing.T) { + cases := []struct { + name string + input string + sentinel error + contains string + }{ + { + name: "TestParsePatchDuplicatePath", + input: "*** Begin Patch\n*** Add File: a\n+x\n*** Add File: a\n+y\n*** End Patch", + sentinel: errApplyPatchDupPath, + contains: "a", + }, + { + name: "TestParsePatchMoveDestDuplicatesExistingPath", + input: "*** Begin Patch\n*** Add File: b\n+z\n*** Update File: a\n*** Move to: b\n@@\n-x\n+y\n*** End Patch", + sentinel: errApplyPatchDupPath, + }, + { + name: "TestParsePatchAddFileBadBodyLine", + input: "*** Begin Patch\n*** Add File: a\nbad\n*** End Patch", + sentinel: errApplyPatchAddBody, + }, + { + name: "TestParsePatchAddFileEmptyBody", + input: "*** Begin Patch\n*** Add File: a\n*** End Patch", + contains: "no content", + }, + { + name: "TestParsePatchUpdateNoHunks", + input: "*** Begin Patch\n*** Update File: a\n*** End Patch", + contains: "no hunks", + }, + { + name: "TestParsePatchHunkBadLine", + input: "*** Begin Patch\n*** Update File: a\n@@\nctx\nbad\n*** End Patch", + sentinel: errApplyPatchHunkLine, + }, + { + name: "TestParsePatchHunkEmptyLine", + input: "*** Begin Patch\n*** Update File: a\n@@\nctx\n\nmore\n*** End Patch", + sentinel: errApplyPatchHunkLine, + }, + { + name: "TestParsePatchMissingHeaderBeforeHunk", + input: "*** Begin Patch\n@@\nctx\n*** End Patch", + sentinel: errApplyPatchHunkExpected, + }, + { + name: "TestParsePatchUnknownHeader", + input: "*** Begin Patch\n*** Rename File: a\n*** End Patch", + sentinel: errApplyPatchUnknownHead, + }, } -} - -func TestParsePatchUnknownHeader(t *testing.T) { - input := "*** Begin Patch\n*** Rename File: a\n*** End Patch" - _, err := parsePatch(input) - if !errors.Is(err, errApplyPatchUnknownHead) { - t.Fatalf("parsePatch err = %v, want %v", err, errApplyPatchUnknownHead) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := parsePatch(tc.input) + if err == nil { + t.Fatal("parsePatch err = nil, want non-nil") + } + if tc.sentinel != nil && !errors.Is(err, tc.sentinel) { + t.Fatalf("parsePatch err = %v, want %v", err, tc.sentinel) + } + if tc.contains != "" && !strings.Contains(err.Error(), tc.contains) { + t.Fatalf("parsePatch err = %q, want it to contain %q", err.Error(), tc.contains) + } + }) } }