Re: [PATCH v10 3/9] grep tests: add missing "grep.patternType" config tests
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 4, 2022, 23:24 UTC
- Message-ID
- <xmqqv8xui5ah.fsf@gitster.g>
- In-Reply-To
- <patch-v10-3.9-3464c76cfd7-20220204T211534Z-avarab@gmail.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 ...
Show 5 quoted lines
> + 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?
Show 10 quoted lines
> + 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.
Show 11 quoted lines
> + 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 ;-).
Show 9 quoted lines
> + 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 ;-).
Show 10 quoted lines
> + 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.
Show 9 quoted lines
> + 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.
Show 10 quoted lines
> + 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.
Show 8 quoted lines
> + 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.
Show 11 quoted lines
> + 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.
Show 15 quoted lines
> + 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.
Show 7 quoted lines
> + 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.