Skip to content

Commit a7e2059

Browse files
authored
test: remove deterministic CI waits (#6144)
* test: remove deterministic CI waits * test: stabilize workspace restoration reload
1 parent e0d9104 commit a7e2059

5 files changed

Lines changed: 270 additions & 73 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,10 @@ root `pnpm tools-pr` script without a new explicit maintainer decision.
268268

269269
## Validation strategy
270270

271+
- Before adding, repairing, or optimizing tests, follow
272+
[`docs/testing/test-efficiency.zh-CN.md`](docs/testing/test-efficiency.zh-CN.md)
273+
for completion signals, virtual-clock usage, isolation, and performance
274+
validation.
271275
- After package, workspace, or command-entry changes, run `pnpm install` so workspace links and generated dist entries stay fresh.
272276
- For agent-stream / parser changes (`apps/daemon/src/runtimes/claude-stream.ts`, `json-event-stream.ts`, `qoder-stream.ts`, etc.), replay a recorded session through the mock CLIs in `mocks/` to verify event shapes round-trip without burning provider budget. PATH-overlay activation: `export PATH="$PWD/mocks/bin:$PATH" OD_MOCKS_TRACE=<8-char-id> OD_MOCKS_NO_DELAY=1`. See `mocks/README.md` for the trace catalog and selection knobs.
273277
- Treat every `pnpm-lock.yaml` change that affects Nix packaging as requiring a Nix pnpm deps hash refresh when you maintain the flake. `nix/pnpm-deps.nix` is a generated lock artifact; use `pnpm nix:update-hash` then re-run `nix flake check --print-build-logs --keep-going` locally. Standalone `.github/workflows/nix.yml` runs flake check when nix/lock inputs change; it is **not** part of core `ci.yml` / `Validate workspace` / merge queue. Docker image smoke/publish lives only in `.github/workflows/docker-image.yml` and is likewise outside the merge gate.

apps/web/tests/components/App.amr-polling.test.tsx

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @vitest-environment jsdom
22

3-
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
3+
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
55

66
import { App } from '../../src/App';
@@ -205,6 +205,12 @@ function deferred<T>() {
205205
return { promise, resolve, reject };
206206
}
207207

208+
async function advanceTestClock(ms: number): Promise<void> {
209+
await act(async () => {
210+
await vi.advanceTimersByTimeAsync(ms);
211+
});
212+
}
213+
208214
describe('App AMR polling', () => {
209215
beforeEach(() => {
210216
mockedDaemonIsLive.mockResolvedValue(true);
@@ -255,20 +261,24 @@ describe('App AMR polling', () => {
255261

256262
afterEach(() => {
257263
cleanup();
264+
vi.useRealTimers();
258265
vi.unstubAllGlobals();
259266
vi.clearAllMocks();
260267
});
261268

262-
it('keeps polling AMR models until the remote catalog replaces the preset list', { timeout: 10_000 }, async () => {
269+
it('keeps polling AMR models until the remote catalog replaces the preset list', async () => {
270+
vi.useFakeTimers();
263271
render(<App />);
264272

265-
await waitFor(() => {
266-
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
267-
});
273+
await advanceTestClock(0);
274+
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
275+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(1);
268276

269-
await waitFor(() => {
270-
expect(screen.getByTestId('amr-model').textContent).toBe('remote-a');
271-
}, { timeout: 4_000 });
277+
await advanceTestClock(1_999);
278+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(2);
279+
await advanceTestClock(1);
280+
281+
expect(screen.getByTestId('amr-model').textContent).toBe('remote-a');
272282
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(3);
273283
});
274284

@@ -409,9 +419,8 @@ describe('App AMR polling', () => {
409419
}
410420
});
411421

412-
it('restarts AMR polling after sign-in when preset refresh previously stopped on a remote error', {
413-
timeout: 10_000,
414-
}, async () => {
422+
it('restarts AMR polling after sign-in when preset refresh previously stopped on a remote error', async () => {
423+
vi.useFakeTimers();
415424
mockedFetchAmrModels.mockReset();
416425
mockedFetchAmrModels
417426
.mockResolvedValueOnce({
@@ -433,19 +442,15 @@ describe('App AMR polling', () => {
433442

434443
render(<App />);
435444

436-
await waitFor(() => {
437-
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(1);
438-
});
439-
440-
await waitFor(() => {
441-
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(2);
442-
}, { timeout: 4_000 });
443-
await waitFor(() => {
444-
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
445-
});
445+
await advanceTestClock(0);
446+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(1);
447+
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
446448

447-
await new Promise((resolve) => setTimeout(resolve, 1_500));
449+
await advanceTestClock(1_000);
450+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(2);
451+
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
448452

453+
await advanceTestClock(1_500);
449454
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(2);
450455
mockedFetchVelaLoginStatus.mockResolvedValue({
451456
loggedIn: true,
@@ -455,15 +460,12 @@ describe('App AMR polling', () => {
455460
});
456461

457462
fireEvent.click(screen.getByText('open settings'));
458-
await waitFor(() => {
459-
expect(screen.getByText('mark amr signed in')).toBeTruthy();
460-
});
463+
expect(screen.getByText('mark amr signed in')).toBeTruthy();
461464
fireEvent.click(screen.getByText('mark amr signed in'));
465+
await advanceTestClock(0);
462466

463-
await waitFor(() => {
464-
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(3);
465-
expect(screen.getByTestId('amr-model').textContent).toBe('remote-a');
466-
}, { timeout: 4_000 });
467+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(3);
468+
expect(screen.getByTestId('amr-model').textContent).toBe('remote-a');
467469
});
468470

469471
it('does not restart AMR model polling for repeated signed-in status snapshots', async () => {
@@ -497,9 +499,8 @@ describe('App AMR polling', () => {
497499
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(2);
498500
});
499501

500-
it('stops polling after the preset retry budget is exhausted when remote never arrives', {
501-
timeout: 20_000,
502-
}, async () => {
502+
it('stops polling after the preset retry budget is exhausted when remote never arrives', async () => {
503+
vi.useFakeTimers();
503504
mockedFetchAmrModels.mockReset();
504505
mockedFetchAmrModels.mockImplementation(async () => ({
505506
source: 'preset',
@@ -509,14 +510,15 @@ describe('App AMR polling', () => {
509510

510511
render(<App />);
511512

512-
await waitFor(() => {
513-
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(11);
514-
}, { timeout: 12_000 });
515-
516-
await new Promise((resolve) => setTimeout(resolve, 1_500));
513+
await advanceTestClock(0);
514+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(1);
515+
await advanceTestClock(10_000);
517516

518517
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(11);
519518
expect(screen.getByTestId('amr-model').textContent).toBe('preset-a');
519+
520+
await advanceTestClock(1_500);
521+
expect(mockedFetchAmrModels).toHaveBeenCalledTimes(11);
520522
});
521523

522524
it('does not merge stale AMR remote models over a rescan with new agent env', async () => {

apps/web/tests/components/ProjectView.run-cleanup.test.tsx

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,18 @@ async function waitForReadyChatPaneProps() {
234234
};
235235
}
236236

237+
async function advanceTestClock(ms: number): Promise<void> {
238+
await act(async () => {
239+
await vi.advanceTimersByTimeAsync(ms);
240+
});
241+
}
242+
243+
async function settleTestClock(): Promise<void> {
244+
for (let step = 0; step < 3; step += 1) {
245+
await advanceTestClock(0);
246+
}
247+
}
248+
237249
describe('terminal replay artifact recovery', () => {
238250
it('only reuses existing artifacts created at or after the current run started', () => {
239251
const runCreatedAt = 1_000;
@@ -2357,6 +2369,7 @@ describe('ProjectView daemon cleanup', () => {
23572369
});
23582370

23592371
it('keeps reattaching after two generic disconnects while daemon status stays running, but backs off before the next retry', async () => {
2372+
vi.useFakeTimers();
23602373
const runCreatedAt = Date.now();
23612374
const genericDisconnect = await createGenericDisconnectError();
23622375

@@ -2419,16 +2432,15 @@ describe('ProjectView daemon cleanup', () => {
24192432
/>,
24202433
);
24212434

2422-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(2), {
2423-
timeout: 2_000,
2424-
});
2425-
expect(reattachDaemonRun.mock.calls.length).toBe(2);
2426-
await new Promise((resolve) => setTimeout(resolve, 500));
2427-
expect(reattachDaemonRun.mock.calls.length).toBe(2);
2428-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(3), {
2429-
timeout: 4_500,
2430-
});
2431-
}, 12_000);
2435+
await settleTestClock();
2436+
expect(reattachDaemonRun).toHaveBeenCalledTimes(2);
2437+
2438+
await advanceTestClock(2_999);
2439+
expect(reattachDaemonRun).toHaveBeenCalledTimes(2);
2440+
await advanceTestClock(1);
2441+
await settleTestClock();
2442+
expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(3);
2443+
});
24322444

24332445
it.each([
24342446
{
@@ -2609,19 +2621,19 @@ describe('ProjectView daemon cleanup', () => {
26092621
);
26102622

26112623
const sendProps = await waitForReadyChatPaneProps();
2624+
vi.useFakeTimers();
26122625
await sendProps!.onSend!('flaky stream', [], []);
26132626

2614-
await waitFor(() => expect(streamViaDaemon).toHaveBeenCalledTimes(1));
2615-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(1), {
2616-
timeout: 2_000,
2617-
});
2618-
expect(reattachDaemonRun.mock.calls.length).toBe(1);
2619-
await new Promise((resolve) => setTimeout(resolve, 500));
2620-
expect(reattachDaemonRun.mock.calls.length).toBe(1);
2621-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(2), {
2622-
timeout: 4_500,
2623-
});
2624-
}, 12_000);
2627+
await settleTestClock();
2628+
expect(streamViaDaemon).toHaveBeenCalledTimes(1);
2629+
expect(reattachDaemonRun).toHaveBeenCalledTimes(1);
2630+
2631+
await advanceTestClock(2_999);
2632+
expect(reattachDaemonRun).toHaveBeenCalledTimes(1);
2633+
await advanceTestClock(1);
2634+
await settleTestClock();
2635+
expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(2);
2636+
});
26252637

26262638
it('keeps a partial live generic disconnect recoverable after the first failure', async () => {
26272639
const runCreatedAt = Date.now();
@@ -2793,6 +2805,7 @@ describe('ProjectView daemon cleanup', () => {
27932805
});
27942806

27952807
it('keeps generic-disconnect cap retryable when the follow-up status probe returns null, but backs off before retrying', async () => {
2808+
vi.useFakeTimers();
27962809
const runCreatedAt = Date.now();
27972810
const genericDisconnect = await createGenericDisconnectError();
27982811

@@ -2873,16 +2886,15 @@ describe('ProjectView daemon cleanup', () => {
28732886
/>,
28742887
);
28752888

2876-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(2), {
2877-
timeout: 2_000,
2878-
});
2879-
expect(reattachDaemonRun.mock.calls.length).toBe(2);
2880-
await new Promise((resolve) => setTimeout(resolve, 500));
2881-
expect(reattachDaemonRun.mock.calls.length).toBe(2);
2882-
await waitFor(() => expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(3), {
2883-
timeout: 4_500,
2884-
});
2885-
}, 12_000);
2889+
await settleTestClock();
2890+
expect(reattachDaemonRun).toHaveBeenCalledTimes(2);
2891+
2892+
await advanceTestClock(2_999);
2893+
expect(reattachDaemonRun).toHaveBeenCalledTimes(2);
2894+
await advanceTestClock(1);
2895+
await settleTestClock();
2896+
expect(reattachDaemonRun.mock.calls.length).toBeGreaterThanOrEqual(3);
2897+
});
28862898

28872899
it('finalizes a reattach generic disconnect as succeeded when the next status poll turns terminal', async () => {
28882900
const runCreatedAt = Date.now();
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
# 测试效率与确定性
2+
3+
本文记录当前测试实现与评审应遵守的效率规则。目标是在不降低隔离性、
4+
覆盖强度和诊断质量的前提下,减少无意义的真实等待、重复初始化与重试。
5+
6+
## 当前策略
7+
8+
- 当前不以统一耗时分档或 guard 拒绝测试变更。
9+
- 优先修复有重复 CI 证据的异常用例,再用收敛后的数据建立耗时基线。
10+
- 测试超时是失败预算,不是测试预期需要消耗的时长。
11+
- CI retry 只用于保留诊断机会;首次失败、重试通过仍视为待修复信号。
12+
- 优化不得通过共享可变运行时、串行化用例或削弱断言来换取速度。
13+
14+
## 等待业务完成信号
15+
16+
测试必须等待后续动作真正依赖的完成信号,不能把中间 UI 状态当作持久化
17+
完成。
18+
19+
例如,上传后立即 reload、切换项目或访问深链接时:
20+
21+
1. 在触发上传前注册对应的 response waiter。
22+
2. 精确匹配请求方法和业务路径。
23+
3. 断言响应成功。
24+
4. 再验证 UI,并执行 reload 或导航。
25+
26+
```ts
27+
const uploaded = page.waitForResponse((response) => {
28+
const url = new URL(response.url());
29+
return response.request().method() === 'POST'
30+
&& url.pathname.endsWith('/upload');
31+
});
32+
33+
await input.setInputFiles(file);
34+
expect((await uploaded).ok()).toBe(true);
35+
await page.reload();
36+
```
37+
38+
仅等待标签、按钮、toast 或乐观列表项出现是不充分的,除非该状态本身就是
39+
用例的最终业务结果,且后续步骤不依赖服务端持久化。
40+
41+
## 时钟驱动逻辑
42+
43+
polling、retry、backoff、debounce、throttle、TTL 和自动关闭等逻辑默认使用
44+
虚拟时钟测试。
45+
46+
- 在定时器被创建前调用 `vi.useFakeTimers()`
47+
- 通过 React `act` 推进时钟并等待异步回调完成。
48+
- 同时断言边界两侧,例如 `2999ms` 尚未执行、再推进 `1ms` 后执行。
49+
-`afterEach` 中恢复真实时钟,避免污染同文件的其他用例。
50+
- 不用扩大 test timeout 代替虚拟时钟。
51+
52+
```ts
53+
vi.useFakeTimers();
54+
55+
await act(async () => {
56+
await vi.advanceTimersByTimeAsync(2_999);
57+
});
58+
expect(retry).not.toHaveBeenCalled();
59+
60+
await act(async () => {
61+
await vi.advanceTimersByTimeAsync(1);
62+
});
63+
expect(retry).toHaveBeenCalledTimes(1);
64+
```
65+
66+
虚拟时钟必须保持被测并发语义。若一个用例专门验证浏览器事件循环、React
67+
提交和未决 I/O 之间的交错,而 fake timer 会把这些阶段合并进同一个
68+
`act`,应保留真实时钟并在代码旁说明该等待保护的语义。
69+
70+
## 真实等待
71+
72+
不要使用 `setTimeout` 或固定 sleep 等待普通异步状态“应该已经完成”。
73+
优先选择:
74+
75+
- 等待网络 response、SSE 终止事件或进程退出;
76+
- 等待可观察的持久化数据;
77+
- 等待明确的 UI actionability 或稳定状态;
78+
- 控制并解决 deferred promise;
79+
- 推进虚拟时钟。
80+
81+
确实需要真实时间时,等待必须对应无法由测试控制的运行时边界,并使用仓库
82+
已有的超时常量。评审时应能从用例名称、断言或邻近注释看出该真实等待保护
83+
的具体行为。
84+
85+
## 隔离与生命周期
86+
87+
- 每个用例独立建立自己依赖的项目、配置、mock 和运行状态。
88+
- 不依赖同文件前序用例或同 worker 前序文件遗留的数据。
89+
- 不为了摊薄启动成本共享可变 daemon、浏览器上下文或数据目录。
90+
- 不用 serial group 隐藏竞争条件。
91+
- 只有在生命周期成本已经被证明为主要瓶颈,且隔离模型仍然明确时,才调整
92+
harness 的预热或复用策略。
93+
94+
## 优化顺序
95+
96+
处理慢测试时按以下顺序判断:
97+
98+
1. 消除固定 sleep 和真实业务计时。
99+
2. 用精确完成信号替换轮询式等待。
100+
3. 减少重复 render、启动和 fixture 构造,但保持用例隔离。
101+
4. 收紧过宽的查询、事件和断言范围。
102+
5. 确认单文件仍是关键路径后,再考虑拆文件或调整 shard。
103+
6. 只有生命周期本身占主导时,才评估 harness 级改动。
104+
105+
大文件名、用例数量或文件总耗时只能用于定位,不能单独证明拆分有收益。
106+
文件拆分如果没有减少执行工作量,通常只会改变调度形状。
107+
108+
## 验证要求
109+
110+
优化后的测试至少满足:
111+
112+
- 聚焦用例重复运行通过;
113+
- 所在测试文件完整通过;
114+
- 相关 package typecheck 通过;
115+
- `pnpm guard` 与根级 `pnpm typecheck` 通过;
116+
- UI 竞态修复在对应 merge lane 的运行模型下验证;
117+
- 记录优化前后的可比测试体耗时,区分测试体、文件和 CI job wall time。
118+
119+
任何 retry-only pass、首次失败或明显偏离基线的耗时都需要解释,不能只报告
120+
最终绿色状态。

0 commit comments

Comments
 (0)