Re: [PATCH 04/29] log: add exhaustive tests for pattern style options & config

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Add exhaustive tests for how the different grep.patternType options &
> the corresponding command-line options affect git-log.
> ...
> The patterns being passed to fixed/basic/extended/PCRE are carefully
> crafted to return the wrong thing if the grep engine were to pick any
> other matching method than the one it's told to use.

That is good; it may be even more helpful to the readers if they are
given a brief in-code comment.  I had to scratch head while reading
them.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/t4202-log.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index f577990716..6d1411abea 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -262,7 +262,23 @@ test_expect_success 'log --grep -i' '
>  
>  test_expect_success 'log -F -E --grep=<ere> uses ere' '
>  	echo second >expect &&
> -	git log -1 --pretty="tformat:%s" -F -E --grep=s.c.nd >actual &&
	# BRE would need \(s\) to do the same.
> +	git log -1 --pretty="tformat:%s" -F -E --grep="(s).c.nd" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success PCRE 'log -F -E --perl-regexp --grep=<pcre> uses PCRE' '
> +	test_when_finished "rm -rf num_commits" &&
> +	git init num_commits &&
> +	(
> +		cd num_commits &&
> +		test_commit 1d &&
> +		test_commit 2e
> +	) &&
> +	echo 2e >expect &&
	# In PCRE \d in [\d] is like saying "0-9", and match '2' in 2e
> +	git -C num_commits log -1 --pretty="tformat:%s" -F -E --perl-regexp --grep="[\d]" >actual &&
> +	test_cmp expect actual &&
> +	echo 1d >expect &&
	# In ERE [\d] is bs or letter 'd' and would not match '2' in '2e'
	# but does match 'd' in '1d'
> +	git -C num_commits log -1 --pretty="tformat:%s" -F -E --grep="[\d]" >actual &&
>  	test_cmp expect actual
>  '
>  
> @@ -280,6 +296,65 @@ test_expect_success 'log with grep.patternType configuration and command line' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'log with various grep.patternType configurations & command-lines' '
> +	git init pattern-type &&
> +	(
> +		cd pattern-type &&
> +		test_commit 1 file A &&
> +		test_commit "(1|2)" file B &&
> +
> +		echo "(1|2)" >expect.fixed &&
> +		cp expect.fixed expect.basic &&
> +		cp expect.fixed expect.extended &&
> +		cp expect.fixed expect.perl &&
> +
		# Fixed finds these literally
> +		git -c grep.patternType=fixed log --pretty=tformat:%s \
> +			--grep="(1|2)" >actual.fixed &&
		# BRE matches (, |, and ) literally
> +		git -c grep.patternType=basic log --pretty=tformat:%s \
> +			--grep="(.|.)" >actual.basic &&
		# ERE needs | quoted with bs to match | literally
> +		git -c grep.patternType=extended log --pretty=tformat:%s \
> +			--grep="\|2" >actual.extended &&

If we use BRE by mistake, wouldn't this pattern actually find
"(1|2)" anyway, without skipping it and show "1 file A" instead?

> +		if test_have_prereq PCRE
> +		then
> +			git -c grep.patternType=perl log --pretty=tformat:%s \
> +				--grep="[\d]\|" >actual.perl
			# Only PCRE would match [\d]\| with "(1|2)" due to [\d]
> +		fi &&
> +		test_cmp expect.fixed actual.fixed &&
> +		test_cmp expect.basic actual.basic &&
> +		test_cmp expect.extended actual.extended &&
> +		if test_have_prereq PCRE
> +		then
> +			test_cmp expect.perl actual.perl
> +		fi &&

It could be just a style thing, but I would actually find it easier
to follow if the above two "only with PCRE" tests, i.e. running test
and taking its output in actual.perl and comparing it with
expect.perl, were inside a single "if test_have_prereq PCRE" block.