Subject: Re: [PATCH 7/9] t9902: fix completion tests for log.d* to match log.diffMerges

From: Sergey Organov <[email-masked]>
Date: Thu, 08 Apr 2021 23:26:38 +0300

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Thu, Apr 08 2021, Sergey Organov wrote:
>
>> Ævar Arnfjörð Bjarmason <[email-masked]> writes:
>>
>>> On Thu, Apr 08 2021, Sergey Organov wrote:
>>>
>>>> There were 3 completion tests failures due to introduction of
>>>> log.diffMerges configuration variable that affected the result of
>>>> completion of log.d. Fixed them accordingly.
>>>>
>>>> Signed-off-by: Sergey Organov <[email-masked]>
>>>> ---
>>>>  t/t9902-completion.sh | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
>>>> index 04ce884ef5ac..4d732d6d4f81 100755
>>>> --- a/t/t9902-completion.sh
>>>> +++ b/t/t9902-completion.sh
>>>> @@ -2306,6 +2306,7 @@ test_expect_success 'git config - variable name' '
>>>>  	test_completion "git config log.d" <<-\EOF
>>>>  	log.date Z
>>>>  	log.decorate Z
>>>> +	log.diffMerges Z
>>>>  	EOF
>>>>  '
>>>>  
>>>> @@ -2327,6 +2328,7 @@ test_expect_success 'git -c - variable name' '
>>>>  	test_completion "git -c log.d" <<-\EOF
>>>>  	log.date=Z
>>>>  	log.decorate=Z
>>>> +	log.diffMerges=Z
>>>>  	EOF
>>>>  '
>>>>  
>>>> @@ -2348,6 +2350,7 @@ test_expect_success 'git clone --config= - variable name' '
>>>>  	test_completion "git clone --config=log.d" <<-\EOF
>>>>  	log.date=Z
>>>>  	log.decorate=Z
>>>> +	log.diffMerges=Z
>>>>  	EOF
>>>>  '
>>>
>>> Commits should be made in such a way as to not break the build/tests
>>> partway through a series, which it seems is happening until this
>>> fixup.
>>
>> Yep.
>>
>> Could these tests be somehow written in a more robust manner, to be
>> protected against future additions of configuration variables that are
>> unrelated to the features being tested? If so, I'd prefer to fix them as
>> a prerequisite to the series rather than adding fixes to unrelated 
>> existing tests into my patches.
>
> Hrm? I mean if you have a commit fixing up failing tests in an earlier
> commit then that change should in one way or the other be made as part
> of that earlier change.
>
> Yes we can skip the tests or something in the meantime, which we do
> sometimes as part of some really large changes, but these can just be
> squashed, no?

I mean I don't want this change at all.

I didn't change completion mechanism, so completion tests should not
suddenly fail because of my changes. I did entirely unrelated change and
noticed the breakage only by accident, as tests even don't fail unless
you *install* git, not only make it. So, for example, just "make test"
doesn't fail, while "make install; make test" will.

It looks like something is wrong here, a bug or misfeature, or even two,
and if it's fixed before these series, I won't need this in my series at
all. Besides, that's yet another reason *not* to squash this change into
an otherwise unrelated commit.

-- Sergey Organov