[PATCH] Prefix shell test output messages with test id

[PATCH] Prefix shell test output messages with test id

[Jan-Marek Glogowski]

This adds the test ID (tXXXX) prefix to the test result message of
all shell tests.  This is especially useful when doing a parallel
check run, where it's currently quite hard to identify the actual
failing test case.

Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
---
 t/t0000-basic.sh        | 28 ++++++++++++++--------------
 t/test-lib-functions.sh | 11 +++++++----
 t/test-lib.sh           | 10 ++++++----
 3 Dateien geändert, 27 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ccb5435..1bbf5b8 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -58,7 +58,7 @@ test_expect_failure 'pretend we have a known breakage' '
 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 &&
+	cat >0000t05-passing-todo.sh <<-EOF &&
 	#!$SHELL_PATH
 
 	test_description='A passing TODO test
@@ -77,14 +77,14 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib
 
 	test_done
 	EOF
-	chmod +x passing-todo.sh &&
-	./passing-todo.sh >out 2>err &&
+	chmod +x 0000t05-passing-todo.sh &&
+	./0000t05-passing-todo.sh >out 2>err &&
 	! test -s err &&
 	sed -e 's/^> //' >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
+	> 0000t05: ok 1 - pretend we have fixed a known breakage # TODO known breakage
+	> 0000t05: # fixed 1 known breakage(s)
+	> 0000t05: # passed all 1 test(s)
+	> 0000t05: 1..1
 	EOF
 	test_cmp expect out)
 "
@@ -141,7 +141,7 @@ test_expect_success 'tests clean up even on failures' "
 	(
 	cd failing-cleanup &&
 
-	cat >failing-cleanup.sh <<-EOF &&
+	cat >0000t12-failing-cleanup.sh <<-EOF &&
 	#!$SHELL_PATH
 
 	test_description='Failing tests with cleanup commands'
@@ -162,23 +162,23 @@ test_expect_success 'tests clean up even on failures' "
 
 	EOF
 
-	chmod +x failing-cleanup.sh &&
-	test_must_fail ./failing-cleanup.sh >out 2>err &&
+	chmod +x 0000t12-failing-cleanup.sh &&
+	test_must_fail ./0000t12-failing-cleanup.sh >out 2>err &&
 	! test -s err &&
 	! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
 	sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
-	> not ok - 1 tests clean up even after a failure
+	> 0000t12: not ok 1 - tests clean up even after a failure
 	> #	Z
 	> #	touch clean-after-failure &&
 	> #	test_when_finished rm clean-after-failure &&
 	> #	(exit 1)
 	> #	Z
-	> not ok - 2 failure to clean up causes the test to fail
+	> 0000t12: not ok 2 - failure to clean up causes the test to fail
 	> #	Z
 	> #	test_when_finished \"(exit 2)\"
 	> #	Z
-	> # failed 2 among 2 test(s)
-	> 1..2
+	> 0000t12: # failed 2 among 2 test(s)
+	> 0000t12: 1..2
 	EOF
 	test_cmp expect out
 	)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 9bc57d2..c81ad7f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -24,6 +24,9 @@
 #
 # In particular, quoting isn't enough, as the path may contain the same quote
 # that we're using.
+
+TID=$(basename ${0%%-*})
+
 test_set_editor () {
 	FAKE_EDITOR="$1"
 	export FAKE_EDITOR
@@ -434,7 +437,7 @@ test_external_without_stderr () {
 test_path_is_file () {
 	if ! [ -f "$1" ]
 	then
-		echo "File $1 doesn't exist. $*"
+		echo "$TID: File $1 doesn't exist. $*"
 		false
 	fi
 }
@@ -442,7 +445,7 @@ test_path_is_file () {
 test_path_is_dir () {
 	if ! [ -d "$1" ]
 	then
-		echo "Directory $1 doesn't exist. $*"
+		echo "$TID: Directory $1 doesn't exist. $*"
 		false
 	fi
 }
@@ -450,7 +453,7 @@ test_path_is_dir () {
 test_path_is_missing () {
 	if [ -e "$1" ]
 	then
-		echo "Path exists:"
+		echo "$TID: Path exists:"
 		ls -ld "$1"
 		if [ $# -ge 1 ]; then
 			echo "$*"
@@ -476,7 +479,7 @@ test_line_count () {
 		error "bug in the test script: not 3 parameters to test_line_count"
 	elif ! test $(wc -l <"$3") "$1" "$2"
 	then
-		echo "test_line_count: line count for $3 !$1 $2"
+		echo "$TID: test_line_count: line count for $3 !$1 $2"
 		cat "$3"
 		return 1
 	fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..6fccbe9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -31,6 +31,8 @@ done,*)
 	;;
 esac
 
+TID=$(basename ${0%%-*})
+
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
 
@@ -185,7 +187,7 @@ if test -n "$color"; then
 			*) test -n "$quiet" && return;;
 		esac
 		shift
-		printf "%s" "$*"
+		printf "$TID: %s" "$*"
 		tput sgr0
 		echo
 		)
@@ -194,12 +196,12 @@ else
 	say_color() {
 		test -z "$1" && test -n "$quiet" && return
 		shift
-		echo "$*"
+		echo "$TID: $*"
 	}
 fi
 
 error () {
-	say_color error "error: $*"
+	say_color error "$TID: error: $*"
 	GIT_EXIT_OK=t
 	exit 1
 }
@@ -262,7 +264,7 @@ test_ok_ () {
 
 test_failure_ () {
 	test_failure=$(($test_failure + 1))
-	say_color error "not ok - $test_count $1"
+	say_color error "not ok $test_count - $1"
 	shift
 	echo "$@" | sed -e 's/^/#	/'
 	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
-- 
1.7.11.5

Re: [PATCH] Prefix shell test output messages with test id

[Thomas Gummerer]

On 09/06, Jan-Marek Glogowski wrote:
> This adds the test ID (tXXXX) prefix to the test result message of
> all shell tests.  This is especially useful when doing a parallel
> check run, where it's currently quite hard to identify the actual
> failing test case.
> 
> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>

This breaks the TAP output format of the tests, which is needed to run
them with prove.  To identify the failing tests more easily when running
the tests in parallel, you may want to add GIT_TEST_TARGET = prove to
your config.mak.

If this change is really needed, I think you should add the test-id after
the message.
> ---
>  t/t0000-basic.sh        | 28 ++++++++++++++--------------
>  t/test-lib-functions.sh | 11 +++++++----
>  t/test-lib.sh           | 10 ++++++----
>  3 Dateien geändert, 27 Zeilen hinzugefügt(+), 22 Zeilen entfernt(-)
> 
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index ccb5435..1bbf5b8 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -58,7 +58,7 @@ test_expect_failure 'pretend we have a known breakage' '
>  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 &&
> +	cat >0000t05-passing-todo.sh <<-EOF &&
>  	#!$SHELL_PATH
>  
>  	test_description='A passing TODO test
> @@ -77,14 +77,14 @@ test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib
>  
>  	test_done
>  	EOF
> -	chmod +x passing-todo.sh &&
> -	./passing-todo.sh >out 2>err &&
> +	chmod +x 0000t05-passing-todo.sh &&
> +	./0000t05-passing-todo.sh >out 2>err &&
>  	! test -s err &&
>  	sed -e 's/^> //' >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
> +	> 0000t05: ok 1 - pretend we have fixed a known breakage # TODO known breakage
> +	> 0000t05: # fixed 1 known breakage(s)
> +	> 0000t05: # passed all 1 test(s)
> +	> 0000t05: 1..1
>  	EOF
>  	test_cmp expect out)
>  "
> @@ -141,7 +141,7 @@ test_expect_success 'tests clean up even on failures' "
>  	(
>  	cd failing-cleanup &&
>  
> -	cat >failing-cleanup.sh <<-EOF &&
> +	cat >0000t12-failing-cleanup.sh <<-EOF &&
>  	#!$SHELL_PATH
>  
>  	test_description='Failing tests with cleanup commands'
> @@ -162,23 +162,23 @@ test_expect_success 'tests clean up even on failures' "
>  
>  	EOF
>  
> -	chmod +x failing-cleanup.sh &&
> -	test_must_fail ./failing-cleanup.sh >out 2>err &&
> +	chmod +x 0000t12-failing-cleanup.sh &&
> +	test_must_fail ./0000t12-failing-cleanup.sh >out 2>err &&
>  	! test -s err &&
>  	! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
>  	sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
> -	> not ok - 1 tests clean up even after a failure
> +	> 0000t12: not ok 1 - tests clean up even after a failure
>  	> #	Z
>  	> #	touch clean-after-failure &&
>  	> #	test_when_finished rm clean-after-failure &&
>  	> #	(exit 1)
>  	> #	Z
> -	> not ok - 2 failure to clean up causes the test to fail
> +	> 0000t12: not ok 2 - failure to clean up causes the test to fail
>  	> #	Z
>  	> #	test_when_finished \"(exit 2)\"
>  	> #	Z
> -	> # failed 2 among 2 test(s)
> -	> 1..2
> +	> 0000t12: # failed 2 among 2 test(s)
> +	> 0000t12: 1..2
>  	EOF
>  	test_cmp expect out
>  	)
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 9bc57d2..c81ad7f 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -24,6 +24,9 @@
>  #
>  # In particular, quoting isn't enough, as the path may contain the same quote
>  # that we're using.
> +
> +TID=$(basename ${0%%-*})
> +
>  test_set_editor () {
>  	FAKE_EDITOR="$1"
>  	export FAKE_EDITOR
> @@ -434,7 +437,7 @@ test_external_without_stderr () {
>  test_path_is_file () {
>  	if ! [ -f "$1" ]
>  	then
> -		echo "File $1 doesn't exist. $*"
> +		echo "$TID: File $1 doesn't exist. $*"
>  		false
>  	fi
>  }
> @@ -442,7 +445,7 @@ test_path_is_file () {
>  test_path_is_dir () {
>  	if ! [ -d "$1" ]
>  	then
> -		echo "Directory $1 doesn't exist. $*"
> +		echo "$TID: Directory $1 doesn't exist. $*"
>  		false
>  	fi
>  }
> @@ -450,7 +453,7 @@ test_path_is_dir () {
>  test_path_is_missing () {
>  	if [ -e "$1" ]
>  	then
> -		echo "Path exists:"
> +		echo "$TID: Path exists:"
>  		ls -ld "$1"
>  		if [ $# -ge 1 ]; then
>  			echo "$*"
> @@ -476,7 +479,7 @@ test_line_count () {
>  		error "bug in the test script: not 3 parameters to test_line_count"
>  	elif ! test $(wc -l <"$3") "$1" "$2"
>  	then
> -		echo "test_line_count: line count for $3 !$1 $2"
> +		echo "$TID: test_line_count: line count for $3 !$1 $2"
>  		cat "$3"
>  		return 1
>  	fi
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 78c4286..6fccbe9 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -31,6 +31,8 @@ done,*)
>  	;;
>  esac
>  
> +TID=$(basename ${0%%-*})
> +
>  # Keep the original TERM for say_color
>  ORIGINAL_TERM=$TERM
>  
> @@ -185,7 +187,7 @@ if test -n "$color"; then
>  			*) test -n "$quiet" && return;;
>  		esac
>  		shift
> -		printf "%s" "$*"
> +		printf "$TID: %s" "$*"
>  		tput sgr0
>  		echo
>  		)
> @@ -194,12 +196,12 @@ else
>  	say_color() {
>  		test -z "$1" && test -n "$quiet" && return
>  		shift
> -		echo "$*"
> +		echo "$TID: $*"
>  	}
>  fi
>  
>  error () {
> -	say_color error "error: $*"
> +	say_color error "$TID: error: $*"
>  	GIT_EXIT_OK=t
>  	exit 1
>  }
> @@ -262,7 +264,7 @@ test_ok_ () {
>  
>  test_failure_ () {
>  	test_failure=$(($test_failure + 1))
> -	say_color error "not ok - $test_count $1"
> +	say_color error "not ok $test_count - $1"
>  	shift
>  	echo "$@" | sed -e 's/^/#	/'
>  	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
> -- 
> 1.7.11.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Prefix shell test output messages with test id

[Jeff King]

On Thu, Sep 06, 2012 at 02:34:00PM +0200, Thomas Gummerer wrote:

> On 09/06, Jan-Marek Glogowski wrote:
> > This adds the test ID (tXXXX) prefix to the test result message of
> > all shell tests.  This is especially useful when doing a parallel
> > check run, where it's currently quite hard to identify the actual
> > failing test case.
> > 
> > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> 
> This breaks the TAP output format of the tests, which is needed to run
> them with prove.  To identify the failing tests more easily when running
> the tests in parallel, you may want to add GIT_TEST_TARGET = prove to
> your config.mak.

Yeah, I would second the suggestion to just use "prove". However, if for
some reason that isn't an option, note that the stock harness stores
results in test-results/. You can just grep it like this:

  grep 'failed [^0]' test-results/*

to find out which scripts had failures, which is usually sufficient (if
you're running the tests in parallel, you won't have any sensible output
anyway, so your first step is going to be to rerun the failed script
with "-v" anyway).

But really, "prove" is much nicer, and I recommend it unless you don't
have perl on your system.

-Peff

Re: [PATCH] Prefix shell test output messages with test id

[Jan-Marek Glogowski]

Am 06.09.2012 14:34, schrieb Thomas Gummerer:
> On 09/06, Jan-Marek Glogowski wrote:
>> This adds the test ID (tXXXX) prefix to the test result message of
>> all shell tests.  This is especially useful when doing a parallel
>> check run, where it's currently quite hard to identify the actual
>> failing test case.
>>
>> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> 
> This breaks the TAP output format of the tests, which is needed to run
> them with prove.  To identify the failing tests more easily when running
> the tests in parallel, you may want to add GIT_TEST_TARGET = prove to
> your config.mak.
> 
> If this change is really needed, I think you should add the test-id after
> the message.

While grep'ing for prove I found t/README...

>From my point of view the patch can be dropped. I can set GIT_PROVE_OPTS
to run prove with multiple CPUS. Is there a known (easy) way to
propagate MAKEFLAGS -j to prove?

And I would like to propose a symlink from Documentation/testing =>
../t/README.

Thanks for the quick review.