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

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

On Mon, Apr 12, 2021 at 01:09:04PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

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.