Subject: Re: [PATCH v2 09/27] userdiff tests: match full hunk headers

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 23 Feb 2021 14:11:40 +0100

On Wed, Feb 17 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <[email-masked]> writes:
>
>>>> +		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.
>>
>> Tests aren't only for testing a subjective "good enough" in the
>> estimation of the author of the code in question, but also for others
>> who later touch the same area and want to avoid regressions.
>>
>> Which is why I think it's an anti-pattern to use "grep SOME-SUBSTR" in
>> lieu of test_cmp if we can easily do the latter.
>
> Sounds good.  It shouldn't be too hard to satisfy both camps,
> i.e. the quoted demonstrates one way to allow test writers to
> give expectation in-place in the single test file, and replacing
> how it uses "grep" to check the output with test_cmp or whatever
> wouldn't make the resulting tests too hard to write and maintain.

It doesn't satisfy both camps, because I'd like to convert all these
tests to test_cmp because for a subsequent refactoring of userdiff.c by
me or others I don't know in advance what might break, so I'd like to
assert the exact current behavior.

Whereas your patch provides a way to opt-in individual tests to a
test_cmp-alike, but leaves the rest at grepping for the "RIGHT"
substring. Failures in the tests who aren't opted-in will be hidden.

It also means that subsequent changes to the behavior in the form of
submitted patches won't be as self-documenting, e.g. I've wondered if we
could introduce a case to balance parens in this code (sometimes C
function declarations stretch across lines), and there's e.g. the
arbitrary limit of 80 bytes on the line (which to be fair, we don't
curretly have tests for).

Anyway, as noted in [1] I don't see how this custom format of grepping
stuff out of plain-text files is simpler, particularly when its behavior
would start to rely on other things like "# HEADER |right()|" whose
behavior is a function of what we grep/sed when/where in the logic
driving the tests.

But if you & Johannes S. disagree with that I don't really say a way
forward with this series. I think e.g. squashing 09/27 into the rest
would make things simpler/less verbose, but the end-state would still be
matching the full hunk line, and if that's not something that's wanted
in any shape or form as a default...

1. https://lore.kernel.org/git/[email-masked]/