skill: merge-base test check — do a PR's new tests fail without the change they cover?
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 0
- Forks
- 1
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 7
Description
Problem
Tests get shipped that pass even when the code they cover is deleted. In one repo this was the most frequent defect class found in review over a week — across several PRs, including one where three of four new tests stayed green with the guard they named removed.
A written agreement ("a guard is not finished until you have watched it fail") did not change behaviour: it was in a file that had already been read. A bespoke mutation-testing tool built to mechanise that agreement did not either, for a reason worth recording — nothing invoked it. It required the same person who skipped the rule to voluntarily hand-author an honest mutation spec and remember to run it. It also only probed what its author already thought of: the author's own mutation matrix scored 24/24 caught, while an adversarially-written set found survivors in most of the same file.
The check
Run the PR's new tests against its merge-base. A test that passes without the change it covers cannot be constraining that change.
git worktree add --detach /tmp/base "$(git merge-base origin/main HEAD)"
# overlay the PR's changed test files onto the base
cd /tmp/base && <run the scoped suite>
No spec to author, nothing pinned to source text that can rot, nothing author-selected. It reads the diff that already exists, and it is language-agnostic — it tests the tree, not the language, so it covers shell, which has no maintained mutation tooling at all.
Evidence
Run against the PR that motivated the original tooling effort: 14 seconds. Of the 11 tests new in that PR, 8 fail on the merge-base and 3 pass — and the 3 that pass are exactly the negative-space tests, the same ones that had stayed green when the guard was deleted. A check with nothing to write isolated the whole vacuous population.
The rule has to be "every", not "at least one"
"At least one new test fails on the merge-base" would have passed that PR — eight of its tests failed — and shipped all three vacuous ones. The workable form is:
Every new test must fail on the merge-base, or carry an in-diff annotation naming its positive-direction partner, e.g.
# passes-on-parent: guards the false-positive direction; the killed-guard case is test_<name>.
Negative-space tests are legitimate. The check forces classification rather than deletion, and the annotation is "I watched it fail" made mechanical, reviewable, and sitting in the diff where a reviewer sees it.
Measurement gotcha, learned the hard way
Determine per-test results by running each test individually and reading its exit status. Parsing unittest -v output with a line-oriented regex gives the wrong answer, because -v prints the test's docstring between the test name and ... ok. That mistake scored every test as failed and nearly shipped a false conclusion.
Known limits
- New source files. A new test referencing a file absent from the merge-base fails with a collection error — technically "failed", but it proves the test references the new code, not that it constrains it. Report the failure kind; treat collection errors as inconclusive rather than verified.
- Refactors legitimately stay green — needs a per-PR skip.
- Characterization tests legitimately pass on the merge-base — same annotation escape hatch.
- Scope to unit suites; long e2e suites and stubs that drift against older trees make this unreliable and slow.
Why a skill rather than documentation
It is several commands with a non-obvious measurement step, repeated for every test-bearing PR. Documentation describing it would be skipped exactly as the working agreement was. Cost is one extra scoped test run — seconds — so it can live in a pre-push hook or a path-gated CI step rather than needing to be remembered.
Acceptance criteria
- A skill/command that finds the merge-base, overlays the PR's changed test files, and runs the scoped suite
- Per-test results determined by individual exit status, not by parsing runner output
- Reports each new test as fails-on-base / passes-on-base, and distinguishes collection errors as inconclusive
- Recognises a
passes-on-parent:annotation and treats an annotated test as classified rather than failing - Refuses to run on a dirty tree; leaves no worktree or scratch state behind
- Documents that a pass is evidence about one probe, not proof the suite is sound
Out of scope
Framing this as proof of coverage. A test that fails on the merge-base is constrained by that change; it says nothing about the rest of the suite. The control that has actually caught every instance of this defect is adversarial review — this makes that review cheaper and should not be described as replacing it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the repository's existing skill or command entry point and its test conventions. Reproduce the described merge-base workflow with the scoped suite, measuring each test through its individual exit status. Done means the acceptance criteria are met, including annotation handling, inconclusive collection errors, dirty-tree refusal, and cleanup of temporary worktree state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, python
- Domain
- devtools, testing-qa, tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100