fix(ci): retain disk samples when npm install fails #10517
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,117 @@ | ||||||||||||||||||||||||||
| import assert from 'node:assert/strict'; | ||||||||||||||||||||||||||
| import { spawnSync } from 'node:child_process'; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| chmodSync, | ||||||||||||||||||||||||||
| mkdtempSync, | ||||||||||||||||||||||||||
| readFileSync, | ||||||||||||||||||||||||||
| rmSync, | ||||||||||||||||||||||||||
| writeFileSync, | ||||||||||||||||||||||||||
| } from 'node:fs'; | ||||||||||||||||||||||||||
| import { tmpdir } from 'node:os'; | ||||||||||||||||||||||||||
| import { dirname, join } from 'node:path'; | ||||||||||||||||||||||||||
| import { describe, it } from 'node:test'; | ||||||||||||||||||||||||||
| import { fileURLToPath } from 'node:url'; | ||||||||||||||||||||||||||
| import { parse } from 'yaml'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const workflowPath = join( | ||||||||||||||||||||||||||
| dirname(fileURLToPath(import.meta.url)), | ||||||||||||||||||||||||||
| '..', | ||||||||||||||||||||||||||
| 'workflows', | ||||||||||||||||||||||||||
| 'ci.yml', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| const testSteps = parse(readFileSync(workflowPath, 'utf8')).jobs.test.steps; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function step(name) { | ||||||||||||||||||||||||||
| const value = testSteps.find((candidate) => candidate.name === name); | ||||||||||||||||||||||||||
| assert.ok(value, `missing ${name} step`); | ||||||||||||||||||||||||||
| return value; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| describe('ci.yml disk-pressure evidence', () => { | ||||||||||||||||||||||||||
| it('starts sampling before npm ci and preserves those samples for upload', () => { | ||||||||||||||||||||||||||
| const install = step('Install dependencies').run; | ||||||||||||||||||||||||||
| const npmCi = install.indexOf('npm ci'); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| assert.match( | ||||||||||||||||||||||||||
| install, | ||||||||||||||||||||||||||
| /DISK_SAMPLES="\$\{RUNNER_TEMP\}\/disk-pressure-samples\.log"/, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| assert.ok(npmCi > install.indexOf('DFSAMPLE ')); | ||||||||||||||||||||||||||
| assert.match(install, /\( while sleep 10; do sample_disk; done \) &/); | ||||||||||||||||||||||||||
| assert.ok(npmCi > install.indexOf('( while sleep 10')); | ||||||||||||||||||||||||||
| assert.match(install, /trap .*SAMPLER_PID.* EXIT/); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const tests = step('Run tests and generate reports').run; | ||||||||||||||||||||||||||
| assert.match( | ||||||||||||||||||||||||||
| tests, | ||||||||||||||||||||||||||
| /DISK_SAMPLES="\$\{RUNNER_TEMP\}\/disk-pressure-samples\.log"\nif \[ ! -s "\$DISK_SAMPLES" \]; then\n {2}echo "DISKCONTEXT .*" > "\$DISK_SAMPLES" 2>\/dev\/null \|\| true\nfi/, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| assert.ok(tests.indexOf('export TMPDIR=') > tests.indexOf('DISK_SAMPLES=')); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const sampleFormat = (script) => { | ||||||||||||||||||||||||||
| const match = script.match( | ||||||||||||||||||||||||||
| /sample="DFSAMPLE .*\/proc\/meminfo 2>\/dev\/null(?: \|\| true)?\)\]"/, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| assert.ok(match); | ||||||||||||||||||||||||||
| return match[0] | ||||||||||||||||||||||||||
| .replaceAll('${RUNNER_TEMP:-/tmp}', '${TMPDIR}') | ||||||||||||||||||||||||||
| .replace( | ||||||||||||||||||||||||||
| ' /proc/meminfo 2>/dev/null || true)]', | ||||||||||||||||||||||||||
| ' /proc/meminfo 2>/dev/null)]', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| const headerLine = (script) => | ||||||||||||||||||||||||||
| script | ||||||||||||||||||||||||||
| .split('\n') | ||||||||||||||||||||||||||
| .find((line) => line.trimStart().startsWith('echo "DISKCONTEXT ')) | ||||||||||||||||||||||||||
| ?.trim(); | ||||||||||||||||||||||||||
| assert.equal(headerLine(install), headerLine(tests)); | ||||||||||||||||||||||||||
| assert.equal(sampleFormat(install), sampleFormat(tests)); | ||||||||||||||||||||||||||
|
Comment on lines
+68
to
+69
Collaborator
There was a problem hiding this comment. [Suggestion] R2-2: The contract suite pins the sampler wiring selectively, and this round demonstrated three behaviours that can silently regress with the whole suite green. (1) The EXIT trap is pinned only by presence, not by ordering against Witness:
Suggested change
The trap pin must keep the trap after 中文说明合约测试套件对采样器接线只做选择性固定（pin），本轮演示了三种可以在整套测试保持绿色的情况下悄悄回归的行为。(1) EXIT trap 只被固定了"存在"，没有固定相对 约束：trap 的固定必须保持 trap 位于 — qwen3.8-max via Qwen Code /review (v0.22.3) |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const upload = step('Upload disk-pressure samples'); | ||||||||||||||||||||||||||
| assert.equal(upload.if, '${{ failure() }}'); | ||||||||||||||||||||||||||
| assert.equal(upload.with['if-no-files-found'], 'ignore'); | ||||||||||||||||||||||||||
|
yiliang114 marked this conversation as resolved.
|
||||||||||||||||||||||||||
| assert.equal( | ||||||||||||||||||||||||||
| upload.with.path, | ||||||||||||||||||||||||||
| '${{ runner.temp }}/disk-pressure-samples.log', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| it('keeps install failure status while writing the pre-install sample', () => { | ||||||||||||||||||||||||||
| const root = mkdtempSync(join(tmpdir(), 'ci-disk-pressure-')); | ||||||||||||||||||||||||||
| const npm = join(root, 'npm'); | ||||||||||||||||||||||||||
| writeFileSync(npm, '#!/usr/bin/env bash\nexit 42\n'); | ||||||||||||||||||||||||||
| chmodSync(npm, 0o755); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| const result = spawnSync( | ||||||||||||||||||||||||||
| 'bash', | ||||||||||||||||||||||||||
| ['-e', '-o', 'pipefail', '-c', step('Install dependencies').run], | ||||||||||||||||||||||||||
|
yiliang114 marked this conversation as resolved.
|
||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| encoding: 'utf8', | ||||||||||||||||||||||||||
| timeout: 30_000, | ||||||||||||||||||||||||||
| env: { | ||||||||||||||||||||||||||
| ...process.env, | ||||||||||||||||||||||||||
| PATH: `${root}:${process.env.PATH}`, | ||||||||||||||||||||||||||
| RUNNER_TEMP: root, | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| assert.equal(result.error, undefined); | ||||||||||||||||||||||||||
| assert.equal( | ||||||||||||||||||||||||||
| result.status, | ||||||||||||||||||||||||||
| 42, | ||||||||||||||||||||||||||
| `signal: ${result.signal}\nerror: ${result.error}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| const samples = readFileSync( | ||||||||||||||||||||||||||
| join(root, 'disk-pressure-samples.log'), | ||||||||||||||||||||||||||
| 'utf8', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| assert.match(samples, /^DISKCONTEXT /m); | ||||||||||||||||||||||||||
| assert.match(samples, /^DFSAMPLE /m); | ||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||
| rmSync(root, { recursive: true, force: true }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R2-1: The
sampleFormatnormalizer carries a branch that can never fire: the.replace(...)call below targets the spelling/proc/meminfo 2>/dev/null || true)](guard inside the command substitution), but neither sampler copy at HEAD has it — the install copy carries its|| trueoutside the assignment's closing quote (ci.yml:485) and the test-step copy has no guard there at all (ci.yml:685). The branch is not merely dead: if a future edit adds|| trueinside one copy's awk substitution, this normalizer silently absorbs the divergence and the parity assertion stays green — the two samplers drift, and a mixed-format timeline reaches the humans correlating ENOSPC failures, the artifact's only consumer. Drop the(?: \|\| true)?optional group from the regex on line 53 as well; the regex already ends its match at)]", which excludes both copies' trailing guards by construction.Witness:
中文说明
sampleFormat归一化器里有一个永远不会触发的分支：下面的.replace(...)针对的是/proc/meminfo 2>/dev/null || true)]（guard 位于命令替换内部）这种写法，但 HEAD 上的两份采样器副本都不是这种写法——install 副本的|| true位于赋值引号之外（ci.yml:485），test 步骤副本在该位置完全没有 guard（ci.yml:685）。这个分支不只是死代码：如果未来某次编辑把|| true加进某个副本的 awk 替换内部，该归一化会悄悄吸收掉这次格式分歧，奇偶断言仍然为绿——两个采样器从此漂移，混杂格式的 timeline 会送到关联 ENOSPC 故障的人工排查者手中，而这个 artifact 只有这一个消费者。请同时去掉第 53 行正则中的(?: \|\| true)?可选组；正则本身以)]"结尾，构造上已经排除了两份副本尾部的 guard。— qwen3.8-max via Qwen Code /review (v0.22.3)