| Message ID | patch-v2-3.8-41e38ebb32c-20211110T013632Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | grep: simplify & delete code by changing obscure cfg variable behavior | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Extend the tests added in my 9df46763ef1 (log: add exhaustive tests > for pattern style options & config, 2017-05-20) to check not only > whether "git log" handles "grep.patternType", but also "git show" > etc. > > It's sufficient to check whether a PCRE regex matches for the purposes > of this test, we otherwise assume that it's running the same code as > "git log", whose behavior is tested more exhaustively by test added in > 9df46763ef1e. I do agree with the reasoning that it is sufficient to check with only one kind, but it is unclear if PCRE is so special and if so why. Wouldn't testing with, say grep.patternType=extended, alone also be sufficient? The reason I am asking it is mostly because the above description is insufficient to answer these questions, but also because we can lose the prerequisite and gain a bit wider test coverage, if we use a pattern that is not PCRE. Knowing that "show", "whatchanged", "reflog", and "format-patch" all share the same backend in builtin/log.c, I am not sure how much value the new tests add. Testing the "--grep-reflog" option and the "git rev-list" command may exercise a bit different code path, though. > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t4202-log.sh | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/t/t4202-log.sh b/t/t4202-log.sh > index 7884e3d46b3..a114c49ef27 100755 > --- a/t/t4202-log.sh > +++ b/t/t4202-log.sh > @@ -449,6 +449,22 @@ test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurati > ) > ' > > +for cmd in show whatchanged reflog format-patch > +do > + myarg= > + if test "$cmd" = "format-patch" > + then > + myarg="HEAD~.." > + fi I prefer to send the output that ought to contain the grep hits consistently to "actual", i.e. case "$cmd" in format-patch) myarg="--stdout -1" ;; *) myarg= ;; esac instead of sending only the filenames. I also prefer "case/esac" as it would be more concise when we need to tweak myarg per command later. > + test_expect_success PCRE "$cmd: understands grep.patternType=perl, like 'log'" ' > + git -c grep.patternType=fixed -C pattern-type $cmd --grep="1(?=\|2)" $myarg >actual && > + test_must_be_empty actual && > + git -c grep.patternType=perl -C pattern-type $cmd --grep="1(?=\|2)" $myarg >actual && > + test_file_not_empty actual > + ' > +done > + > test_expect_success 'log --author' ' > cat >expect <<-\EOF && > Author: <BOLD;RED>A U<RESET> Thor <author@example.com> Thanks.
diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 7884e3d46b3..a114c49ef27 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -449,6 +449,22 @@ test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurati ) ' +for cmd in show whatchanged reflog format-patch +do + myarg= + if test "$cmd" = "format-patch" + then + myarg="HEAD~.." + fi + + test_expect_success PCRE "$cmd: understands grep.patternType=perl, like 'log'" ' + git -c grep.patternType=fixed -C pattern-type $cmd --grep="1(?=\|2)" $myarg >actual && + test_must_be_empty actual && + git -c grep.patternType=perl -C pattern-type $cmd --grep="1(?=\|2)" $myarg >actual && + test_file_not_empty actual + ' +done + test_expect_success 'log --author' ' cat >expect <<-\EOF && Author: <BOLD;RED>A U<RESET> Thor <author@example.com>
Extend the tests added in my 9df46763ef1 (log: add exhaustive tests for pattern style options & config, 2017-05-20) to check not only whether "git log" handles "grep.patternType", but also "git show" etc. It's sufficient to check whether a PCRE regex matches for the purposes of this test, we otherwise assume that it's running the same code as "git log", whose behavior is tested more exhaustively by test added in 9df46763ef1e. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t4202-log.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)