Explorar el Código

test(ci): clean up fixture workers after test timeouts

Tianyi Cui hace 2 semanas
padre
commit
59ec2d3e94

+ 2 - 2
.agents/notes/implemented/testing/2026-09-08-ci-readiness-and-completion.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/testing/2026-09-08-ci-readiness-and-completion.md
-2026-09-08-ci-readiness-and-completion.md: 3382904eccd46264a902c394f6ed6a6250fef926
-2026-09-08-ci-readiness-and-completion.zh.md: 72f524cc024efb0d373cfa4f30855d6454e1bd82
+2026-09-08-ci-readiness-and-completion.md: a28e8bb1d6d6d52d57abc47c96976fa0225c1531
+2026-09-08-ci-readiness-and-completion.zh.md: 4dbd74af71d2528e17148ef2d304113ca91bd414

+ 3 - 3
.agents/notes/implemented/testing/2026-09-08-ci-readiness-and-completion.md

@@ -10,9 +10,9 @@ The [empty master PR run](https://github.com/deepseek-harness/deepseek-harness/a
 
 ## Decision
 
-The [webhook browser test](../../../../apps/web/tests/github-ready-review.e2e.ts) arms the scaffold's turn-completion observation before delivery, awaits completion, then checks Session registration and the model request. The [feedback test](../../../../apps/web/tests/feedback-command.e2e.ts) waits for the empty composer and enabled attachment control before comparing ARIA output. Matching consecutive snapshots cannot prove that the command RPC has settled: its event stream can publish the acknowledgement first.
+The [webhook browser test](../../../../apps/web/tests/github-ready-review.e2e.ts) observes the model request caused by delivery before checking Session registration. The [feedback test](../../../../apps/web/tests/feedback-command.e2e.ts) waits for the empty composer and enabled attachment control before comparing ARIA output. Matching consecutive snapshots cannot prove that the command RPC has settled: its event stream can publish the acknowledgement first.
 
-The [desktop transaction test](../../../../apps/desktop/tests/project-manager.spec.ts) gives the worker readiness marker the active test's execution budget. Its cleanup releases and awaits the blocked worker even when an ownership assertion fails. The [PowerShell background-input test](../../../../packages/shell/pwsh-local/tests/executor.spec.ts) awaits process completion before checking the complete output, completed status, and exit code, and disposes its context in cleanup. Consuming reads remain covered by their separate streaming tests.
+The [desktop transaction test](../../../../apps/desktop/tests/project-manager.spec.ts) gives the worker readiness marker the active test's execution budget. Its independent `afterEach` releases and awaits workers before deleting private roots, including when the runner abandons a timed-out test body. The poll observes runner cancellation, and teardown reports transaction failures independently from assertion failures. The [PowerShell tests](../../../../packages/shell/pwsh-local/tests/executor.spec.ts) register each helper-created Context before plugin initialization and dispose those Contexts before deleting temporary directories. The background-input case awaits process completion before checking complete output, completed status, and exit code. Consuming reads remain covered by their separate streaming tests.
 
 The [subagent teardown decision](2026-09-07-subagent-teardown-test-budgets.md) owns lifecycle cleanup budgets. The [persistent PowerShell decision](2026-09-07-pwsh-ci-observable-completion.md) owns exact versus inferred terminal readiness; a one-shot process's completion promise has different semantics.
 
@@ -26,4 +26,4 @@ The [subagent teardown decision](2026-09-07-subagent-teardown-test-budgets.md) o
 
 ## Consequences
 
-Readiness and output assertions preserve their original content and ownership checks. Controlled worker-start and command-response delays reproduce the original failures and pass with the completion waits. The execution lane still bounds hangs; native Windows execution remains necessary to verify PowerShell and process cleanup there.
+Readiness and output assertions preserve their original content and ownership checks. Controlled desktop readiness, webhook preflight, command-response, and PowerShell output delays reproduce the original failures and pass with the completion waits. A stalled desktop-worker control still reports a test timeout while proving that teardown drains the child before removing its directory. The execution lane bounds test bodies and cleanup hooks separately; native Windows execution remains necessary to verify PowerShell and process cleanup there.

+ 3 - 3
.agents/notes/implemented/testing/2026-09-08-ci-readiness-and-completion.zh.md

@@ -10,9 +10,9 @@ Status: implemented
 
 ## 决策
 
-[Webhook 浏览器测试](../../../../apps/web/tests/github-ready-review.e2e.ts)在投递前注册 scaffold 的轮次完成观察,等待完成后再检查 Session 注册和模型请求。[反馈测试](../../../../apps/web/tests/feedback-command.e2e.ts)在比较 ARIA 输出前等待输入框清空且附件按钮启用。连续两次快照相同不能证明命令 RPC 已完成:事件流可能先发布确认消息。
+[Webhook 浏览器测试](../../../../apps/web/tests/github-ready-review.e2e.ts)观察投递触发的模型请求后再检查 Session 注册。[反馈测试](../../../../apps/web/tests/feedback-command.e2e.ts)在比较 ARIA 输出前等待输入框清空且附件按钮启用。连续两次快照相同不能证明命令 RPC 已完成:事件流可能先发布确认消息。
 
-[Desktop 事务测试](../../../../apps/desktop/tests/project-manager.spec.ts)为 worker 就绪标记使用当前测试的执行预算。即使所有权断言失败,清理也释放并等待阻塞的 worker。[PowerShell 后台输入测试](../../../../packages/shell/pwsh-local/tests/executor.spec.ts)等待进程完成后检查完整输出、完成状态与退出码,并在清理中处置其 Context。消费式读取仍由独立的流式测试覆盖。
+[Desktop 事务测试](../../../../apps/desktop/tests/project-manager.spec.ts)为 worker 就绪标记使用当前测试的执行预算。独立的 `afterEach` 在删除私有目录前释放并等待 worker,包括运行器放弃超时测试体的情况。轮询观察运行器的取消信号,teardown 独立报告事务失败,不覆盖断言失败。[PowerShell 测试](../../../../packages/shell/pwsh-local/tests/executor.spec.ts)在初始化插件前登记每个 helper 创建的 Context,并在删除临时目录前处置这些 Context。后台输入用例等待进程完成后检查完整输出、完成状态与退出码。消费式读取仍由独立的流式测试覆盖。
 
 [子 Agent 拆卸决策](2026-09-07-subagent-teardown-test-budgets.zh.md)负责生命周期清理预算。[持久 PowerShell 决策](2026-09-07-pwsh-ci-observable-completion.zh.md)负责精确与推断的终端就绪状态;一次性进程的完成 Promise 具有不同语义。
 
@@ -26,4 +26,4 @@ Status: implemented
 
 ## 后果
 
-就绪与输出断言保留原有的内容和所有权检查。受控的 worker 启动延迟与命令响应延迟可复现原始失败,并在采用完成等待后通过。执行通道仍为挂起设置上限;PowerShell 和进程清理仍需在原生 Windows 上验证。
+就绪与输出断言保留原有的内容和所有权检查。受控的 Desktop 就绪、Webhook 预检、命令响应及 PowerShell 输出延迟可复现原始失败,并在采用完成等待后通过。阻塞 Desktop worker 的控制用例仍报告测试超时,同时证明 teardown 在删除目录前等待子进程退出。执行通道分别限制测试体和清理 hook;PowerShell 和进程清理仍需在原生 Windows 上验证。

+ 27 - 13
apps/desktop/tests/project-manager.spec.ts

@@ -18,6 +18,7 @@ import type { DesktopRelease } from '../src/release.ts'
 import { archivePnpmStore } from '../src/seed-store.ts'
 
 const roots: string[] = []
+const releaseWorkers: Array<() => Promise<void>> = []
 
 function temporaryRoot(): string {
   const root = mkdtempSync(join(tmpdir(), 'dsh-desktop-test-'))
@@ -162,8 +163,13 @@ function release(version = '1.0.0'): DesktopRelease {
   }
 }
 
-afterEach(() => {
-  for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true })
+afterEach(async () => {
+  const cleanups = releaseWorkers.splice(0)
+  const directories = roots.splice(0)
+  const results = await Promise.allSettled(cleanups.map(cleanup => cleanup()))
+  for (const root of directories) rmSync(root, { recursive: true, force: true })
+  const failures: unknown[] = results.flatMap((result): unknown[] => result.status === 'rejected' ? [result.reason] : [])
+  if (failures.length > 0) throw new AggregateError(failures, 'desktop worker cleanup failed')
 })
 
 describe('desktop package policy', () => {
@@ -293,7 +299,7 @@ describe('desktop project transactions', () => {
     expect(existsSync(paths.pending)).toBe(false)
   })
 
-  it('records the live pnpm worker as transaction owner until it exits', async ({ task }) => {
+  it('records the live pnpm worker as transaction owner until it exits', async ({ task, signal }) => {
     const root = temporaryRoot()
     const seed = join(root, 'seed')
     const ready = join(root, 'pnpm-ready')
@@ -306,17 +312,25 @@ describe('desktop project transactions', () => {
     const runtime = { node: process.execPath, pnpm: writeBlockingFakePnpm(root, ready, releaseWorker) }
     const manager = new DesktopProjectManager(paths, runtime)
     const installing = manager.applyRelease(seed, '1.0.0', hooks())
-    try {
-      // Child startup shares the test's execution budget; it has no one-second latency contract.
-      await expect.poll(() => existsSync(ready), { timeout: task.timeout }).toBe(true)
-      const workerPid = Number.parseInt(readFileSync(ready, 'utf8'), 10)
-      expect(readFileSync(paths.lock, 'utf8')).toBe(`${String(workerPid)}\n`)
-      const competing = new DesktopProjectManager(paths, runtime)
-      await expect(competing.applyRelease(seed, '1.0.0', hooks())).rejects.toThrow(/another package transaction is active/u)
-    } finally {
+    // Teardown observes failures even if the runner has abandoned the test body.
+    const completed = installing.then(value => ({ value }), (error: unknown) => ({ error }))
+    releaseWorkers.push(async () => {
       writeFileSync(releaseWorker, 'continue')
-      await expect(installing).resolves.toBe(true)
-    }
+      const outcome = await completed
+      if ('error' in outcome) throw outcome.error
+    })
+    // Child startup shares the test budget; an aborted poll must not resume ownership assertions.
+    await expect.poll(() => {
+      signal.throwIfAborted()
+      return existsSync(ready)
+    }, { timeout: task.timeout }).toBe(true)
+    signal.throwIfAborted()
+    const workerPid = Number.parseInt(readFileSync(ready, 'utf8'), 10)
+    expect(readFileSync(paths.lock, 'utf8')).toBe(`${String(workerPid)}\n`)
+    const competing = new DesktopProjectManager(paths, runtime)
+    await expect(competing.applyRelease(seed, '1.0.0', hooks())).rejects.toThrow(/another package transaction is active/u)
+    writeFileSync(releaseWorker, 'continue')
+    await expect(installing).resolves.toBe(true)
     expect(existsSync(paths.lock)).toBe(false)
   })
 

+ 10 - 2
packages/shell/pwsh-local/tests/executor.spec.ts

@@ -30,8 +30,14 @@ afterAll(() => {
 
 /** Per-test temp dirs, removed after each test. */
 const tempDirs: string[] = []
-afterEach(() => {
-  for (const dir of tempDirs.splice(0)) rmSync(dir, { recursive: true, force: true })
+const contexts: Context[] = []
+afterEach(async () => {
+  const ownedContexts = contexts.splice(0)
+  const directories = tempDirs.splice(0)
+  const results = await Promise.allSettled(ownedContexts.map(ctx => ctx.fiber.dispose()))
+  for (const dir of directories) rmSync(dir, { recursive: true, force: true })
+  const failures: unknown[] = results.flatMap((result): unknown[] => result.status === 'rejected' ? [result.reason] : [])
+  if (failures.length > 0) throw new AggregateError(failures, 'PowerShell fixture cleanup failed')
 })
 
 // The probe follows the executor's own resolution (Program Files installs on
@@ -51,6 +57,7 @@ function samePath(actual: string, expected: string): boolean {
 
 async function setup(config: ConstructorParameters<typeof PwshLocalExecutor>[1] = {}) {
   const ctx = new Context()
+  contexts.push(ctx)
   await ctx.plugin(LocalSubprocessRuntime)
   ;(ctx.subprocess as LocalSubprocessRuntime).internals = { spillDir }
   // A short kill grace via the REAL config path, so escalation tests stay fast.
@@ -422,6 +429,7 @@ describe.skipIf(!hasPwsh)('PwshLocalExecutor.start (background process handles)'
     }))
     const partialOutput = await readUntil(proc, '[bg-env][bg-dsh-env]', task.timeout)
     await proc.done
+    expect(proc.status).toBe('completed')
     const output = partialOutput + lf(proc.readOutput().delta)
     expect(output).toBe('bg-stdin\n[bg-env][bg-dsh-env]\n')
     expect(proc.exitCode).toBe(0)