From: Johannes Sixt <j6t@kdbg.org>
Date: Tue, 16 Feb 2021 21:45:30 GMT
Subject: Re: [PATCH v2 09/27] userdiff tests: match full hunk headers
Message-ID: <1b2cb670-b49b-b478-7f69-6d4c356c8118@kdbg.org>
In-Reply-To: <xmqqsg5vrhha.fsf@gitster.c.googlers.com>

Am 16.02.21 um 19:32 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>>>    t/t4018-diff-funcname.sh                      |  7 +++---
>>>    t/t4018/README                                | 22 +++++++++----------
>>>    t/t4018/README.ctx                            |  1 +
>>>    t/t4018/bash-arithmetic-function.ctx          |  1 +
>>>    t/t4018/bash-bashism-style-compact.ctx        |  1 +
>>>    [...and so on...]
>>
>> This is what I meant by "without burdening test writers with lots of
>> subtleties".
>>
>> I'm not a friend of this change :-(
>>
>> I think you are going overboard with required test precision. To have
>> useful tests for userdiff patterns that demonstrate its features,
>> authors should write *many* tests. The right balance should be on the
>> coverage of userdiff pattern features, not on the subtle details of
>> each and everyone of it. Requiring that many additional context files
>> makes it *really hard* to comply.
> 
> Yeah, the first time I saw the t4018 test framework appeared in my
> tree, I truly appreciated its simplicity, how the test input file is
> self-documenting and self-contained, with the clever use of "RIGHT",
> "broken" and "ChangeMe" magic tokens, admired the cleverness of the
> approach, and wished I was clever enough to invent that pattern to
> apply to other tests myself.
> 
> A little new for each and every test for the miniscule gain of
> checking which part of the function header line is extracted feels a
> bit too much noise and rubs my sense of aesthetics, spoiled by the
> existing t4018 tests, the wrong way.
> 
> This is a rough sketch of a different approach aiming for the same.
> I converted only a few files, but I hope that this is enough to
> illustrate the idea.
> 
>   t/t4018-diff-funcname.sh         | 17 ++++++++++++++---
>   t/t4018/README                   |  9 ++++++---
>   t/t4018/bash-arithmetic-function |  3 +++
>   3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git c/t/t4018-diff-funcname.sh w/t/t4018-diff-funcname.sh
> index 9675bc17db..dd79c99fc5 100755
> --- c/t/t4018-diff-funcname.sh
> +++ w/t/t4018-diff-funcname.sh
> @@ -107,10 +107,21 @@ do
>   	else
>   		result=success
>   	fi
> -	test_expect_$result "hunk header: $i" "
> +
> +	test_expect_$result "hunk header: $i" '
> +		HEAD=$(sed -n \
> +			-e "s/^.*HEADER.*|\(.*\)right\(.*\)|.*/ \1RIGHT\2/p" "$i") &&
> +
>   		git diff -U1 $i >actual &&
> -		grep '@@ .* @@.*RIGHT' actual
> -	"
> +
> +		sed -ne "s/^@@[^@]*@@//p" actual |
> +		if test -n "$HEAD"
> +		then
> +			grep -F "$HEAD"
> +		else
> +			grep "^.*RIGHT"
> +		fi
> +	'
>   done
>   
>   test_done

> diff --git c/t/t4018/bash-arithmetic-function w/t/t4018/bash-arithmetic-function
> index c0b276cb50..935f18d96d 100644
> --- c/t/t4018/bash-arithmetic-function
> +++ w/t/t4018/bash-arithmetic-function
> @@ -2,3 +2,6 @@ RIGHT() ((
>   
>       ChangeMe = "$x" + "$y"
>   ))
> +
> +
> +# HEADER |right()|
> 

Clever! Opt-in for those who desire precise tests.

-- Hannes