Fail CI if a critical test case disappears#65
Merged
Conversation
Requiring the fixtures (#63) catches a fixture that fails to generate, but not a TEST_CASE deleted outright or gated on something other than a fixture. CI now checks by name for the cases covering bugs that reached main and were expensive to find: the seek contract on both the plain and B-frame fixtures, the EOF drain, position-counter drift, and seek past the end. Checking names rather than a total count was deliberate. A count needs bumping on every test addition, which trains people to update it reflexively -- including when it drops for a bad reason -- and it reports "the number changed" rather than which coverage was lost. Names fail with exactly what went missing and need no maintenance when tests are added. Verified in both directions: all five resolve against the current binary, and a deliberately bogus name is reported and fails the step. The whitespace strip matters -- the heredoc is indented for YAML, and without it no name would ever match and the check would pass vacuously, which is the same silent-success shape this is meant to prevent. Fixes #58 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The check only printed on failure, so a pass that matched nothing looked exactly like a pass that matched everything -- the same silent-success shape the guard exists to catch, reintroduced in the guard itself. It now echoes each match and fails if it verified zero names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 18, 2026
kmatzen
added a commit
that referenced
this pull request
Jul 19, 2026
) ctest reports pass/fail for the binary as a whole, and --output-on-failure hides Catch2's summary on success, so a green build recorded nothing about how many cases actually ran. A case that starts skipping stays present in --list-tests (so the #65 guard misses it) and needs no fixture (so the #63 guard misses it too), yet exercises nothing. That is not hypothetical: #70 was a completely broken FFV1 path hidden behind exactly this, for however long it had been there. Every codec these tests need is installed in CI, so a skip there is a real gap rather than graceful degradation, and the check fails the build. It uses the default console reporter deliberately -- --reporter compact omits the skip count entirely, so the grep would never fire. Verified both directions against real output: the line from a run with skips matches, the current clean run does not. Also bumps actions/checkout@v4 -> v5 and actions/setup-python@v5 -> v6 across all three workflows. Both target Node 20, which GitHub is currently force-running on Node 24 behind a shim it calls temporary; when that goes they break, for reasons unrelated to any change here. And adds __pycache__/ to .gitignore, which has been dirtying git status since the Python bindings landed. Fixes #66, fixes #68 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #58.
#63 makes CI fail if a fixture fails to generate. That covers the mechanism by which the suite had actually been shrinking, but not a
TEST_CASEdeleted outright, or one gated on something other than a fixture.CI now checks by name for the cases covering bugs that reached
mainand were expensive to find:Names, not a count
#58 suggested a minimum test-case count. I argued against it in the issue and still think names are the better shape:
Verified both directions
All five names resolve against the current binary; a deliberately bogus name is reported and fails the step.
The whitespace strip is load-bearing and worth a look in review. The heredoc has to be indented to satisfy YAML, so each name arrives with leading spaces and would never match the
--list-testsoutput. Without the strip, the loop matches nothing and the step passes — the same silent-success shape this check exists to prevent. I only caught it because I ran the logic before trusting it.🤖 Generated with Claude Code