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

From: Sergey Organov <sorganov@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Philip Oakley <philipoakley@iee.email>,
	Elijah Newren <newren@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 7/9] t9902: fix completion tests for log.d* to match log.diffMerges
Date: Thu, 08 Apr 2021 17:41:04 +0300	[thread overview]
Message-ID: <875z0wdekf.fsf@osv.gnss.ru> (raw)
In-Reply-To: <87y2dtitlp.fsf@evledraar.gmail.com> (=?utf-8?B?IsOGdmFyIEFy?= =?utf-8?B?bmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 08 Apr 2021 01:05:06 +0200")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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 <sorganov@gmail.com>
>> ---
>>  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.

> Having read this far most of what you have in this 9 patch series
> could/should be squashed into something much smaller, e.g. tests being
> added for code added in previous steps, let's add the tests along with
> the code since this isn't such a large change.

In general, I try to make commits as small as possible, but if you
prefer tests to be included with the code in the same commit, – that's
fine with me too.

Will meld new tests into code commits for the next re-roll.

Thanks!

-- Sergey Organov