Uh oh!
There was an error while loading. Please reload this page.
fix(test): isolate Windows background terminal tests - #310
Conversation
d071569 to
0837bffComparett-a1i
commented
Aug 31, 2026
推进结果(head Windows 实跑已完成,当前不能合并:CI run 33397613622。Node 22 / Node 24 CI 通过;Windows 原有 manager 生命周期步骤通过,但新增完整入口步骤失败并在 5 分钟上限处超时。
本地 check/test:1088 Node passed / 1 platform skip,30 Vitest passed。Standards / Spec 对本次 CI 接线未发现新增代码问题;这不替代失败的 Windows 验收。 |
tt-a1i
left a comment
There was a problem hiding this comment.
Review: changes requested — Windows acceptance remains blocked
Reviewed exact head 0837bff87ea81ffc2c97bb8e3281d4e65a156d25.
Problem / value / approach
Serializing the Windows background-terminal test group addresses contention while preserving normal parallel execution elsewhere. The partitioning approach is appropriately small; no new production-code defect was found in that change.
Merge blocker: the actual Windows full-suite entry point is not passing
Run 33397613622, Windows job passed the original manager lifecycle step, but the newly added bun run test step failed and reached its five-minute timeout. The ordinary Node group did not finish, so the serial background group and Vitest were not reached. Please do not treat the manager-only pass or Linux passes as full Windows acceptance.
The subsequent diagnosis identifies existing test-fixture problems, not regressions introduced by the contributor's grouping change:
- Confirmed native Windows failure:file-mutation-display/render.test.ts:68 expects
src/long-file.ts, while the native Windows output issrc\\long-file.ts. The success-row test fails on this assertion. - High-confidence explanation for the subsequent stall:git-info/index.test.ts creates extensionless Unix shell stubs, joins PATH with
:at line 62, and waits without a deadline for anisRepository: trueevent at line 113. Windows uses;for PATH and cannot use these fixtures as ordinary native executables. If the fake git is not executed, the real refresh can publishisRepository: false; that never resolves this promise, and cleanup is behind the await.- Three local paired fault-injection runs: the valid stub passed 3/3; removing the stub from PATH published
isRepository: falseand hit a diagnostic two-second deadline 3/3. Cleanup completed once the deadline released the await. - These were macOS fault-injection runs, not native Windows isolated-test passes. They explain a reproducible failure path consistent with the CI stall; the exact Windows hang still needs confirmation by a native rerun.
- Three local paired fault-injection runs: the valid stub passed 3/3; removing the stub from PATH published
- Static follow-up:
tests/test-discovery.test.tsalso contains slash-specific path checks. This was identified by inspection, not reported as an observed failure in that timed-out Windows run.
These relevant fixtures are unchanged from base b02ed884d8f38f4e05cb55b933eca19b59ead124.
Smallest follow-up
Use cross-platform command fixtures and path expectations, and give the expected-event wait a bounded failure with cleanup. Then rerun the native Windows full test entry point through both Node groups and Vitest. No production lifecycle rewrite, retries, or simply increasing the CI timeout is warranted by the current evidence. The follow-up can be isolated if preferred, but this PR should not be merged with its required Windows check failing.
Verification boundaries
- Exact-head local
bun run check: passed. - Exact-head local
bun run test: Node 1088 passed / 1 skipped; Vitest 30 passed. - Linux Node 22/24 CI: passed.
- Native Windows full-suite CI: failed / timed out.
- This review does not claim Windows acceptance, merge, or release.
tt-a1i
commented
Aug 31, 2026
Review 已提交:Changes requested,固定 head 0837bff。分组方案未发现新增生产代码缺陷;阻挡项是新增的 Windows 全量 CI 未通过。已把路径断言的原生失败、git-info 测试夹具导致无限等待的本地对照证据,以及仍需 Windows 复跑的限制分别写清。建议只修跨平台测试夹具、路径断言和有界等待,不扩大生产代码改动;本轮未再改代码、未合并。 |
0837bff to
44de99fCompare
Problem
Fixes#304.
scripts/run-tests.mjsdiscovers every Node test file and currently passes the complete list to onenode --testinvocation. Node runs test files concurrently by default. Thebackground-terminalssuite is different from ordinary unit tests: on Windows it creates real process trees, invokes and awaitstaskkill, observes settlement callbacks, polls descendant liveness, and cleans up OS resources.When those tests overlap unrelated Node test files, Windows scheduling and process-tree teardown become timing-sensitive. The result is intermittent failures in the same roughly 3.3-3.8 second teardown window, even though the affected tests are stable alone or when the directory is run serially.
Value
This restores a reliable meaning to the full test result on Windows:
prepublishOnlyno longer depends on retrying a flaky full suite.Approach
The runner now partitions discovered Node tests with a small platform-aware helper.
tests/extensions/background-terminalsrun in a separatenode --test --test-concurrency=1invocation.The branch has now been rebased onto
main@b02ed884(which includes the separately merged LF policy from #302). LF policy changes are not part of this PR's diff.Original author validation (before maintainer rebase)
bun run checkpasses: config contract, discipline ledger, format, lint, and typecheck.node --test --experimental-strip-types tests/scripts/node-test-groups.test.tspasses (2/2).bun run testin this Windows checkout is still blocked by pre-existing path-separator assertions (tests expecting/while Node returns\\), unrelated to this runner change.Impact
Maintainer verification — 2026-08-31
Current head:
0837bff87ea81ffc2c97bb8e3281d4e65a156d25; base:main@b02ed884d8f38f4e05cb55b933eca19b59ead124.bun run test验证步骤(5 分钟执行上限),真实经过本 PR 修改的入口;不跳过、不重试、不使用 continue-on-error。bun install --frozen-lockfile、bun run check、bun run test通过;Node 1088 passed / 1 platform skip / 0 failed;Vitest 30 passed。tests/extensions/file-mutation-display/render.test.ts:236(read fixture 正则位于第 68 行附近)只接受src/long-file.ts,Windows 实际输出src\\long-file.ts:10-29。该文件及相关生产渲染实现与 base main 完全相同,不属于本 PR 的分组改动。