Re: [PATCH v3 2/4] Revert and amend "test-lib-functions: assert correct parameter count"

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2021, 03:59 UTC
Message-ID: <xmqqmtth91wx.fsf@gitster.g>
In-Reply-To: <patch-2.4-3a570321770-20210423T072824Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 12 quoted lines

> The goal here is to get rid of the verbosity of having e.g. a "test 2
> -ne 2" line for every "test_cmp". We use "$@" as an argument to "test"
> to intentionally feed the "test" operator too many arguments if the
> functions are called with too many arguments, thus piggy-backing on it
> to check the number of arguments we get.
>
> Before this for each test_cmp invocation we'd emit:
>
>     + test_cmp expect actual
>     + test 2 -ne 2
>     + eval diff -u "$@"
>     + diff -u expect actual

Quite honestly, I do not see what's bad about that and am not all that sympathetic to the cause behind this series.

The developer is asking to debug the test script by passing '-x', and see where the test script is broken. Especially since ...

Show 5 quoted lines

> Now we'll get just:
>
>     + test_cmp expect actual
>     + eval diff -u "$@"
>     + diff -u expect actual

... we are not removing the 'eval diff -u "$@"' line, I do not think how a few extra trace output lines coming from sanity check hurts all that much. It is worth paying for to have such basic sanity checks expressed in a more natural way (than "oh, this hack would not work when 'one -a two' is used as a path"), I would think.

Thanks.