Re: [PATCH 15/16] Revert and amend "test-lib-functions: assert correct parameter count"
flat view
From: SZEDER Gábor <hidden>
Date: 2021-04-15 22:05:35
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.
quoted hunk ↗ jump to hunk
The same goes for "cmp" and "diff -u" (which we typically use for test_cmp). Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- 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