git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v4 0/8] --valgrind improvements
@ 2013-06-23 18:12 Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 1/8] test-lib: enable MALLOC_* for the actual tests Thomas Rast
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

> Please hold off; Fredrik found an issue with the test automation that
> bisects to "verbose mode for only tests matching a pattern"; when
> running with the 'test' target (not with prove however), the suite
> reports
> 
>   failed test(s):  
> 
>   fixed   0
>   success 9788
>   failed  2
>   broken  69
>   total   9989
> 
> Even worse, I botched a rebase that makes the commit before --
> "self-test that --verbose works" untestable.  I'm pretty sure that it
> has the same problem, too, though I'm still investigating the actual
> issue.

This should fix it.  The culprit was that the test suite generates
.counts files when not under $TEST_HARNESS, and these are seen and
reported by aggregate-results.  So this version changes 4/8 to set
TEST_HARNESS, instead of unsetting it.

Sorry for the trouble.


Thomas Rast (8):
  test-lib: enable MALLOC_* for the actual tests
  test-lib: refactor $GIT_SKIP_TESTS matching
  test-lib: rearrange start/end of test_expect_* and test_skip
  test-lib: self-test that --verbose works
  test-lib: verbose mode for only tests matching a pattern
  test-lib: valgrind for only tests matching a pattern
  test-lib: allow prefixing a custom string before "ok N" etc.
  test-lib: support running tests under valgrind in parallel

 t/README                |  10 ++
 t/t0000-basic.sh        |  61 +++++++++++-
 t/test-lib-functions.sh |   6 +-
 t/test-lib.sh           | 246 ++++++++++++++++++++++++++++++++++++++----------
 t/valgrind/valgrind.sh  |   3 +
 5 files changed, 274 insertions(+), 52 deletions(-)

-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 1/8] test-lib: enable MALLOC_* for the actual tests
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 2/8] test-lib: refactor $GIT_SKIP_TESTS matching Thomas Rast
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the
MALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit
their effect to only the test runs.  However, they were actually
enabled only during test cleanup.  Call setup/teardown_malloc_check
also around the evaluation of the actual test snippet.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/test-lib.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index eff3a65..35da859 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -337,8 +337,10 @@ test_eval_ () {
 test_run_ () {
 	test_cleanup=:
 	expecting_failure=$2
+	setup_malloc_check
 	test_eval_ "$1"
 	eval_ret=$?
+	teardown_malloc_check
 
 	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
 	then
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 2/8] test-lib: refactor $GIT_SKIP_TESTS matching
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 1/8] test-lib: enable MALLOC_* for the actual tests Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 3/8] test-lib: rearrange start/end of test_expect_* and test_skip Thomas Rast
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

It's already used twice, and we will have more of the same kind of
matching in a minute.

Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/test-lib.sh | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 35da859..4fa141a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -328,6 +328,20 @@ test_debug () {
 	test "$debug" = "" || eval "$1"
 }
 
+match_pattern_list () {
+	arg="$1"
+	shift
+	test -z "$*" && return 1
+	for pattern_
+	do
+		case "$arg" in
+		$pattern_)
+			return 0
+		esac
+	done
+	return 1
+}
+
 test_eval_ () {
 	# This is a separate function because some tests use
 	# "return" to end a test_expect_success block early.
@@ -358,14 +372,10 @@ test_run_ () {
 test_skip () {
 	test_count=$(($test_count+1))
 	to_skip=
-	for skp in $GIT_SKIP_TESTS
-	do
-		case $this_test.$test_count in
-		$skp)
-			to_skip=t
-			break
-		esac
-	done
+	if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
+	then
+		to_skip=t
+	fi
 	if test -z "$to_skip" && test -n "$test_prereq" &&
 	   ! test_have_prereq "$test_prereq"
 	then
@@ -630,15 +640,12 @@ cd -P "$TRASH_DIRECTORY" || exit 1
 
 this_test=${0##*/}
 this_test=${this_test%%-*}
-for skp in $GIT_SKIP_TESTS
-do
-	case "$this_test" in
-	$skp)
-		say_color info >&3 "skipping test $this_test altogether"
-		skip_all="skip all tests in $this_test"
-		test_done
-	esac
-done
+if match_pattern_list "$this_test" $GIT_SKIP_TESTS
+then
+	say_color info >&3 "skipping test $this_test altogether"
+	skip_all="skip all tests in $this_test"
+	test_done
+fi
 
 # Provide an implementation of the 'yes' utility
 yes () {
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 3/8] test-lib: rearrange start/end of test_expect_* and test_skip
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 1/8] test-lib: enable MALLOC_* for the actual tests Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 2/8] test-lib: refactor $GIT_SKIP_TESTS matching Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 4/8] test-lib: self-test that --verbose works Thomas Rast
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

This moves

* the early setup part from test_skip to a new function test_start_

* the final common parts of test_expect_* to a new function
  test_finish_

to make the next commit more obvious.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/test-lib-functions.sh | 6 ++++--
 t/test-lib.sh           | 9 ++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8828ff7..a7e9aac 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -343,6 +343,7 @@ test_declared_prereq () {
 }
 
 test_expect_failure () {
+	test_start_
 	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
@@ -357,10 +358,11 @@ test_expect_failure () {
 			test_known_broken_failure_ "$1"
 		fi
 	fi
-	echo >&3 ""
+	test_finish_
 }
 
 test_expect_success () {
+	test_start_
 	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
@@ -375,7 +377,7 @@ test_expect_success () {
 			test_failure_ "$@"
 		fi
 	fi
-	echo >&3 ""
+	test_finish_
 }
 
 # test_external runs external test scripts that provide continuous
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4fa141a..e99b0ea 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -369,8 +369,15 @@ test_run_ () {
 	return "$eval_ret"
 }
 
-test_skip () {
+test_start_ () {
 	test_count=$(($test_count+1))
+}
+
+test_finish_ () {
+	echo >&3 ""
+}
+
+test_skip () {
 	to_skip=
 	if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
 	then
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 4/8] test-lib: self-test that --verbose works
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
                   ` (2 preceding siblings ...)
  2013-06-23 18:12 ` [PATCH v4 3/8] test-lib: rearrange start/end of test_expect_* and test_skip Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 5/8] test-lib: verbose mode for only tests matching a pattern Thomas Rast
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

t0000 contains some light self-tests of test-lib.sh, but --verbose was
not covered.  Add a test.

The only catch is that the presence of a test harness influences the
output (specifically, the presence of some empty lines).  So we need
to unset TEST_HARNESS or set it to a known value.  Leaving it unset
leads to spurious test failures in the final summary, which come from
the subtest.  So we always set it.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/t0000-basic.sh | 37 ++++++++++++++++++++++++++++++++++++-
 t/test-lib.sh    |  2 ++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 0f13180..4b4103f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -47,8 +47,13 @@ test_expect_failure 'pretend we have a known breakage' '
 
 run_sub_test_lib_test () {
 	name="$1" descr="$2" # stdin is the body of the test code
+	shift 2
 	mkdir "$name" &&
 	(
+		# Pretend we're a test harness.  This prevents
+		# test-lib from writing the counts to a file that will
+		# later be summarized, showing spurious "failed" tests
+		export HARNESS_ACTIVE=t &&
 		cd "$name" &&
 		cat >"$name.sh" <<-EOF &&
 		#!$SHELL_PATH
@@ -65,7 +70,7 @@ run_sub_test_lib_test () {
 		cat >>"$name.sh" &&
 		chmod +x "$name.sh" &&
 		export TEST_DIRECTORY &&
-		./"$name.sh" >out 2>err
+		./"$name.sh" "$@" >out 2>err
 	)
 }
 
@@ -215,6 +220,36 @@ test_expect_success 'pretend we have a mix of all possible results' "
 	EOF
 "
 
+test_expect_success 'test --verbose' '
+	test_must_fail run_sub_test_lib_test \
+		test-verbose "test verbose" --verbose <<-\EOF &&
+	test_expect_success "passing test" true
+	test_expect_success "test with output" "echo foo"
+	test_expect_success "failing test" false
+	test_done
+	EOF
+	mv test-verbose/out test-verbose/out+
+	grep -v "^Initialized empty" test-verbose/out+ >test-verbose/out &&
+	check_sub_test_lib_test test-verbose <<-\EOF
+	> expecting success: true
+	> Z
+	> ok 1 - passing test
+	> Z
+	> expecting success: echo foo
+	> foo
+	> Z
+	> ok 2 - test with output
+	> Z
+	> expecting success: false
+	> Z
+	> not ok 3 - failing test
+	> #	false
+	> Z
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF
+'
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e99b0ea..10827a4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -414,6 +414,8 @@ test_at_end_hook_ () {
 test_done () {
 	GIT_EXIT_OK=t
 
+	# Note: t0000 relies on $HARNESS_ACTIVE disabling the .counts
+	# output file
 	if test -z "$HARNESS_ACTIVE"
 	then
 		test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 5/8] test-lib: verbose mode for only tests matching a pattern
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
                   ` (3 preceding siblings ...)
  2013-06-23 18:12 ` [PATCH v4 4/8] test-lib: self-test that --verbose works Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 6/8] test-lib: valgrind " Thomas Rast
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

With the new --verbose-only=<pattern> option, one can enable --verbose
at a per-test granularity.  The pattern is matched against the test
number, e.g.

  ./t0000-basic.sh --verbose-only='2[0-2]'

to see only the full output of test 20-22, while showing the rest in the
one-liner format.

As suggested by Jeff King, this takes care to wrap the entire
test_expect_* block, but nothing else, in the verbose toggling.  We
can use the test_start/end functions from the previous commit for the
purpose.

This is arguably not *too* useful on its own, but makes the next patch
easier to follow.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/README         |  5 +++++
 t/t0000-basic.sh | 24 ++++++++++++++++++++++++
 t/test-lib.sh    | 31 +++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/t/README b/t/README
index ec52468..ec8ab79 100644
--- a/t/README
+++ b/t/README
@@ -76,6 +76,11 @@ appropriately before running "make".
 	command being run and their output if any are also
 	output.
 
+--verbose-only=<pattern>::
+	Like --verbose, but the effect is limited to tests with
+	numbers matching <pattern>.  The number matched against is
+	simply the running count of the test within the file.
+
 --debug::
 	This may help the person who is developing a new test.
 	It causes the command defined with test_debug to run.
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 4b4103f..5c32288 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -250,6 +250,30 @@ test_expect_success 'test --verbose' '
 	EOF
 '
 
+test_expect_success 'test --verbose-only' '
+	test_must_fail run_sub_test_lib_test \
+		test-verbose-only-2 "test verbose-only=2" \
+		--verbose-only=2 <<-\EOF &&
+	test_expect_success "passing test" true
+	test_expect_success "test with output" "echo foo"
+	test_expect_success "failing test" false
+	test_done
+	EOF
+	check_sub_test_lib_test test-verbose-only-2 <<-\EOF
+	> ok 1 - passing test
+	> Z
+	> expecting success: echo foo
+	> foo
+	> Z
+	> ok 2 - test with output
+	> Z
+	> not ok 3 - failing test
+	> #	false
+	> # failed 1 among 3 test(s)
+	> 1..3
+	EOF
+'
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 10827a4..5729702 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -184,6 +184,9 @@ do
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
 		verbose=t; shift ;;
+	--verbose-only=*)
+		verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
 	-q|--q|--qu|--qui|--quie|--quiet)
 		# Ignore --quiet under a TAP::Harness. Saying how many tests
 		# passed without the ok/not ok details is always an error.
@@ -342,6 +345,32 @@ match_pattern_list () {
 	return 1
 }
 
+maybe_teardown_verbose () {
+	test -z "$verbose_only" && return
+	exec 4>/dev/null 3>/dev/null
+	verbose=
+}
+
+last_verbose=t
+maybe_setup_verbose () {
+	test -z "$verbose_only" && return
+	if match_pattern_list $test_count $verbose_only
+	then
+		exec 4>&2 3>&1
+		# Emit a delimiting blank line when going from
+		# non-verbose to verbose.  Within verbose mode the
+		# delimiter is printed by test_expect_*.  The choice
+		# of the initial $last_verbose is such that before
+		# test 1, we do not print it.
+		test -z "$last_verbose" && echo >&3 ""
+		verbose=t
+	else
+		exec 4>/dev/null 3>/dev/null
+		verbose=
+	fi
+	last_verbose=$verbose
+}
+
 test_eval_ () {
 	# This is a separate function because some tests use
 	# "return" to end a test_expect_success block early.
@@ -371,10 +400,12 @@ test_run_ () {
 
 test_start_ () {
 	test_count=$(($test_count+1))
+	maybe_setup_verbose
 }
 
 test_finish_ () {
 	echo >&3 ""
+	maybe_teardown_verbose
 }
 
 test_skip () {
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 6/8] test-lib: valgrind for only tests matching a pattern
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
                   ` (4 preceding siblings ...)
  2013-06-23 18:12 ` [PATCH v4 5/8] test-lib: verbose mode for only tests matching a pattern Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 7/8] test-lib: allow prefixing a custom string before "ok N" etc Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 8/8] test-lib: support running tests under valgrind in parallel Thomas Rast
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

With the new --valgrind-only=<pattern> option, one can enable
--valgrind at a per-test granularity, exactly analogous to
--verbose-only from the previous commit.

The options are wired such that --valgrind implies --verbose (as
before), but --valgrind-only=<pattern> implies
--verbose-only=<pattern> unless --verbose is also in effect.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/README               |  5 +++++
 t/test-lib.sh          | 36 +++++++++++++++++++++++++++++++++++-
 t/valgrind/valgrind.sh |  3 +++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/t/README b/t/README
index ec8ab79..2167125 100644
--- a/t/README
+++ b/t/README
@@ -126,6 +126,11 @@ appropriately before running "make".
 	the 't/valgrind/' directory and use the commands under
 	't/valgrind/bin/'.
 
+--valgrind-only=<pattern>::
+	Like --valgrind, but the effect is limited to tests with
+	numbers matching <pattern>.  The number matched against is
+	simply the running count of the test within the file.
+
 --tee::
 	In addition to printing the test output to the terminal,
 	write it to files named 't/test-results/$TEST_NAME.out'.
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5729702..a926828 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -201,6 +201,9 @@ do
 	--valgrind=*)
 		valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
 		shift ;;
+	--valgrind-only=*)
+		valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
 	--tee)
 		shift ;; # was handled already
 	--root=*)
@@ -211,7 +214,14 @@ do
 	esac
 done
 
-test -n "$valgrind" && verbose=t
+if test -n "$valgrind_only"
+then
+	test -z "$valgrind" && valgrind=memcheck
+	test -z "$verbose" && verbose_only="$valgrind_only"
+elif test -n "$valgrind"
+then
+	verbose=t
+fi
 
 if test -n "$color"
 then
@@ -371,6 +381,25 @@ maybe_setup_verbose () {
 	last_verbose=$verbose
 }
 
+maybe_teardown_valgrind () {
+	test -z "$GIT_VALGRIND" && return
+	GIT_VALGRIND_ENABLED=
+}
+
+maybe_setup_valgrind () {
+	test -z "$GIT_VALGRIND" && return
+	if test -z "$valgrind_only"
+	then
+		GIT_VALGRIND_ENABLED=t
+		return
+	fi
+	GIT_VALGRIND_ENABLED=
+	if match_pattern_list $test_count $valgrind_only
+	then
+		GIT_VALGRIND_ENABLED=t
+	fi
+}
+
 test_eval_ () {
 	# This is a separate function because some tests use
 	# "return" to end a test_expect_success block early.
@@ -401,10 +430,12 @@ test_run_ () {
 test_start_ () {
 	test_count=$(($test_count+1))
 	maybe_setup_verbose
+	maybe_setup_valgrind
 }
 
 test_finish_ () {
 	echo >&3 ""
+	maybe_teardown_valgrind
 	maybe_teardown_verbose
 }
 
@@ -590,6 +621,9 @@ then
 	export GIT_VALGRIND
 	GIT_VALGRIND_MODE="$valgrind"
 	export GIT_VALGRIND_MODE
+	GIT_VALGRIND_ENABLED=t
+	test -n "$valgrind_only" && GIT_VALGRIND_ENABLED=
+	export GIT_VALGRIND_ENABLED
 elif test -n "$GIT_TEST_INSTALLED"
 then
 	GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
index 6b87c91..4215303 100755
--- a/t/valgrind/valgrind.sh
+++ b/t/valgrind/valgrind.sh
@@ -4,6 +4,9 @@ base=$(basename "$0")
 
 TOOL_OPTIONS='--leak-check=no'
 
+test -z "$GIT_VALGRIND_ENABLED" &&
+exec "$GIT_VALGRIND"/../../"$base" "$@"
+
 case "$GIT_VALGRIND_MODE" in
 memcheck-fast)
 	;;
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 7/8] test-lib: allow prefixing a custom string before "ok N" etc.
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
                   ` (5 preceding siblings ...)
  2013-06-23 18:12 ` [PATCH v4 6/8] test-lib: valgrind " Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  2013-06-23 18:12 ` [PATCH v4 8/8] test-lib: support running tests under valgrind in parallel Thomas Rast
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

This is not really meant for external use, and thus not documented. It
allows the next commit to neatly distinguish between sub-tests and the
main run.

The format is intentionally not valid TAP.  The use in the next commit
would not result in anything valid either way, and it seems better to
make it obvious.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/test-lib.sh | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a926828..682459b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -209,6 +209,9 @@ do
 	--root=*)
 		root=$(expr "z$1" : 'z[^=]*=\(.*\)')
 		shift ;;
+	--statusprefix=*)
+		statusprefix=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
 	*)
 		echo "error: unknown test option '$1'" >&2; exit 1 ;;
 	esac
@@ -316,12 +319,12 @@ trap 'die' EXIT
 
 test_ok_ () {
 	test_success=$(($test_success + 1))
-	say_color "" "ok $test_count - $@"
+	say_color "" "${statusprefix}ok $test_count - $@"
 }
 
 test_failure_ () {
 	test_failure=$(($test_failure + 1))
-	say_color error "not ok $test_count - $1"
+	say_color error "${statusprefix}not ok $test_count - $1"
 	shift
 	echo "$@" | sed -e 's/^/#	/'
 	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
@@ -329,12 +332,12 @@ test_failure_ () {
 
 test_known_broken_ok_ () {
 	test_fixed=$(($test_fixed+1))
-	say_color error "ok $test_count - $@ # TODO known breakage vanished"
+	say_color error "${statusprefix}ok $test_count - $@ # TODO known breakage vanished"
 }
 
 test_known_broken_failure_ () {
 	test_broken=$(($test_broken+1))
-	say_color warn "not ok $test_count - $@ # TODO known breakage"
+	say_color warn "${statusprefix}not ok $test_count - $@ # TODO known breakage"
 }
 
 test_debug () {
@@ -458,8 +461,8 @@ test_skip () {
 			of_prereq=" of $test_prereq"
 		fi
 
-		say_color skip >&3 "skipping test: $@"
-		say_color skip "ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
+		say_color skip >&3 "${statusprefix}skipping test: $@"
+		say_color skip "${statusprefix}ok $test_count # skip $1 (missing $missing_prereq${of_prereq})"
 		: true
 		;;
 	*)
@@ -497,11 +500,11 @@ test_done () {
 
 	if test "$test_fixed" != 0
 	then
-		say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
+		say_color error "${statusprefix}# $test_fixed known breakage(s) vanished; please update test(s)"
 	fi
 	if test "$test_broken" != 0
 	then
-		say_color warn "# still have $test_broken known breakage(s)"
+		say_color warn "${statusprefix}# still have $test_broken known breakage(s)"
 	fi
 	if test "$test_broken" != 0 || test "$test_fixed" != 0
 	then
@@ -524,9 +527,9 @@ test_done () {
 		then
 			if test $test_remaining -gt 0
 			then
-				say_color pass "# passed all $msg"
+				say_color pass "${statusprefix}# passed all $msg"
 			fi
-			say "1..$test_count$skip_all"
+			say "${statusprefix}1..$test_count$skip_all"
 		fi
 
 		test -d "$remove_trash" &&
@@ -540,8 +543,8 @@ test_done () {
 	*)
 		if test $test_external_has_tap -eq 0
 		then
-			say_color error "# failed $test_failure among $msg"
-			say "1..$test_count"
+			say_color error "${statusprefix}# failed $test_failure among $msg"
+			say "${statusprefix}1..$test_count"
 		fi
 
 		exit 1 ;;
-- 
1.8.3.1.727.gcbe3af3

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

* [PATCH v4 8/8] test-lib: support running tests under valgrind in parallel
  2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
                   ` (6 preceding siblings ...)
  2013-06-23 18:12 ` [PATCH v4 7/8] test-lib: allow prefixing a custom string before "ok N" etc Thomas Rast
@ 2013-06-23 18:12 ` Thomas Rast
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Rast @ 2013-06-23 18:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Fredrik Gustafsson, Jeff King

With the new --valgrind-parallel=<n> option, we support running the
tests in a single test script under valgrind in parallel using 'n'
processes.

This really follows the dumbest approach possible, as follows:

* We spawn the test script 'n' times, using a throw-away
  TEST_OUTPUT_DIRECTORY.  Each of the instances is given options that
  ensures that it only runs every n-th test under valgrind, but
  together they cover the entire range.

* We add up the numbers from the individual tests, and provide the
  usual output.

This is really a gross hack at this point, and should be improved.  In
particular we should keep the actual outputs somewhere more easily
discoverable, and summarize them to the user.

Nevertheless, this is already workable and gives a speedup of more
than 2 on a dual-core (hyperthreaded) machine, using n=4.  This is
expected since the overhead of valgrind is so big (on the order of 20x
under good conditions, and a large startup overhead at every git
invocation) that redundantly running the non-valgrind tests in between
is not that expensive.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 t/test-lib.sh | 106 ++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 84 insertions(+), 22 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 682459b..9753641 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -204,6 +204,15 @@ do
 	--valgrind-only=*)
 		valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
 		shift ;;
+	--valgrind-parallel=*)
+		valgrind_parallel=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
+	--valgrind-only-stride=*)
+		valgrind_only_stride=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
+	--valgrind-only-offset=*)
+		valgrind_only_offset=$(expr "z$1" : 'z[^=]*=\(.*\)')
+		shift ;;
 	--tee)
 		shift ;; # was handled already
 	--root=*)
@@ -217,7 +226,7 @@ do
 	esac
 done
 
-if test -n "$valgrind_only"
+if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
 then
 	test -z "$valgrind" && valgrind=memcheck
 	test -z "$verbose" && verbose_only="$valgrind_only"
@@ -367,7 +376,9 @@ maybe_teardown_verbose () {
 last_verbose=t
 maybe_setup_verbose () {
 	test -z "$verbose_only" && return
-	if match_pattern_list $test_count $verbose_only
+	if match_pattern_list $test_count $verbose_only ||
+		{ test -n "$valgrind_only_stride" &&
+		expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null; }
 	then
 		exec 4>&2 3>&1
 		# Emit a delimiting blank line when going from
@@ -391,7 +402,7 @@ maybe_teardown_valgrind () {
 
 maybe_setup_valgrind () {
 	test -z "$GIT_VALGRIND" && return
-	if test -z "$valgrind_only"
+	if test -z "$valgrind_only" && test -z "$valgrind_only_stride"
 	then
 		GIT_VALGRIND_ENABLED=t
 		return
@@ -400,6 +411,10 @@ maybe_setup_valgrind () {
 	if match_pattern_list $test_count $valgrind_only
 	then
 		GIT_VALGRIND_ENABLED=t
+	elif test -n "$valgrind_only_stride" &&
+		expr $test_count "%" $valgrind_only_stride - $valgrind_only_offset = 0 >/dev/null
+	then
+		GIT_VALGRIND_ENABLED=t
 	fi
 }
 
@@ -552,6 +567,9 @@ test_done () {
 	esac
 }
 
+
+# Set up a directory that we can put in PATH which redirects all git
+# calls to 'valgrind git ...'.
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -599,33 +617,42 @@ then
 		make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit
 	}
 
-	# override all git executables in TEST_DIRECTORY/..
-	GIT_VALGRIND=$TEST_DIRECTORY/valgrind
-	mkdir -p "$GIT_VALGRIND"/bin
-	for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
-	do
-		make_valgrind_symlink $file
-	done
-	# special-case the mergetools loadables
-	make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
-	OLDIFS=$IFS
-	IFS=:
-	for path in $PATH
-	do
-		ls "$path"/git-* 2> /dev/null |
-		while read file
+	# In the case of --valgrind-parallel, we only need to do the
+	# wrapping once, in the main script.  The worker children all
+	# have $valgrind_only_stride set, so we can skip based on that.
+	if test -z "$valgrind_only_stride"
+	then
+		# override all git executables in TEST_DIRECTORY/..
+		GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+		mkdir -p "$GIT_VALGRIND"/bin
+		for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
 		do
-			make_valgrind_symlink "$file"
+			make_valgrind_symlink $file
 		done
-	done
-	IFS=$OLDIFS
+		# special-case the mergetools loadables
+		make_symlink "$GIT_BUILD_DIR"/mergetools "$GIT_VALGRIND/bin/mergetools"
+		OLDIFS=$IFS
+		IFS=:
+		for path in $PATH
+		do
+			ls "$path"/git-* 2> /dev/null |
+			while read file
+			do
+				make_valgrind_symlink "$file"
+			done
+		done
+		IFS=$OLDIFS
+	fi
 	PATH=$GIT_VALGRIND/bin:$PATH
 	GIT_EXEC_PATH=$GIT_VALGRIND/bin
 	export GIT_VALGRIND
 	GIT_VALGRIND_MODE="$valgrind"
 	export GIT_VALGRIND_MODE
 	GIT_VALGRIND_ENABLED=t
-	test -n "$valgrind_only" && GIT_VALGRIND_ENABLED=
+	if test -n "$valgrind_only" || test -n "$valgrind_only_stride"
+	then
+		GIT_VALGRIND_ENABLED=
+	fi
 	export GIT_VALGRIND_ENABLED
 elif test -n "$GIT_TEST_INSTALLED"
 then
@@ -711,6 +738,41 @@ then
 else
 	mkdir -p "$TRASH_DIRECTORY"
 fi
+
+# Gross hack to spawn N sub-instances of the tests in parallel, and
+# summarize the results.  Note that if this is enabled, the script
+# terminates at the end of this 'if' block.
+if test -n "$valgrind_parallel"
+then
+	for i in $(test_seq 1 $valgrind_parallel)
+	do
+		root="$TRASH_DIRECTORY/vgparallel-$i"
+		mkdir "$root"
+		TEST_OUTPUT_DIRECTORY="$root" \
+			${SHELL_PATH} "$0" \
+			--root="$root" --statusprefix="[$i] " \
+			--valgrind="$valgrind" \
+			--valgrind-only-stride="$valgrind_parallel" \
+			--valgrind-only-offset="$i" &
+		pids="$pids $!"
+	done
+	trap "kill $pids" INT TERM HUP
+	wait $pids
+	trap - INT TERM HUP
+	for i in $(test_seq 1 $valgrind_parallel)
+	do
+		root="$TRASH_DIRECTORY/vgparallel-$i"
+		eval "$(cat "$root/test-results/$(basename "$0" .sh)"-*.counts |
+			sed 's/^\([a-z][a-z]*\) \([0-9][0-9]*\)/inner_\1=\2/')"
+		test_count=$(expr $test_count + $inner_total)
+		test_success=$(expr $test_success + $inner_success)
+		test_fixed=$(expr $test_fixed + $inner_fixed)
+		test_broken=$(expr $test_broken + $inner_broken)
+		test_failure=$(expr $test_failure + $inner_failed)
+	done
+	test_done
+fi
+
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$TRASH_DIRECTORY" || exit 1
-- 
1.8.3.1.727.gcbe3af3

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

end of thread, other threads:[~2013-06-23 18:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-23 18:12 [PATCH v4 0/8] --valgrind improvements Thomas Rast
2013-06-23 18:12 ` [PATCH v4 1/8] test-lib: enable MALLOC_* for the actual tests Thomas Rast
2013-06-23 18:12 ` [PATCH v4 2/8] test-lib: refactor $GIT_SKIP_TESTS matching Thomas Rast
2013-06-23 18:12 ` [PATCH v4 3/8] test-lib: rearrange start/end of test_expect_* and test_skip Thomas Rast
2013-06-23 18:12 ` [PATCH v4 4/8] test-lib: self-test that --verbose works Thomas Rast
2013-06-23 18:12 ` [PATCH v4 5/8] test-lib: verbose mode for only tests matching a pattern Thomas Rast
2013-06-23 18:12 ` [PATCH v4 6/8] test-lib: valgrind " Thomas Rast
2013-06-23 18:12 ` [PATCH v4 7/8] test-lib: allow prefixing a custom string before "ok N" etc Thomas Rast
2013-06-23 18:12 ` [PATCH v4 8/8] test-lib: support running tests under valgrind in parallel Thomas Rast

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