From c8904fc4243db40f7acee1ccb4deef2162cc92c9 Mon Sep 17 00:00:00 2001 From: Joris Wouter Jonkers Date: Mon, 19 Oct 2026 19:21:28 +0200 Subject: [PATCH] fix: the mutation report runs Stryker for real and stays short Stryker's Vitest runner switches no mutant on under Vitest 5, and its sandbox loses imports from outside the web directory, so a project whose code imports its contract ran a fraction of its tests and every mutant read as survived. Stryker now mutates only the changed lines, one changed file at a time, in place, with the command runner running vitest related on that file. The comment lists survivors up to 30, counts unreached mutants per file instead of listing each, and leaves out generated code. Closes #167 --- .github/workflows/ci.yml | 7 +++ .github/workflows/mutation-report.yml | 44 +++++++++++--- scripts/changed_ranges.py | 50 +++++++++++++++ scripts/mutation_report.py | 76 ++++++++++++++++++----- tests/test_changed_ranges.py | 64 +++++++++++++++++++ tests/test_mutation_report.py | 88 +++++++++++++++++++++++++-- 6 files changed, 298 insertions(+), 31 deletions(-) create mode 100644 scripts/changed_ranges.py create mode 100644 tests/test_changed_ranges.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d52f7c..d0205b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,6 +114,13 @@ jobs: .venv-tests/bin/python -m coverage run --branch --source=scripts.mutation_report -m unittest tests.test_mutation_report .venv-tests/bin/python -m coverage report --show-missing --fail-under=100 + # The ranges decide which lines Stryker mutates, so a wrong one reports on lines nobody changed. + - name: Run changed-ranges tests with branch coverage + if: ${{ !cancelled() }} + run: | + .venv-tests/bin/python -m coverage run --branch --source=scripts.changed_ranges -m unittest tests.test_changed_ranges + .venv-tests/bin/python -m coverage report --show-missing --fail-under=100 + # A quarantine check that passes when it should fail lets a skipped test stay skipped forever. - name: Run quarantine check tests with branch coverage if: ${{ !cancelled() }} diff --git a/.github/workflows/mutation-report.yml b/.github/workflows/mutation-report.yml index 953fb0c..3add7fc 100644 --- a/.github/workflows/mutation-report.yml +++ b/.github/workflows/mutation-report.yml @@ -5,8 +5,11 @@ name: Mutation Report # Vue. One comment is kept up to date per pull request. It is a report and never fails the build # (workspace docs/testing-standards.md, section 6). # -# The caller's toolchain comes from its mise.toml. A web directory needs @stryker-mutator/core and -# @stryker-mutator/vitest-runner as devDependencies, so Stryker runs with the project's own vitest. +# The caller's toolchain comes from its mise.toml. A web directory tests with Vitest and has +# @stryker-mutator/core as a devDependency. Stryker mutates only the changed lines, one changed file +# at a time, in place, and runs `vitest related` on that file against each mutant: its Vitest test +# runner switches no mutant on under Vitest 5, and its sandbox loses imports from outside the web +# directory, such as a contract file beside it. on: workflow_call: @@ -75,29 +78,50 @@ jobs: working-directory: ${{ inputs.web-directory }} env: BASE: origin/${{ github.base_ref }} + RANGES: ${{ github.workspace }}/.github-workflows/scripts/changed_ranges.py run: | - changed=$(git diff --name-only --diff-filter=AM --relative "$BASE"...HEAD -- src \ - | grep -E '\.(ts|vue)$' | grep -Ev '\.(spec|test)\.ts$|\.gen\.ts$|/infrastructure/api/' | paste -sd, - || true) - if [ -z "$changed" ]; then + git diff -U0 --diff-filter=AM --relative "$BASE"...HEAD -- src \ + | python3 "$RANGES" \ + | awk -F'\t' '$1 ~ /\.(ts|vue)$/ && $1 !~ /\.(spec|test|gen)\.ts$/ && $1 !~ /\/infrastructure\/api\//' \ + > "$RUNNER_TEMP/ranges.tsv" || true + if [ ! -s "$RUNNER_TEMP/ranges.tsv" ]; then echo "No changed TypeScript or Vue source to mutate." exit 0 fi pnpm install --frozen-lockfile - # pnpm keeps each package's dependencies apart, so Stryker is told where its runner is. - pnpm exec stryker run --plugins @stryker-mutator/vitest-runner --testRunner vitest \ - --reporters json,clear-text --mutate "$changed" - cp reports/mutation/mutation.json "$RUNNER_TEMP/mutation.json" + i=0 + while IFS=$'\t' read -r file spans; do + i=$((i + 1)) + jq -n --arg file "$file" --arg out "$RUNNER_TEMP/stryker-$i.json" '{ + testRunner: "command", + commandRunner: {command: ("pnpm exec vitest related --run " + $file)}, + coverageAnalysis: "off", + inPlace: true, + reporters: ["json"], + jsonReporter: {fileName: $out} + }' > "$RUNNER_TEMP/stryker-$i.conf.json" + echo "::group::$file" + pnpm exec stryker run "$RUNNER_TEMP/stryker-$i.conf.json" --mutate "$spans" --concurrency "$(nproc)" + echo "::endgroup::" + done < "$RUNNER_TEMP/ranges.tsv" - name: Write the report env: GO_OUTCOME: ${{ steps.go.outcome }} WEB_OUTCOME: ${{ steps.web.outcome }} + GO_DIRECTORY: ${{ inputs.go-directory }} + WEB_DIRECTORY: ${{ inputs.web-directory }} run: | failed=() [ "$GO_OUTCOME" = failure ] && failed+=(--failed gremlins) [ "$WEB_OUTCOME" = failure ] && failed+=(--failed Stryker) + stryker=() + for report in "$RUNNER_TEMP"/stryker-*.json; do + case "$report" in *.conf.json) ;; *) [ -f "$report" ] && stryker+=(--stryker "$report") ;; esac + done python3 .github-workflows/scripts/mutation_report.py \ - --gremlins "$RUNNER_TEMP/gremlins.json" --stryker "$RUNNER_TEMP/mutation.json" \ + --gremlins "$RUNNER_TEMP/gremlins.json" "${stryker[@]}" \ + --go-root "${GO_DIRECTORY:-.}" --web-root "${WEB_DIRECTORY:-.}" \ "${failed[@]}" --out "$RUNNER_TEMP/comment.md" >> "$GITHUB_STEP_SUMMARY" - name: Comment on the pull request diff --git a/scripts/changed_ranges.py b/scripts/changed_ranges.py new file mode 100644 index 0000000..ace17ad --- /dev/null +++ b/scripts/changed_ranges.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +"""The lines a pull request adds or changes, per file, as Stryker's --mutate ranges. + + git diff -U0 origin/main...HEAD --relative -- src | changed_ranges.py + +prints one line per file: the file, a tab, and its ranges, `src/a.ts:12-14,src/a.ts:20-20`. +Stryker then mutates only those lines, as gremlins --diff does for Go, so the report is about what +the pull request changed and a large file costs no more than its changed lines. +""" + +from __future__ import annotations + +import re +import sys + +# The new side of a hunk: `@@ -a,b +c,d @@`, where an absent d means one line and 0 means none. +HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") + + +def ranges(diff: str) -> dict[str, list[tuple[int, int]]]: + """Each file's added or changed line ranges, first to last, from a `git diff -U0`.""" + out: dict[str, list[tuple[int, int]]] = {} + current: str | None = None + for line in diff.splitlines(): + if line.startswith("+++ "): + path = line[4:] + current = path[2:] if path.startswith("b/") else None + continue + match = HUNK.match(line) + if not match or current is None: + continue + start, count = int(match.group(1)), int(match.group(2) or "1") + if count: + out.setdefault(current, []).append((start, start + count - 1)) + return out + + +def render(changed: dict[str, list[tuple[int, int]]]) -> str: + return "".join( + f"{path}\t{','.join(f'{path}:{a}-{b}' for a, b in spans)}\n" for path, spans in sorted(changed.items()) + ) + + +def main() -> int: # pragma: no cover + sys.stdout.write(render(ranges(sys.stdin.read()))) + return 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/scripts/mutation_report.py b/scripts/mutation_report.py index c5689f4..ccfe227 100644 --- a/scripts/mutation_report.py +++ b/scripts/mutation_report.py @@ -1,11 +1,14 @@ #!/usr/bin/env python3 """Turn gremlins' and Stryker's JSON reports into one pull request comment. -Both tools run only on the lines a pull request changes. The comment lists every -mutant that survived (a test ran the line but no test failed when it changed) and, -apart, every mutant no test reaches. It is a report: it never fails the build. +Both tools run only on the lines a pull request changes. The comment lists the +mutants that survived (a test ran the line but no test failed when it changed) and +counts, per file, the mutants no test reaches. Generated code is left out: a pull +request that regenerates a client changes thousands of lines nobody wrote. It is a +report: it never fails the build. - mutation_report.py --gremlins gremlins.json --stryker mutation.json --out comment.md + mutation_report.py --gremlins gremlins.json --stryker a.json --stryker b.json \ + --go-root . --web-root web --out comment.md A report that is absent counts as no mutants, so a repository with only Go or only TypeScript passes one flag. A tool that crashed also leaves no report, so the @@ -17,12 +20,20 @@ import argparse import json +from collections import Counter from dataclasses import dataclass from pathlib import Path # The comment is found again by this line, so a later run edits it instead of adding another. MARKER = "" +# How many survivors and files the comment lists before it says how many more there are. +MOST_SURVIVORS = 30 +MOST_FILES = 15 + +# What generators write at the top of a file: Go's convention, and hey-api's and sqlc's own. +GENERATED = ("code generated", "auto-generated", "autogenerated", "do not edit") + @dataclass(frozen=True, order=True) class Mutant: @@ -60,20 +71,46 @@ def from_stryker(report: dict) -> tuple[list[Mutant], list[Mutant]]: def stryker_worked(report: dict) -> bool: - """Whether Stryker's results can be trusted: it recorded which tests reach the mutants, and ran - a test against every mutant it calls survived. A runner that does not support the project's - Vitest does neither, and every mutant then reads as survived.""" + """Whether Stryker's results can be trusted: it switched mutants on, which shows as tests + recorded against them or as a mutant a test killed, and ran a test against every mutant it + calls survived. A runner that cannot switch mutants on reads every mutant as survived.""" mutants = [m for f in (report.get("files") or {}).values() for m in f.get("mutants") or []] if not mutants: return True - covered = any(m.get("coveredBy") for m in mutants) + switched_on = any(m.get("coveredBy") or m["status"] in ("Killed", "Timeout") for m in mutants) untested = any(m["status"] == "Survived" and not m.get("testsCompleted") for m in mutants) - return covered and not untested + return switched_on and not untested + + +def generated(path: Path) -> bool: + """Whether a file says in its first lines that a generator wrote it.""" + try: + with path.open(encoding="utf-8", errors="replace") as f: + head = "".join(f.readline() for _ in range(5)).lower() + except OSError: + return False + return any(marker in head for marker in GENERATED) + + +def written(mutants: list[Mutant], root: str) -> list[Mutant]: + """The mutants in files people write, the paths read against the tool's own directory.""" + return [m for m in mutants if not generated(Path(root) / m.file)] def _table(mutants: list[Mutant]) -> list[str]: rows = ["| where | mutation |", "|---|---|"] - rows += [f"| `{m.file}:{m.line}` | {m.mutator} |" for m in sorted(mutants)] + rows += [f"| `{m.file}:{m.line}` | {m.mutator} |" for m in sorted(mutants)[:MOST_SURVIVORS]] + if len(mutants) > MOST_SURVIVORS: + rows += ["", f"…and {len(mutants) - MOST_SURVIVORS} more: see the job summary."] + return rows + + +def _per_file(mutants: list[Mutant]) -> list[str]: + counts = sorted(Counter(m.file for m in mutants).items(), key=lambda item: (-item[1], item[0])) + rows = ["| file | mutants |", "|---|---|"] + rows += [f"| `{path}` | {n} |" for path, n in counts[:MOST_FILES]] + if len(counts) > MOST_FILES: + rows += ["", f"…and {len(counts) - MOST_FILES} more files."] return rows @@ -94,7 +131,8 @@ def render( for tool in failed or []: lines += ["", f"{tool} did not finish, so its mutants are not listed: see the job log."] if unreached: - lines += ["", "#### No test reaches these lines", "", *_table(unreached)] + noun = "mutant" if len(unreached) == 1 else "mutants" + lines += ["", f"#### No test reaches {len(unreached)} {noun} on the changed lines", "", *_per_file(unreached)] for problem in problems or []: lines += ["", f"- {problem}"] lines += ["", "This report never fails the build."] @@ -114,23 +152,29 @@ def _read(path: str | None, name: str, problems: list[str]) -> dict: def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description="Turn mutation reports into one pull request comment.") parser.add_argument("--gremlins", help="gremlins' JSON report (--output)") - parser.add_argument("--stryker", help="Stryker's JSON report (json reporter)") + parser.add_argument("--stryker", action="append", default=[], help="a Stryker JSON report; one per run") + parser.add_argument("--go-root", default=".", help="the directory gremlins' paths are relative to") + parser.add_argument("--web-root", default=".", help="the directory Stryker's paths are relative to") parser.add_argument("--out", required=True, help="where to write the comment") parser.add_argument("--failed", action="append", default=[], help="a tool whose run did not finish") args = parser.parse_args(argv) problems: list[str] = [] g_survived, g_unreached = from_gremlins(_read(args.gremlins, "gremlins", problems)) - stryker = _read(args.stryker, "Stryker", problems) + stryker = {"files": {}} + for path in args.stryker: + stryker["files"].update(_read(path, "Stryker", problems).get("files") or {}) s_survived, s_unreached = from_stryker(stryker) if not stryker_worked(stryker): s_survived, s_unreached = [], [] args.failed.append("Stryker") problems.append( - "Stryker ran no test against its mutants, which happens when its test runner does not " - "support the project's Vitest." + "Stryker ran no test against its mutants, so it could not switch them on: its " + "survivors would all be false." ) - comment = render(g_survived + s_survived, g_unreached + s_unreached, problems, args.failed) + survived = written(g_survived, args.go_root) + written(s_survived, args.web_root) + unreached = written(g_unreached, args.go_root) + written(s_unreached, args.web_root) + comment = render(survived, unreached, problems, args.failed) Path(args.out).write_text(comment) print(comment) return 0 diff --git a/tests/test_changed_ranges.py b/tests/test_changed_ranges.py new file mode 100644 index 0000000..1ec9e97 --- /dev/null +++ b/tests/test_changed_ranges.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +import unittest + +from scripts.changed_ranges import ranges, render + +DIFF = """diff --git a/src/a.ts b/src/a.ts +index 1..2 100644 +--- a/src/a.ts ++++ b/src/a.ts +@@ -3,0 +4,2 @@ export function a() { ++ const x = 1 ++ const y = 2 +@@ -10 +12 @@ +-old ++new +@@ -20,3 +21,0 @@ +-gone +-gone +-gone +diff --git a/src/gone.ts b/src/gone.ts +deleted file mode 100644 +--- a/src/gone.ts ++++ /dev/null +@@ -1,2 +0,0 @@ +-a +-b +diff --git a/src/b.vue b/src/b.vue +--- a/src/b.vue ++++ b/src/b.vue +@@ -1,0 +1,3 @@ ++ +""" + + +class ReadingADiff(unittest.TestCase): + def test_each_hunk_that_adds_lines_is_a_range_of_its_new_lines(self) -> None: + self.assertEqual(ranges(DIFF)["src/a.ts"], [(4, 5), (12, 12)]) + + def test_a_hunk_that_only_removes_lines_is_no_range(self) -> None: + self.assertNotIn((21, 20), ranges(DIFF)["src/a.ts"]) + + def test_a_deleted_file_has_no_ranges(self) -> None: + self.assertNotIn("src/gone.ts", ranges(DIFF)) + + def test_every_changed_file_is_read(self) -> None: + self.assertEqual(ranges(DIFF)["src/b.vue"], [(1, 3)]) + + def test_an_empty_diff_changes_nothing(self) -> None: + self.assertEqual(ranges(""), {}) + + +class Rendering(unittest.TestCase): + def test_each_file_is_a_line_of_its_stryker_ranges_in_path_order(self) -> None: + self.assertEqual( + render(ranges(DIFF)), + "src/a.ts\tsrc/a.ts:4-5,src/a.ts:12-12\nsrc/b.vue\tsrc/b.vue:1-3\n", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_mutation_report.py b/tests/test_mutation_report.py index f9964a0..aa6305a 100644 --- a/tests/test_mutation_report.py +++ b/tests/test_mutation_report.py @@ -7,7 +7,7 @@ from contextlib import redirect_stdout from pathlib import Path -from scripts.mutation_report import MARKER, Mutant, from_gremlins, from_stryker, main, render +from scripts.mutation_report import MARKER, Mutant, from_gremlins, from_stryker, main, render, stryker_worked, written GREMLINS = { "go_module": "github.com/example/app", @@ -73,10 +73,27 @@ def test_each_survivor_is_listed_with_where_it_lives(self) -> None: self.assertIn("| `a.go:3` | INVERT_NEGATIVES |", text) self.assertIn("1 mutant survived", text) - def test_unreached_mutants_get_their_own_section(self) -> None: - text = render([], [Mutant("b.ts", 9, "StringLiteral")]) - self.assertIn("No test reaches these lines", text) - self.assertIn("| `b.ts:9` | StringLiteral |", text) + def test_unreached_mutants_are_counted_per_file_not_listed(self) -> None: + text = render([], [Mutant("b.ts", 9, "StringLiteral"), Mutant("b.ts", 12, "BooleanLiteral"), Mutant("a.ts", 1, "X")]) + self.assertIn("#### No test reaches 3 mutants on the changed lines", text) + self.assertIn("| `b.ts` | 2 |", text) + self.assertNotIn("StringLiteral", text) + + def test_one_unreached_mutant_is_counted_in_the_singular(self) -> None: + self.assertIn("No test reaches 1 mutant on the changed lines", render([], [Mutant("b.ts", 9, "X")])) + + def test_the_files_with_the_most_unreached_mutants_come_first_and_the_rest_are_counted(self) -> None: + unreached = [Mutant(f"f{i:02}.ts", 1, "X") for i in range(20)] + [Mutant("z.ts", n, "X") for n in range(3)] + text = render([], unreached) + self.assertLess(text.index("`z.ts`"), text.index("`f00.ts`")) + self.assertIn("…and 6 more files.", text) + self.assertNotIn("`f19.ts`", text) + + def test_survivors_past_the_limit_are_counted_not_listed(self) -> None: + text = render([Mutant("a.go", n, "X") for n in range(1, 36)], []) + self.assertIn("`a.go:30`", text) + self.assertNotIn("`a.go:31`", text) + self.assertIn("…and 5 more: see the job summary.", text) def test_the_comment_says_it_never_fails_the_build(self) -> None: self.assertIn("This report never fails the build.", render([], [])) @@ -161,6 +178,58 @@ def test_a_report_that_is_not_json_is_named_but_does_not_fail(self) -> None: self.assertIn("could not be read", out.read_text()) +class GeneratedCode(unittest.TestCase): + def test_a_file_a_generator_wrote_is_left_out(self) -> None: + with tempfile.TemporaryDirectory() as root: + (Path(root) / "client_gen.go").write_text("// Code generated by ogen, DO NOT EDIT.\npackage oas\n") + (Path(root) / "service.go").write_text("package app\n") + mutants = [Mutant("client_gen.go", 9, "X"), Mutant("service.go", 3, "Y")] + self.assertEqual(written(mutants, root), [Mutant("service.go", 3, "Y")]) + + def test_a_file_that_cannot_be_read_is_kept(self) -> None: + with tempfile.TemporaryDirectory() as root: + self.assertEqual(written([Mutant("absent.ts", 1, "X")], root), [Mutant("absent.ts", 1, "X")]) + + def test_generated_mutants_never_reach_the_comment(self) -> None: + report = {"files": [{"file_name": "oas_client_gen.go", "mutations": [{"type": "X", "status": "NOT COVERED", "line": 1}]}]} + with tempfile.TemporaryDirectory() as directory: + (Path(directory) / "oas_client_gen.go").write_text("// Code generated by ogen, DO NOT EDIT.\n") + gremlins, out = Path(directory) / "g.json", Path(directory) / "comment.md" + gremlins.write_text(json.dumps(report)) + with redirect_stdout(io.StringIO()): + main(["--gremlins", str(gremlins), "--go-root", directory, "--out", str(out)]) + self.assertNotIn("oas_client_gen.go", out.read_text()) + + +class StrykerRuns(unittest.TestCase): + def test_reports_of_several_runs_make_one_comment(self) -> None: + other = {"files": {"src/b.ts": {"mutants": [ + {"id": "9", "mutatorName": "StringLiteral", "status": "Survived", "testsCompleted": 2, "location": {"start": {"line": 4}}}, + {"id": "10", "mutatorName": "StringLiteral", "status": "Killed", "testsCompleted": 1, "location": {"start": {"line": 5}}}, + ]}}} + with tempfile.TemporaryDirectory() as directory: + a, b, out = Path(directory) / "a.json", Path(directory) / "b.json", Path(directory) / "comment.md" + a.write_text(json.dumps(STRYKER)) + b.write_text(json.dumps(other)) + with redirect_stdout(io.StringIO()): + main(["--stryker", str(a), "--stryker", str(b), "--out", str(out)]) + text = out.read_text() + self.assertIn("`src/features/home/useLauncher.ts:14`", text) + self.assertIn("`src/b.ts:4`", text) + + def test_a_run_without_coverage_that_killed_mutants_is_trusted(self) -> None: + # The command runner switches mutants on through the environment and records no coverage. + report = {"files": {"src/b.ts": {"mutants": [ + {"mutatorName": "X", "status": "Killed", "testsCompleted": 1}, + {"mutatorName": "Y", "status": "Survived", "testsCompleted": 1}, + ]}}} + self.assertTrue(stryker_worked(report)) + + def test_a_run_that_switched_no_mutant_on_is_not_trusted(self) -> None: + report = {"files": {"src/b.ts": {"mutants": [{"mutatorName": "Y", "status": "Survived", "testsCompleted": 3}]}}} + self.assertFalse(stryker_worked(report)) + + if __name__ == "__main__": unittest.main() @@ -172,3 +241,12 @@ def test_gremlins_runs_every_test_against_each_mutant(self) -> None: workflow = (Path(__file__).resolve().parents[1] / ".github/workflows/mutation-report.yml").read_text() unleash = next(line for line in workflow.splitlines() if "gremlins\" unleash" in line) self.assertIn("--integration", unleash) + + def test_stryker_runs_in_place_on_the_changed_lines_with_the_command_runner(self) -> None: + # Its Vitest runner switches no mutant on under Vitest 5, and its sandbox loses imports from + # outside the web directory; the command runner switches each mutant on through the environment. + workflow = (Path(__file__).resolve().parents[1] / ".github/workflows/mutation-report.yml").read_text() + self.assertIn('testRunner: "command"', workflow) + self.assertIn("inPlace: true", workflow) + self.assertIn('--mutate "$spans"', workflow) + self.assertNotIn("clear-text", workflow)