fix(coverage): quote coverage paths baked into the DEBUG trap - #1246
Merged
Conversation
A coverage path containing a space produced a trap that does not parse:
"syntax error near unexpected token" on every executed line, 75 of them for a
one-test run, coverage reported 0% because record_line never ran, and the test
itself was marked failed although its assertion passed.
build_trap_glob bakes the paths into a `case` as syntax on purpose -- `|`
arriving through a variable would not split -- but the literal segments were
interpolated unquoted. Quote those and leave `*` and `|` as syntax. That also
reads more correctly: a coverage path is a literal, so `[` or `?` in a
directory name must not act as a pattern.
An apostrophe has to be escaped as '\'', and that cannot be written inline:
the replacement in ${var//pat/repl} processes backslashes and hands it back
mangled, so it is built from variables and verified by round-tripping through
a real `case`.
Three tests asserted the glob's spelling and broke on the quoting although the
behaviour was unchanged. The file already had a glob_matches helper that evals
the pattern; they now assert what the glob admits, which is also the only way
to tell a pattern that parses from one that parses and matches nothing.
Closes #1245
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.
🤔 Background
Related #1245
--coverage-paths "my src/"produced a DEBUG trap that does not parse:The worst part is the third line: the test is reported failed while its own assertion passed, so a correct suite goes red for a reason unrelated to the code under test. Coverage also silently reads 0%.
💡 Changes
*and|as syntax — the alternation still has to reach the trap as syntax, which is why the paths are baked in at all'\'', built from variables because the replacement in${var//pat/repl}processes backslashes and mangles the inline spelling; verified by round-tripping through a realcaseglob_matcheshelper the same file already had — a spelling assertion cannot tell a pattern that parses from one that parses and matches nothingQuoting is also the more correct reading: a coverage path is a literal, so a
[or?in a directory name should not act as a pattern.