test(autofix): single-source the infra-signature list from the workflow - #7565
Conversation
A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`.
…signatures in tests (#7562)
#6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard.
The infra-rerun test re-typed INFRA_FAILURE_SIGNATURES as an inline mirror of the workflow's env value. Two copies that must be hand-synced can drift — the test could keep passing against a stale list while production changed, or vice versa. That is exactly the copy the git-transport follow-up had to remember to update in two places. Extract the list from the workflow source instead, the same extract-from-source idiom the file already uses for NON_BLOCKING_CHECKS, so there is only one copy and drift is impossible. A toContain guard fails loudly if the env is renamed or the regex breaks, rather than letting an empty pattern match every line and silently pass.
|
Thanks for the PR! Template looks good ✓ Problem: observed maintenance hazard — Direction: aligned — eliminating a hand-synced copy in test infrastructure is squarely within scope. No CHANGELOG reference needed for a test-only change. Size: not applicable (no core paths touched; single test file, +10/−2). Approach: the scope is exactly right — one regex extraction, one guard assertion, one inline-literal replacement. Mirrors the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的维护隐患—— 方向：对齐——消除测试基础设施中的手工同步副本完全在范围内。纯测试改动无需 CHANGELOG 引用。 规模：不适用（未触及核心路径；单个测试文件，+10/−2）。 方案：范围恰好——一处正则提取、一处守卫断言、一处内联字面量替换。与第 346 行已有的 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given two copies of Comparison with the diff: the PR does exactly this. The regex TestingTest-only change with no user-visible behavior — the real-scenario test here is running the test suite itself. Infrastructure test (targeted): Mutation verification (rename Guard fails loudly in ~42ms, as claimed — the test is now coupled to the production value. Full suite: 96/96 green. No flakes on this run. 中文说明代码审查独立方案： 面对两份可能漂移的 与 diff 对比： PR 完全如此。正则与 测试纯测试改动，无用户可见行为——真实场景测试即运行测试套件本身。 基础设施测试（定向）： 1 通过，95 跳过。 变异验证： 改名后守卫约 42ms 内响亮失败——测试已耦合到生产值。 全量套件： 96/96 全绿。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is exactly the kind of follow-up that keeps test infrastructure honest. The two-copy pattern already bit once during #7562's git-transport edit, and the fix applies the same extract-from-source idiom the file uses for #7562 has merged and this PR is now based on 中文说明置信度：5/5——每个阶段都干净；毫不犹豫可合并。 这正是保持测试基础设施诚实的那种跟进。两副本模式在 #7562 的 git-transport 编辑时已经咬过一次，修复复用了文件对 #7562 已合并，本 PR 现已基于 — Qwen Code · qwen3.8-max-preview 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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
…nature-single-source The base advanced under #7562's takeover: the bot's "paginate annotations and filter Autofix runs" commit reflowed the infra test's run() signature (added crName/wfName) exactly where this branch inserts the INFRA_SIGNATURES extraction. Union both — keep the extraction + toContain guard and the new multi-line signature. The mirror → INFRA_SIGNATURES replacement auto-merged (base left that line unchanged). Test-only; infra test + mutation green against the new base.
#7562 (this PR's original base) squash-merged into main, which orphaned the stack: retargeting straight to main would show a misleading diff re-adding all of #7562. Merge current main in instead so the branch sits directly on top of it — the PR diff collapses to just this change (extract INFRA_FAILURE_SIGNATURES instead of mirroring it). Both conflicts were this change vs main's pre-change state; kept this side. #7569's bilingual test (also now on main) is preserved. No force-push.
|
@qwen-code /triage |
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. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: 6a (attacker mindset) — the agent returned substantively twice but the automated transcript check reports it never opened its brief file. Not reviewed: Agent 6a: Undirected audit — attacker mindset — never opened its brief (/home/github-runner/actions-runner-test-10/_work/qwen-code/qwen-code/.qwen/tmp/qwen-review-pr-7565-fetch-prompts/6a.brief.md), so it reviewed without the instructions it was launched to follow.
— qwen3.7-max via Qwen Code /review
gwinthis
left a comment
gwinthis
left a comment
There was a problem hiding this comment.
Review: APPROVE (C=0)
Summary
Clean, minimal test-only change (+10/-2) that eliminates a hand-synced copy of INFRA_FAILURE_SIGNATURES by extracting it from the workflow source — the same single-source idiom already used for NON_BLOCKING_CHECKS in this file.
Findings
None blocking. The change is well-structured:
- Extraction regex (
/INFRA_FAILURE_SIGNATURES: '([^']*)'/) is appropriate for the single-quoted, single-line YAML scalar format. Prettier/YAML keeps it on one line. toContainguard fails loudly if the env is renamed or regex breaks — preventing the empty-pattern-silently-matches-everything failure mode.- Follows established pattern —
NON_BLOCKING_CHECKSalready uses this exact extract-from-source idiom in the same file. - Mutation-verified per the PR description — renaming the env makes the guard fail in ~18ms.
Engineering Pattern
Single-source of truth for test fixtures: When a test needs a value that exists in production config, extract it from the source rather than re-typing it. Add a guard assertion that fails loudly if extraction breaks. This prevents the "test passes against stale copy" drift bug.
Verification
- Test-only change, no production behavior modified
- Stacked on #7562 (merge that first)
中文说明
评审：APPROVE (C=0)
概要
干净、最小化的纯测试改动（+10/-2），通过从工作流源提取 INFRA_FAILURE_SIGNATURES 来消除手工同步的副本——与文件中 NON_BLOCKING_CHECKS 已使用的 single-source 惯例一致。
发现
无阻断项。 改动结构良好：
- 提取正则 适用于单引号、单行 YAML 标量格式
toContain守卫 在 env 改名或正则失效时响亮报错- 遵循既有模式 —
NON_BLOCKING_CHECKS已在同文件使用完全相同的惯例 - 变异验证 — 改名后守卫约 18ms 令测试变红
工程模式
测试 fixture 的单一事实来源： 当测试需要生产配置中的值时，从源提取而非重新誊写。添加守卫断言在提取失败时响亮报错。
— qwen3.7-max via Qwen Code /review
4 participants
What this PR does
Removes a hand-synced copy: the infra-rerun test re-typed
INFRA_FAILURE_SIGNATURESas an inline mirror of the workflow's value. This extracts it from the workflow source instead, so there is exactly one copy and the two can never drift.Why it's needed
INFRA_FAILURE_SIGNATURESlived in two places: the workflow env (production) and an inline literal in the test. Two copies that must be hand-synced can drift — the test can keep passing against a stale list while production changed, or vice versa. The git-transport follow-up on #7562 had to remember to edit both; the next change might not.The file already solves this for a sibling value:
NON_BLOCKING_CHECKSis extracted from the workflow source and reused, so there is nothing to keep in sync. This applies the same idiom to the signature list.How
The infra test reads the module-level
workflowsource (already loaded at the top of the file) and pulls the value withworkflow.match(/INFRA_FAILURE_SIGNATURES: '([^']*)'/), then feeds that into the extracted bash block — replacing the inline literal. AtoContain('lost communication with the server')guard sits right after the extraction: if the env is renamed or the regex breaks, the extraction returns'', and rather than an empty pattern silently matching every annotation line (which would mask the failure), the guard fails the test loudly.Reviewer Test Plan
How to verify
npx vitest run scripts/tests/qwen-autofix-workflow.test.js -t infrastructure— green; the test now runs against the workflow's own value.INFRA_FAILURE_SIGNATURES→…_X) makes the extraction return''and thetoContainguard fails the test in ~18ms — proving the test is now coupled to the production value instead of a private copy.eligibility recheck/permanent API failures terminal, green on re-run). Prettier clean. Test-only change — no workflow edit, so no bash/actionlint delta.Evidence (Before & After)
Tested on
Risk & Scope
toContainguard would catch it immediately).Linked Issues
Follow-up to #7562 (which introduced
INFRA_FAILURE_SIGNATURESand the two-copy pattern this removes).中文说明
本 PR 做了什么
去掉一份需手工同步的副本:infra 重跑测试把
INFRA_FAILURE_SIGNATURES重新誊写为工作流值的内联镜像。本 PR 改为从工作流源提取,于是全局只有一份,二者永不漂移。为什么需要
INFRA_FAILURE_SIGNATURES存在两处:工作流 env(生产)与测试里的内联字面量。两份手工同步的副本会漂移 —— 测试可能一直对着陈旧列表通过,而生产已改,反之亦然。#7562 上的 git-transport 跟进就必须记得两处都改;下一次未必。文件对同类值已有解:
NON_BLOCKING_CHECKS是从工作流源提取再复用,无需同步。本 PR 把同一惯例用于签名列表。怎么做
infra 测试读取文件顶部已加载的模块级
workflow源,用workflow.match(/INFRA_FAILURE_SIGNATURES: '([^']*)'/)取值,喂给提取出的 bash 块 —— 替换内联字面量。提取后紧跟一句toContain('lost communication with the server')守卫:若 env 被改名或正则失效,提取返回'',此时空模式会静默匹配每一行 annotation(掩盖失败),而守卫会让测试响亮地变红。评审验证
npx vitest run ... -t infrastructure—— 绿;测试现在对着工作流自身的值运行。INFRA_FAILURE_SIGNATURES→…_X),提取返回'',toContain守卫约 18ms 令测试变红 —— 证明测试已耦合到生产值而非私有副本。eligibility recheck/permanent API failures terminal,重跑即绿)。prettier clean。纯测试改动,无工作流编辑,无 bash/actionlint 增量。风险与范围
toContain守卫会立即捕获)。关联 Issue
#7562 的跟进(该 PR 引入了
INFRA_FAILURE_SIGNATURES与本 PR 去除的两副本模式)。