Subject: Re: [PATCH 1/2] pretty: add %(describe)

From: René Scharfe. <[email-masked]>
Date: Tue, 2 Mar 2021 17:00:25 +0100

Am 01.03.21 um 18:45 schrieb Junio C Hamano:
> René Scharfe. <[email-masked]> writes:
>
>> Subject: [PATCH] t4205: assert %(describe) test coverage
>>
>> Document that the test is covering both describable and
>> undescribable commits.
>>
>> Suggested-by: Ævar Arnfjörð Bjarmason <[email-masked]>
>> Signed-off-by: René Scharfe <[email-masked]>
>> ---
>>  t/t4205-log-pretty-formats.sh | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
>> index b47a0bd9eb..cabdf7d57a 100755
>> --- a/t/t4205-log-pretty-formats.sh
>> +++ b/t/t4205-log-pretty-formats.sh
>> @@ -965,8 +965,17 @@ test_expect_success 'log --pretty=reference is colored appropriately' '
>>  test_expect_success '%(describe) vs git describe' '
>>  	git log --format="%H" | while read hash
>>  	do
>> -		echo "$hash $(git describe $hash)"
>> +		if desc=$(git describe $hash)
>> +		then
>> +			: >expect-contains-good
>> +		else
>> +			: >expect-contains-bad
>> +		fi &&
>> +		echo "$hash $desc"
>>  	done >expect &&
>> +	test_path_exists expect-contains-good &&
>> +	test_path_exists expect-contains-bad &&
>
> Hmph, I am not sure why we want temporary files for this (and I
> doubt this "documenting" adds that much value to the tests to begin
> with), but OK.  Will queue.

Variables would suffice, but make debugging harder.  test_path_exists
will at least print a suggestive file name.  Perhaps we should add a
test_assert?

The added checks guard against neutering the test accidentally e.g.
by tagging the currently undescribable commit in the setup phase.
That would be hard to detect without it.

>
>>  	git log --format="%H %(describe)" >actual 2>err &&
>>  	test_cmp expect actual &&
>>  	test_must_be_empty err
>> --
>> 2.30.1