Subject: Re: [PATCH 15/16] Revert and amend "test-lib-functions: assert correct parameter count"

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 17 Apr 2021 01:48:35 +0200

On Fri, Apr 16 2021, SZEDER Gábor wrote:

> On Mon, Apr 12, 2021 at 01:09:04PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> This reverts and amends my my own e7884b353b7 (test-lib-functions:
>> assert correct parameter count, 2021-02-12) in order to improve the -x
>> output.
>> 
>> When I added these BUG assertions in e7884b353b7 I missed that this
>> made the -x output much more verbose.
>> 
>> E.g. for each test_cmp invocation we'd now emit:
>> 
>>     + test_cmp expect actual
>>     + test 2 -ne 2
>>     + eval diff -u "$@"
>>     + diff -u expect actual
>> 
>> That "test 2 -ne 2" line is new in e7884b353b7. As noted in
>> 45a2686441b (test-lib-functions: remove bug-inducing "diagnostics"
>> helper param, 2021-02-12) we had buggy invocations of some of these
>> functions with too many parameters.
>> 
>> Let's instead use "$@" instead of "$1" to achieve the same ends with
>> no extra -x output verbosity. The "test" operator will die with an
>> error if given more than one argument in these contexts, so using "$@"
>> achieves the same goal.
>
> I prefer the current check for its explicitness over the implicit and
> somewhat cryptic approach introduced in this patch.

Fair enough, I think it's worth it to have a bit of a non-obvious pattern there for less trace verbosity across the board.

> I hope that sooner or later I'll finish up my patch series to suppress
> '-x' output from test helper functions, and then this issue will
> become moot anyway.

That sounds like an interesting feature for those who want it, but it's
entirely orthagonal to the direction this patch is taking.

I'd like to have trace output for when tests descend into
test-lib-functions.sh and friends, I'd just like the most frequently
used code there to not be needlessly verbose.

That's not the same as wanting to hide it entirely, i.e. treat
test_path_is_file et al as though they were shell built-ins.