Skip to content

Commit 435d413

Browse files
authored
test(desktop): pin the journal snapshot before recovery, keep start going when the journal cannot load, and run tmux in the canary E2E (#8739)
* test(desktop): pin the journal snapshot before recovery, keep start going when the journal cannot load, and run tmux in the canary E2E * test(desktop): wait for the journal to empty, not for recovery to be called
1 parent 61ac093 commit 435d413

3 files changed

Lines changed: 66 additions & 8 deletions

File tree

‎.github/workflows/desktop-e2e.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ jobs:
129129
working-directory: apps/desktop
130130
run: bunx playwright install chromium
131131

132+
# The terminal-cancel suite drives a real tmux server; without tmux its tmux scenarios skip.
133+
- name: Install tmux
134+
run: brew install tmux
135+
132136
- name: Run Playwright _electron smoke suite
133137
working-directory: apps/desktop
134138
run: bunx playwright test

‎apps/desktop/src/main/desktop-executor/service.test.ts‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,31 @@ import { describe, expect, it, vi } from 'vitest'
66

77
vi.mock('electron', () => import('@/test/electron-mock'))
88

9+
/** How many journal loads still fail, like a read that throws. */
10+
const journalFaults = vi.hoisted(() => ({ loads: 0 }))
11+
12+
vi.mock('@/main/desktop-executor/journal', async () => {
13+
const actual = await vi.importActual<typeof import('@/main/desktop-executor/journal')>(
14+
'@/main/desktop-executor/journal'
15+
)
16+
return {
17+
...actual,
18+
createExecutorJournal: (...args: Parameters<typeof actual.createExecutorJournal>) => {
19+
const journal = actual.createExecutorJournal(...args)
20+
return {
21+
...journal,
22+
load: () => {
23+
if (journalFaults.loads <= 0) return journal.load()
24+
journalFaults.loads -= 1
25+
return Promise.reject(new Error('The disk went away.'))
26+
},
27+
}
28+
},
29+
}
30+
})
31+
932
import { net } from 'electron'
33+
import { DesktopExecutor } from '@/main/desktop-executor/executor'
1034
import { createExecutorJournal } from '@/main/desktop-executor/journal'
1135
import { createDesktopExecutorService, deviceName } from '@/main/desktop-executor/service'
1236

@@ -156,18 +180,40 @@ describe('results recovery will hand to the model', () => {
156180
executionToken: 't1',
157181
completion: { status: 'success', message: 'running', data: { status: 'running' } },
158182
})
159-
const { sim, desktopExecutor } = await service(1, userData)
183+
// Recovery returns only once Sim has the result and the journal no longer holds it, so a
184+
// snapshot taken any time after recovery started would come back empty.
185+
const recover = DesktopExecutor.prototype.recover
186+
const recovered = vi
187+
.spyOn(DesktopExecutor.prototype, 'recover')
188+
.mockImplementation(async function (this: DesktopExecutor) {
189+
await recover.call(this)
190+
await vi.waitFor(async () => expect(await createExecutorJournal(path).load()).toEqual([]))
191+
})
192+
try {
193+
const { sim, desktopExecutor } = await service(1, userData)
194+
desktopExecutor.start()
195+
await vi.waitFor(() => expect(sim.registrations).toHaveLength(1))
196+
sim.registrations[0]?.(true)
197+
await vi.waitFor(() => expect(sim.requests).toContain('POST /api/desktop/tool/complete'))
198+
await vi.waitFor(async () => expect(await createExecutorJournal(path).load()).toEqual([]))
199+
200+
expect([...(await desktopExecutor.pendingResults())]).toEqual(['handed-back'])
201+
await desktopExecutor.signOut()
202+
} finally {
203+
recovered.mockRestore()
204+
}
205+
})
206+
207+
it('counts a journal that cannot be loaded at all as holding none, and still starts', async () => {
208+
journalFaults.loads = 1
209+
const { sim, desktopExecutor } = await service()
210+
211+
expect([...(await desktopExecutor.pendingResults())]).toEqual([])
160212
desktopExecutor.start()
161213
await vi.waitFor(() => expect(sim.registrations).toHaveLength(1))
162214
sim.registrations[0]?.(true)
163215

164-
// Recovery hands the result to Sim and drops it from the journal.
165-
await vi.waitFor(async () => {
166-
expect(sim.requests).toContain('POST /api/desktop/tool/complete')
167-
expect(await createExecutorJournal(path).load()).toEqual([])
168-
})
169-
170-
expect([...(await desktopExecutor.pendingResults())]).toEqual(['handed-back'])
216+
await vi.waitFor(() => expect(sim.requests).toContain('GET /api/desktop/inbox'))
171217
await desktopExecutor.signOut()
172218
})
173219
})

‎apps/desktop/src/main/desktop-executor/service.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ export function createDesktopExecutorService(
154154
)
155155
)
156156
)
157+
// The journal reads an unreadable file as empty, so this is for a load that throws anyway:
158+
// it holds none, and the executor still starts.
159+
.catch((error: unknown) => {
160+
logger.warn('Could not read the executor journal for pending results', {
161+
error: getErrorMessage(error),
162+
})
163+
return new Set<string>()
164+
})
157165
return pendingSnapshot
158166
}
159167
/** Bumped on sign-out, so work started for the previous session cannot resume it. */

0 commit comments

Comments
 (0)