Skip to content

chore(common-utils): add Stryker mutation testing - #2755

Open
jordan-simonovski wants to merge 1 commit into
mainfrom
feat/mutation-testing-common-utils
Open

chore(common-utils): add Stryker mutation testing#2755
jordan-simonovski wants to merge 1 commit into
mainfrom
feat/mutation-testing-common-utils

Conversation

@jordan-simonovski

Copy link
Copy Markdown
Contributor

Adds Stryker to common-utils as a local dev tool. Nothing runs in CI; nothing is gated on it.

Why

common-utils has a coverage ratchet in jest.config.js sitting at 86% statements, with a comment saying it should only go up. That's a good floor, but coverage only proves a line executed — not that a test would fail if the line were wrong. Mutation testing closes that gap: Stryker edits the source in small ways and reports which edits no test caught. A surviving mutant is a missing assertion.

It found real gaps immediately. From a run over five files:

File                    | % score | killed | survived | no cov |
 dashboardValidation.ts |    0.00 |      0 |        0 |     50 |
 filters.ts             |   50.47 |    207 |      183 |     27 |
 guards.ts              |   82.76 |     24 |        3 |      2 |
 macros.ts              |   85.16 |    108 |       17 |      2 |
 rawSqlParams.ts        |  100.00 |      6 |        0 |      0 |

Two concrete examples from filters.ts, both surviving:

  • if (/\bDate32\b/.test(chType)) -> if (false) — nothing asserts on the Date32 branch.
  • chType.match(/DateTime64\((\d+)/) -> (\d) — no test uses a two-digit precision.

dashboardValidation.ts has no unit test reaching it at all.

None of these are fixed here. This PR is just the tooling.

Usage

From packages/common-utils:

yarn dev:mutation --mutate src/filters.ts

Scope it with --mutate; a whole-package run is tens of minutes. Full notes in CONTRIBUTING.md.

The one thing to look at

The root package.json gets a resolutions entry pinning @stryker-mutator/core/minimatch to ^9.

We have a blanket "brace-expansion": "^2.1.2" resolution, which forces v2 (CJS) onto every consumer in the tree. minimatch v10's ESM build does import { expand } from 'brace-expansion', which needs v5's named exports, so Stryker crashes on startup. Pinning its minimatch to v9 sidesteps it without touching the blanket resolution.

Worth noting the blanket resolution looks obsolete and slightly harmful on its own terms. The advisory it was presumably added for (CVE-2025-5889) is fixed in 1.1.12 / 2.0.2 / 3.0.1 / 4.0.1, and nothing in the tree requests a range that would resolve below those today — the requested ranges are ^1.1.7, ^2.0.1, ^5.0.2 and ^5.0.5. So the override forces v2 onto packages asking for v1 and v5 without buying any security. Narrowing or dropping it would let this pin go away too, but that's a security-adjacent change and deserves its own PR rather than riding along with dev tooling.

@stryker-mutator/core is the only Stryker package depending on minimatch, and it only uses new Minimatch() and minimatch(), both unchanged between v9 and v10.

Checks

  • yarn install --immutable clean; the lockfile change is purely additive (no existing package's resolved version moved).
  • yarn knip clean.
  • common-utils ci:lint and ci:unit pass (1556 tests).
  • Stryker's typescript-checker works against the repo's TypeScript 6 — it discards non-compiling mutants rather than erroring, which is what keeps runs affordable.

Coverage says a line ran, not that a test would fail if it were wrong.
Stryker edits the source and reports which edits the tests miss.

Set up in common-utils only, run on demand via `yarn dev:mutation`. Not
wired into CI.

Pins @stryker-mutator/core/minimatch to ^9: the blanket brace-expansion
resolution forces v2, and minimatch v10's ESM build needs v5's named
exports.
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Jul 30, 2026 10:01am
hyperdx-storybook Ready Ready Preview Jul 30, 2026 10:01am

Request Review

@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: fc1b883

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Jul 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 4
  • Production lines changed: 32
  • Branch: feat/mutation-testing-common-utils
  • Author: jordan-simonovski

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds local Stryker mutation-testing support for common-utils.

  • Adds Stryker core, Jest runner, TypeScript checker, package script, and configuration.
  • Pins Stryker’s minimatch dependency to v9 to remain compatible with the repository-wide brace-expansion resolution.
  • Documents scoped mutation-testing workflows and ignores generated reports, cache files, and temporary files.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The mutation-testing command, Jest and TypeScript configuration paths, source globs, generated-output ignores, dependency declarations, and documented package-relative workflow are mutually consistent.

Important Files Changed

Filename Overview
packages/common-utils/stryker.config.js Configures Stryker to use the package’s existing Jest and TypeScript configurations, exclude tests, cache results, and generate local reports.
packages/common-utils/package.json Adds aligned Stryker development dependencies and a local mutation-testing command without affecting production dependencies.
package.json Adds a targeted Yarn resolution that supplies minimatch v9 specifically to Stryker core.
yarn.lock Records the added Stryker toolchain and its transitive dependencies, including minimatch v9.
CONTRIBUTING.md Documents scoped mutation-test usage, output interpretation, performance expectations, and the dependency workaround.

Reviews (1): Last reviewed commit: "chore(common-utils): add Stryker mutatio..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 243 passed • 1 skipped • 1026s

Status Count
✅ Passed 243
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is dev-only tooling: no production code path, no CI gate, and packages/common-utils is private: true, so nothing here can reach a published artifact or a Docker image. Nothing rose above P2 once re-graded. Verified clean during review: the mutate exclusion !src/**/__tests__/** correctly covers both unit (**/__tests__/*.test.ts) and integration (**/__tests__/*.int.test.ts) suites, so no test file is mutated as source; and the three .gitignore patterns match every path Stryker actually writes (.stryker-tmp/, the html reporter's reports/mutation/, and the default incrementalFile at reports/stryker-incremental.json).

🟡 P2 — recommended

  • package.json:80 — The nested @stryker-mutator/core/minimatch pin is load-bearing for Stryker startup, but its only explanation lives in CONTRIBUTING.md, and it sits in a resolutions block whose other 14 entries are all transitive security overrides — so it reads as a CVE mitigation to anyone auditing that block and gives no signal that narrowing the sibling brace-expansion entry, or bumping @stryker-mutator/core past a minimatch v9 requirement, will break yarn dev:mutation with an opaque module-resolution error that no CI job would catch.
    • Fix: Add a self-documenting marker key adjacent to the pin (or split dev-tooling overrides into their own labelled block) naming both the Stryker dependency and the brace-expansion coupling, so the constraint is visible at the point of edit rather than only in prose.
    • maintainability, project-standards
  • CONTRIBUTING.md:150ignoreStatic: true removes module-level mutants from both execution and the score denominator — by the config's own estimate ~10% of mutants in this package — but the "how to read the output" paragraph documents only Survived and NoCoverage, so a developer who reads the guide without opening stryker.config.js can take a high score as evidence that module-level constants, regexes, and lookup tables are asserted on when they were never mutated.
    • Fix: Extend the output-interpretation paragraph to state that static/module-level mutants are skipped entirely and are excluded from the score.
    • testing
🔵 P3 nitpicks (5)
  • CONTRIBUTING.md:140 — The "everything you've changed off main" recipe resolves origin/main from the contributor's local remote-tracking ref, which is only as fresh as their last git fetch, so a stale ref silently narrows or widens the --mutate file list with no warning; it also hardcodes origin/main where nx.json sets affected.defaultBase to main.
    • Fix: Prepend a git fetch origin main to the recipe and align the base with the repo's existing nx affected.defaultBase convention.
    • testing, project-standards
  • CONTRIBUTING.md:140 — When the pipeline matches no files, paste -sd, - emits an empty string, so the command degrades to --mutate "" — which either mutates nothing while appearing to have run, or falls through to the config's whole-package mutate glob and starts the tens-of-minutes run this same section tells readers to avoid.
    • Fix: Guard the recipe by capturing the file list into a variable and skipping the run when it is empty.
  • AGENTS.md:127 — The packages/common-utils test-command block still lists only ci:unit, dev:unit, ci:int, and dev:int; the new capability is documented in CONTRIBUTING.md and agent_docs/development.md with overlapping but non-identical wording, leaving the auto-loaded agent guide as the one place that never mentions it and three copies to keep in sync.
    • Fix: Add a one-line dev:mutation entry to the common-utils block in AGENTS.md pointing at the CONTRIBUTING.md section, and keep the detailed prose in a single source.
    • maintainability, project-standards
  • nx.json:27 — The production named input explicitly excludes jest.config.[jt]s and eslint.config.mjs as dev-only files but has no entry for the newly added stryker.config.js, so once any target adopts inputs: ['production'] an edit to a mutation-testing config will invalidate the build cache for common-utils and its dependents.
    • Fix: Add "!{projectRoot}/stryker.config.js" alongside the existing dev-config exclusions in the production named input.
    • maintainability
  • CONTRIBUTING.md:147 — The hard-coded "~365 files" figure is a snapshot used as justification for scoping runs, and it will drift as the package grows without anything flagging it.
    • Fix: Drop the specific count and rely on the "tens of minutes" framing already given above it.
    • maintainability

Reviewers (4): testing, maintainability, project-standards, correctness-scope (orchestrator).

Coverage limitations: Bash, Grep, and Glob were non-functional for the entire session (bwrap: Can't create file at /home/.mcp.json), network fetch was unavailable, and node_modules/ is not installed in this checkout. The diff was therefore reconstructed by reading the working tree rather than from git diff, and three dispatched reviewers (correctness, security, agent-native) had not returned when this report was finalized. project-standards returned zero findings, having explicitly checked and cleared the missing .changeset/ entry under the documented "internal tooling" exemption for a private: true package.

Unverified claims — worth confirming before merge:

  • Yarn Berry precedence between the new nested @stryker-mutator/core/minimatch descriptor and any future blanket minimatch resolution was not empirically resolved. If the nested entry wins, a later security override for minimatch would be silently defeated inside that subtree.
  • Whether incremental: true can surface carried-over results from a previously differently-scoped --mutate run as if they were produced by the current run — i.e. whether a scoped run's reported score mixes fresh and cached data — depends on Stryker's incremental-matching semantics and could not be checked without executing the tool.
  • @stryker-mutator/typescript-checker was not inspected against the package's typescript: ^6.0.3 or against the isolatedModules: true / declaration: true combination in tsconfig.json, since dependencies are not installed.

Testing gaps:

  • Code reachable only from *.int.test.ts will always report NoCoverage, because the config drives jest.config.js only; the output does not distinguish "genuinely unasserted" from "integration-covered", leaving that entirely to reader judgment.
  • No mutation-score baseline or threshold is recorded anywhere, so unlike the coverage ratchet in jest.config.js there is no reference point against which a score regression would be noticeable.
  • Nothing verifies that the minimatch pin still matches what Stryker requires, or that dev:mutation stays out of ci:* / make ci-* targets; both would only surface when a developer runs the tool locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant