From: Stefano Lattarini Date: Sun, 7 Aug 2011 13:20:00 +0000 (+0200) Subject: testsuite: TAP tests properly decide when to remove tempdirs X-Git-Tag: ng-0.5a~89^2~134 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5c1bb4e240c3538a6bfa7026dfe5f590ac747863;p=thirdparty%2Fautomake.git testsuite: TAP tests properly decide when to remove tempdirs Before this change, the TAP tests in the Automake testsuite were removing the temporary test directory even when they failed or were skipped, thus making debugging more difficult. * tests/tap-functions.sh (incr_tap_count): Removed, superseded by ... (incr_): ... this function, which can increment the value of any variable passed to it. (result_): Updated to use `incr_' instead of the now-removed `incr_tap_count_'. Keep count of failures, xfailures, xpasses, and skips, using ... ($tap_skip_count_, $tap_bad_count, _$tap_xfail_count_): ... these new variables. * tests/defs (trap): Try to use their values to decide whether the temporary directory being used by the test script should be removed or not. Other code reorganizations. And move the code for the removal of the temporary directory out to ... (rm_rf_): ... this new subroutine. (Main code): Use that instead of duplicating the code. --- diff --git a/ChangeLog b/ChangeLog index f18162467..01ff2a59a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2011-08-07 Stefano Lattarini + + testsuite: TAP tests properly decide when to remove tempdirs + Before this change, the TAP tests in the Automake testsuite were + removing the temporary test directory even when they failed or + were skipped, thus making debugging more difficult. + * tests/tap-functions.sh (incr_tap_count): Removed, superseded + by ... + (incr_): ... this function, which can increment the value of any + variable passed to it. + (result_): Updated to use `incr_' instead of the now-removed + `incr_tap_count_'. Keep count of failures, xfailures, xpasses, + and skips, using ... + ($tap_skip_count_, $tap_bad_count, _$tap_xfail_count_): ... + these new variables. + * tests/defs (trap): Try to use their values to decide whether + the temporary directory being used by the test script should be + removed or not. Other code reorganizations. And move the code + for the removal of the temporary directory out to ... + (rm_rf_): ... this new subroutine. + (Main code): Use that instead of duplicating the code. + 2011-08-07 Stefano Lattarini testsuite: improve and refactor our custom TAP shell library diff --git a/tests/defs b/tests/defs index e9e241e4a..bfe7fa451 100644 --- a/tests/defs +++ b/tests/defs @@ -322,6 +322,20 @@ seq_ () done } +# rm_rf_ [FILES OR DIRECTORIES ...] +# --------------------------------- +# Recursively remove the given files or directory, also handling the case +# of non-writable subdirectories. +rm_rf_ () +{ + test $# -gt 0 || return 0 + # Ignore failures in find, we are only interested in failures of the + # final rm. + find "$@" -type d ! -perm -700 -exec chmod u+rwx {} \; || : + rm -rf "$@" +} + + # count_test_results total=N pass=N fail=N xpass=N xfail=N skip=N error=N # ----------------------------------------------------------------------- # Check that a testsuite run driven by the parallel-tests harness has @@ -802,10 +816,7 @@ distdir=$me-1.0 # this directory will be kept, to facilitate debugging. testSubDir=$me.dir -test ! -d $testSubDir || { - find $testSubDir -type d ! -perm -700 -exec chmod u+rwx {} ";" - rm -rf $testSubDir -} +test ! -d $testSubDir || rm_rf_ $testSubDir mkdir $testSubDir cd ./$testSubDir @@ -814,31 +825,40 @@ if test "$sh_errexit_works" = yes; then trap 'exit_status=$? set +e cd "$testbuilddir" - # This is to ensure that a test script does give a SKIP outcome just - # because a command in it happens to exit with status 77. This - # behaviour, while from time to time useful to developers, is not - # meant to be enabled by default, as it could cause spurious failures - # in the wild. Thus it will be enabled only when the variable - # "am_explicit_skips" is set to a "true" value. - case $am_explicit_skips in - [yY]|[yY]es|1) - if test $exit_status -eq 77 && test $am__test_skipped != yes; then - exit_status=78 - fi - ;; - esac - case $exit_status,$keep_testdirs in - 0,) - find $testSubDir -type d ! -perm -700 -exec chmod u+rwx {} ";" - rm -rf $testSubDir - ;; - esac + if test -n "$keep_testdirs"; then + keep_testdirs=yes + else + keep_testdirs=no + fi + if test $using_tap = yes; then + if test $have_tap_plan_ != yes; then + late_plan_ + fi + test $exit_status -eq 0 \ + && test $tap_xfail_count_ -eq 0 \ + && test $tap_skip_count_ -eq 0 \ + && test $tap_bad_count_ -eq 0 \ + || keep_testdirs=yes + else + # This is to ensure that a test script does give a SKIP outcome just + # because a command in it happens to exit with status 77. This + # behaviour, while from time to time useful to developers, is not + # meant to be enabled by default, as it could cause spurious failures + # in the wild. Thus it will be enabled only when the variable + # "am_explicit_skips" is set to a "true" value. + case $am_explicit_skips in + [yY]|[yY]es|1) + if test $exit_status -eq 77 && test $am__test_skipped != yes; then + echo "$me: implicit skip turned into failure" + exit_status=78 + fi;; + esac + test $exit_status -eq 0 || keep_testdirs=yes + fi + test $keep_testdirs = no && rm_rf_ $testSubDir set +x test "$signal" != 0 && echo "$me: caught signal $signal" echo "$me: exit $exit_status" - if test $using_tap = yes && test $have_tap_plan_ != yes; then - late_plan_ - fi exit $exit_status ' 0 for signal in 1 2 13 15; do diff --git a/tests/tap-functions.sh b/tests/tap-functions.sh index b638e6f0e..d685c182a 100644 --- a/tests/tap-functions.sh +++ b/tests/tap-functions.sh @@ -24,15 +24,22 @@ # The count of the TAP test results seen so far. tap_count_=0 +# The count of skipped tests. +tap_skip_count_=0 +# The count of tests that experienced an expected failure. +tap_xfail_count_=0 +# The count of tests with unexpected outcomes (i.e., failed and xpassed). +tap_bad_count_=0 # The first "test -n" tries to avoid extra forks when possible. if test -n "${ZSH_VERSION}${BASH_VERSION}" \ || (eval 'test $((1 + 1)) = 2') >/dev/null 2>&1 then - # Use of 'eval' needed to protect dumber shells from parsing errors. - eval 'incr_tap_count_ () { tap_count_=$(($tap_count_ + 1)); }' + # Outer use of 'eval' needed to protect dumber shells from parsing + # errors. + eval 'incr_ () { eval "$1=\$((\${$1} + 1))"; }' else - incr_tap_count_ () { tap_count_=`expr $tap_count_ + 1`; } + incr_ () { eval "$1=\`expr \${$1} + 1\`"; } fi # plan_ NUMBER-OF-PLANNED-TESTS @@ -113,7 +120,14 @@ result_ () ""|TODO|SKIP) ;; *) bailout_ "result_: invalid directive '$directive_'" ;; esac - incr_tap_count_ + incr_ tap_count_ + case $tap_result_,$tap_directive_ in + ok,) ;; # Passed. + not\ ok,TODO) incr_ tap_xfail_count_;; # Expected failure. + not\ ok,*|ok,TODO) incr_ tap_bad_count_ ;; # Failed or xpassed. + ok,SKIP) incr_ tap_skip_count_ ;; # Skipped. + *) bailout_ "internal error in 'result_'";; # Can't happen. + esac tap_text_="$tap_result_ $tap_count_" if test x"$*" != x; then tap_text_="$tap_text_ - $*"