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)