git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1
@ 2010-08-17  9:43 Ævar Arnfjörð Bjarmason
  2010-08-17 17:57 ` Sverre Rabbelier
  0 siblings, 1 reply; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-17  9:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1
  2010-08-17  9:43 [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1 Ævar Arnfjörð Bjarmason
@ 2010-08-17 17:57 ` Sverre Rabbelier
  2010-08-17 19:12   ` Junio C Hamano
  0 siblings, 1 reply; 20+ messages in thread
From: Sverre Rabbelier @ 2010-08-17 17:57 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano

Heya,

On Tue, Aug 17, 2010 at 04:43, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1
  2010-08-17 17:57 ` Sverre Rabbelier
@ 2010-08-17 19:12   ` Junio C Hamano
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                       ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Junio C Hamano @ 2010-08-17 19:12 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Ævar Arnfjörð Bjarmason, git

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?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test
  2010-08-17 19:12   ` Junio C Hamano
@ 2010-08-18 13:34     ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
                         ` (9 more replies)
  2010-08-18 13:34     ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
                       ` (3 subsequent siblings)
  4 siblings, 10 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-18 13:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

On Tue, Aug 17, 2010 at 19:12, Junio C Hamano <gitster@pobox.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../
  2010-08-17 19:12   ` Junio C Hamano
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
@ 2010-08-18 13:34     ` Ævar Arnfjörð Bjarmason
  2010-08-18 13:34     ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-18 13:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  2010-08-17 19:12   ` Junio C Hamano
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
  2010-08-18 13:34     ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
@ 2010-08-18 13:34     ` Ævar Arnfjörð Bjarmason
  2010-08-18 13:34     ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
  2010-08-18 13:34     ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-18 13:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY
  2010-08-17 19:12   ` Junio C Hamano
                       ` (2 preceding siblings ...)
  2010-08-18 13:34     ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
@ 2010-08-18 13:34     ` Ævar Arnfjörð Bjarmason
  2010-08-18 13:34     ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
  4 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-18 13:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
  2010-08-17 19:12   ` Junio C Hamano
                       ` (3 preceding siblings ...)
  2010-08-18 13:34     ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
@ 2010-08-18 13:34     ` Ævar Arnfjörð Bjarmason
  2010-08-19 17:39       ` Sverre Rabbelier
  4 siblings, 1 reply; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-18 13:34 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
                         ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

On Tue, Aug 17, 2010 at 19:12, Junio C Hamano <gitster@pobox.com> wrote:
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
                         ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:05       ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:06       ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
                         ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:05 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (2 preceding siblings ...)
  2010-08-19 16:05       ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:06       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:06       ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
                         ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:06 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (3 preceding siblings ...)
  2010-08-19 16:06       ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:06       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:08       ` [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:06 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (4 preceding siblings ...)
  2010-08-19 16:06       ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:08       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:18         ` Jeff King
  2010-08-19 16:08       ` [PATCH v3 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
                         ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (5 preceding siblings ...)
  2010-08-19 16:08       ` [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:08       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:08       ` [PATCH v3 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
                         ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (6 preceding siblings ...)
  2010-08-19 16:08       ` [PATCH v3 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:08       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:08       ` [PATCH v3 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
  2010-08-19 16:08       ` [PATCH v3 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 3/4] test-lib: Allow overriding of TEST_DIRECTORY
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (7 preceding siblings ...)
  2010-08-19 16:08       ` [PATCH v3 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:08       ` Ævar Arnfjörð Bjarmason
  2010-08-19 16:08       ` [PATCH v3 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
  2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
                         ` (8 preceding siblings ...)
  2010-08-19 16:08       ` [PATCH v3 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:08       ` Ævar Arnfjörð Bjarmason
  9 siblings, 0 replies; 20+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2010-08-19 16:08 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason

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(-)

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test
  2010-08-19 16:08       ` [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
@ 2010-08-19 16:18         ` Jeff King
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff King @ 2010-08-19 16:18 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano

On Thu, Aug 19, 2010 at 04:08:08PM +0000, Ævar Arnfjörð Bjarmason wrote:

> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib
  2010-08-18 13:34     ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
@ 2010-08-19 17:39       ` Sverre Rabbelier
  0 siblings, 0 replies; 20+ messages in thread
From: Sverre Rabbelier @ 2010-08-19 17:39 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2010-08-19 17:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-17  9:43 [PATCH] t/t0000-basic.sh: Don't run a passing TODO unless TEST_PASSING_TODO=1 Ævar Arnfjörð Bjarmason
2010-08-17 17:57 ` Sverre Rabbelier
2010-08-17 19:12   ` Junio C Hamano
2010-08-18 13:34     ` [PATCH v2 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
2010-08-19 16:05       ` Ævar Arnfjörð Bjarmason
2010-08-19 16:05       ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
2010-08-19 16:05       ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
2010-08-19 16:06       ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
2010-08-19 16:06       ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
2010-08-19 16:08       ` [PATCH v3 0/4] Support for running tests outside t/ + don't run a TODO test Ævar Arnfjörð Bjarmason
2010-08-19 16:18         ` Jeff King
2010-08-19 16:08       ` [PATCH v3 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
2010-08-19 16:08       ` [PATCH v3 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
2010-08-19 16:08       ` [PATCH v3 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
2010-08-19 16:08       ` [PATCH v3 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
2010-08-18 13:34     ` [PATCH v2 1/4] test-lib: Use $TEST_DIRECTORY or $GIT_BUILD_DIR instead of $(pwd) and ../ Ævar Arnfjörð Bjarmason
2010-08-18 13:34     ` [PATCH v2 2/4] test-lib: Use "$GIT_BUILD_DIR" instead of "$TEST_DIRECTORY"/../ Ævar Arnfjörð Bjarmason
2010-08-18 13:34     ` [PATCH v2 3/4] test-lib: Allow overriding of TEST_DIRECTORY Ævar Arnfjörð Bjarmason
2010-08-18 13:34     ` [PATCH v2 4/4] t/t0000-basic.sh: Run the passing TODO test inside its own test-lib Ævar Arnfjörð Bjarmason
2010-08-19 17:39       ` Sverre Rabbelier

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).