Re: [PATCH v10 3/9] grep tests: add missing "grep.patternType" config tests

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

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

> +	test_expect_success "grep $L with grep.extendedRegexp is last-one-wins" '

The statement is true, but ...

> +		echo "${HC}ab:a+bc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=true \
> +			-c grep.patternType=basic \
> +			-c grep.extendedRegexp=false \

... I do not think that is what is tested here.  It checks that
the value of grep.extendedRegexp is irrelevant when grep.patternType
is anything but default, no?  In other words, if you lose the last
one that overrides the earlier .extendedRegexp=true, this should
still pass, no?

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp is last-one-wins & defers to grep.patternType" '
> +		echo "${HC}ab:abc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=true \
> +			-c grep.patternType=extended \
> +			-c grep.extendedRegexp=false \

Likewise.  grep.extendedRegexp being last-one-wins has no relevance
to this result, just like the previous one.  I would understand:

	grep $L with grep.patternType=extended pays no atention to grep.extendedRegexp

though.

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp and grep.patternType are both last-one-wins independently (ERE)" '
> +		echo "${HC}ab:abc" >expected &&
> +		git \
> +			-c grep.patternType=fixed \
> +			-c grep.extendedRegexp=true \
> +			-c grep.patternType=default \
> +			grep "a+b*c" $H ab >actual &&

.patternType=default lets .extendedRegexp to decide, and we get ERE.

This does have a correct name and tests the right thing (but
grep.extendedRegexp is set only once in this, so "last-one-wins" is
technically correct but may not be very useful thing to stress upon
;-).

> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp and grep.patternType are both last-one-wins independently (ERE)" '
> +		echo "${HC}ab:abc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=false \
> +			-c grep.patternType=default \
> +			-c grep.extendedRegexp=true \

.patternType=default lets .extendedRegexp to decide, and we get ERE.

Future readers might wonder why we are are testing the same thing
again, without enough imagination to guess what faulty implementation
is possible to require this test ;-).

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp and grep.patternType are both last-one-wins independently (BRE)" '
> +		echo "${HC}ab:a+bc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=true \
> +			-c grep.extendedRegexp=false \
> +			grep "a+b*c" $H ab >actual &&

.patternType=default that is implicit is the "last" value and lets
.extendedRegexp to decide, and we get BRE.

> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp and grep.patternType are both last-one-wins independently (BRE)" '
> +		echo "${HC}ab:abc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=false \
> +			-c grep.extendedRegexp=true \
> +			-c grep.patternType=default \

Similar.  As the default for .patternType is the default anyway,
even with an implementation that commits the choice too early would
get this right, as it assumes that .patternType is the default when
it sees true given to .extendedRegexp variable.

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.extendedRegexp and grep.patternType are both last-one-wins independently (BRE)" '
> +		echo "${HC}ab:a+bc" >expected &&
> +		git \
> +			-c grep.patternType=default \
> +			-c grep.extendedRegexp=true \
> +			-c grep.patternType=basic \

The last value for .patternType not being "default" makes it the
only thing that matters.  This one also tests the right thing.

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +	test_expect_success "grep $L with grep.patternType=extended and grep.patternType=default" '
> +		echo "${HC}ab:a+bc" >expected &&
> +		git \
> +			-c grep.patternType=extended \
> +			-c grep.patternType=default \

The last value for .patternType being "default" lets .extendedRegexp
to decide, but .extendedRegexp is left to its default, which is
false, so we should get BRE.

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep $L with grep.patternType=[extended -> default -> fixed]" '
> +		echo "${HC}ab:a+b*c" >expected &&
> +		git \
> +			-c grep.patternType=extended \
> +			-c grep.patternType=default \
> +			-c grep.patternType=fixed \
> +			grep "a+b*c" $H ab >actual &&

OK.

> +		test_cmp expected actual
> +	'
> +
>  	test_expect_success "grep $L with grep.patternType=extended and grep.extendedRegexp=false" '
>  		echo "${HC}ab:abc" >expected &&
>  		git \
> @@ -478,6 +565,15 @@ do
>  		test_cmp expected actual
>  	'
>  
> +	test_expect_success "grep $L with grep.extendedRegexp=false and grep.patternType=default" '
> +		echo "${HC}ab:abc" >expected &&
> +		git \
> +			-c grep.extendedRegexp=false \
> +			-c grep.patternType=extended \

Again, .patternType not being "default" makes .extendedRegexp not to
matter at all, and we get ERE.

> +			grep "a+b*c" $H ab >actual &&
> +		test_cmp expected actual
> +	'
> +
>  	test_expect_success "grep $L with grep.extendedRegexp=true and grep.patternType=basic" '
>  		echo "${HC}ab:a+bc" >expected &&
>  		git \

All tests look correct.  The first two had labels that missed the
point of what is being tested, which need fixing.  Other tests were
correct, I know that all of them may have helped to catch mistakes
made in past iterations of this series, but without knowing the past
mistakes, new readers may not get exactly the point of these tests.

Thanks.