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. 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.
> The same goes for "cmp" and "diff -u" (which we typically use for
> test_cmp).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> t/test-lib-functions.sh | 41 ++++++++++++++++-------------------------
> 1 file changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index c46bf0ff09c..2cf72b56851 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -759,39 +759,35 @@ test_external_without_stderr () {
> # debugging-friendly alternatives to "test [-f|-d|-e]"
> # The commands test the existence or non-existence of $1
> test_path_is_file () {
> - test "$#" -ne 1 && BUG "1 param"
> - if ! test -f "$1"
> + if ! test -f "$@"
> then
> - echo "File $1 doesn't exist"
> + echo "File $@ doesn't exist"
> return 1
> fi
> }
>
> test_path_is_dir () {
> - test "$#" -ne 1 && BUG "1 param"
> - if ! test -d "$1"
> + if ! test -d "$@"
> then
> - echo "Directory $1 doesn't exist"
> + echo "Directory $@ doesn't exist"
> return 1
> fi
> }
>
> test_path_exists () {
> - test "$#" -ne 1 && BUG "1 param"
> - if ! test -e "$1"
> + if ! test -e "$@"
> then
> - echo "Path $1 doesn't exist"
> + echo "Path $@ doesn't exist"
> return 1
> fi
> }
>
> # Check if the directory exists and is empty as expected, barf otherwise.
> test_dir_is_empty () {
> - test "$#" -ne 1 && BUG "1 param"
> - test_path_is_dir "$1" &&
> - if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
> + test_path_is_dir "$@" &&
> + if test -n "$(ls -a1 "$@" | egrep -v '^\.\.?$')"
> then
> - echo "Directory '$1' is not empty, it contains:"
> + echo "Directory '$@' is not empty, it contains:"
> ls -la "$1"
> return 1
> fi
> @@ -799,17 +795,15 @@ test_dir_is_empty () {
>
> # Check if the file exists and has a size greater than zero
> test_file_not_empty () {
> - test "$#" = 2 && BUG "2 param"
> - if ! test -s "$1"
> + if ! test -s "$@"
> then
> - echo "'$1' is not a non-empty file."
> + echo "'$@' is not a non-empty file."
> return 1
> fi
> }
>
> test_path_is_missing () {
> - test "$#" -ne 1 && BUG "1 param"
> - if test -e "$1"
> + if test -e "$@"
> then
> echo "Path $1 exists!"
> false
> @@ -1013,7 +1007,6 @@ test_expect_code () {
> # - not all diff versions understand "-u"
>
> test_cmp () {
> - test "$#" -ne 2 && BUG "2 param"
> eval "$GIT_TEST_CMP" '"$@"'
> }
>
> @@ -1043,7 +1036,6 @@ test_cmp_config () {
> # test_cmp_bin - helper to compare binary files
>
> test_cmp_bin () {
> - test "$#" -ne 2 && BUG "2 param"
> cmp "$@"
> }
>
> @@ -1104,12 +1096,11 @@ verbose () {
> # otherwise.
>
> test_must_be_empty () {
> - test "$#" -ne 1 && BUG "1 param"
> - test_path_is_file "$1" &&
> - if test -s "$1"
> + test_path_is_file "$@" &&
> + if test -s "$@"
> then
> - echo "'$1' is not empty, it contains:"
> - cat "$1"
> + echo "'$@' is not empty, it contains:"
> + cat "$@"
> return 1
> fi
> }
> --
> 2.31.1.634.gb41287a30b0
>
