@matheusbsbs / matheus-bagliano / commits / 165293af3c

tests: send "bug in the test script" errors to the script's stderr

Some of the functions in our test library check that they were invoked properly with conditions like this: test "$#" = 2 || error "bug in the test script: not 2 parameters to test-expect-success" If this particular condition is triggered, then 'error' will abort the whole test script with a bold red error message [1] right away. However, under certain circumstances the test script will be aborted completely silently, namely if: - a similar condition in a test helper function like 'test_line_count' is triggered, - which is invoked from the test script's "main" shell [2], - and the test script is run manually (i.e. './t1234-foo.sh' as opposed to 'make t1234-foo.sh' or 'make test') [3] - and without the '--verbose' option, because the error message is printed from within 'test_eval_', where standard output is redirected either to /dev/null or to a log file. The only indication that something is wrong is that not all tests in the script are executed and at the end of the test script's output there is no "# passed all N tests" message, which are subtle and can easily go unnoticed, as I had to experience myself. Send these "bug in the test script" error messages directly to the test scripts standard error and thus to the terminal, so those bugs will be much harder to overlook. Instead of updating all ~20 such 'error' calls with a redirection, let's add a BUG() function to 'test-lib.sh', wrapping an 'error' call with the proper redirection and also including the common prefix of those error messages, and convert all those call sites [4] to use this new BUG() function instead. [1] That particular error message from 'test_expect_success' is printed in color only when running with or without '--verbose'; with '--tee' or '--verbose-log' the error is printed without color, but it is printed to the terminal nonetheless. [2] If such a condition is triggered in a subshell of a test, then 'error' won't be able to abort the whole test script, but only the subshell, which in turn causes the test to fail in the usual way, indicating loudly and clearly that something is wrong. [3] Well, 'error' aborts the test script the same way when run manually or by 'make' or 'prove', but both 'make' and 'prove' pay attention to the test script's exit status, and even a silently aborted test script would then trigger those tools' usual noticable error messages. [4] Strictly speaking, not all those 'error' calls need that redirection to send their output to the terminal, see e.g. 'test_expect_success' in the opening example, but I think it's better to be consistent. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Nov 19, 2018 at 14:13 UTC 165293af3ce4535efa72eb51587383144a2b3f01

7 files changed +26 -23

t/perf/perf-lib.sh

+2 -2

@@ -82,7 +82,7 @@ test_perf_do_repo_symlink_config_ () {

83 test_perf_create_repo_from () {

84 test "$#" = 2 ||

85 - error "bug in the test script: not 2 parameters to test-create-repo"

85 + BUG "not 2 parameters to test-create-repo"

86 repo="$1"

87 source="$2"

88 source_git="$("$MODERN_GIT" -C "$source" rev-parse --git-dir)"

@@ -184,7 +184,7 @@ test_wrapper_ () {

184 test_start_

185 test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=

186 test "$#" = 2 ||

187 - error "bug in the test script: not 2 or 3 parameters to test-expect-success"

187 + BUG "not 2 or 3 parameters to test-expect-success"

188 export test_prereq

189 if ! test_skip "$@"

190 then

t/t0001-init.sh

+2 -2

@@ -319,14 +319,14 @@ test_lazy_prereq GETCWD_IGNORES_PERMS '

319 base=GETCWD_TEST_BASE_DIR &&

320 mkdir -p $base/dir &&

321 chmod 100 $base ||

322 - error "bug in test script: cannot prepare $base"

322 + BUG "cannot prepare $base"

323

324 (cd $base/dir && /bin/pwd -P)

325 status=$?

326

327 chmod 700 $base &&

328 rm -rf $base ||

329 - error "bug in test script: cannot clean $base"

329 + BUG "cannot clean $base"

330 return $status

331 '

332

t/t4013-diff-various.sh

+1 -1

@@ -129,7 +129,7 @@ do

129 case "$magic" in

130 noellipses) ;;

131 *)

132 - die "bug in t4103: unknown magic $magic" ;;

132 + BUG "unknown magic $magic" ;;

133 esac ;;

134 *)

135 cmd="$magic $cmd" magic=

t/t5516-fetch-push.sh

+1 -1

@@ -95,7 +95,7 @@ mk_child() {

96 check_push_result () {

97 test $# -ge 3 ||

98 - error "bug in the test script: check_push_result requires at least 3 parameters"

98 + BUG "check_push_result requires at least 3 parameters"

100 repo_name="$1"

101 shift

t/t9902-completion.sh

+1 -1

@@ -1249,7 +1249,7 @@ test_expect_success 'teardown after ref completion' '

1249

1250 test_path_completion ()

1251 {

1252 - test $# = 2 || error "bug in the test script: not 2 parameters to test_path_completion"

1252 + test $# = 2 || BUG "not 2 parameters to test_path_completion"

1253

1254 local cur="$1" expected="$2"

1255 echo "$expected" >expected &&

t/test-lib-functions.sh

+12 -13

@@ -418,14 +418,14 @@ test_declared_prereq () {

418 test_verify_prereq () {

419 test -z "$test_prereq" ||

420 expr >/dev/null "$test_prereq" : '[A-Z0-9_,!]*$' ||

421 - error "bug in the test script: '$test_prereq' does not look like a prereq"

421 + BUG "'$test_prereq' does not look like a prereq"

422 }

423

424 test_expect_failure () {

425 test_start_

426 test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=

427 test "$#" = 2 ||

428 - error "bug in the test script: not 2 or 3 parameters to test-expect-failure"

428 + BUG "not 2 or 3 parameters to test-expect-failure"

429 test_verify_prereq

430 export test_prereq

431 if ! test_skip "$@"

@@ -445,7 +445,7 @@ test_expect_success () {

445 test_start_

446 test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=

447 test "$#" = 2 ||

448 - error "bug in the test script: not 2 or 3 parameters to test-expect-success"

448 + BUG "not 2 or 3 parameters to test-expect-success"

449 test_verify_prereq

450 export test_prereq

451 if ! test_skip "$@"

@@ -472,7 +472,7 @@ test_expect_success () {

472 test_external () {

473 test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=

474 test "$#" = 3 ||

475 - error >&5 "bug in the test script: not 3 or 4 parameters to test_external"

475 + BUG "not 3 or 4 parameters to test_external"

476 descr="$1"

477 shift

478 test_verify_prereq

@@ -613,7 +613,7 @@ test_path_is_missing () {

613 test_line_count () {

614 if test $# != 3

615 then

616 - error "bug in the test script: not 3 parameters to test_line_count"

616 + BUG "not 3 parameters to test_line_count"

617 elif ! test $(wc -l <"$3") "$1" "$2"

618 then

619 echo "test_line_count: line count for $3 !$1 $2"

@@ -793,13 +793,12 @@ test_i18ngrep () {

793 eval "last_arg=\${$#}"

794

795 test -f "$last_arg" ||

796 - error "bug in the test script: test_i18ngrep requires a file" \

797 - "to read as the last parameter"

796 + BUG "test_i18ngrep requires a file to read as the last parameter"

797

798 if test $# -lt 2 ||

799 { test "x!" = "x$1" && test $# -lt 3 ; }

800 then

802 - error "bug in the test script: too few parameters to test_i18ngrep"

801 + BUG "too few parameters to test_i18ngrep"

802 fi

803

804 if test_have_prereq !C_LOCALE_OUTPUT

@@ -871,7 +870,7 @@ test_seq () {

870 case $# in

871 1) set 1 "$@" ;;

872 2) ;;

874 - *) error "bug in the test script: not 1 or 2 parameters to test_seq" ;;

873 + *) BUG "not 1 or 2 parameters to test_seq" ;;

874 esac

875 test_seq_counter__=$1

876 while test "$test_seq_counter__" -le "$2"

@@ -909,7 +908,7 @@ test_when_finished () {

908 # doing so on Bash is better than nothing (the test will

909 # silently pass on other shells).

910 test "${BASH_SUBSHELL-0}" = 0 ||

912 - error "bug in test script: test_when_finished does nothing in a subshell"

911 + BUG "test_when_finished does nothing in a subshell"

912 test_cleanup="{ $*

913 } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"

914 }

@@ -918,7 +917,7 @@ test_when_finished () {

917 # Usage: test_create_repo <directory>

918 test_create_repo () {

919 test "$#" = 1 ||

921 - error "bug in the test script: not 1 parameter to test-create-repo"

920 + BUG "not 1 parameter to test-create-repo"

921 repo="$1"

922 mkdir -p "$repo"

923 (

@@ -1231,7 +1230,7 @@ test_oid_cache () {

1230

1231 if ! expr "$k" : '[a-z0-9][a-z0-9]*$' >/dev/null

1232 then

1234 - error 'bug in the test script: bad hash algorithm'

1233 + BUG 'bad hash algorithm'

1234 fi &&

1235 eval "test_oid_${k}_$tag=\"\$v\""

1236 done

@@ -1246,7 +1245,7 @@ test_oid () {

1245 # key-hash pair, so exit with an error.

1246 if eval "test -z \"\${$var+set}\""

1247 then

1249 - error "bug in the test script: undefined key '$1'" >&2

1248 + BUG "undefined key '$1'"

1249 fi &&

1250 eval "printf '%s' \"\${$var}\""

1251 }

t/test-lib.sh

+7 -3

@@ -402,6 +402,10 @@ error () {

402 exit 1

403 }

404

405 +BUG () {

406 + error >&7 "bug in the test script: $*"

407 +}

408 +

409 say () {

410 say_color info "$*"

411 }

@@ -729,7 +733,7 @@ test_run_ () {

733 if $(printf '%s\n' "$1" | sed -f "$GIT_BUILD_DIR/t/chainlint.sed" | grep -q '?![A-Z][A-Z]*?!') ||

734 test "OK-117" != "$(test_eval_ "(exit 117) && $1${LF}${LF}echo OK-\$?" 3>&1)"

735 then

732 - error "bug in the test script: broken &&-chain or run-away HERE-DOC: $1"

736 + BUG "broken &&-chain or run-away HERE-DOC: $1"

737 fi

738 trace=$trace_tmp

739 fi

@@ -1231,7 +1235,7 @@ test_lazy_prereq SANITY '

1235 chmod -w SANETESTD.1 &&

1236 chmod -r SANETESTD.1/x &&

1237 chmod -rx SANETESTD.2 ||

1234 - error "bug in test sript: cannot prepare SANETESTD"

1238 + BUG "cannot prepare SANETESTD"

1239

1240 ! test -r SANETESTD.1/x &&

1241 ! rm SANETESTD.1/x && ! test -f SANETESTD.2/x

@@ -1239,7 +1243,7 @@ test_lazy_prereq SANITY '

1243

1244 chmod +rwx SANETESTD.1 SANETESTD.2 &&

1245 rm -rf SANETESTD.1 SANETESTD.2 ||

1242 - error "bug in test sript: cannot clean SANETESTD"

1246 + BUG "cannot clean SANETESTD"

1247 return $status

1248 '

1249