test(corpus): retire the stale pdf.js exclude list - #1346
Merged
Conversation
The 118-entry exclude list froze in the mozjs-default era (#84, closed); under the post-#507 upstream tree-sitter-javascript grammar every entry parses without ERROR nodes, so the list goes away entirely and the asserted file count rises 266 -> 384. The issue's probe had reported 3 residual failures, but `rg ERROR` over `bca dump` output was matching source text (MAX_ERROR, an ERRORS: property, a regex literal), not parse-error nodes, which dump renders as a `{ERROR:...}` kind. The submodule bump refreshes the 118 orphaned mozjs-era snapshots to the current output format and deletes the 5 DeepSpeech orphan snapshots (files still excluded under #86), so on-disk snapshot counts equal asserted counts for every corpus. The tests/README.md corpus table rows are corrected to match. Fixes #1282
Found by the post-#1282 audit-tests pass: insta is loud about a missing snapshot, but a stale extra .snap is asserted by nothing and silently pins values no test verifies -- the exact mechanism that let 118 pdf.js and 5 DeepSpeech snapshots pin grammar-era-old output for years. The harness now asserts every on-disk .snap under snapshots/<repo>/ corresponds to a resolved corpus file and fails naming the orphans. Verified by planting snapshots/php/bogus.php.snap: the suite passed before this change and fails naming the file after it. Refs #1282
Review follow-up to the #1282 guard: it was verified only by a one-off manual plant, so nothing committed failed if its filter silently broke, and it re-derived the corpus-file-to-snapshot mapping that act_on_file also computes, 100 lines away. Extract expected_snapshot_path as the single source of that mapping (act_on_file now derives insta's snapshot_path from it) and orphan_snapshots as a testable function. The new test runs read-only over the committed PHP snapshots, withholding one file from the expected set to simulate an orphan. Perturbation-verified: changing the guard's extension filter to match nothing fails only the new test while every corpus test stays green, which is exactly the silent breakage the corpus tests cannot see. Refs #1282
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 #1282.
What changed
tests/corpus/pdf_js_test.rs: the 118-entry mozjs-era exclude list (fix(tests,mozjs): tree-sitter-javascript parse failures skip pdf.js tests #84, closed) is retired entirely;expected_filesrises 266 → 384.big-code-analysis-output@94b60c00(already on itsmain): the 118 orphaned mozjs-era snapshots refreshed to the current output format, plus the 5 DeepSpeech orphan snapshots deleted (files still excluded under track(grammar): tree-sitter-cpp blockers behind mozcpp/deepspeech skips (#83) #86), so on-disk snapshot counts equal asserted counts for every corpus.tests/common/mod.rs): the corpus harness now fails when a.snapon disk matches no resolved corpus file, naming the orphans — closing the mechanism that let 123 stale snapshots accumulate silently.expected_snapshot_pathis the single source of the file→snapshot mapping shared byact_on_fileand the guard, andorphan_snapshotsis pinned by a read-only unit test over the committed PHP snapshots (tests/corpus/snapshot_guard_test.rs).tests/README.mdcorpus-table rows corrected (pdf.js 384/384 with no excludes; DeepSpeech 1042/1042, exclude count 7 → 5) plus a paragraph on the orphan guard; CHANGELOG entry under Unreleased.Why the fix diverges from the issue
The issue's probe (
bca dump | rg ERROR) matched source text —MAX_ERROR,ERRORS: 0, a regex literal containingNEXT ERROR— not parse-error nodes, whichbca dumprenders as a{ERROR:…}node kind. Re-probing all 118 files withrg '\{(ERROR|MISSING)'(validated against the 5 genuinely-broken DeepSpeech excludes) found zero remaining failures, so the list was deleted rather than shrunk to 3 and no successor tracking issue is needed.Validation
n1/mi_*keys → current naming, newnpa/cdafields), no structural changes except onestart_line: 2 → 1matching current behaviour; the 266 previously-asserted snapshots are byte-unchanged.make pre-commit:BCA_GATE: passon each commit.