diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index f1bc1f5a..8f01bcf2 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -30,17 +30,48 @@ jobs: - name: Setup uses: ./.github/actions/setup - - name: Mutation test every file + # Incremental mode (https://stryker-mutator.io/docs/stryker-js/incremental/): Stryker keeps + # the result of every mutant in reports/stryker-incremental.json and, on the next run, only + # tests the mutants whose code or covering tests changed. A pull request starts from its own + # last run, or else from main's. Stryker cannot see a change outside the mutated files and the + # test files (a constants table, a test helper, a dependency, the config), so those files are + # hashed into the key: when one of them changes, no earlier result is reused. + - name: Restore the previous mutation results + id: incremental + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: reports/stryker-incremental.json + key: stryker-incremental-${{ hashFiles('src/**/constants.ts', 'src/_internals/constants/**', 'src/_internals/test/**', 'src/index.ts', 'package-lock.json', 'stryker.config.json', 'vite.config.ts') }}-${{ github.ref }}-${{ github.sha }} + restore-keys: | + stryker-incremental-${{ hashFiles('src/**/constants.ts', 'src/_internals/constants/**', 'src/_internals/test/**', 'src/index.ts', 'package-lock.json', 'stryker.config.json', 'vite.config.ts') }}-${{ github.ref }}- + stryker-incremental-${{ hashFiles('src/**/constants.ts', 'src/_internals/constants/**', 'src/_internals/test/**', 'src/index.ts', 'package-lock.json', 'stryker.config.json', 'vite.config.ts') }}-refs/heads/main- + + # Stryker only compares a mutant's own code and its tests' source: the prune script drops the + # results of every file that changed or imports a changed file (a shared helper), and every + # result that came from a test reaching a changed file, so those mutants run again. + - name: Mutation test the changed code if: ${{ github.event_name != 'push' }} - run: npm run test:mutation + run: | + node ./scripts/prune-stryker-incremental.ts + npm run test:mutation -- --incremental - # On `main` the dashboard reporter also publishes the score to dashboard.stryker-mutator.io; - # pull requests keep the local reporters only, so a fork never needs the key. + # On `main` every mutant is tested again (--force), so the published score and the baseline + # the pull requests start from never carry an old result. The dashboard reporter also + # publishes the score to dashboard.stryker-mutator.io; pull requests keep the local reporters + # only, so a fork never needs the key. - name: Mutation test every file and publish the score if: ${{ github.event_name == 'push' }} env: STRYKER_DASHBOARD_API_KEY: ${{ secrets.STRYKER_DASHBOARD_API_KEY }} - run: npm run test:mutation -- --reporters clear-text,progress,html,json,dashboard + run: npm run test:mutation -- --incremental --force --reporters clear-text,progress,html,json,dashboard + + # Saved even when a mutant survives: the results of the mutants that were tested stay valid. + - name: Save the mutation results + if: ${{ always() && hashFiles('reports/stryker-incremental.json') != '' }} + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: reports/stryker-incremental.json + key: ${{ steps.incremental.outputs.cache-primary-key }} - name: Upload the mutation report if: ${{ always() }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 35cdeb5b..9e271944 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -288,9 +288,18 @@ pull request so the CI result is not a surprise. - **Mutation testing** (`npm run test:mutation`, [Stryker](https://stryker-mutator.io), config in `stryker.config.json`): mutates every source file except tests, constants and the test runtime shims, and runs the vitest suite against each mutant. The score must stay at or above - the `thresholds.break` value in the config. The `Mutation tests` workflow runs the whole suite on - every pull request and on every push to `main`, like the other checks (about 3 minutes); the - HTML report is attached to the run as the `mutation-report` artifact. A surviving mutant + the `thresholds.break` value in the config. The `Mutation tests` workflow runs on every pull request and on + every push to `main`. It uses Stryker's + [incremental mode](https://stryker-mutator.io/docs/stryker-js/incremental/): a pull request + only tests the mutants whose code or covering tests changed since its last run (or since + `main`'s), and `main` tests every mutant again. Before the run, `scripts/prune-stryker-incremental.ts` + drops from the saved results every file that changed or imports a changed file, and every result + that came from a test reaching a changed file, because Stryker alone would keep the results of a + function whose helper changed. A change to a constants table, a + test helper, a dependency or the config makes every mutant run. Locally, + `node scripts/prune-stryker-incremental.ts && npm run test:mutation -- --incremental` does the same + against your previous run. The HTML report is attached to the run as the + `mutation-report` artifact. A surviving mutant means a test is missing (add one, with a literal expectation) or the code has a branch that can never matter (simplify it). Only when a mutant is truly equivalent, use `// Stryker disable next-line : ` right above the line; that is the one diff --git a/scripts/prune-stryker-incremental.ts b/scripts/prune-stryker-incremental.ts new file mode 100644 index 00000000..6d9f7c96 --- /dev/null +++ b/scripts/prune-stryker-incremental.ts @@ -0,0 +1,207 @@ +#!/usr/bin/env node + +/** + * Removes from Stryker's incremental report every saved result that a change can have made + * stale, so that `stryker run --incremental` tests those mutants again: + * + * - every mutant of a mutated file that changed, or that imports (directly or through other + * files) a file that changed; + * - every other mutant whose result came from a test in a test file that imports (directly or + * through other files) a file that changed: the test that killed a killed mutant, or any + * covering test of a mutant with another status. + * + * Stryker reuses a mutant's result when the mutant's own code and the source of its covering + * tests did not change, and looks at nothing else. A change to `_internals/format` would keep + * every `format*` mutant's old result (the first case), and a change to `format-cpf` that stops + * calling a helper would keep that helper's mutants "killed" by the `format-cpf` tests (the + * second). A mutant missing from the report is a new one to Stryker. A killed mutant whose + * killing test reaches no changed file is still killed, so the tests that only cover it (like + * `index.test.ts`, which imports every module) do not send it back. The previous source of + * every file is in the report itself, so no git history is needed. Constants, test helpers, + * dependencies and the config are not mutated and are covered by the cache key in the + * `Mutation tests` workflow. + * + * Usage: + * node scripts/prune-stryker-incremental.ts + * Rewrite `reports/stryker-incremental.json` (or the config's `incrementalFile`) in place. + * Does nothing when the file does not exist. + */ + +import { existsSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { dirname, isAbsolute, join, matchesGlob, relative, resolve } from "node:path"; + +const ROOT = join(import.meta.dirname, ".."); + +type StrykerConfig = { mutate: string[]; incrementalFile?: string }; +type Mutant = { status: string; killedBy?: string[]; coveredBy?: string[] }; +type IncrementalReport = { + files: Record; + testFiles?: Record; +}; + +const isStrykerConfig = (value: unknown): value is StrykerConfig => + typeof value === "object" && + value !== null && + "mutate" in value && + Array.isArray(value.mutate) && + value.mutate.every((pattern) => typeof pattern === "string") && + (!("incrementalFile" in value) || typeof value.incrementalFile === "string"); + +const isIncrementalReport = (value: unknown): value is IncrementalReport => + typeof value === "object" && + value !== null && + "files" in value && + typeof value.files === "object" && + value.files !== null && + Object.entries(value.files).every( + ([, file]: [string, unknown]) => + typeof file === "object" && + file !== null && + "source" in file && + typeof file.source === "string" && + "mutants" in file && + Array.isArray(file.mutants) && + file.mutants.every( + (mutant: unknown) => + typeof mutant === "object" && + mutant !== null && + "status" in mutant && + typeof mutant.status === "string", + ), + ) && + (!("testFiles" in value) || + (typeof value.testFiles === "object" && + value.testFiles !== null && + Object.entries(value.testFiles).every( + ([, file]: [string, unknown]) => + typeof file === "object" && file !== null && "tests" in file && Array.isArray(file.tests), + ))); + +const parsedConfig: unknown = JSON.parse(readFileSync(join(ROOT, "stryker.config.json"), "utf8")); +if (!isStrykerConfig(parsedConfig)) throw new Error("stryker.config.json has no mutate patterns"); +const config = parsedConfig; +const reportPath = join(ROOT, config.incrementalFile ?? "reports/stryker-incremental.json"); + +/** + * Whether Stryker mutates the file, by the `mutate` patterns of the config (a `!` pattern + * excludes). + * @param {string} file - The path relative to the repository root, with `/` separators. + * @returns {boolean} `true` when the file is mutated. + */ +function isMutated(file: string): boolean { + const included = config.mutate.filter((pattern) => !pattern.startsWith("!")); + const excluded = config.mutate + .filter((pattern) => pattern.startsWith("!")) + .map((pattern) => pattern.slice(1)); + return ( + included.some((pattern) => matchesGlob(file, pattern)) && + !excluded.some((pattern) => matchesGlob(file, pattern)) + ); +} + +/** + * Resolves a relative import specifier the way the source is written: without an extension, + * pointing at a `.ts` file or at a directory's `index.ts`. + * @param {string} importer - The importing file, relative to the repository root. + * @param {string} specifier - The specifier as written in the import. + * @returns {string | undefined} The imported file relative to the repository root, or + * `undefined` for a package import or a path that does not exist. + */ +function resolveImport(importer: string, specifier: string): string | undefined { + if (!specifier.startsWith(".")) return undefined; + const base = join(dirname(importer), specifier); + const candidates = [base, `${base}.ts`, join(base, "index.ts")]; + return candidates.find( + (candidate) => candidate.endsWith(".ts") && existsSync(join(ROOT, candidate)), + ); +} + +/** + * Lists, for every source file, the files that import it. + * @param {string[]} files - Every `.ts` file under `src`, relative to the repository root. + * @returns {Map>} The importers of each imported file. + */ +function collectImporters(files: string[]): Map> { + const importers = new Map>(); + const specifierPattern = /(?:\bfrom\s*|\bimport\s*\(?\s*)["']([^"']+)["']/g; + for (const file of files) { + const source = readFileSync(join(ROOT, file), "utf8"); + for (const [, specifier] of source.matchAll(specifierPattern)) { + const imported = resolveImport(file, specifier); + if (imported === undefined) continue; + const set = importers.get(imported) ?? new Set(); + set.add(file); + importers.set(imported, set); + } + } + return importers; +} + +if (!existsSync(reportPath)) { + console.log(`No incremental report at ${relative(ROOT, reportPath)}, nothing to prune.`); + process.exit(0); +} + +const parsedReport: unknown = JSON.parse(readFileSync(reportPath, "utf8")); +if (!isIncrementalReport(parsedReport)) + throw new Error(`${relative(ROOT, reportPath)} is not a Stryker report`); +const report = parsedReport; +const toRelative = (file: string): string => + (isAbsolute(file) ? relative(ROOT, file) : relative(ROOT, resolve(ROOT, file))) + .split("\\") + .join("/"); +const previousSources = new Map( + Object.entries(report.files).map(([file, { source }]) => [toRelative(file), source]), +); + +const sourceFiles = readdirSync(join(ROOT, "src"), { recursive: true, encoding: "utf8" }) + .map((file) => `src/${file.split("\\").join("/")}`) + .filter((file) => file.endsWith(".ts")); + +// A mutated file is changed when its source differs from the report's, or when the report does +// not have it (a new file). +const changed = sourceFiles.filter( + (file) => isMutated(file) && previousSources.get(file) !== readFileSync(join(ROOT, file), "utf8"), +); + +const importers = collectImporters(sourceFiles); +const stale = new Set(changed); +const queue = [...changed]; +for (let file = queue.pop(); file !== undefined; file = queue.pop()) { + for (const importer of importers.get(file) ?? []) { + if (stale.has(importer)) continue; + stale.add(importer); + queue.push(importer); + } +} + +// The tests of every test file that `stale` reached: their source did not change, but what they +// call did. +const staleTests = new Set( + Object.entries(report.testFiles ?? {}) + .filter(([file]) => stale.has(toRelative(file))) + .flatMap(([, { tests }]) => tests.map(({ id }) => id)), +); +const isStaleResult = (mutant: Mutant): boolean => + (mutant.status === "Killed" ? (mutant.killedBy ?? []) : (mutant.coveredBy ?? [])).some((id) => + staleTests.has(id), + ); + +let droppedFiles = 0; +let droppedMutants = 0; +for (const [file, entry] of Object.entries(report.files)) { + if (stale.has(toRelative(file))) { + droppedFiles += 1; + droppedMutants += entry.mutants.length; + delete report.files[file]; + continue; + } + const kept = entry.mutants.filter((mutant) => !isStaleResult(mutant)); + droppedMutants += entry.mutants.length - kept.length; + entry.mutants = kept; +} +writeFileSync(reportPath, JSON.stringify(report)); + +console.log( + `${changed.length} mutated file(s) changed, ${staleTests.size} test(s) reach a change; dropped ${droppedMutants} mutant result(s), ${droppedFiles} whole file(s), from ${relative(ROOT, reportPath)}.`, +);