test: 五条把 main 打红的 Windows 计时型用例改成断言行为 - #1616
Conversation
`握手超时(open 后 hello-ack 一直不来)→ 强制断开走退避重连` 用固定 `await tick(50)` 等 15ms 握手看门狗 + 退避重连落地。Windows CI 两分片并跑时 事件循环调度远超名义毫秒数,50ms 内 watchdog 可能还没换连接,断言 `sockets.length >= 2` 拿到 1 → main HEAD (e931a57) 的 client-ci 红灯。 改成同文件已有的有界等待模式(与紧邻的「open 从未到来」用例一致): `for (let i = 0; i < 40 && h.sockets.length < 2; i++) await tick(10)`。 断言语义不变,只把"固定窗口"换成"等到发生或超时",上界 400ms。 纯测试改动,不动产品代码。 验证: `pnpm --filter @cindy/device-link test` 163 passed。 Signed-off-by: Chris <tkdv42k4mg@privaterelay.appleid.com> Signed-off-by: Chris <4436110+zqchris@users.noreply.github.com>
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/skillhub/registry/tests/lock.test.ts | 将不同 key 并行性的墙钟耗时阈值改为直接检查三个临界区存在重叠。 |
| packages/device-link/src/tests/client.test.ts | 将心跳、令牌超时和握手重连相关测试的固定等待改为有界状态轮询。 |
| packages/maker-core/src/contacts/tests/manager.test.ts | 仅在 Windows 上将该文件的 Vitest 用例超时放宽到 30 秒,其他平台保持 5 秒。 |
Reviews (4): Last reviewed commit: "test(device-link): 心跳僵死用例改有界等待(第五条打红 mai..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
本 PR 针对 @cindy/device-link 的单元测试用例做稳定性修复:将“握手超时(open 后 hello-ack 一直不来)→ 强制断开走退避重连”的断言等待方式从固定时长等待改为有界等待,以避免 Windows CI 负载/并跑下事件循环调度抖动导致的偶发失败。
Changes:
- 将原先固定
await tick(50)的等待方式改为最多 400ms 的有界轮询等待(直到第二个 socket 出现或超时结束)。 - 补充更明确的注释说明该用例在 Windows CI 上的调度抖动原因与断言语义保持不变。
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`withLock > 不同 key 并行,耗时近似单次而非 N 倍` 断言 `elapsed < delay * 2.5`(20ms × 2.5 = 50ms)。三个 20ms sleep 串行是 60ms —— 阈值 离"串行"只差 10ms,等于拿调度抖动当被测行为。Windows CI 两分片并跑时实测 52ms, 直接把 main 打红(makecindy#1618 的 client-ci 就是被这条挡下的,与它的 diff 毫无关系)。 改成直接量"并行"本身:记每个临界区的进入 / 离开时刻,断言 `max(进入) < min(离开)` —— 存在一个瞬间三者同时在临界区内。串行执行下第二个的进入 必然晚于第一个的离开,这条一定不成立;而机器多慢都不影响它成立。 被测语义没变(不同 key 不互相串行),只是把判据从"跑得够快"换成"确实重叠"。 验证: `vitest run src/main/skillhub/registry/__tests__/lock.test.ts` 5 passed。 Signed-off-by: Chris <tkdv42k4mg@privaterelay.appleid.com> Signed-off-by: Chris <4436110+zqchris@users.noreply.github.com>
同一根因的第三、四条(前两条见本 PR 前两个 commit)。 ① `packages/device-link` client.test.ts —— `getToken 挂起超过 getTokenTimeoutMs → 走 退避重连,不永久卡在 connecting` 用固定 `await tick(30)` 赌「10ms getToken 超时 + ≤5ms 退避 + 第二轮 getToken 都能在 30ms 内跑完」。Windows 两分片并跑时 socket 还没建出来 → `expected 1, received 0` (实测 makecindy#1616 的 client-ci)。改成与同文件其它用例一致的有界等待,断言语义不变。 ② `packages/maker-core` contacts/manager.test.ts —— `Test timed out in 5000ms` 这条**不是**墙钟断言,是纯正确性用例被默认超时判死。该文件每个用例都在 os.tmpdir() 里真开 better-sqlite3 落库(建目录 → 建表 → v1→v2 迁移 → FTS5 重建 → 删目录), Windows CI 两分片并跑、叠上 Defender 对新建文件的实时扫描,这些**同步** IO 会超过 vitest 默认的 5s。实测 makecindy#1618 的 client-ci 就是被它挡下的(makecindy#1618 的 diff 只碰 mobile HTML 预览,与 contacts 毫无关系);此前值班日志也记过同一文件 4 个用例同时 5s 超时。 packages/maker-core 没有自己的 vitest 配置,拿不到 apps/desktop 那份 `testTimeout: win32 ? 20_000 : 5_000`,所以按仓内既有写法(git-integration 系列用例的 `vi.setConfig`)在该文件单独放宽到 win32 30s。**只放宽时间,不放宽任何断言** —— 它测的是迁移正确性,从来不是"迁移够快"。 验证: - `pnpm --filter @cindy/device-link exec vitest run src/__tests__/client.test.ts` → 100 passed - `pnpm --filter @cindy/maker-core exec vitest run src/contacts` → 122 passed(6 文件) - `pnpm --filter desktop exec vitest run src/main/skillhub` → 281 passed(24 文件) - @cindy/device-link / @cindy/maker-core / desktop 三个 package 的 typecheck 均通过 Signed-off-by: Chris <tkdv42k4mg@privaterelay.appleid.com> Signed-off-by: Chris <4436110+zqchris@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/main/skillhub/registry/tests/lock.test.ts:55
- 这里用于判定“并行重叠”的时间戳仍然取自
Date.now()(墙钟时间)。在 CI 上若发生 NTP 校时回拨/跳变,可能导致进入/离开顺序断言偶发失败;建议改用单调时钟performance.now()来彻底摆脱墙钟干扰。
const section = async (): Promise<void> => {
enter.push(Date.now());
await new Promise<void>((r) => setTimeout(r, 20));
leave.push(Date.now());
};
`心跳:连续无 pong 超限 → terminate + 重连` 用固定 `await tick(40)` 赌「8ms ping 周期 × 2 轮都能在 40ms 内跑完」。刚合并的 main(0e1e791)的 client-ci 就是被它挡下的: `expected false to be true`(terminate 还没发生)。改成有界等待到 terminate 真的发生。 同时对整个 client.test.ts 做了一次清扫,结论记在 issue 里(见 PR 描述):全文件共 35 处 `await tick(>=10)`,其中约 15 处属本类(固定等待后断言"某事已发生",慢机器会假失败), 另有约 15 处是反向形态(等一段时间断言"某事没发生"),后者不能用轮询修 —— 它要求真实经过 的时间**小于**某个阈值,慢机器上等越久越危险,只能调被测时间参数,属另一件事。 本 PR 只收正在发作的那几条,不把整份清扫塞进来。 验证: `pnpm --filter @cindy/device-link test` → 163 passed(6 文件)。 Signed-off-by: Chris <tkdv42k4mg@privaterelay.appleid.com> Signed-off-by: Chris <4436110+zqchris@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/maker-core/src/contacts/tests/manager.test.ts:19
- 这里的 vi.setConfig 同时把非 Windows 平台也显式设回 5s。既然注释里说明问题只发生在 win32(默认 5s 会在 Windows CI 下被同步 IO 撑爆),建议只在 win32 分支覆盖 testTimeout,避免未来全局/包级 vitest 配置调整时被这行固定住。
vi.setConfig({ testTimeout: process.platform === 'win32' ? 30_000 : 5_000 });
apps/desktop/src/main/skillhub/registry/tests/lock.test.ts:55
- 这个用例的目标是避免“墙钟耗时阈值”导致的抖动,但当前仍用 Date.now() 记录进入/离开时刻;Date.now() 受系统时间校准影响且并不保证单调递增,理论上仍可能引入不必要的抖动。这里可以完全不依赖时间,改为记录“当前同时处于临界区内的任务数”并断言峰值为 3,更直接也更稳。
const section = async (): Promise<void> => {
enter.push(Date.now());
await new Promise<void>((r) => setTimeout(r, 20));
leave.push(Date.now());
};
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04e174f248
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MagicLizi
left a comment
There was a problem hiding this comment.
代码审查通过。把四条 Windows CI 计时型用例从墙钟断言改为行为断言,思路正确:lock 并行测试改为检查三个临界区时间段重叠、device-link 和 contacts 测试改为有界等待条件成立。不改变任何被测逻辑的断言语义,只消除调度抖动导致的假阳。无 P0/P1 问题。
|
根治了 Windows CI 计时用例的调度抖动假阳——改为断言行为而非墙钟,思路干净。 |
* main: (39 commits) fix(desktop): preserve legacy plugin permission approvals (makecindy#1657) fix(im): 控制命令收归主人专属,群成员的 !stop / slash 不再生效 (makecindy#1658) fix(pi): 第一方 MCP 工具走 host 审批策略,与 Claude 的 auto-review 判定对齐 (makecindy#1639) test(desktop): validate Python probe execution (makecindy#1653) fix(plugin): 支持真实包权限变化后重新确认 (makecindy#1648) fix(desktop): sync pinned sessions to mobile (makecindy#1492) test: 五条把 main 打红的 Windows 计时型用例改成断言行为 (makecindy#1616) fix: 补齐任务列表导出菜单 (makecindy#1620) feat(mobile): HTML 渲染态透传同目录资源,多文件产物不再缺图缺样式(重开 makecindy#1455) (makecindy#1618) fix(device-link): 暴露待命状态并诊断不稳定连接 (makecindy#1611) fix(desktop): 修复 Claude Opus 套餐错误提示 (makecindy#1607) feat(hook-control): 官方 Telegram 卡消除配置重叠,并支持默认工作目录 (makecindy#1595) feat(pi): PI 子代理(只读画像),与 Claude / Codex 共用同一张子代理卡 (makecindy#1458) fix(desktop): 让原生分类器故障观察器的漏检在日志里可见 (makecindy#1588) fix: 把「审阅器不可用」与「模型判定危险」拆开,前者提示一次 (makecindy#1597) test(maker-core): 钉住 MCP fail-closed 闸绑定 resolver 而非界面 (makecindy#1587) feat(mobile): HTML 生成物在手机端进渲染态(WebView 离线沙箱) (makecindy#1441) feat(desktop): 统一 @ 资源引用与搜索 (makecindy#1557) feat(codex): 子代理卡补齐实时状态,与 Claude 子代理卡形态统一 (makecindy#1438) test(maker-core): 锁定 auto-review 送审用目录模型 id 而非 wire 串 (makecindy#1582) ...
改动内容
五条用时间当判据的用例改成断言被测行为本身。3 个测试文件,
+41 −13,纯测试改动,不动任何产品代码,不放宽任何断言语义。
device-link握手超时 → 退避重连tick(50)后必须已有 2 个 socketdevice-linkgetToken 挂起 → 退避重连tick(30)后必须已有 1 个 socketdevice-link心跳僵死 → terminate + 重连tick(40)后必须已 terminatedesktopskillhub 不同 key 并行elapsed < 50msmaker-corecontacts v1→v2 迁移为什么
**这五条各自把 main 或别人的 PR 打红过,没有一条是产品代码的问题。**一天之内实测 5 次发作:
前四条里的 skillhub 那条最典型:断言
elapsed < 20 × 2.5 = 50ms,而三个 20ms sleep串行是 60ms —— 阈值离「串行」只差 10ms,等于拿调度抖动当被测行为。改成记录每个临界区
的进入 / 离开时刻,断言
max(进入) < min(离开)(存在一个瞬间三者同时在临界区内):串行执行下第二个的进入必然晚于第一个的离开,这条一定不成立;而机器多慢都不影响它成立。
第 ④ 条性质不同,不是墙钟断言,是纯正确性用例被默认超时判死。该文件每个用例都在
os.tmpdir()里真开 better-sqlite3 落库(建目录 → 建表 → v1→v2 迁移 → FTS5 重建 → 删目录),Windows 两分片并跑叠上 Defender 实时扫描,这些同步 IO 会超过 vitest 默认的 5s。
根因是
packages/maker-core没有自己的 vitest 配置,拿不到apps/desktop/vitest.config.ts里那份
testTimeout: win32 ? 20_000 : 5_000;按仓内既有写法(git-integration 系列用例的vi.setConfig)在该文件单独放宽到 win32 30s。只放宽时间,不放宽任何断言 —— 它测的是迁移正确性,从来不是「迁移够快」。
范围:只收正在发作的,存量另开 issue
顺手对
client.test.ts做了一次全文件清扫,共 35 处await tick(>=10),分两类:—— 不能用轮询修,它要求真实经过的时间小于某阈值,慢机器上等越久越危险,只能调
被测的时间参数,需要逐个理解被测语义。
完整清单(含行号)、修法配方,以及「补门禁防新增」的几个可选做法,记在 #1625。
本 PR 不把整份清扫塞进来 —— 那会从 3 个文件涨到十几个站点、两种修法混在一起。
为什么单开一个 PR 而不是各自修
这类失败红灯落在谁的 PR 上纯看运气。把五条收在一起,而不是塞进各自撞见它的功能 PR:
后者会把 device-link / skillhub / maker-core 三拨评审面拖进一个 mobile 预览 PR
(#1618 实际撞上了两次)。
验证
pnpm --filter @cindy/device-link test→ 163 passed(6 文件)pnpm --filter @cindy/maker-core exec vitest run src/contacts→ 122 passed(6 文件)pnpm --filter desktop exec vitest run src/main/skillhub→ 281 passed(24 文件)@cindy/device-link/@cindy/maker-core/desktop三个 package typecheck 均通过风险
极低。3 个测试文件,没有触及产品代码。四条把判据从「跑得够快」换成「行为确实发生了」,
一条只放宽了单个文件在 Windows 上的超时。