Skip to content

Commit b2f63d3

Browse files
zoomote[bot]roomote
zoomote[bot]
and
roomote
authored
[Fix] Mutation gate fails for stale and current pull requests (Zoo-Code-Org#1499)
* fix(ci): test mutation gate against merge result * fix(ci): resolve package-local vitest binaries * improve(ci): explain mutation gate failures * fix(ci): preserve mutation annotation punctuation * fix(ci): align mutation selectors with merge result --------- Co-authored-by: Roomote <roomote@roomote.dev>
1 parent 79cd12f commit b2f63d3

3 files changed

Lines changed: 517 additions & 32 deletions

File tree

‎.github/workflows/mutation-testing.yml‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@ jobs:
2525
echo "## Changed-code mutation testing" >> "$GITHUB_STEP_SUMMARY"
2626
echo "Mutation testing was enforced on each pull request before it entered the merge queue." >> "$GITHUB_STEP_SUMMARY"
2727
28-
- name: Checkout pull request head
28+
- name: Checkout pull request merge result
2929
if: github.event_name == 'pull_request'
3030
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
3131
with:
32-
repository: ${{ github.event.pull_request.head.repo.full_name }}
33-
ref: ${{ github.event.pull_request.head.sha }}
32+
ref: ${{ github.sha }}
3433
fetch-depth: 0
3534
persist-credentials: false
3635

@@ -55,14 +54,26 @@ jobs:
5554
if: github.event_name == 'pull_request'
5655
env:
5756
BASE_SHA: ${{ github.event.pull_request.base.sha }}
58-
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
57+
HEAD_SHA: ${{ github.sha }}
5958
run: node scripts/stryker-diff.mjs ci --base "$BASE_SHA" --head "$HEAD_SHA"
6059

6160
- name: Upload mutation reports
61+
id: mutation_report
6262
if: always() && github.event_name == 'pull_request'
6363
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
6464
with:
6565
name: changed-code-mutation-report
6666
path: reports/mutation/
6767
if-no-files-found: ignore
6868
retention-days: 7
69+
70+
- name: Link mutation report artifact
71+
if: always() && github.event_name == 'pull_request' && steps.mutation_report.outputs.artifact-url != ''
72+
env:
73+
ARTIFACT_URL: ${{ steps.mutation_report.outputs.artifact-url }}
74+
run: |
75+
{
76+
echo ""
77+
echo "### Download mutation reports"
78+
echo "[Open the changed-code-mutation-report artifact]($ARTIFACT_URL), then open the package's mutation.html file."
79+
} >> "$GITHUB_STEP_SUMMARY"

‎scripts/stryker-diff.mjs‎

Lines changed: 192 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -304,14 +304,23 @@ export function preferDirectTestFiles(testFiles, sourceFiles) {
304304
return direct.length > 0 ? direct : testFiles
305305
}
306306

307-
function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) {
307+
export function resolveVitestBinary(repoRoot, packageEntry) {
308+
const packageRoot = path.join(repoRoot, packageEntry.root)
309+
const runRoot = path.join(repoRoot, packageEntry.runRoot ?? packageEntry.root)
310+
const candidates = [...new Set([runRoot, packageRoot, repoRoot])].map((root) =>
311+
path.join(root, "node_modules/.bin/vitest"),
312+
)
313+
return candidates.find((candidate) => fs.existsSync(candidate)) ?? candidates.at(-1)
314+
}
315+
316+
export function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) {
308317
const packageRoot = path.join(repoRoot, packageEntry.root)
309318
const runRoot = path.join(repoRoot, packageEntry.runRoot ?? packageEntry.root)
310319
const outputFile = path.join(reportDirectory, "vitest-related.json")
311320
const configFile = path.relative(runRoot, path.join(packageRoot, packageEntry.vitestConfig)).replaceAll("\\", "/")
312321
const sourceFiles = [...new Set(packageEntry.selectors.map(selectorFile))]
313322
const result = spawnSync(
314-
path.join(repoRoot, "node_modules/.bin/vitest"),
323+
resolveVitestBinary(repoRoot, packageEntry),
315324
["related", ...sourceFiles, "--run", "--config", configFile, "--reporter=json", `--outputFile=${outputFile}`],
316325
{
317326
cwd: runRoot,
@@ -325,6 +334,9 @@ function discoverRelatedTestFiles(repoRoot, packageEntry, reportDirectory) {
325334
if (result.error?.code === "ETIMEDOUT") {
326335
throw new Error(`${packageEntry.id} related-test discovery exceeded 5 minutes`)
327336
}
337+
if (result.error) {
338+
throw new Error(`${packageEntry.id} related-test discovery could not start: ${result.error.message}`)
339+
}
328340
if (result.status !== 0) {
329341
throw new Error(
330342
`${packageEntry.id} related-test discovery failed:\n${stripAnsi(`${result.stdout ?? ""}${result.stderr ?? ""}`).trim()}`,
@@ -378,6 +390,11 @@ function runStryker(repoRoot, packageEntry, reportRoot, dryRunOnly) {
378390
`${packageEntry.id} mutation run exceeded 12 minutes. Split the PR or obtain a maintainer-reviewed narrow exclusion.`,
379391
)
380392
}
393+
if (result.error) {
394+
throw new Error(
395+
`${packageEntry.id} Stryker ${dryRunOnly ? "preflight" : "run"} could not start: ${result.error.message}`,
396+
)
397+
}
381398
if (result.status !== 0) {
382399
throw new Error(
383400
`${packageEntry.id} Stryker ${dryRunOnly ? "preflight" : "run"} failed:\n${stripAnsi(output).trim()}`,
@@ -405,17 +422,19 @@ export function mutantCounts(report) {
405422
return counts
406423
}
407424

408-
function escapeWorkflowValue(value) {
409-
return String(value)
410-
.replaceAll("%", "%25")
411-
.replaceAll("\r", "%0D")
412-
.replaceAll("\n", "%0A")
413-
.replaceAll(":", "%3A")
414-
.replaceAll(",", "%2C")
425+
function escapeWorkflowData(value) {
426+
return String(value).replaceAll("%", "%25").replaceAll("\r", "%0D").replaceAll("\n", "%0A")
427+
}
428+
429+
function escapeWorkflowProperty(value) {
430+
return escapeWorkflowData(value).replaceAll(":", "%3A").replaceAll(",", "%2C")
415431
}
416432

417-
export function formatAnnotations(blockingMutants, packageRoot) {
418-
const perFile = new Map()
433+
export function formatAnnotationCommand(annotation) {
434+
return `::error file=${escapeWorkflowProperty(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowData(annotation.message)}`
435+
}
436+
437+
export function formatAnnotations(blockingMutants, packageRoot, state = { total: 0, perFile: new Map() }) {
419438
const annotations = []
420439

421440
for (const mutant of blockingMutants.sort((left, right) => {
@@ -424,9 +443,8 @@ export function formatAnnotations(blockingMutants, packageRoot) {
424443
})) {
425444
const repositoryPath = path.posix.join(packageRoot, mutant.filePath.replaceAll("\\", "/"))
426445
const key = `${repositoryPath}:${mutant.location.start.line}`
427-
const fileCount = perFile.get(repositoryPath) ?? 0
428-
if (annotations.some((annotation) => annotation.key === key) || fileCount >= 7 || annotations.length >= 20)
429-
continue
446+
const fileCount = state.perFile.get(repositoryPath) ?? 0
447+
if (annotations.some((annotation) => annotation.key === key) || fileCount >= 7 || state.total >= 20) continue
430448

431449
const replacement = String(mutant.replacement ?? "")
432450
.replace(/\s+/g, " ")
@@ -438,17 +456,59 @@ export function formatAnnotations(blockingMutants, packageRoot) {
438456
line: mutant.location.start.line,
439457
message:
440458
`${mutant.status} ${mutant.mutatorName} mutant${replacement ? ` (replacement: ${replacement})` : ""}. ` +
441-
"Add or strengthen a focused test that fails under this mutation, or add a maintainer-approved targeted exclusion with a reason.",
459+
"See the job summary for the complete list and resolution guidance.",
442460
})
443-
perFile.set(repositoryPath, fileCount + 1)
461+
state.perFile.set(repositoryPath, fileCount + 1)
462+
state.total++
444463
}
445464

446465
return annotations
447466
}
448467

449-
function appendSummary(rows, failures) {
450-
if (!process.env.GITHUB_STEP_SUMMARY) return
468+
function markdownCell(value) {
469+
return String(value ?? "—")
470+
.replace(/\s+/g, " ")
471+
.trim()
472+
.replaceAll("|", "\\|")
473+
.slice(0, 120)
474+
}
475+
476+
export function testsFromMutationReport(report, fallback = []) {
477+
const testFiles = Object.keys(report.testFiles ?? {})
478+
return testFiles.length > 0 ? testFiles : fallback
479+
}
480+
481+
export function formatBlockingMutants(blockingMutants, packageRoot) {
482+
const grouped = new Map()
483+
for (const mutant of [...blockingMutants].sort((left, right) => {
484+
const pathOrder = left.filePath.localeCompare(right.filePath)
485+
return pathOrder || left.location.start.line - right.location.start.line
486+
})) {
487+
const repositoryPath = path.posix.join(packageRoot, mutant.filePath.replaceAll("\\", "/"))
488+
const group = grouped.get(repositoryPath) ?? []
489+
group.push(mutant)
490+
grouped.set(repositoryPath, group)
491+
}
492+
493+
const lines = []
494+
for (const [filePath, mutants] of grouped) {
495+
lines.push(
496+
`#### \`${filePath}\``,
497+
"",
498+
"| Line | Status | Mutator | Replacement |",
499+
"| ---: | --- | --- | --- |",
500+
)
501+
for (const mutant of mutants) {
502+
lines.push(
503+
`| ${mutant.location.start.line} | ${markdownCell(mutant.status)} | ${markdownCell(mutant.mutatorName)} | ${markdownCell(mutant.replacement)} |`,
504+
)
505+
}
506+
lines.push("")
507+
}
508+
return lines
509+
}
451510

511+
export function formatSummary(rows, failures, manifest = {}) {
452512
const lines = [
453513
"## Changed-code mutation testing",
454514
"",
@@ -461,8 +521,89 @@ function appendSummary(rows, failures) {
461521
)
462522
}
463523
if (rows.length === 0) lines.push("| — | 0 | 0 | 0 | 0 | 0 | 0 | Not applicable |")
464-
if (failures.length > 0) lines.push("", ...failures.map((failure) => `- ${failure}`))
465-
fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${lines.join("\n")}\n`)
524+
525+
if (rows.length > 0) {
526+
lines.push("", "### Focused tests")
527+
for (const row of rows) {
528+
const cwd = row.runRoot ?? row.root
529+
if (row.testFiles?.length > 0) {
530+
lines.push(`- **${row.id}** (cwd \`${cwd}\`): ${row.testFiles.map((file) => `\`${file}\``).join(", ")}`)
531+
} else {
532+
lines.push(
533+
`- **${row.id}** (cwd \`${cwd}\`): the mutation run did not complete far enough to report its selected tests; use the exact reproduction command below.`,
534+
)
535+
}
536+
}
537+
}
538+
539+
const blockingRows = rows.filter((row) => row.blocking?.length > 0)
540+
if (blockingRows.length > 0) {
541+
lines.push(
542+
"",
543+
"### All surviving and uncovered mutants",
544+
"",
545+
"Annotations highlight up to 20 unique locations (maximum 7 per file). This summary lists every blocking mutant.",
546+
"",
547+
)
548+
for (const row of blockingRows) {
549+
lines.push(`### ${row.id}`, "", ...formatBlockingMutants(row.blocking, row.runRoot ?? row.root))
550+
}
551+
lines.push(
552+
"### Resolve a mutation gap",
553+
"",
554+
"Add or strengthen a focused test that fails under the mutation. If the mutant is equivalent, request maintainer approval for the narrowest mutator-specific exclusion and explain why it cannot change behavior:",
555+
"",
556+
"```ts",
557+
"// Stryker disable next-line ConditionalExpression: normalized input cannot reach the alternate branch",
558+
"const result = condition ? value : fallback",
559+
"```",
560+
"",
561+
"Broad `all` exclusions and exclusions without a concrete reason are rejected by the gate.",
562+
)
563+
}
564+
565+
if (manifest.baseSha && manifest.headSha) {
566+
lines.push(
567+
"",
568+
"### Reproduce locally",
569+
"",
570+
"From a full checkout containing both commits:",
571+
"",
572+
"```bash",
573+
"pnpm install --frozen-lockfile",
574+
`node scripts/stryker-diff.mjs ci --base ${manifest.baseSha} --head ${manifest.headSha}`,
575+
"```",
576+
)
577+
}
578+
579+
if (rows.length > 0) {
580+
lines.push("", "### Mutation reports", "")
581+
for (const row of rows) lines.push(`- **${row.id}:** \`${row.reportPath}\``)
582+
lines.push(
583+
"",
584+
"The workflow uploads generated reports in the `changed-code-mutation-report` artifact. A direct artifact link appears below after upload.",
585+
)
586+
}
587+
588+
if (failures.length > 0) {
589+
lines.push(
590+
"",
591+
"### Failures",
592+
"",
593+
...failures.map((failure) => {
594+
const detail =
595+
failure.length > 4_000 ? `${failure.slice(0, 4_000)}\n[truncated; see the step log]` : failure
596+
return `- ${detail.replaceAll("\n", "\n ")}`
597+
}),
598+
)
599+
}
600+
601+
return `${lines.join("\n")}\n`
602+
}
603+
604+
function appendSummary(rows, failures, manifest) {
605+
if (!process.env.GITHUB_STEP_SUMMARY) return
606+
fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, formatSummary(rows, failures, manifest))
466607
}
467608

468609
export function evaluateReport(report, packageEntry) {
@@ -491,9 +632,13 @@ export function evaluateReport(report, packageEntry) {
491632
export function runManifest(repoRoot, manifest, reportRoot) {
492633
const rows = []
493634
const failures = []
635+
const annotationState = { total: 0, perFile: new Map() }
494636

495637
for (const packageEntry of manifest.packages) {
496638
let counts
639+
const reportPath = path
640+
.relative(repoRoot, path.join(reportRoot, packageEntry.id, "mutation.html"))
641+
.replaceAll("\\", "/")
497642
try {
498643
const reportDirectory = path.join(reportRoot, packageEntry.id)
499644
fs.mkdirSync(reportDirectory, { recursive: true })
@@ -514,6 +659,11 @@ export function runManifest(repoRoot, manifest, reportRoot) {
514659
if (generatedMutants === 0) {
515660
rows.push({
516661
id: packageEntry.id,
662+
root: packageEntry.root,
663+
runRoot: packageEntry.runRoot,
664+
selectors: packageEntry.selectors,
665+
testFiles: packageEntry.testFiles ?? [],
666+
reportPath,
517667
changedLines: packageEntry.changedExecutableLines,
518668
valid: 0,
519669
killed: 0,
@@ -526,17 +676,25 @@ export function runManifest(repoRoot, manifest, reportRoot) {
526676
}
527677

528678
runStryker(repoRoot, packageEntry, reportRoot, false)
529-
const reportPath = path.join(reportRoot, packageEntry.id, "mutation.json")
530-
const report = JSON.parse(fs.readFileSync(reportPath, "utf8"))
679+
const jsonReportPath = path.join(reportRoot, packageEntry.id, "mutation.json")
680+
const report = JSON.parse(fs.readFileSync(jsonReportPath, "utf8"))
681+
packageEntry.testFiles = testsFromMutationReport(report, packageEntry.testFiles)
531682
counts = mutantCounts(report)
532-
for (const annotation of formatAnnotations(counts.blocking, packageEntry.runRoot ?? packageEntry.root)) {
533-
console.log(
534-
`::error file=${escapeWorkflowValue(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowValue(annotation.message)}`,
535-
)
683+
for (const annotation of formatAnnotations(
684+
counts.blocking,
685+
packageEntry.runRoot ?? packageEntry.root,
686+
annotationState,
687+
)) {
688+
console.log(formatAnnotationCommand(annotation))
536689
}
537690
evaluateReport(report, packageEntry)
538691
rows.push({
539692
id: packageEntry.id,
693+
root: packageEntry.root,
694+
runRoot: packageEntry.runRoot,
695+
selectors: packageEntry.selectors,
696+
testFiles: packageEntry.testFiles ?? [],
697+
reportPath,
540698
changedLines: packageEntry.changedExecutableLines,
541699
...counts,
542700
result: "Passed",
@@ -545,18 +703,24 @@ export function runManifest(repoRoot, manifest, reportRoot) {
545703
failures.push(error.message)
546704
rows.push({
547705
id: packageEntry.id,
706+
root: packageEntry.root,
707+
runRoot: packageEntry.runRoot,
708+
selectors: packageEntry.selectors,
709+
testFiles: packageEntry.testFiles ?? [],
710+
reportPath,
548711
changedLines: packageEntry.changedExecutableLines,
549712
valid: counts?.valid ?? 0,
550713
killed: counts?.killed ?? 0,
551714
timeout: counts?.timeout ?? 0,
552715
survived: counts?.survived ?? 0,
553716
noCoverage: counts?.noCoverage ?? 0,
717+
blocking: counts?.blocking ?? [],
554718
result: "Failed",
555719
})
556720
}
557721
}
558722

559-
appendSummary(rows, failures)
723+
appendSummary(rows, failures, manifest)
560724
if (failures.length > 0) throw new Error(failures.join("\n"))
561725
return rows
562726
}
@@ -579,7 +743,7 @@ function main() {
579743
const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation")
580744
const manifest = selectFromGit(repoRoot, baseSha, headSha)
581745
if (manifest.packages.length === 0) {
582-
appendSummary([], [])
746+
appendSummary([], [], manifest)
583747
console.log("No changed executable lines in mutation-tested packages; mutation testing is not applicable.")
584748
return
585749
}

0 commit comments

Comments
 (0)