ci: serialize helper test files - #10371
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — the release-note classifier helper test has been failing intermittently in required CI while passing in isolation, which is the classic signature of cross-file interference under parallel Direction: aligned — stabilizing a required CI gate is squarely in scope. The CHANGELOG check isn't applicable to a CI-only change. Size: not applicable — no core paths touched; this is 4 changed lines in Approach: this is the minimal fix. Risk: no elevated risk signals — Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:真实观测到的问题,而非理论加固——release-note classifier helper 测试在必要 CI 中间歇性失败、单独运行却能通过,这正是 方向:对齐——稳定必要 CI 门禁完全在项目范围内。CHANGELOG 检查不适用于纯 CI 改动。 规模:不适用——未触及核心路径;仅修改 方案:这是最小修复。 风险:无升级风险信号—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent proposal for "helper test files interfere when run concurrently" was exactly this: pass Nothing to flag:
No Critical findings, no convention violations. Testing evidence (CI, via API — no PR code executed)This run's evidence is the PR's own CI on the reviewed commit; no PR-derived code was built or run locally. The notable check here is that the in-progress Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 No failures so far; the ubuntu Test job (which contains the serialized helper-test step) is still running. Not verified: flake-free behavior over repeated runs — by nature a flake fix is statistical, and one green run shows the flag is accepted, not that the intermittent failure is gone for good. That settles with time on No sandboxed-verification line is warranted here: the claim is about CI scheduling, not runtime behavior — nothing a 中文说明代码审查在读 diff 之前,我对"helper 测试文件并发运行互相干扰"的独立方案正是这个:在 没有需要指出的问题:
无 Critical 问题,无规范违规。 测试证据(来自 PR 自身 CI 的 API,未执行任何 PR 代码)本次证据是被审提交上 PR 自身的 CI;本地未构建或运行任何 PR 派生代码。值得注意的是,正在进行的 目前无失败;包含串行化 helper 测试步骤的 ubuntu Test job 仍在运行。未验证项:多次运行下不再 flake——flake 修复本质上是统计性的,一次绿色运行只能证明参数被接受,不能证明间歇性失败从此消失,这需要在 此处不需要沙箱验证行:该声明关于 CI 调度而非运行时行为—— — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — minimal, well-targeted CI flake fix; would merge without hesitation. Stepping back: the motivation is real (an intermittent required-CI failure, watched happen), and the fix is the smallest tool that addresses it — serialize the test files, keep everything else as-is. My independent proposal before reading the diff was the same flag at the same two call sites, and the PR lands exactly there with no drive-by changes. The diff carries nothing beyond the stated goal. The honest reservation is the one inherent to any flake fix: one green run proves the flag is accepted, not that the interference is gone — that's statistical and settles with time on Approval is deferred only because CI hasn't finished: the 中文说明信心:5/5 —— 最小化、目标明确的 CI flake 修复,可以毫不犹豫地合并。 整体来看:动机真实(必要 CI 的间歇性失败,作者亲眼观察到),修复是解决问题的最小手段——串行化测试文件,其余一切保持不变。我在读 diff 之前的独立方案就是在同样两处调用点加同样的参数,PR 恰好如此,没有夹带任何顺带改动。diff 中没有超出既定目标的内容。 唯一诚实的保留意见是所有 flake 修复与生俱来的:一次绿色运行只能证明参数被接受,不能证明干扰已消除——那是统计性的,需要在 暂缓批准只是因为 CI 尚未结束:被审提交上的 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Required Ubuntu CI is green on attempt 2. Attempt 1 was an unrelated runner |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.2)
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. Approving.
Reviewed at head 11df18dc — two commits over main:
.github/workflows/ci.yml: adds--test-concurrency=1to bothHELPER_TESTSinvocations (lines 358 and 499).scripts/tests/ci-platform-lanes.test.js: adds a'GitHub helper tests'suite asserting every step whoserunreferencesenv.HELPER_TESTScarries--test-concurrency=1.
What I checked:
- Sibling entrances:
HELPER_TESTSis defined at theci.ymlworkflow-levelenv:block and is not accessible from any other workflow file. Grepping all 54 workflow files confirms zero outside references. Both in-file invocations are updated. --test-concurrency=1semantics: flag limits file-level concurrency in Node's test runner only; within-file test ordering is unaffected. PR description is accurate.- Enforcement test (commit 2): the new test reads
ci.ymlvia theyamlpackage (same path already used by the rest of the suite at line 33), collects all steps matchingenv.HELPER_TESTS, guards non-empty withnot.toHaveLength(0), and asserts--test-concurrency=1in each. Valid — catches flag deletion at either site, and catches a future third invocation without the flag. No circular fixture. - Cross-check: the
qwen-code-ci-botsuggestion (mutation-verified: stripping the flag left all existing contract suites green) is addressed by commit 2. Finding resolved.
CI: Tests are still queued/in-progress at this head (Classify PR and secret/CVE scans in progress; ubuntu, macOS, Windows test jobs queued). The prior head's CI was fully green across 15 checks for the identical ci.yml change. The new commit adds only a test file; the test itself will run under Test (ubuntu-latest, Node 22.x). I am approving before that result lands; if it comes back red, the test assertion itself would be the defect to fix.
Scope: .github/workflows/ci.yml + scripts/tests/ci-platform-lanes.test.js only. No production code, no SDK changes.
Reviewed with AI assistance.
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed at 21d537c — no findings. --test-concurrency=1 is added at both helper-test invocations (the linters-setup leg and the full-profile leg), serializing file execution while in-file cases keep the runner default — consistent with the evidence (fails concurrent, passes alone and with concurrency limited to one). Self-PR: GitHub blocks self-approval, so this is a comment review; merge approvals need to come from other maintainers.
|
Released in v0.22.3. |
What this PR does
Runs the repository's GitHub helper test files serially in both CI entry points. Test cases inside each file still use Node's normal test runner behavior.
Why it's needed
The helper suite shares process-level and repository-level resources across files. Running all files concurrently caused the release-note classifier test to fail intermittently in required CI, while the same test passed alone and the suite passed when file concurrency was limited to one.
Reviewer Test Plan
How to verify
Confirm both helper-test invocations pass
--test-concurrency=1, and that the helper test step completes without the prior cross-file interference.Evidence (Before & After)
N/A — CI-only change.
Tested on
Environment (optional)
Node.js 22.22.0.
Risk & Scope
Linked Issues
N/A.
中文说明
本 PR 的修改
在 CI 的两个入口中串行运行仓库的 GitHub helper 测试文件。每个文件内部的测试用例仍保留 Node 测试运行器的正常行为。
修改原因
helper 测试套件的不同文件会共享进程级和仓库级资源。并发运行所有文件时,release-note classifier 测试曾在必要 CI 中间歇失败;同一测试单独运行可以通过,将文件并发数限制为一后整套 CI 也通过。
Reviewer 测试计划
验证方式
确认两个 helper-test 调用都传入
--test-concurrency=1,并确认 helper 测试步骤不再出现此前的跨文件干扰。证据(修改前后)
N/A——仅 CI 修改。
测试环境
环境(可选)
Node.js 22.22.0。
风险与范围
关联 Issue
N/A。