The history check passed with its safeguards removed
Problem
A check proves that main holds only the first commit and merges of pull requests made by Bonsung. It had tests for the rules that judge a list of commits. The code that reads that list from git had no test at all, and the rule that a merge has exactly two parents could be deleted without anything failing. The check looked protected and was not.
Symptoms
- Adding
--mergesto the command that reads the chain hid every direct commit from the check, and the suite stayed green. - Removing the two-parent condition stayed green.
- Nothing pinned which commit the history had to start from.
What Didn't Work
- Testing only the judging rules with hand-made commit lists. Those lists never went through the reader, so the reader could change freely.
Solution
The reader got a test with a scratch repository built in the test: a first commit, a branch, a merge with a pull request subject, and a commit made straight onto the main branch. The test asserted that the reader returned exactly those commits, oldest first, with their parents and subjects. A separate test pinned the repository's first commit, so a history rewritten from the start, or one from another repository, failed.
Each safeguard was then removed or altered on purpose. Adding the merges-only flag, removing the first-parent flag and removing the two-parent condition each turned a test red.
Why This Works
The scratch repository exercises the real command against real git behaviour, so a change to the command changes the result. Deliberate damage shows that each safeguard is held by at least one test.
Prevention
- Test the code that reads from the outside world, not only the rules that judge what it returns.
- For a guard, remove each safeguard in turn and confirm a test fails. A safeguard no test notices is not a safeguard.
- Pin the anchor a check depends on, such as the first commit, with its own test.
Related Issues
- Tests that could not fail describes the habit of seeing a new test fail first.