[PATCH 0/3] test-lib-functions.sh: trickery to make -x less verbose

This series makes the "-x" output less verbose for our various helper functions, mainly using the trickery of feeding "$@" to "test" to rely on it to die when given too many arguments.

It was split off from v2 of my larger test-lib.sh series as noted in its CL: https://lore.kernel.org/git/cover-00.12-00000000000-20210417T124424Z-avarab@gmail.com/

The range-diff below is to v1 of the relevant part of that series, showing what's changed just for these commits.

Ævar Arnfjörð Bjarmason (3):
  test-lib-functions: normalize test_path_is_missing() debugging
  Revert and amend "test-lib-functions: assert correct parameter count"
  test-lib-functions: remove last two parameter count assertions

 t/test-lib-functions.sh | 64 +++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 41 deletions(-)

Range-diff:
1:  0813aa8e34e = 1:  c22e3f7764f test-lib-functions: normalize test_path_is_missing() debugging
2:  b6e9d971b40 < -:  ----------- test-lib-functions: use "return 1" instead of "false"
3:  0cd511206c4 ! 2:  6f9e09a2017 Revert and amend "test-lib-functions: assert correct parameter count"
    @@ Commit message
         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.
    +    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.
     
    -    E.g. for each test_cmp invocation we'd now emit:
    +    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
     
    -    That "test 2 -ne 2" line is new in e7884b353b7. As noted in
    +    That "test 2 -ne 2" line is new in my 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.
    +    Now we'll get just:
     
    -    The same goes for "cmp" and "diff -u" (which we typically use for
    -    test_cmp).
    +        + test_cmp expect actual
    +        + eval diff -u "$@"
    +        + diff -u expect actual
    +
    +    This does not to the "right" thing in cases like:
    +
    +        test_path_is_file x -a y
    +
    +    Which will now turn into:
    +
    +        test -f x -a y
    +
    +    I consider that to be OK given the trade-off that any extra checking
    +    would produce more verbose trace output. As shown in 45a2686441b we
    +    had issues with these functions being invoked with multiple
    +    parameters (e.g. a glob) by accident, we don't need to be paranoid in
    +    guarding against hostile misuse from our own test suite.
    +
    +    While I'm at it change a few functions that relied on a "false" being
    +    the last statement in the function to use an explicit "return 1" like
    +    the other functions in this file.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ t/test-lib-functions.sh: test_external_without_stderr () {
     +	if ! test -f "$@"
      	then
     -		echo "File $1 doesn't exist"
    -+		echo "File $@ doesn't exist"
    - 		return 1
    +-		false
    ++		echo "File $* doesn't exist"
    ++		return 1
      	fi
      }
      
    @@ t/test-lib-functions.sh: test_external_without_stderr () {
     +	if ! test -d "$@"
      	then
     -		echo "Directory $1 doesn't exist"
    -+		echo "Directory $@ doesn't exist"
    - 		return 1
    +-		false
    ++		echo "Directory $* doesn't exist"
    ++		return 1
      	fi
      }
      
    @@ t/test-lib-functions.sh: test_external_without_stderr () {
     +	if ! test -e "$@"
      	then
     -		echo "Path $1 doesn't exist"
    -+		echo "Path $@ doesn't exist"
    - 		return 1
    +-		false
    ++		echo "Path $* doesn't exist"
    ++		return 1
      	fi
      }
      
    @@ t/test-lib-functions.sh: test_external_without_stderr () {
     +	if test -n "$(ls -a1 "$@" | egrep -v '^\.\.?$')"
      	then
     -		echo "Directory '$1' is not empty, it contains:"
    -+		echo "Directory '$@' is not empty, it contains:"
    ++		echo "Directory '$*' is not empty, it contains:"
      		ls -la "$1"
      		return 1
      	fi
    @@ t/test-lib-functions.sh: test_dir_is_empty () {
     +	if ! test -s "$@"
      	then
     -		echo "'$1' is not a non-empty file."
    -+		echo "'$@' is not a non-empty file."
    - 		return 1
    +-		false
    ++		echo "'$*' is not a non-empty file."
    ++		return 1
      	fi
      }
      
    @@ t/test-lib-functions.sh: test_dir_is_empty () {
     -	if test -e "$1"
     +	if test -e "$@"
      	then
    - 		echo "Path $1 exists!"
    +-		echo "Path $1 exists!"
    ++		echo "Path $* exists!"
      		false
    + 	fi
    + }
     @@ t/test-lib-functions.sh: test_expect_code () {
      # - not all diff versions understand "-u"
      
    @@ t/test-lib-functions.sh: verbose () {
      	then
     -		echo "'$1' is not empty, it contains:"
     -		cat "$1"
    -+		echo "'$@' is not empty, it contains:"
    ++		echo "'$*' is not empty, it contains:"
     +		cat "$@"
      		return 1
      	fi
4:  b4a018a63f3 ! 3:  b7b11a60bcd test-lib-functions: remove last two parameter count assertions
    @@ Metadata
      ## Commit message ##
         test-lib-functions: remove last two parameter count assertions
     
    -    Remove a couple of parameter count assertions where we'll now silently
    -    do the wrong thing if given too many parameters, unlike the "$@" cases
    -    in the preceding commit where "test" etc. handle the check for usi.
    +    Remove a couple of parameter count assertions where, unlike the
    +    preceding commit's migration to 'test -$x "$@"', we'll now silently do
    +    the "wrong" thing if given too many parameters. The benefit is less
    +    verbose trace output, as noted in the preceding commit.
     
         In the case of "test_file_size", the "test-tool" we're invoking is
         happy to accept N parameters (it'll print out all N sizes). Let's just
    -    use "$@" in that case anyway, there's only a few callers, and
    +    use "$@" in that case anyway. There's only a few callers, and
         eventually those should probably be moved to use the test-tool
         directly.