docs(plans): record why CI test time is bound by module import, not scheduling - #10909
docs(plans): record why CI test time is bound by module import, not scheduling#10909yiliang114 wants to merge 5 commits into
Conversation
…cheduling
Release runs spend more time importing modules than running assertions: in
run 33713579913 the cli workspace reported 2223s of collect against 1372s of
tests, and the three shards finished within a minute of each other, so the
sharding itself is balanced and adding shards only divides a fixed unit cost.
The unit cost is the core barrel. Every `import { X } from
'@qwen-code/qwen-code-core'` evaluates roughly 612 modules regardless of how
little it uses, and about 70% of cli sources import it that way. Two
independent paths converge on the same floor: rewriting an import to the
defining module drops a file from ~11.5s to ~2.1s, and the suites that
already replace the barrel with a mock factory have always run at ~1.9s.
This plan records the measurements, the phased remediation and its projected
payoff, and the open risks — chiefly that mixing resolution forms would put
two copies of a module in the bundle, that the barrel currently resolves to
core sources rather than dist for reasons not yet established, and that the
existing barrel mocks would fail silently if their targets are not migrated
alongside the code under test.
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent baseline first: for "record why CI test time is bound by module import, not scheduling", I'd write a dated Spot checks against the code at the reviewed commit:
One non-blocking note:
No critical blockers, no convention violations. The local-machine timings are carried as ratios with the caveat that absolute seconds don't extrapolate to CI hardware — the doc says so itself where those numbers appear, which is the honest way to carry them. Testing evidenceDocs-only change; nothing user-visible to drive in a terminal — real-scenario testing:
Both 中文说明代码审查先说独立基线：要记录"CI 测试时间受限于模块导入而非调度"，我会写一篇带日期的 按被审提交抽查代码：
一条非阻塞提示：
无关键阻塞项，无规范违规。本机实测耗时以比值呈现、并注明绝对秒数不可外推到 CI 硬件 —— 文档在引用这些数字的地方也是这么写的，这是诚实的处理方式。 测试证据纯文档改动，无用户可见内容可在终端驱动 —— 真实场景测试： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — evidence independently verified against the real run logs; one non-blocking nit (§5's jsdom phase was already landed by #10890). This is what a good plan document looks like. The diagnosis is built on measurements, not vibes: the collect-vs-tests split reproduces from the actual release-run logs, the root cause is isolated by two independent paths (deep imports vs barrel-mocked suites) converging on the same ~2s floor, and the doc is unusually honest about what it doesn't know — §6.2's barrel-resolution question is flagged as must-answer-before-acting rather than buried. The migration risks it leads with (§6.1 module identity in a bundled CLI, §6.3 Scope stayed minimal: one file, no code, no drive-by edits. In six months the confidence table and the "ratios yes, absolute seconds no" caveats are what will keep this document trustworthy instead of misleading. The one reservation, named in the review: #10890 landed the jsdom opt-in this morning, so phase ③ in the §5 table is no longer open work. That doesn't touch the core thesis — barrel evaluation still dominates, and #10890 did nothing to it — and a one-line note or follow-up settles it. Approving, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 证据已与真实 run 日志独立核对一致；仅一条非阻塞提示（§5 的 jsdom 阶段已由 #10890 落地）。 这是一篇高质量的计划文档。诊断建立在实测而非感觉上：collect/tests 比例可从真实 release run 日志复现；根因由两条独立路径（深路径导入与 barrel mock 套件）收敛到同一 ~2s 下界来锁定；文档对未知项也异常诚实 —— §6.2 的 barrel 解析问题被标为"动手前必须查清"而不是藏起来。它优先列出的迁移风险（§6.1 单文件 bundle 下的模块同一性、§6.3 范围保持最小：单文件、无代码、无顺手改动。六个月后，可信度表和"比值可用、绝对秒数不可外推"的标注会让这份文档保持可信而非误导。 唯一保留意见已在审查中点名：#10890 今天上午落地了 jsdom 按需，§5 表中的阶段 ③ 不再是待办。这不动摇核心结论 —— barrel 求值仍是大头，#10890 未触及 —— 一行说明或一次跟进即可解决。 批准，固定在被审提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
The open question about which files the package root resolves to is answered: cli's own tsconfig carries a paths block mapping both the package root and a wildcard subpath onto core's TypeScript sources, and esbuild honours it, so neither the build nor the test run ever consults the package's exports map. Both chains land on core sources and never on the build output. That fixes the spelling a migration has to use — the package specifier plus a path under core's source tree — and explains the duplicate module the earlier experiment produced: a specifier naming the build output misses the wildcard rule, falls back to node resolution, and lands on a second copy of a module the package root had already pulled in from source. It also surfaces a prerequisite that has to come first: the test runner mirrors those path mappings by hand and currently lists only four named subpaths, so deep imports do not resolve there at all until a wildcard entry is added.
|
📄 Docs-only change — the automatic review ran at
Reviewed head: 中文说明📄 纯文档变更 —— 自动评审以 |
|
Cross-linking the implementation: #10917 is open and it is the step this document names. §6.1 says the wildcard alias is the prerequisite — "vitest alias：目前只有 4 个具名 subpath …… 没有通配。深路径导入今天在测试里会解析失败，必须先补一条通配 alias，这是 phase ① 的第一步" — and that is exactly what #10917 does, plus two migrated files as an end-to-end proof. It resolves An independent datapoint for §1, from a PR run rather than a release run. This document's
Worth recording alongside it, because it bounds how much of this is contention rather than per-file cost — the DFSAMPLE lines from that job (the 128-core host, load ~290, 114–187 concurrent vitest processes, with ~160 GB memory free and disk at 41%. So it is CPU oversubscription, not resource exhaustion — and One number to reconcile: §6.3 says 135 cli test files carry 中文说明互相链接一下实现侧：#10917 已经开着，而它做的正是本文档指定的那一步。 §6.1 写明通配 alias 是前置 —— 「vitest alias：目前只有 4 个具名 subpath …… 没有通配。深路径导入今天在测试里会解析失败，必须先补一条通配 alias，这是 phase ① 的第一步」—— #10917 做的就是这件事，另外带了两个迁移文件做端到端验证。它把 给 §1 补一个独立数据点，来自 PR 运行而非 release 运行。 本文档的 那里 值得并列记录的是，这能界定其中有多少是争抢、多少是单文件成本 —— 该 job 的 DFSAMPLE 行（ 128 核主机、load 约 290、并发 114–187 个 vitest 进程，同时还有约 160 GB 内存空闲、磁盘 41%。所以这是 CPU 超订而非资源耗尽 —— 而被饿死的正是 有一个数字需要对齐： §6.3 写的是 135 个 cli 测试文件带 |
…st batch taught The impact section was measured on a checkout three weeks behind main and undercounted the surface: 453 cli modules import the package root for a value, not 380, and 138 tests mock the package, not 113. The gap over three weeks matches the growth rate the document already cites. It also treated every barrel mock as an equal obstacle. They are not: a mock that replaces the package outright never evaluates it, so the code under test breaks the moment it imports a module directly, while a mock that spreads the real package and overrides a few names only matters when an overridden name is one the file imports. Separating the two is what makes 142 modules movable today without touching a single test, and it changes the batching advice from "by directory" to "by mock topology" — files in one directory have nothing in common where this is concerned. Two things the first batch cost us are written down so the next one does not repeat them: a symbol map built from re-export chains can name the module that re-exports a symbol rather than the one declaring it, which needs its own verification pass; and Prettier invoked against paths outside its working directory skips them while still reporting success. The worry about code splitting is resolved rather than left open — the helper that anchors sibling asset lookups already normalizes both possible chunk locations, so import granularity cannot move a module anywhere that matters.
CI triggers on pull requests against main and release branches only, so a PR based on another feature branch picks up the TUI gates and the bot jobs and nothing else — no unit suite, no lint. Splitting this migration into reviewable layers walked straight into that: two batches totalling well over two hundred files sat green on seven or eight passing checks without a single test having run, which is precisely the validation the whole approach leans on. The section records what to do instead — point the top of the stack at main so the cumulative tree gets a real run — and the detail that changing a base does not restart anything, since GitHub reports that as an edit rather than a push.
2 participants
What this PR does
Adds a plan document recording where release-run test time actually goes, what causes it, and a phased way out with its projected payoff and open risks. Documentation only — no code, config or workflow changes.
Why it's needed
Release runs spend more time importing modules than running assertions: in run 33713579913 the cli workspace reported 2223s of collect against 1372s of tests, and core reported 546s against 251s. The three shards finished within a minute of each other, so the sharding is already balanced and adding shards only divides a fixed per-file cost.
That cost is the core barrel. Every import from the package root evaluates a closure of roughly 612 modules regardless of how little it uses, and about 70% of cli source files import it that way. Rewriting a file's import to the defining module drops it from ~11.5s to ~2.1s, and the suites that already replace the barrel with a mock factory have always run at ~1.9s — two independent paths landing on the same floor.
This matters because test files grew 87% in ten weeks. Sharding buys a constant factor that gets consumed in about fifteen weeks at that rate, which is why the last month has produced a steady stream of timeout, retry and concurrency adjustments. Lowering the per-file cost, and adding a lint rule so it cannot come back, is the part that keeps working as the suite grows.
Reviewer Test Plan
How to verify
The claims are checkable without running anything locally. The workspace timings come from the release run's job logs, which can be read with
gh api /repos/QwenLM/qwen-code/actions/jobs/<id>/logs --allow-escape-sequencesand grepped for the per-workspaceDuration (… collect …, tests …)lines. The import-cost figures were measured on a development machine with a single worker and coverage disabled, so the ratios hold but the absolute seconds should not be extrapolated to CI hardware — the document says so where those numbers appear.What is worth a reviewer's judgement is the section on risks, particularly the two that would decide how a migration is written: whether subpath imports should point at core's sources or its build output, and how the existing barrel mocks are handled. Both are recorded as unresolved rather than settled.
Evidence (Before & After)
N/A — documentation only.
Tested on
Risk & Scope
docs/plans/and changes no behaviour. The risk is that the plan is acted on before its two open questions are answered, which is why they are called out in the document rather than buried.Linked Issues
Refs #10908
中文说明
这个 PR 做了什么
新增一份计划文档，记录 release run 的测试时间到底花在哪、根因是什么，以及分阶段的解决路径、预期收益和未决风险。纯文档，不涉及任何代码、配置或 workflow 改动。
为什么需要
Release run 里导入模块的时间比跑断言还长：run 33713579913 中 cli 的 collect 是 2223s、tests 只有 1372s，core 是 546s 对 251s。三个分片耗时相差不到一分钟，说明分片本身已经是均衡的，继续加分片只能摊薄一个固定的单文件成本。
这个成本来自 core 的 barrel。任何从包根导入的语句都会把约 612 个模块的闭包完整求值一遍，无论实际只用了多少，而约 70% 的 cli 源文件就是这么导入的。把某个文件的导入改成指向定义模块，耗时从 ~11.5s 降到 ~2.1s；而套件里本来就用 mock 工厂替换 barrel 的那些文件，一直是 ~1.9s —— 两条独立路径落在同一个下界上。
这件事之所以要紧，是因为测试文件数十周涨了 87%。按这个速度，分片买来的常数因子约十五周就被吃完，这也解释了过去一个月为什么持续在调超时、重试和并发。降低单文件成本、再加一条 lint 规则防止它回来，才是随套件增长仍然有效的部分。
审查者验证计划
如何验证
文中结论不需要在本地跑任何东西就能核对。各 workspace 的耗时来自 release run 的 job 日志，用
gh api /repos/QwenLM/qwen-code/actions/jobs/<id>/logs --allow-escape-sequences取出后，grep 每个 workspace 的Duration (… collect …, tests …)行即可。导入成本的数字是在一台开发机上以单 worker、关闭 coverage 测得的，因此倍数关系成立、绝对秒数不应外推到 CI 机器 —— 文档在引用这些数字的地方都做了说明。真正需要审查者判断的是风险一节，尤其是决定迁移写法的那两条：subpath 导入应该指向 core 的源码还是构建产物，以及现有的 barrel mock 如何处理。这两条都按「未决」记录，而不是当作已有定论。
证据（前后对比）
N/A —— 纯文档。
风险与范围
docs/plans/下新增一个文件，不改变任何行为。真正的风险是在两个未决问题被回答之前就照着计划动手，所以它们被写在文档显眼处而不是藏在正文里。关联 Issue
Refs #10908