threads / patch / 24767

patcht/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1

Subject: [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1

## tl;dr

20 messages between Aug 17, 2010 and Aug 19, 2010. Diffs are folded; open one to read it.

replies: 19people: 4as markdown or json

Ævar Arnfjörð Bjarmason· Aug 17, 2010, 09:43 UTC · lore

Change the sanity tests in t/t0000-basic.sh to not to run a passing TODO test unless the TEST_PASSING_TODO environment variable is set.

The motivation is to have nothing out of the ordinary on a normal test run for test smoking purposes.

If every normal test run has a passing TODO you're more likely to turn a blind eye to it and not to investigate cases where things really are passing unexpectedly.

It also makes the prove(1) output less noisy. Before:
    All tests successful.
    Test Summary Report
    -------------------
    ./t0000-basic.sh                                   (Wstat: 0 Tests: 46 Failed: 0)
      TODO passed:   5
    Files=484, Tests=6229, 143 wallclock secs ( 4.00 usr  4.15 sys + 104.77 cusr 351.57 csys = 464.49 CPU)
    Result: PASS
And after:
    All tests successful.
    Files=484, Tests=6228, 139 wallclock secs ( 4.07 usr  4.25 sys + 104.54 cusr 350.85 csys = 463.71 CPU)
    Result: PASS
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0000-basic.sh |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)
Show changes to t/t0000-basic.sh +8 −3
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index f2c7336..2f6a17b 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -54,9 +54,14 @@ test_expect_success 'success is reported like this' '
 test_expect_failure 'pretend we have a known breakage' '
     false
 '
-test_expect_failure 'pretend we have fixed a known breakage' '
-    :
-'
+if test -z "$TEST_PASSING_TODO"
+then
+	say "Not testing a known breakage, set TEST_PASSING_TODO=1 to enable"
+else
+	test_expect_failure 'pretend we have fixed a known breakage' '
+	    :
+	'
+fi
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '

-- 
1.7.2.1.389.gc3d0b
Sverre Rabbelier· Aug 17, 2010, 17:57 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1

Heya,
On Tue, Aug 17, 2010 at 04:43, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 9 quoted lines
> Change the sanity tests in t/t0000-basic.sh to not to run a passing
> TODO test unless the TEST_PASSING_TODO environment variable is set.
>
> The motivation is to have nothing out of the ordinary on a normal test
> run for test smoking purposes.
>
> If every normal test run has a passing TODO you're more likely to turn
> a blind eye to it and not to investigate cases where things really are
> passing unexpectedly.

Thanks, this has bothered me ever since I first ran the test suite. While I understand that the test suite needs testing to, it should be done in such a way that it doesn't produce false positives. That is, my preference would be to still keep the test in the suite as-is, but somehow get it to show up as a passing test instead of a fixed one. E.g., something like:

test_in_new_harness 0 1 0 0 '
    test_expect_failure 'pretend we have fixed a known breakage' '
           :
     '
'

Where there '0 1 0 0' stands for '0 failures, 1 fixed, 0 broken, 0 success'. Not sure there's any other use for a test_in_new_harness function though...

PS: To clarify, test_in_new_harness would not change any of the
counters (test_failure, test_count, test_fixed, test_broken,
test_success), but would instead compare those against the values
specified as argument to test_in_new_harness.
PSPS: I have no idea how easy it would be to implement something like this.
-- 
Cheers,

Sverre Rabbelier
Junio C Hamano· Aug 17, 2010, 19:12 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1

Sverre Rabbelier <srabbelier@gmail.com> writes:
Show 7 quoted lines
> E.g., something like:
>
> test_in_new_harness 0 1 0 0 '
>     test_expect_failure 'pretend we have fixed a known breakage' '
>            :
>      '
> '
Now the question becomes who tests "test_in_new_harness" ;-)

I'd rather prefer not to touch this one. Isn't whatever the outer wrapper used flexible enough to be taught about this kind of thing and filter it?

Ævar Arnfjörð Bjarmason· Aug 18, 2010, 13:34 UTC · re: Junio C Hamano · lore

[PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test

On Tue, Aug 17, 2010 at 19:12, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
>> E.g., something like:
>>
>> test_in_new_harness 0 1 0 0 '
>>     test_expect_failure 'pretend we have fixed a known breakage' '
>>            :
>>      '
>> '
>
> Now the question becomes who tests "test_in_new_harness" ;-)
>
> I'd rather prefer not to touch this one.  Isn't whatever the outer wrapper
> used flexible enough to be taught about this kind of thing and filter it?

The problem is that you have to modify *all* the outer wrappers. Since the test suite can be run with any TAP consumer that isn't viable, and at the very least having to do:

    prove --source=Some::Custom::Git Thing ...
Is tedious, and requires us to maintain the Some::Custom::Git thingy.

Anyway, here's a better patch. It allows us to run a pasing TODO test, check if it works, and if so not mark *that* test as passing TODO, but simply as passing.

As a side benefit it makes the test lib more awesome.
Ævar Arnfjörð Bjarmason (4):
  test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and
    ../
  test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  test-lib: Allow overriding of TEST_DIRECTORY
  t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 t/test-lib.sh    |   39 +++++++++++++++++++++++----------------
 2 files changed, 54 insertions(+), 16 deletions(-)
-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test

On Tue, Aug 17, 2010 at 19:12, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> Sverre Rabbelier <srabbelier@gmail.com> writes:
>
>> E.g., something like:
>>
>> test_in_new_harness 0 1 0 0 '
>>     test_expect_failure 'pretend we have fixed a known breakage' '
>>            :
>>      '
>> '
>
> Now the question becomes who tests "test_in_new_harness" ;-)
>
> I'd rather prefer not to touch this one.  Isn't whatever the outer wrapper
> used flexible enough to be taught about this kind of thing and filter it?

The problem is that you have to modify *all* the outer wrappers. Since the test suite can be run with any TAP consumer that isn't viable, and at the very least having to do:

    prove --source=Some::Custom::Git Thing ...
Is tedious, and requires us to maintain the Some::Custom::Git thingy.

Anyway, here's a better patch. It allows us to run a pasing TODO test, check if it works, and if so not mark *that* test as passing TODO, but simply as passing.

As a side benefit it makes the test lib more awesome.
Ævar Arnfjörð Bjarmason (4):
  test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and
    ../
  test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  test-lib: Allow overriding of TEST_DIRECTORY
  t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 t/test-lib.sh    |   39 +++++++++++++++++++++++----------------
 2 files changed, 54 insertions(+), 16 deletions(-)
-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../

Change the redundant calls to $(pwd) to use $TEST_DIRECTORY instead. None of these were being executed after we cd'd somewhere else so they weren't actually needed.

This also makes it easier to add support for overriding the test library location and run tests in a different directory than t/.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Show changes to t/test-lib.sh +8 −8
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5bb7662..0e460f9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -830,14 +830,14 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		PATH="$TEST_DIRECTORY/..:$PATH"
 	fi
 fi
-GIT_BUILD_DIR=$(pwd)/..
-GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
 export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
 
-. ../GIT-BUILD-OPTIONS
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 
 if test -z "$GIT_TEST_CMP"
 then
@@ -849,22 +849,22 @@ then
 	fi
 fi
 
-GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
+GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d ../templates/blt || {
+test -d "$TEST_DIRECTORY"/../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d ../git_remote_helpers/build || {
+	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x ../test-chmtime; then
+if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:05 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../

Change code that used $TEST_DIRECTORY/.. to use $GIT_BUILD_DIR instead, the two are equivalent, but the latter is easier to read.

This required moving the assignment od GIT_BUILD_DIR to earlier in the test-lib.sh file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Note that the usage of GIT_BUILD_DIR in the first hunk isn't an error, since it's inside a function GIT_BUILD_DIR will be defined by the time it's used.

 t/test-lib.sh |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)
Show changes to t/test-lib.sh +15 −14
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0e460f9..689aa29 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -685,7 +685,7 @@ test_create_repo () {
 	repo="$1"
 	mkdir -p "$repo"
 	cd "$repo" || error "Cannot setup test environment"
-	"$GIT_EXEC_PATH/git-init" "--template=$TEST_DIRECTORY/../templates/blt/" >&3 2>&4 ||
+	"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
 	error "cannot run git init -- have you built things yet?"
 	mv .git/hooks .git/hooks-disabled
 	cd "$owd"
@@ -748,6 +748,8 @@ test_done () {
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
 TEST_DIRECTORY=$(pwd)
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -774,7 +776,7 @@ then
 		test -x "$1" || return
 
 		base=$(basename "$1")
-		symlink_target=$TEST_DIRECTORY/../$base
+		symlink_target=$GIT_BUILD_DIR/$base
 		# do not override scripts
 		if test -x "$symlink_target" &&
 		    test ! -d "$symlink_target" &&
@@ -793,7 +795,7 @@ then
 	# override all git executables in TEST_DIRECTORY/..
 	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
 	mkdir -p "$GIT_VALGRIND"/bin
-	for file in $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-*
+	for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
 	do
 		make_valgrind_symlink $file
 	done
@@ -814,10 +816,10 @@ then
 elif test -n "$GIT_TEST_INSTALLED" ; then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
 	error "Cannot run git from $GIT_TEST_INSTALLED."
-	PATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH
+	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
 	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
-	git_bin_dir="$TEST_DIRECTORY/../bin-wrappers"
+	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
 	if ! test -x "$git_bin_dir/git" ; then
 		if test -z "$with_dashes" ; then
 			say "$git_bin_dir/git is not executable; using GIT_EXEC_PATH"
@@ -825,13 +827,12 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		with_dashes=t
 	fi
 	PATH="$git_bin_dir:$PATH"
-	GIT_EXEC_PATH=$TEST_DIRECTORY/..
+	GIT_EXEC_PATH=$GIT_BUILD_DIR
 	if test -n "$with_dashes" ; then
-		PATH="$TEST_DIRECTORY/..:$PATH"
+		PATH="$GIT_BUILD_DIR:$PATH"
 	fi
 fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
-GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
+GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
@@ -849,22 +850,22 @@ then
 	fi
 fi
 
-GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
+GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d "$TEST_DIRECTORY"/../templates/blt || {
+test -d "$GIT_BUILD_DIR"/templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$GIT_BUILD_DIR/git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
+	test -d "$GIT_BUILD_DIR"/git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
+if ! test -x "$GIT_BUILD_DIR"/test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:06 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY

Tests that test the test-lib.sh itself need to be executed in the dynamically created trash directory, so we can't assume $TEST_DIRECTORY is ../ for those.

As a side benefit this change also makes it easy for us to move the t/*.sh tests into subdirectories if we ever want to do that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to t/test-lib.sh +7 −1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 689aa29..01ddf3e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -747,7 +747,13 @@ test_done () {
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
-TEST_DIRECTORY=$(pwd)
+if test -z "$TEST_DIRECTORY"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/, e.g. for running tests on the test library
+	# itself.
+	TEST_DIRECTORY=$(pwd)
+fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
 if test -n "$valgrind"

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:06 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib

Change the passing TODO test in t0000-basic.sh to run inside its own test-lib.sh. The motivation is to have nothing out of the ordinary on a normal test run for test smoking purposes.

If every normal test run has a passing TODO you're more likely to turn a blind eye to it and not to investigate cases where things really are passing unexpectedly.

It also makes the prove(1) output less noisy. Before:
    All tests successful.
    Test Summary Report
    -------------------
    ./t0000-basic.sh                                   (Wstat: 0 Tests: 46 Failed: 0)
      TODO passed:   5
    Files=484, Tests=6229, 143 wallclock secs ( 4.00 usr  4.15 sys + 104.77 cusr 351.57 csys = 464.49 CPU)
    Result: PASS
And after:
    All tests successful.
    Files=484, Tests=6228, 139 wallclock secs ( 4.07 usr  4.25 sys + 104.54 cusr 350.85 csys = 463.71 CPU)
    Result: PASS
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
Show changes to t/t0000-basic.sh +31 −0
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 9602085..1a977db 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -54,9 +54,40 @@ test_expect_success 'success is reported like this' '
 test_expect_failure 'pretend we have a known breakage' '
     false
 '
+
+test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' "
+    mkdir passing-todo &&
+    (cd passing-todo &&
+    cat >passing-todo.sh <<EOF &&
+#!/bin/sh
+
+test_description='A passing TODO test
+
+This is run in a sub test-lib so that we don't get incorrect passing
+metrics
+'
+
+# Point to the t/test-lib.sh, which isn't in ../ as usual
+TEST_DIRECTORY=\"$TEST_DIRECTORY\"
+. \"\$TEST_DIRECTORY\"/test-lib.sh
+
 test_expect_failure 'pretend we have fixed a known breakage' '
     :
 '
+
+test_done
+EOF
+    chmod +x passing-todo.sh &&
+    ./passing-todo.sh >out 2>err &&
+    ! test -s err &&
+cat >expect <<EOF &&
+ok 1 - pretend we have fixed a known breakage # TODO known breakage
+# fixed 1 known breakage(s)
+# passed all 1 test(s)
+1..1
+EOF
+    test_cmp expect out)
+"
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test

This *really* v3, I just sent another copy of v2 unchanged accidentally (this is really not my day). This one:

 * Fixes a trivial shell syntax error in the last patch (oops!)
 * Uses #!$SHELL_PATH for a shebang instead of #!/bin/sh in the file
   we're about to write out. This now passes e.g. on Solaris which has
   an insane /bin/sh.

As an aside there's a bunch of other things in our tests that are writing out shellscripts with a #!/bin/sh shebang instead of #!$SHELL_PATH. These are passing purely by accident since they just happen to use shell syntax that doesn't run afoul of grumpy old shells like Solaris's /bin/sh.

Ævar Arnfjörð Bjarmason (4):
  test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and
    ../
  test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  test-lib: Allow overriding of TEST_DIRECTORY
  t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 t/test-lib.sh    |   39 +++++++++++++++++++++++----------------
 2 files changed, 54 insertions(+), 16 deletions(-)
-- 
1.7.2.1.446.g168052
Jeff King· Aug 19, 2010, 16:18 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test

On Thu, Aug 19, 2010 at 04:08:08PM +0000, Ævar Arnfjörð Bjarmason wrote:
Show 5 quoted lines
> As an aside there's a bunch of other things in our tests that are
> writing out shellscripts with a #!/bin/sh shebang instead of
> #!$SHELL_PATH. These are passing purely by accident since they just
> happen to use shell syntax that doesn't run afoul of grumpy old shells
> like Solaris's /bin/sh.

It's not an accident. I (and others) checked them all when $SHELL_PATH became available to the test scripts, and declared them all trivial. But I would not be opposed to making them all #!$SHELL_PATH.

-Peff
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../

Change the redundant calls to $(pwd) to use $TEST_DIRECTORY instead. None of these were being executed after we cd'd somewhere else so they weren't actually needed.

This also makes it easier to add support for overriding the test library location and run tests in a different directory than t/.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Show changes to t/test-lib.sh +8 −8
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5bb7662..0e460f9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -830,14 +830,14 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		PATH="$TEST_DIRECTORY/..:$PATH"
 	fi
 fi
-GIT_BUILD_DIR=$(pwd)/..
-GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
 export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
 
-. ../GIT-BUILD-OPTIONS
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 
 if test -z "$GIT_TEST_CMP"
 then
@@ -849,22 +849,22 @@ then
 	fi
 fi
 
-GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
+GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d ../templates/blt || {
+test -d "$TEST_DIRECTORY"/../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d ../git_remote_helpers/build || {
+	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x ../test-chmtime; then
+if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.446.g168052
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../

Change code that used $TEST_DIRECTORY/.. to use $GIT_BUILD_DIR instead, the two are equivalent, but the latter is easier to read.

This required moving the assignment od GIT_BUILD_DIR to earlier in the test-lib.sh file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)
Show changes to t/test-lib.sh +15 −14
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0e460f9..689aa29 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -685,7 +685,7 @@ test_create_repo () {
 	repo="$1"
 	mkdir -p "$repo"
 	cd "$repo" || error "Cannot setup test environment"
-	"$GIT_EXEC_PATH/git-init" "--template=$TEST_DIRECTORY/../templates/blt/" >&3 2>&4 ||
+	"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
 	error "cannot run git init -- have you built things yet?"
 	mv .git/hooks .git/hooks-disabled
 	cd "$owd"
@@ -748,6 +748,8 @@ test_done () {
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
 TEST_DIRECTORY=$(pwd)
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -774,7 +776,7 @@ then
 		test -x "$1" || return
 
 		base=$(basename "$1")
-		symlink_target=$TEST_DIRECTORY/../$base
+		symlink_target=$GIT_BUILD_DIR/$base
 		# do not override scripts
 		if test -x "$symlink_target" &&
 		    test ! -d "$symlink_target" &&
@@ -793,7 +795,7 @@ then
 	# override all git executables in TEST_DIRECTORY/..
 	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
 	mkdir -p "$GIT_VALGRIND"/bin
-	for file in $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-*
+	for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
 	do
 		make_valgrind_symlink $file
 	done
@@ -814,10 +816,10 @@ then
 elif test -n "$GIT_TEST_INSTALLED" ; then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
 	error "Cannot run git from $GIT_TEST_INSTALLED."
-	PATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH
+	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
 	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
-	git_bin_dir="$TEST_DIRECTORY/../bin-wrappers"
+	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
 	if ! test -x "$git_bin_dir/git" ; then
 		if test -z "$with_dashes" ; then
 			say "$git_bin_dir/git is not executable; using GIT_EXEC_PATH"
@@ -825,13 +827,12 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		with_dashes=t
 	fi
 	PATH="$git_bin_dir:$PATH"
-	GIT_EXEC_PATH=$TEST_DIRECTORY/..
+	GIT_EXEC_PATH=$GIT_BUILD_DIR
 	if test -n "$with_dashes" ; then
-		PATH="$TEST_DIRECTORY/..:$PATH"
+		PATH="$GIT_BUILD_DIR:$PATH"
 	fi
 fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
-GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
+GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
@@ -849,22 +850,22 @@ then
 	fi
 fi
 
-GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
+GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d "$TEST_DIRECTORY"/../templates/blt || {
+test -d "$GIT_BUILD_DIR"/templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$GIT_BUILD_DIR/git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
+	test -d "$GIT_BUILD_DIR"/git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
+if ! test -x "$GIT_BUILD_DIR"/test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.446.g168052
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 3/4] test-lib: Allow overriding of TEST_DIRECTORY

Tests that test the test-lib.sh itself need to be executed in the dynamically created trash directory, so we can't assume $TEST_DIRECTORY is ../ for those.

As a side benefit this change also makes it easy for us to move the t/*.sh tests into subdirectories if we ever want to do that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to t/test-lib.sh +7 −1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 689aa29..01ddf3e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -747,7 +747,13 @@ test_done () {
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
-TEST_DIRECTORY=$(pwd)
+if test -z "$TEST_DIRECTORY"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/, e.g. for running tests on the test library
+	# itself.
+	TEST_DIRECTORY=$(pwd)
+fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
 if test -n "$valgrind"

-- 
1.7.2.1.446.g168052
Ævar Arnfjörð Bjarmason· Aug 19, 2010, 16:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v3 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib

Change the passing TODO test in t0000-basic.sh to run inside its own test-lib.sh. The motivation is to have nothing out of the ordinary on a normal test run for test smoking purposes.

If every normal test run has a passing TODO you're more likely to turn a blind eye to it and not to investigate cases where things really are passing unexpectedly.

It also makes the prove(1) output less noisy. Before:
    All tests successful.
    Test Summary Report
    -------------------
    ./t0000-basic.sh                                   (Wstat: 0 Tests: 46 Failed: 0)
      TODO passed:   5
    Files=484, Tests=6229, 143 wallclock secs ( 4.00 usr  4.15 sys + 104.77 cusr 351.57 csys = 464.49 CPU)
    Result: PASS
And after:
    All tests successful.
    Files=484, Tests=6228, 139 wallclock secs ( 4.07 usr  4.25 sys + 104.54 cusr 350.85 csys = 463.71 CPU)
    Result: PASS
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
Show changes to t/t0000-basic.sh +31 −0
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 9602085..f688bd3 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -54,9 +54,40 @@ test_expect_success 'success is reported like this' '
 test_expect_failure 'pretend we have a known breakage' '
     false
 '
+
+test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' "
+    mkdir passing-todo &&
+    (cd passing-todo &&
+    cat >passing-todo.sh <<EOF &&
+#!$SHELL_PATH
+
+test_description='A passing TODO test
+
+This is run in a sub test-lib so that we do not get incorrect passing
+metrics
+'
+
+# Point to the t/test-lib.sh, which isn't in ../ as usual
+TEST_DIRECTORY=\"$TEST_DIRECTORY\"
+. \"\$TEST_DIRECTORY\"/test-lib.sh
+
 test_expect_failure 'pretend we have fixed a known breakage' '
     :
 '
+
+test_done
+EOF
+    chmod +x passing-todo.sh &&
+    ./passing-todo.sh >out 2>err &&
+    ! test -s err &&
+cat >expect <<EOF &&
+ok 1 - pretend we have fixed a known breakage # TODO known breakage
+# fixed 1 known breakage(s)
+# passed all 1 test(s)
+1..1
+EOF
+    test_cmp expect out)
+"
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '

-- 
1.7.2.1.446.g168052
Ævar Arnfjörð Bjarmason· Aug 18, 2010, 13:34 UTC · re: Junio C Hamano · lore

[PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../

Change the redundant calls to $(pwd) to use $TEST_DIRECTORY instead. None of these were being executed after we cd'd somewhere else so they weren't actually needed.

This also makes it easier to add support for overriding the test library location and run tests in a different directory than t/.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Show changes to t/test-lib.sh +8 −8
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5bb7662..0e460f9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -830,14 +830,14 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		PATH="$TEST_DIRECTORY/..:$PATH"
 	fi
 fi
-GIT_BUILD_DIR=$(pwd)/..
-GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
 export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOBAL
 
-. ../GIT-BUILD-OPTIONS
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 
 if test -z "$GIT_TEST_CMP"
 then
@@ -849,22 +849,22 @@ then
 	fi
 fi
 
-GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
+GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d ../templates/blt || {
+test -d "$TEST_DIRECTORY"/../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d ../git_remote_helpers/build || {
+	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x ../test-chmtime; then
+if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 18, 2010, 13:34 UTC · re: Junio C Hamano · lore

[PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../

Change code that used $TEST_DIRECTORY/.. to use $GIT_BUILD_DIR instead, the two are equivalent, but the latter is easier to read.

This required moving the assignment od GIT_BUILD_DIR to earlier in the test-lib.sh file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Note that the usage of GIT_BUILD_DIR in the first hunk isn't an error, since it's inside a function GIT_BUILD_DIR will be defined by the time it's used.

 t/test-lib.sh |   29 +++++++++++++++--------------
 1 files changed, 15 insertions(+), 14 deletions(-)
Show changes to t/test-lib.sh +15 −14
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0e460f9..689aa29 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -685,7 +685,7 @@ test_create_repo () {
 	repo="$1"
 	mkdir -p "$repo"
 	cd "$repo" || error "Cannot setup test environment"
-	"$GIT_EXEC_PATH/git-init" "--template=$TEST_DIRECTORY/../templates/blt/" >&3 2>&4 ||
+	"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
 	error "cannot run git init -- have you built things yet?"
 	mv .git/hooks .git/hooks-disabled
 	cd "$owd"
@@ -748,6 +748,8 @@ test_done () {
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
 TEST_DIRECTORY=$(pwd)
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -774,7 +776,7 @@ then
 		test -x "$1" || return
 
 		base=$(basename "$1")
-		symlink_target=$TEST_DIRECTORY/../$base
+		symlink_target=$GIT_BUILD_DIR/$base
 		# do not override scripts
 		if test -x "$symlink_target" &&
 		    test ! -d "$symlink_target" &&
@@ -793,7 +795,7 @@ then
 	# override all git executables in TEST_DIRECTORY/..
 	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
 	mkdir -p "$GIT_VALGRIND"/bin
-	for file in $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-*
+	for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
 	do
 		make_valgrind_symlink $file
 	done
@@ -814,10 +816,10 @@ then
 elif test -n "$GIT_TEST_INSTALLED" ; then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
 	error "Cannot run git from $GIT_TEST_INSTALLED."
-	PATH=$GIT_TEST_INSTALLED:$TEST_DIRECTORY/..:$PATH
+	PATH=$GIT_TEST_INSTALLED:$GIT_BUILD_DIR:$PATH
 	GIT_EXEC_PATH=${GIT_TEST_EXEC_PATH:-$GIT_EXEC_PATH}
 else # normal case, use ../bin-wrappers only unless $with_dashes:
-	git_bin_dir="$TEST_DIRECTORY/../bin-wrappers"
+	git_bin_dir="$GIT_BUILD_DIR/bin-wrappers"
 	if ! test -x "$git_bin_dir/git" ; then
 		if test -z "$with_dashes" ; then
 			say "$git_bin_dir/git is not executable; using GIT_EXEC_PATH"
@@ -825,13 +827,12 @@ else # normal case, use ../bin-wrappers only unless $with_dashes:
 		with_dashes=t
 	fi
 	PATH="$git_bin_dir:$PATH"
-	GIT_EXEC_PATH=$TEST_DIRECTORY/..
+	GIT_EXEC_PATH=$GIT_BUILD_DIR
 	if test -n "$with_dashes" ; then
-		PATH="$TEST_DIRECTORY/..:$PATH"
+		PATH="$GIT_BUILD_DIR:$PATH"
 	fi
 fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
-GIT_TEMPLATE_DIR="$TEST_DIRECTORY"/../templates/blt
+GIT_TEMPLATE_DIR="$GIT_BUILD_DIR"/templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_CONFIG_NOGLOBAL=1
@@ -849,22 +850,22 @@ then
 	fi
 fi
 
-GITPERLLIB="$TEST_DIRECTORY"/../perl/blib/lib:"$TEST_DIRECTORY"/../perl/blib/arch/auto/Git
+GITPERLLIB="$GIT_BUILD_DIR"/perl/blib/lib:"$GIT_BUILD_DIR"/perl/blib/arch/auto/Git
 export GITPERLLIB
-test -d "$TEST_DIRECTORY"/../templates/blt || {
+test -d "$GIT_BUILD_DIR"/templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
 if test -z "$GIT_TEST_INSTALLED" && test -z "$NO_PYTHON"
 then
-	GITPYTHONLIB="$TEST_DIRECTORY/../git_remote_helpers/build/lib"
+	GITPYTHONLIB="$GIT_BUILD_DIR/git_remote_helpers/build/lib"
 	export GITPYTHONLIB
-	test -d "$TEST_DIRECTORY"/../git_remote_helpers/build || {
+	test -d "$GIT_BUILD_DIR"/git_remote_helpers/build || {
 		error "You haven't built git_remote_helpers yet, have you?"
 	}
 fi
 
-if ! test -x "$TEST_DIRECTORY"/../test-chmtime; then
+if ! test -x "$GIT_BUILD_DIR"/test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 	exit 1

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 18, 2010, 13:34 UTC · re: Junio C Hamano · lore

[PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY

Tests that test the test-lib.sh itself need to be executed in the dynamically created trash directory, so we can't assume $TEST_DIRECTORY is ../ for those.

As a side benefit this change also makes it easy for us to move the t/*.sh tests into subdirectories if we ever want to do that.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to t/test-lib.sh +7 −1
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 689aa29..01ddf3e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -747,7 +747,13 @@ test_done () {
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
-TEST_DIRECTORY=$(pwd)
+if test -z "$TEST_DIRECTORY"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/, e.g. for running tests on the test library
+	# itself.
+	TEST_DIRECTORY=$(pwd)
+fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
 if test -n "$valgrind"

-- 
1.7.2.1.414.g9bf49
Ævar Arnfjörð Bjarmason· Aug 18, 2010, 13:34 UTC · re: Junio C Hamano · lore

[PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib

Change the passing TODO test in t0000-basic.sh to run inside its own test-lib.sh. The motivation is to have nothing out of the ordinary on a normal test run for test smoking purposes.

If every normal test run has a passing TODO you're more likely to turn a blind eye to it and not to investigate cases where things really are passing unexpectedly.

It also makes the prove(1) output less noisy. Before:
    All tests successful.
    Test Summary Report
    -------------------
    ./t0000-basic.sh                                   (Wstat: 0 Tests: 46 Failed: 0)
      TODO passed:   5
    Files=484, Tests=6229, 143 wallclock secs ( 4.00 usr  4.15 sys + 104.77 cusr 351.57 csys = 464.49 CPU)
    Result: PASS
And after:
    All tests successful.
    Files=484, Tests=6228, 139 wallclock secs ( 4.07 usr  4.25 sys + 104.54 cusr 350.85 csys = 463.71 CPU)
    Result: PASS
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0000-basic.sh |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
Show changes to t/t0000-basic.sh +31 −0
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 9602085..1a977db 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -54,9 +54,40 @@ test_expect_success 'success is reported like this' '
 test_expect_failure 'pretend we have a known breakage' '
     false
 '
+
+test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' "
+    mkdir passing-todo &&
+    (cd passing-todo &&
+    cat >passing-todo.sh <<EOF &&
+#!/bin/sh
+
+test_description='A passing TODO test
+
+This is run in a sub test-lib so that we don't get incorrect passing
+metrics
+'
+
+# Point to the t/test-lib.sh, which isn't in ../ as usual
+TEST_DIRECTORY=\"$TEST_DIRECTORY\"
+. \"\$TEST_DIRECTORY\"/test-lib.sh
+
 test_expect_failure 'pretend we have fixed a known breakage' '
     :
 '
+
+test_done
+EOF
+    chmod +x passing-todo.sh &&
+    ./passing-todo.sh >out 2>err &&
+    ! test -s err &&
+cat >expect <<EOF &&
+ok 1 - pretend we have fixed a known breakage # TODO known breakage
+# fixed 1 known breakage(s)
+# passed all 1 test(s)
+1..1
+EOF
+    test_cmp expect out)
+"
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '

-- 
1.7.2.1.414.g9bf49
Sverre Rabbelier· Aug 19, 2010, 17:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib

Heya,
On Wed, Aug 18, 2010 at 08:34, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Change the passing TODO test in t0000-basic.sh to run inside its own
> test-lib.sh. The motivation is to have nothing out of the ordinary on
> a normal test run for test smoking purposes.

I like the effect of this patch, but the patch itself is a bit obscure perhaps. I'll defer judgement on whether this is acceptable to others :).

-- 
Cheers,

Sverre Rabbelier

← back to recent threads