test(grep): compare _grep_python against a real grep (closes #63) - #79
Merged
shauryagangrade merged 2 commits intoAug 14, 2026
Merged
Conversation
…angrade#63) _grep_python is the fallback for machines with no grep binary, so the two implementations serve the same calls and were only ever checked against hand-written expected strings. That lets them drift on exactly the axes the issue names: regex flavour, glob handling, case folding, binary detection. Eleven cases over one corpus (text, nested dirs, a NUL-bearing binary, files in and out of a glob). Output is reduced to {(relative posix path, lineno, text)} before comparing, since grep joins with "/" and os.walk with os.sep -- the raw strings differ on Windows even when the matches are identical, and the assertion is about which lines matched. Gated on grep being *present* rather than on sys.platform, which is a deliberate departure from the issue text: Git for Windows ships GNU grep, so skipping by platform would skip exactly where the fallback is the default path and divergence matters most. These run green here on Windows. Two anchors keep "both agree on nothing" from passing: the plain-regex and glob cases assert the concrete file set as well as agreement. The comparison already found one real divergence. Given a single file rather than a directory, GNU grep drops the filename prefix ("1:needle at the top") while _grep_python always emits it -- so the grep tool's output shape depends on whether a grep binary is installed. Pinned in its own test rather than fixed: aligning them changes tool output and is the maintainer's call.
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
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.
Closes #63.
What
Eleven differential cases over one fixture corpus — text, nested dirs, a NUL-bearing binary, files in and out of a glob — requiring
_grep_pythonand the systemgrepto agree on: plain regex,-i, glob filter, no-match, binary skipping, regex metacharacters (a[0-9]b,^needle,line$,n..dle), and a single-file target.Output from both sides is reduced to
{(relative posix path, lineno, text)}before comparing. grep joins its argument to each entry with/whileos.walkusesos.sep, so the raw strings differ on Windows even when the matches are identical — normalising keeps the assertion about which lines matched, which is the thing that can actually drift.Two tests assert the concrete file set as well as agreement, so "both implementations agree on nothing" can't pass.
One deliberate departure from the issue
The acceptance criterion says "passes on macOS/Linux, skips cleanly on Windows". I gated on
grepbeing present instead of onsys.platform, for two reasons:_grep_pythonis the default code path and divergence matters most.grepgenuinely is absent, which is the same protection.Say the word if you'd rather have the platform gate and I'll swap it.
It already found something
Given a single file rather than a directory, GNU grep drops the filename prefix:
So
grep(pattern, path="a.txt")returns a different shape depending on whether a grep binary happens to be installed — precisely the class of drift this issue exists to catch.I pinned it in
test_single_file_output_format_divergesrather than fixing it: aligning the two changes thegreptool's output, which is your call rather than a drive-by in a test-only PR. Adding-Hto the tool's flags would do it, and I'm happy to send that separately.Verification
pytest tests/test_grep_differential.py— 11 passed.ruff checkclean.Full suite on this branch: 63 passed, 2 failed — the 2 are
test_tools.py::test_grep/::test_grep_ignore_case, which fail on a Windows checkout ofmainfor an unrelated reason I've sent as #78. On a Linux runner the whole suite should be green.