Skip to content

Commit d8f2d47

Browse files
[Improve] Make mutation findings advisory (#1610)
* ci: make mutation findings advisory * fix(ci): surface mutation advisories --------- Co-authored-by: Roomote <roomote@roomote.dev>
1 parent a6c4864 commit d8f2d47

3 files changed

Lines changed: 140 additions & 65 deletions

File tree

.github/workflows/mutation-testing.yml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ jobs:
2222
- name: Record merge-queue enforcement
2323
if: github.event_name == 'merge_group'
2424
run: |
25-
echo "## Changed-code mutation testing" >> "$GITHUB_STEP_SUMMARY"
26-
echo "Mutation testing was enforced on each pull request before it entered the merge queue." >> "$GITHUB_STEP_SUMMARY"
25+
{
26+
echo "## Changed-code mutation testing"
27+
echo "Mutation testing was enforced on each pull request before it entered the merge queue."
28+
} >> "$GITHUB_STEP_SUMMARY" || echo "::warning title=Mutation test advisory::Could not write the job summary"
2729
2830
- name: Checkout pull request merge result
2931
if: github.event_name == 'pull_request'
@@ -50,7 +52,7 @@ jobs:
5052
if: github.event_name == 'pull_request'
5153
run: pnpm test:mutation-ci
5254

53-
- name: Mutate changed executable lines
55+
- name: Enforce executable-line scope and run advisory mutation testing
5456
if: github.event_name == 'pull_request'
5557
env:
5658
BASE_SHA: ${{ github.event.pull_request.base.sha }}
@@ -60,6 +62,7 @@ jobs:
6062
- name: Upload mutation reports
6163
id: mutation_report
6264
if: always() && github.event_name == 'pull_request'
65+
continue-on-error: true
6366
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
6467
with:
6568
name: changed-code-mutation-report
@@ -76,4 +79,4 @@ jobs:
7679
echo ""
7780
echo "### Download mutation reports"
7881
echo "[Open the changed-code-mutation-report artifact]($ARTIFACT_URL), then open the package's mutation.html file."
79-
} >> "$GITHUB_STEP_SUMMARY"
82+
} >> "$GITHUB_STEP_SUMMARY" || echo "::warning title=Mutation test advisory::Could not write the job summary"

scripts/stryker-diff.mjs

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ export function packageForPath(filePath) {
198198

199199
export function buildManifest(entries, readSource, diffForPath) {
200200
const packages = new Map()
201+
const advisories = []
201202

202203
for (const entry of entries) {
203204
if (!new Set(["A", "M", "R"]).has(entry.status)) continue
@@ -211,7 +212,11 @@ export function buildManifest(entries, readSource, diffForPath) {
211212
? new Set(Array.from({ length: sourceLineCount }, (_, index) => index + 1))
212213
: parseChangedLines(diffForPath(entry.path))
213214

214-
validateDisableDirectives(source, new Set(source.split(/\r?\n/).map((_, index) => index + 1)), entry.path)
215+
try {
216+
validateDisableDirectives(source, new Set(source.split(/\r?\n/).map((_, index) => index + 1)), entry.path)
217+
} catch (error) {
218+
advisories.push(error.message)
219+
}
215220
const executableLines = executableChangedLines(source, changedLines, entry.path)
216221
if (executableLines.size === 0) continue
217222

@@ -243,7 +248,7 @@ export function buildManifest(entries, readSource, diffForPath) {
243248
}
244249
}
245250

246-
return { packages: [...packages.values()] }
251+
return { packages: [...packages.values()], advisories }
247252
}
248253

249254
function validateSha(value, name) {
@@ -446,7 +451,11 @@ function escapeWorkflowProperty(value) {
446451
}
447452

448453
export function formatAnnotationCommand(annotation) {
449-
return `::error file=${escapeWorkflowProperty(annotation.file)},line=${annotation.line},title=Mutation test gap::${escapeWorkflowData(annotation.message)}`
454+
return `::warning file=${escapeWorkflowProperty(annotation.file)},line=${annotation.line},title=Mutation test advisory::${escapeWorkflowData(annotation.message)}`
455+
}
456+
457+
export function formatAdvisoryCommand(advisory) {
458+
return `::warning title=Mutation test advisory::${escapeWorkflowData(advisory)}`
450459
}
451460

452461
export function formatAnnotations(blockingMutants, packageRoot, state = { total: 0, perFile: new Map() }) {
@@ -523,7 +532,7 @@ export function formatBlockingMutants(blockingMutants, packageRoot) {
523532
return lines
524533
}
525534

526-
export function formatSummary(rows, failures, manifest = {}) {
535+
export function formatSummary(rows, advisories, manifest = {}) {
527536
const lines = [
528537
"## Changed-code mutation testing",
529538
"",
@@ -557,7 +566,7 @@ export function formatSummary(rows, failures, manifest = {}) {
557566
"",
558567
"### All surviving and uncovered mutants",
559568
"",
560-
"Annotations highlight up to 20 unique locations (maximum 7 per file). This summary lists every blocking mutant.",
569+
"Warning annotations highlight up to 20 unique locations (maximum 7 per file). This summary lists every advisory mutant.",
561570
"",
562571
)
563572
for (const row of blockingRows) {
@@ -573,7 +582,7 @@ export function formatSummary(rows, failures, manifest = {}) {
573582
"const result = condition ? value : fallback",
574583
"```",
575584
"",
576-
"Broad `all` exclusions and exclusions without a concrete reason are rejected by the gate.",
585+
"Broad `all` exclusions and exclusions without a concrete reason are reported as advisories.",
577586
)
578587
}
579588

@@ -600,14 +609,14 @@ export function formatSummary(rows, failures, manifest = {}) {
600609
)
601610
}
602611

603-
if (failures.length > 0) {
612+
if (advisories.length > 0) {
604613
lines.push(
605614
"",
606-
"### Failures",
615+
"### Advisory findings",
607616
"",
608-
...failures.map((failure) => {
617+
...advisories.map((advisory) => {
609618
const detail =
610-
failure.length > 4_000 ? `${failure.slice(0, 4_000)}\n[truncated; see the step log]` : failure
619+
advisory.length > 4_000 ? `${advisory.slice(0, 4_000)}\n[truncated; see the step log]` : advisory
611620
return `- ${detail.replaceAll("\n", "\n ")}`
612621
}),
613622
)
@@ -616,37 +625,45 @@ export function formatSummary(rows, failures, manifest = {}) {
616625
return `${lines.join("\n")}\n`
617626
}
618627

619-
function appendSummary(rows, failures, manifest) {
628+
export function appendSummary(rows, advisories, manifest) {
629+
for (const advisory of advisories) console.warn(formatAdvisoryCommand(advisory))
620630
if (!process.env.GITHUB_STEP_SUMMARY) return
621-
fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, formatSummary(rows, failures, manifest))
631+
try {
632+
fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, formatSummary(rows, advisories, manifest))
633+
} catch (error) {
634+
console.warn(
635+
`::warning title=Mutation test advisory::Could not write the job summary: ${escapeWorkflowData(error.message)}`,
636+
)
637+
}
622638
}
623639

624640
export function evaluateReport(report, packageEntry) {
625641
const counts = mutantCounts(report)
642+
const advisories = []
626643
if (counts.valid > MAX_MUTANTS) {
627-
throw new Error(
644+
advisories.push(
628645
`${packageEntry.id} generated ${counts.valid} valid mutants (limit ${MAX_MUTANTS}). ` +
629646
"Split the PR or obtain a maintainer-reviewed narrow exclusion.",
630647
)
631648
}
632649
if (counts.timeout > 10 || (counts.valid > 0 && counts.timeout / counts.valid > 0.15)) {
633-
throw new Error(
650+
advisories.push(
634651
`${packageEntry.id} timed out ${counts.timeout} of ${counts.valid} valid mutants. ` +
635-
"The result is inconclusive; fix flaky or slow tests, or reduce the changed scope before merge.",
652+
"The result is inconclusive; consider fixing flaky or slow tests, or reducing the changed scope.",
636653
)
637654
}
638655
if (counts.blocking.length > 0) {
639-
throw new Error(
656+
advisories.push(
640657
`${packageEntry.id} has ${counts.survived} surviving and ${counts.noCoverage} uncovered changed-code mutants. ` +
641-
"Add or strengthen focused tests before merge.",
658+
"Consider adding or strengthening focused tests.",
642659
)
643660
}
644-
return counts
661+
return { ...counts, advisories }
645662
}
646663

647664
export function runManifest(repoRoot, manifest, reportRoot) {
648665
const rows = []
649-
const failures = []
666+
const advisories = [...(manifest.advisories ?? [])]
650667
const annotationState = { total: 0, perFile: new Map() }
651668

652669
for (const packageEntry of manifest.packages) {
@@ -694,15 +711,15 @@ export function runManifest(repoRoot, manifest, reportRoot) {
694711
const jsonReportPath = path.join(reportRoot, packageEntry.id, "mutation.json")
695712
const report = JSON.parse(fs.readFileSync(jsonReportPath, "utf8"))
696713
packageEntry.testFiles = testsFromMutationReport(report, packageEntry.testFiles)
697-
counts = mutantCounts(report)
714+
counts = evaluateReport(report, packageEntry)
698715
for (const annotation of formatAnnotations(
699716
counts.blocking,
700717
packageEntry.runRoot ?? packageEntry.root,
701718
annotationState,
702719
)) {
703720
console.log(formatAnnotationCommand(annotation))
704721
}
705-
evaluateReport(report, packageEntry)
722+
advisories.push(...counts.advisories)
706723
rows.push({
707724
id: packageEntry.id,
708725
root: packageEntry.root,
@@ -712,10 +729,10 @@ export function runManifest(repoRoot, manifest, reportRoot) {
712729
reportPath,
713730
changedLines: packageEntry.changedExecutableLines,
714731
...counts,
715-
result: "Passed",
732+
result: counts.advisories.length > 0 ? "Advisory findings" : "Passed",
716733
})
717734
} catch (error) {
718-
failures.push(error.message)
735+
advisories.push(error.message)
719736
rows.push({
720737
id: packageEntry.id,
721738
root: packageEntry.root,
@@ -730,13 +747,12 @@ export function runManifest(repoRoot, manifest, reportRoot) {
730747
survived: counts?.survived ?? 0,
731748
noCoverage: counts?.noCoverage ?? 0,
732749
blocking: counts?.blocking ?? [],
733-
result: "Failed",
750+
result: "Advisory incomplete",
734751
})
735752
}
736753
}
737754

738-
appendSummary(rows, failures, manifest)
739-
if (failures.length > 0) throw new Error(failures.join("\n"))
755+
appendSummary(rows, advisories, manifest)
740756
return rows
741757
}
742758

@@ -758,7 +774,7 @@ function main() {
758774
const reportRoot = path.resolve(repoRoot, argument("--reports") ?? "reports/mutation")
759775
const manifest = selectFromGit(repoRoot, baseSha, headSha)
760776
if (manifest.packages.length === 0) {
761-
appendSummary([], [], manifest)
777+
appendSummary([], manifest.advisories, manifest)
762778
console.log("No changed executable lines in mutation-tested packages; mutation testing is not applicable.")
763779
return
764780
}

0 commit comments

Comments
 (0)