From f1dd90bd193637eeef772890c37afe3529a665d0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 10 Oct 2014 02:07:00 -0400 Subject: [PATCH 1/3] t5304: use test_path_is_* instead of "test -f" This is slightly more robust (checking "! test -f" would not notice a directory of the same name, though that is not likely to happen here). It also makes debugging easier, as the test script will output a message on failure. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5304-prune.sh | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index 01c6a3fc1..b0ffb056b 100755 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -14,7 +14,7 @@ add_blob() { BLOB=$(echo aleph_0 | git hash-object -w --stdin) && BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") && test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && test-chmtime =+0 $BLOB_FILE } @@ -35,9 +35,9 @@ test_expect_success 'prune stale packs' ' : > .git/objects/tmp_2.pack && test-chmtime =-86501 .git/objects/tmp_1.pack && git prune --expire 1.day && - test -f $orig_pack && - test -f .git/objects/tmp_2.pack && - ! test -f .git/objects/tmp_1.pack + test_path_is_file $orig_pack && + test_path_is_file .git/objects/tmp_2.pack && + test_path_is_missing .git/objects/tmp_1.pack ' @@ -46,11 +46,11 @@ test_expect_success 'prune --expire' ' add_blob && git prune --expire=1.hour.ago && test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && test-chmtime =-86500 $BLOB_FILE && git prune --expire 1.day && test $before = $(git count-objects | sed "s/ .*//") && - ! test -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -60,11 +60,11 @@ test_expect_success 'gc: implicit prune --expire' ' test-chmtime =-$((2*$week-30)) $BLOB_FILE && git gc && test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && test-chmtime =-$((2*$week+1)) $BLOB_FILE && git gc && test $before = $(git count-objects | sed "s/ .*//") && - ! test -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -110,7 +110,7 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' ' git commit --allow-empty -m "detached commit" && # verify that there is no reflogs # (should be removed and disabled by previous test) - test ! -e .git/logs && + test_path_is_missing .git/logs && git prune -n >prune_actual && : >prune_expected && test_cmp prune_actual prune_expected @@ -145,7 +145,7 @@ test_expect_success 'gc --no-prune' ' git config gc.pruneExpire 2.days.ago && git gc --no-prune && test 1 = $(git count-objects | sed "s/ .*//") && - test -f $BLOB_FILE + test_path_is_file $BLOB_FILE ' @@ -153,10 +153,10 @@ test_expect_success 'gc respects gc.pruneExpire' ' git config gc.pruneExpire 5002.days.ago && git gc && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git config gc.pruneExpire 5000.days.ago && git gc && - test ! -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -165,9 +165,9 @@ test_expect_success 'gc --prune=' ' add_blob && test-chmtime =-$((5001*$day)) $BLOB_FILE && git gc --prune=5002.days.ago && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git gc --prune=5000.days.ago && - test ! -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -175,9 +175,9 @@ test_expect_success 'gc --prune=never' ' add_blob && git gc --prune=never && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git gc --prune=now && - test ! -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -186,10 +186,10 @@ test_expect_success 'gc respects gc.pruneExpire=never' ' git config gc.pruneExpire never && add_blob && git gc && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git config gc.pruneExpire now && git gc && - test ! -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -197,9 +197,9 @@ test_expect_success 'prune --expire=never' ' add_blob && git prune --expire=never && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git prune && - test ! -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ' @@ -210,10 +210,10 @@ test_expect_success 'gc: prune old objects after local clone' ' ( cd aclone && test 1 = $(git count-objects | sed "s/ .*//") && - test -f $BLOB_FILE && + test_path_is_file $BLOB_FILE && git gc --prune && test 0 = $(git count-objects | sed "s/ .*//") && - ! test -f $BLOB_FILE + test_path_is_missing $BLOB_FILE ) ' @@ -250,7 +250,7 @@ test_expect_success 'prune .git/shallow' ' grep $SHA1 .git/shallow && grep $SHA1 out && git prune && - ! test -f .git/shallow + test_path_is_missing .git/shallow ' test_done From 8ad16524183baf196d1db82b99ef52d05ca438e9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 10 Oct 2014 02:11:14 -0400 Subject: [PATCH 2/3] t5304: use helper to report failure of "test foo = bar" For small outputs, we sometimes use: test "$(some_cmd)" = "something we expect" instead of a full test_cmp. The downside of this is that when it fails, there is no output at all from the script. Let's introduce a small helper to make tests easier to debug. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5304-prune.sh | 16 ++++++++-------- t/test-lib-functions.sh | 9 +++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index b0ffb056b..e32e46dee 100755 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -13,7 +13,7 @@ add_blob() { before=$(git count-objects | sed "s/ .*//") && BLOB=$(echo aleph_0 | git hash-object -w --stdin) && BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") && - test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && + verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && test_path_is_file $BLOB_FILE && test-chmtime =+0 $BLOB_FILE } @@ -45,11 +45,11 @@ test_expect_success 'prune --expire' ' add_blob && git prune --expire=1.hour.ago && - test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && + verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && test_path_is_file $BLOB_FILE && test-chmtime =-86500 $BLOB_FILE && git prune --expire 1.day && - test $before = $(git count-objects | sed "s/ .*//") && + verbose test $before = $(git count-objects | sed "s/ .*//") && test_path_is_missing $BLOB_FILE ' @@ -59,11 +59,11 @@ test_expect_success 'gc: implicit prune --expire' ' add_blob && test-chmtime =-$((2*$week-30)) $BLOB_FILE && git gc && - test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && + verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && test_path_is_file $BLOB_FILE && test-chmtime =-$((2*$week+1)) $BLOB_FILE && git gc && - test $before = $(git count-objects | sed "s/ .*//") && + verbose test $before = $(git count-objects | sed "s/ .*//") && test_path_is_missing $BLOB_FILE ' @@ -144,7 +144,7 @@ test_expect_success 'gc --no-prune' ' test-chmtime =-$((5001*$day)) $BLOB_FILE && git config gc.pruneExpire 2.days.ago && git gc --no-prune && - test 1 = $(git count-objects | sed "s/ .*//") && + verbose test 1 = $(git count-objects | sed "s/ .*//") && test_path_is_file $BLOB_FILE ' @@ -209,10 +209,10 @@ test_expect_success 'gc: prune old objects after local clone' ' git clone --no-hardlinks . aclone && ( cd aclone && - test 1 = $(git count-objects | sed "s/ .*//") && + verbose test 1 = $(git count-objects | sed "s/ .*//") && test_path_is_file $BLOB_FILE && git gc --prune && - test 0 = $(git count-objects | sed "s/ .*//") && + verbose test 0 = $(git count-objects | sed "s/ .*//") && test_path_is_missing $BLOB_FILE ) ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index dafd6ad21..b7957b87b 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -634,6 +634,15 @@ test_cmp_bin() { cmp "$@" } +# Call any command "$@" but be more verbose about its +# failure. This is handy for commands like "test" which do +# not output anything when they fail. +verbose () { + "$@" && return 0 + echo >&2 "command failed: $(git rev-parse --sq-quote "$@")" + return 1 +} + # Check if the file expected to be empty is indeed empty, and barfs # otherwise. From a136f6d8ff290ab486c85cd12d1a9a47d53b432d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 10 Oct 2014 02:47:27 -0400 Subject: [PATCH 3/3] test-lib.sh: support -x option for shell-tracing Usually running a test under "-v" makes it clear which command is failing. However, sometimes it can be useful to also see a complete trace of the shell commands being run in the test. You can do so without any support from the test suite by running "sh -x tXXXX-foo.sh". However, this produces quite a large bit of output, as we see a trace of the entire test suite. This patch instead introduces a "-x" option to the test scripts (i.e., "./tXXXX-foo.sh -x"). When enabled, this turns on "set -x" only for the tests themselves. This can still be a bit verbose, but should keep things to a more manageable level. You can even use "--verbose-only" to see the trace only for a specific test. The implementation is a little invasive. We turn on the "set -x" inside the "eval" of the test code. This lets the eval itself avoid being reported in the trace (which would be long, and redundant with the verbose listing we already showed). And then after the eval runs, we do some trickery with stderr to avoid showing the "set +x" to the user. We also show traces for test_cleanup functions (since they can impact the test outcome, too). However, we do avoid running the noop ":" cleanup (the default if the test does not use test_cleanup at all), as it creates unnecessary noise in the "set -x" output. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/README | 6 ++++++ t/test-lib.sh | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/t/README b/t/README index 52c77ae93..995226129 100644 --- a/t/README +++ b/t/README @@ -82,6 +82,12 @@ appropriately before running "make". numbers matching . The number matched against is simply the running count of the test within the file. +-x:: + Turn on shell tracing (i.e., `set -x`) during the tests + themselves. Implies `--verbose`. Note that this can cause + failures in some tests which redirect and test the + output of shell functions. Use with caution. + -d:: --debug:: This may help the person who is developing a new test. diff --git a/t/test-lib.sh b/t/test-lib.sh index 0f4a67bfc..cf19339cc 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -233,6 +233,10 @@ do --root=*) root=$(expr "z$1" : 'z[^=]*=\(.*\)') shift ;; + -x) + trace=t + verbose=t + shift ;; *) echo "error: unknown test option '$1'" >&2; exit 1 ;; esac @@ -517,10 +521,39 @@ maybe_setup_valgrind () { fi } +# This is a separate function because some tests use +# "return" to end a test_expect_success block early +# (and we want to make sure we run any cleanup like +# "set +x"). +test_eval_inner_ () { + # Do not add anything extra (including LF) after '$*' + eval " + test \"$trace\" = t && set -x + $*" +} + test_eval_ () { - # This is a separate function because some tests use - # "return" to end a test_expect_success block early. - eval &3 2>&4 "$*" + # We run this block with stderr redirected to avoid extra cruft + # during a "-x" trace. Once in "set -x" mode, we cannot prevent + # the shell from printing the "set +x" to turn it off (nor the saving + # of $? before that). But we can make sure that the output goes to + # /dev/null. + # + # The test itself is run with stderr put back to &4 (so either to + # /dev/null, or to the original stderr if --verbose was used). + { + test_eval_inner_ "$@" &3 2>&4 + test_eval_ret_=$? + if test "$trace" = t + then + set +x + if test "$test_eval_ret_" != 0 + then + say_color error >&4 "error: last command exited with \$?=$test_eval_ret_" + fi + fi + } 2>/dev/null + return $test_eval_ret_ } test_run_ () { @@ -531,7 +564,8 @@ test_run_ () { eval_ret=$? teardown_malloc_check - if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure" + if test -z "$immediate" || test $eval_ret = 0 || + test -n "$expecting_failure" && test "$test_cleanup" != ":" then setup_malloc_check test_eval_ "$test_cleanup"