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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 24 Feb 2021 12:12:27 +0100

On Tue, Feb 23 2021, Johannes Sixt wrote:

> Am 23.02.21 um 14:11 schrieb Ævar Arnfjörð Bjarmason:
>> On Wed, Feb 17 2021, Junio C Hamano wrote:
>>> 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]/
>
> I could live with a version of Junio's suggestion that is not opt-in,
> i.e., the checks are mandatory and exact. The important point is that
> there is only one file per test case; that would still count as
> "sufficiently simple" in my book.

So something where the tests in 14-15,17,19/27 would need to go back to
having the config that's part of the test driven by setup in
t/t4018-diff-funcname.sh, and the contents of a test file being e.g. (to
take the test from 25/27):

    # HEADER |package main|
    package main

    import "fmt"
    // ChangeMe

?