From 68830470712b370d5ea231f76babd60a8859c105 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 28 Dec 2013 04:29:15 -0500 Subject: [PATCH 1/3] t0000: set TEST_OUTPUT_DIRECTORY for sub-tests Running t0000 produces more trash directories than expected and does not clean up after itself: $ ./t0000-basic.sh [...] $ ls -d trash\ directory.* trash directory.failing-cleanup trash directory.mixed-results1 trash directory.mixed-results2 trash directory.partial-pass trash directory.test-verbose trash directory.test-verbose-only-2 These scratch areas for sub-tests should be under the t0000 trash directory, but because TEST_OUTPUT_DIRECTORY defaults to TEST_DIRECTORY, which is exported to help sub-tests find test-lib.sh, the sub-test trash directories are created under the toplevel t/ directory instead. Because some of the sub-tests simulate failures, their trash directories are kept around. Fix it by explicitly setting TEST_OUTPUT_DIRECTORY appropriately for sub-tests. An alternative fix would be to pass the --root parameter that only specifies where to put the trash directories, which would also work. However, using TEST_OUTPUT_DIRECTORY is more futureproof in case tests want to write more output in addition to the test-results/ (which are already suppressed in sub-tests using the HARNESS_ACTIVE setting) and trash directories. This fixes a regression introduced by 38b074d (t/test-lib.sh: fix TRASH_DIRECTORY handling, 2013-04-14). Before that commit, the TEST_OUTPUT_DIRECTORY setting was not respected consistently so most tests did their work in a "trash" subdirectory of the current directory instead of the output dir. Signed-off-by: Jeff King Clarified-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 10be52beed..bc4e3e27b5 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -71,6 +71,8 @@ run_sub_test_lib_test () { cat >>"$name.sh" && chmod +x "$name.sh" && export TEST_DIRECTORY && + TEST_OUTPUT_DIRECTORY=$(pwd) && + export TEST_OUTPUT_DIRECTORY && ./"$name.sh" "$@" >out 2>err ) } From a63c12c9be28bf455816b1ece4a52b463ecf6241 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 28 Dec 2013 04:31:49 -0500 Subject: [PATCH 2/3] t0000: simplify HARNESS_ACTIVE hack Commit 517cd55 set HARNESS_ACTIVE unconditionally in sub-tests, because that value affects the output of "--verbose". t0000 needs stable output from its sub-tests, and we may or may not be running under a TAP harness. That commit made the decision to always set the variable, since it has another useful side effect, which is suppressing writes to t/test-results by the sub-tests (which would just pollute the real results). Since the last commit, though, the sub-tests have their own test-results directories, so this is no longer an issue. We can now update a few comments that are no longer accurate nor necessary. We can also revisit the choice of HARNESS_ACTIVE. Since we must choose one value for stability, it's probably saner to have it off. This means that future patches could test things like the test-results writing, or the "--quiet" option, which is currently ignored when run under a harness. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 14 +++++--------- t/test-lib.sh | 2 -- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index bc4e3e27b5..e6c5b63839 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -50,11 +50,11 @@ run_sub_test_lib_test () { shift 2 mkdir "$name" && ( - # Pretend we're a test harness. This prevents - # test-lib from writing the counts to a file that will - # later be summarized, showing spurious "failed" tests - HARNESS_ACTIVE=t && - export HARNESS_ACTIVE && + # Pretend we're not running under a test harness, whether we + # are or not. The test-lib output depends on the setting of + # this variable, so we need a stable setting under which to run + # the sub-test. + sane_unset HARNESS_ACTIVE && cd "$name" && cat >"$name.sh" <<-EOF && #!$SHELL_PATH @@ -235,16 +235,13 @@ test_expect_success 'test --verbose' ' grep -v "^Initialized empty" test-verbose/out+ >test-verbose/out && check_sub_test_lib_test test-verbose <<-\EOF > expecting success: true - > Z > ok 1 - passing test > Z > expecting success: echo foo > foo - > Z > ok 2 - test with output > Z > expecting success: false - > Z > not ok 3 - failing test > # false > Z @@ -267,7 +264,6 @@ test_expect_success 'test --verbose-only' ' > Z > expecting success: echo foo > foo - > Z > ok 2 - test with output > Z > not ok 3 - failing test diff --git a/t/test-lib.sh b/t/test-lib.sh index b25249ec4c..f54a77c292 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -477,8 +477,6 @@ test_at_end_hook_ () { test_done () { GIT_EXIT_OK=t - # Note: t0000 relies on $HARNESS_ACTIVE disabling the .counts - # output file if test -z "$HARNESS_ACTIVE" then test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results" From 738a8beac42d5e2c6b882f997b7fc6577363c544 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 28 Dec 2013 04:33:40 -0500 Subject: [PATCH 3/3] t0000: drop "known breakage" test Having a simulated "known breakage" test means that the test suite will always tell us there is a bug to be fixed, even though it is only simulated. The right way to test this is in a sub-test, that can also check that we provide the correct exit status and output. Fortunately, we already have such a test (added much later by 5ebf89e). We could arguably get rid of the simulated success test immediately above, as well, as it is also redundant with the tests added in 5ebf89e. However, it does not have the annoying behavior of the "known breakage" test. It may also be easier to debug if the test suite is truly broken, since it is not a test-within-a-test, as the later tests are. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index e6c5b63839..a2bb63ce8e 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -41,9 +41,6 @@ test_expect_success '.git/objects should have 3 subdirectories' ' test_expect_success 'success is reported like this' ' : ' -test_expect_failure 'pretend we have a known breakage' ' - false -' run_sub_test_lib_test () { name="$1" descr="$2" # stdin is the body of the test code