[PATCH 1/8] Add valgrind support in test scripts

[PATCH 1/8] Add valgrind support in test scripts

[Johannes Schindelin]

This patch adds the ability to use valgrind's memcheck tool to
diagnose memory problems in Git while running the test scripts.

It requires valgrind 3.4.0 or newer.

It works by creating symlinks to a valgrind script, which have the same
name as our Git binaries, and then putting that directory in front of
the test script's PATH as well as set GIT_EXEC_PATH to that directory.
Git scripts are symlinked from that directory directly.

That way, Git binaries called by Git scripts are valgrinded, too.

Valgrind can be used by specifying "GIT_TEST_OPTS=--valgrind" in the
make invocation. Any invocation of git that finds any errors under
valgrind will exit with failure code 126. Any valgrind output will go
to the usual stderr channel for tests (i.e., /dev/null, unless -v has
been specified).

If you need to pass options to valgrind -- you might want to run
another tool than memcheck, for example -- you can set the environment
variable GIT_VALGRIND_OPTIONS.

A few default suppressions are included, since libz seems to trigger
quite a few false positives. We'll assume that libz works and that we
can ignore any errors which are reported there.

Note: it is safe to run the valgrind tests in parallel, as the links in
t/valgrind/bin/ are created using proper locking.

Initial patch and all the hard work by Jeff King.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/README                |    8 +++++-
 t/test-lib.sh           |   58 +++++++++++++++++++++++++++++++++++++++++++++-
 t/valgrind/.gitignore   |    2 +
 t/valgrind/default.supp |   21 +++++++++++++++++
 t/valgrind/valgrind.sh  |   13 ++++++++++
 5 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100644 t/valgrind/.gitignore
 create mode 100644 t/valgrind/default.supp
 create mode 100755 t/valgrind/valgrind.sh

diff --git a/t/README b/t/README
index f208cf1..7560db5 100644
--- a/t/README
+++ b/t/README
@@ -39,7 +39,8 @@ this:
     * passed all 3 test(s)
 
 You can pass --verbose (or -v), --debug (or -d), and --immediate
-(or -i) command line argument to the test.
+(or -i) command line argument to the test, or by setting GIT_TEST_OPTS
+appropriately before running "make".
 
 --verbose::
 	This makes the test more verbose.  Specifically, the
@@ -58,6 +59,11 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate
 	This causes additional long-running tests to be run (where
 	available), for more exhaustive testing.
 
+--valgrind::
+	Execute all Git binaries with valgrind and exit with status
+	126 on errors (just like regular tests, this will only stop
+	the test script when running under -i).  Valgrind errors
+	go to stderr, so you might want to pass the -v option, too.
 
 Skipping Tests
 --------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c1839f7..aa9f938 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -94,6 +94,8 @@ do
 	--no-python)
 		# noop now...
 		shift ;;
+	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
+		valgrind=t; shift ;;
 	*)
 		break ;;
 	esac
@@ -492,8 +494,60 @@ 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)
-PATH=$TEST_DIRECTORY/..:$PATH
-GIT_EXEC_PATH=$(pwd)/..
+if test -z "$valgrind"
+then
+	PATH=$TEST_DIRECTORY/..:$PATH
+	GIT_EXEC_PATH=$TEST_DIRECTORY/..
+else
+	make_symlink () {
+		test -h "$2" &&
+		test "$1" = "$(readlink "$2")" || {
+			# be super paranoid
+			if mkdir "$2".lock
+			then
+				rm -f "$2" &&
+				ln -s "$1" "$2" &&
+				rm -r "$2".lock
+			else
+				while test -d "$2".lock
+				do
+					say "Waiting for lock on $2."
+					sleep 1
+				done
+			fi
+		}
+	}
+
+	make_valgrind_symlink () {
+		# handle only executables
+		test -x "$1" || return
+
+		base=$(basename "$1")
+		symlink_target=$TEST_DIRECTORY/../$base
+		# do not override scripts
+		if test -x "$symlink_target" &&
+		    test ! -d "$symlink_target" &&
+		    test "#!" != "$(head -c 2 < "$symlink_target")"
+		then
+			symlink_target=../valgrind.sh
+		fi
+		# create the link, or replace it if it is out of date
+		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 $TEST_DIRECTORY/../git* $TEST_DIRECTORY/../test-*
+	do
+		make_valgrind_symlink $file
+	done
+	PATH=$GIT_VALGRIND/bin:$PATH
+	GIT_EXEC_PATH=$GIT_VALGRIND/bin
+	export GIT_VALGRIND
+
+	make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit
+fi
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
 unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
diff --git a/t/valgrind/.gitignore b/t/valgrind/.gitignore
new file mode 100644
index 0000000..d4ae667
--- /dev/null
+++ b/t/valgrind/.gitignore
@@ -0,0 +1,2 @@
+/bin/
+/templates
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
new file mode 100644
index 0000000..2482b3b
--- /dev/null
+++ b/t/valgrind/default.supp
@@ -0,0 +1,21 @@
+{
+	ignore-zlib-errors-cond
+	Memcheck:Cond
+	obj:*libz.so*
+}
+
+{
+	ignore-zlib-errors-value4
+	Memcheck:Value4
+	obj:*libz.so*
+}
+
+{
+	writing-data-from-zlib-triggers-errors
+	Memcheck:Param
+	write(buf)
+	obj:/lib/ld-*.so
+	fun:write_in_full
+	fun:write_buffer
+	fun:write_loose_object
+}
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
new file mode 100755
index 0000000..dc92612
--- /dev/null
+++ b/t/valgrind/valgrind.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+base=$(basename "$0")
+
+exec valgrind -q --error-exitcode=126 \
+	--leak-check=no \
+	--suppressions="$GIT_VALGRIND/default.supp" \
+	--gen-suppressions=all \
+	--track-origins=yes \
+	--log-fd=4 \
+	--input-fd=4 \
+	$GIT_VALGRIND_OPTIONS \
+	"$GIT_VALGRIND"/../../"$base" "$@"
-- 
1.6.1.2.582.g3fdd5

[PATCH 2/8] valgrind: ignore ldso and more libz errors

[Johannes Schindelin]

From: Jeff King <peff@peff.net>

On some Linux systems, we get a host of Cond and Addr errors
from calls to dlopen that are caused by nss modules. We
should be able to safely ignore anything happening in
ld-*.so as "not our problem."

[Johannes: I added some more... unfortunately using valgrind 3.4.0 syntax]

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/valgrind/default.supp |   30 ++++++++++++++++++++++++++----
 1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 2482b3b..5f341b8 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -5,17 +5,39 @@
 }
 
 {
+	ignore-zlib-errors-value8
+	Memcheck:Value8
+	obj:*libz.so*
+}
+
+{
 	ignore-zlib-errors-value4
 	Memcheck:Value4
 	obj:*libz.so*
 }
 
 {
-	writing-data-from-zlib-triggers-errors
+	ignore-ldso-cond
+	Memcheck:Cond
+	obj:*ld-*.so
+}
+
+{
+	ignore-ldso-addr8
+	Memcheck:Addr8
+	obj:*ld-*.so
+}
+
+{
+	ignore-ldso-addr4
+	Memcheck:Addr4
+	obj:*ld-*.so
+}
+
+{
+	writing-data-from-zlib-triggers-even-more-errors
 	Memcheck:Param
 	write(buf)
-	obj:/lib/ld-*.so
-	fun:write_in_full
-	fun:write_buffer
+	...
 	fun:write_loose_object
 }
-- 
1.6.1.2.582.g3fdd5

[PATCH 3/8] Valgrind support: check for more than just programming errors

[Johannes Schindelin]

This patch makes --valgrind try to override _all_ Git binaries in the
PATH, and it makes it an error to call *.sh and *.perl scripts directly.

While it is not strictly necessary to look through the whole PATH to
find git binaries to override, it is in line with running an expensive
test (which valgrind is) to make extra sure that only binaries are
tested that actually come from the git.git checkout.

In the same spirit, we can test that neither our test suite nor our
scripts try to run the *.sh or *.perl scripts directly.

It's more like a "because we can" than a "this is tightly connected
to valgrind", but in the author's opinion "because we can" is "so we
should" in this case.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/test-lib.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index aa9f938..2567c74 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -531,6 +531,10 @@ else
 		then
 			symlink_target=../valgrind.sh
 		fi
+		case "$base" in
+		*.sh|*.perl)
+			symlink_target=../unprocessed-script
+		esac
 		# create the link, or replace it if it is out of date
 		make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit
 	}
@@ -542,6 +546,17 @@ else
 	do
 		make_valgrind_symlink $file
 	done
+	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
 	PATH=$GIT_VALGRIND/bin:$PATH
 	GIT_EXEC_PATH=$GIT_VALGRIND/bin
 	export GIT_VALGRIND
-- 
1.6.1.2.582.g3fdd5

[PATCH 4/8] test-lib.sh: optionally output to test-results/$TEST.out, too

[Johannes Schindelin]

When tests are run in parallel and a few tests fail, it does not help
that the output of the terminal is totally confusing, as you rarely know
which test which line came from.

So introduce the option '--tee' which triggers that the output of the
tests will be written to t/test-results/$TEST.out in addition to the
terminal, where $TEST is the basename of the script.

Unfortunately, there seems to be no way to redirect a given file
descriptor to a specified subprocess in POSIX shell, only redirection
to a file is supported via 'exec > $FILE'.

At least with bash, one might think that 'exec >($COMMAND)' would work
as intended, but it does not.

The common way to work around the lack of proper tools support is to
work with named pipes, alas, one of our most beloved platforms does not
really support named pipes.  Besides, we would need a pipe for every
script, as the whole point of this patch is to allow parallel execution.

Therefore, we handle the redirection in the following way: when '--tee'
was passed to the test script, the variable GIT_TEST_TEE_STARTED is set
(to avoid triggering that code path again) and the script is started
_again_, in a subshell, redirected to the command "tee".

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/README      |    6 ++++++
 t/test-lib.sh |   18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/t/README b/t/README
index 7560db5..ed1ebb6 100644
--- a/t/README
+++ b/t/README
@@ -65,6 +65,12 @@ appropriately before running "make".
 	the test script when running under -i).  Valgrind errors
 	go to stderr, so you might want to pass the -v option, too.
 
+--tee::
+	In addition to printing the test output to the terminal,
+	write it to files named 't/test-results/$TEST_NAME.out'.
+	As the names depend on the tests' file names, it is safe to
+	run the tests with this option in parallel.
+
 Skipping Tests
 --------------
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2567c74..2fb2274 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -3,6 +3,22 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
+# if --tee was passed, write the output not only to the terminal, but
+# additionally to the file test-results/$BASENAME.out, too.
+case "$GIT_TEST_TEE_STARTED, $* " in
+done,*)
+	# do not redirect again
+	;;
+*' --tee '*)
+	mkdir -p test-results
+	BASE=test-results/$(basename "$0" .sh)
+	(GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1;
+	 echo $? > $BASE.exit) | tee $BASE.out
+	test "$(cat $BASE.exit)" = 0
+	exit
+	;;
+esac
+
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
 
@@ -96,6 +112,8 @@ do
 		shift ;;
 	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
 		valgrind=t; shift ;;
+	--tee)
+		shift ;; # was handled already
 	*)
 		break ;;
 	esac
-- 
1.6.1.2.582.g3fdd5

[PATCH 5/8] t/Makefile: provide a 'valgrind' target

[Johannes Schindelin]

It is easy to forget running valgrinded tests without -v, and it is
also easy to forget to redirect the output to "tee" (lest the output
scroll out of the terminal's buffer).  Running "make valgrind" will
take care of all that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/Makefile |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index ed49c20..e544493 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -38,4 +38,7 @@ full-svn-test:
 	$(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=1 LC_ALL=C
 	$(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8
 
-.PHONY: pre-clean $(T) aggregate-results clean
+valgrind:
+	GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k
+
+.PHONY: pre-clean $(T) aggregate-results clean valgrind
-- 
1.6.1.2.582.g3fdd5

[PATCH 6/8] Add a script to coalesce the valgrind outputs

[Johannes Schindelin]

After running the valgrind tests with GIT_TEST_TREE=t, the test output
is in the test-results/$TEST.out files.

Call ./valgrind/analyze.sh in $GIT_ROOT/t/ to group the valgrind errors
by backtrace.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/valgrind/analyze.sh |  123 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 123 insertions(+), 0 deletions(-)
 create mode 100755 t/valgrind/analyze.sh

diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh
new file mode 100755
index 0000000..d8105d9
--- /dev/null
+++ b/t/valgrind/analyze.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+out_prefix=$(dirname "$0")/../test-results/valgrind.out
+output=
+count=0
+total_count=0
+missing_message=
+new_line='
+'
+
+# start outputting the current valgrind error in $out_prefix.++$count,
+# and the test case which failed in the corresponding .message file
+start_output () {
+	test -z "$output" || return
+
+	# progress
+	total_count=$(($total_count+1))
+	test -t 2 && printf "\rFound %d errors" $total_count >&2
+
+	count=$(($count+1))
+	output=$out_prefix.$count
+	: > $output
+
+	echo "*** $1 ***" > $output.message
+}
+
+finish_output () {
+	test ! -z "$output" || return
+	output=
+
+	# if a test case has more than one valgrind error, we need to
+	# copy the last .message file to the previous errors
+	test -z "$missing_message" || {
+		while test $missing_message -lt $count
+		do
+			cp $out_prefix.$count.message \
+				$out_prefix.$missing_message.message
+			missing_message=$(($missing_message+1))
+		done
+		missing_message=
+	}
+}
+
+# group the valgrind errors by backtrace
+output_all () {
+	last_line=
+	j=0
+	i=1
+	while test $i -le $count
+	do
+		# output <number> <backtrace-in-one-line>
+		echo "$i $(tr '\n' ' ' < $out_prefix.$i)"
+		i=$(($i+1))
+	done |
+	sort -t ' ' -k 2 | # order by <backtrace-in-one-line>
+	while read number line
+	do
+		# find duplicates, do not output backtrace twice
+		if test "$line" != "$last_line"
+		then
+			last_line=$line
+			j=$(($j+1))
+			printf "\nValgrind error $j:\n\n"
+			cat $out_prefix.$number
+			printf "\nfound in:\n"
+		fi
+		# print the test case where this came from
+		printf "\n"
+		cat $out_prefix.$number.message
+	done
+}
+
+handle_one () {
+	OLDIFS=$IFS
+	IFS="$new_line"
+	while read line
+	do
+		case "$line" in
+		# backtrace, possibly a new one
+		==[0-9]*)
+
+			# Does the current valgrind error have a message yet?
+			case "$output" in
+			*.message)
+				test -z "$missing_message" &&
+				missing_message=$count
+				output=
+			esac
+
+			start_output $(basename $1)
+			echo "$line" |
+			sed 's/==[0-9]*==/==valgrind==/' >> $output
+			;;
+		# end of backtrace
+		'}')
+			test -z "$output" || {
+				echo "$line" >> $output
+				test $output = ${output%.message} &&
+				output=$output.message
+			}
+			;;
+		# end of test case
+		'')
+			finish_output
+			;;
+		# normal line; if $output is set, print the line
+		*)
+			test -z "$output" || echo "$line" >> $output
+			;;
+		esac
+	done < $1
+	IFS=$OLDIFS
+
+	# just to be safe
+	finish_output
+}
+
+for test_script in "$(dirname "$0")"/../test-results/*.out
+do
+	handle_one $test_script
+done
+
+output_all
-- 
1.6.1.2.582.g3fdd5

[PATCH 7/8] Tests: let --valgrind imply --verbose and --tee

[Johannes Schindelin]

It does not make much sense to run the (expensive) valgrind tests and
not look at the output.

To prevent output from scrolling out of reach, the parameter --tee is
implied, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/Makefile    |    2 +-
 t/README      |    4 ++++
 t/test-lib.sh |    4 ++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index e544493..0962341 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -39,6 +39,6 @@ full-svn-test:
 	$(MAKE) $(TSVN) GIT_SVN_NO_OPTIMIZE_COMMITS=0 LC_ALL=en_US.UTF-8
 
 valgrind:
-	GIT_TEST_OPTS='--valgrind -v --tee' $(MAKE) -k
+	GIT_TEST_OPTS=--valgrind $(MAKE)
 
 .PHONY: pre-clean $(T) aggregate-results clean valgrind
diff --git a/t/README b/t/README
index ed1ebb6..d8f6c7d 100644
--- a/t/README
+++ b/t/README
@@ -65,6 +65,10 @@ appropriately before running "make".
 	the test script when running under -i).  Valgrind errors
 	go to stderr, so you might want to pass the -v option, too.
 
+	Since it makes no sense to run the tests with --valgrind and
+	not see any output, this option implies --verbose.  For
+	convenience, it also implies --tee.
+
 --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 2fb2274..495d55d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -9,7 +9,7 @@ case "$GIT_TEST_TEE_STARTED, $* " in
 done,*)
 	# do not redirect again
 	;;
-*' --tee '*)
+*' --tee '*|*' --va'*)
 	mkdir -p test-results
 	BASE=test-results/$(basename "$0" .sh)
 	(GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1;
@@ -111,7 +111,7 @@ do
 		# noop now...
 		shift ;;
 	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
-		valgrind=t; shift ;;
+		valgrind=t; verbose=t; shift ;;
 	--tee)
 		shift ;; # was handled already
 	*)
-- 
1.6.1.2.582.g3fdd5

[PATCH 8/8] test-lib: avoid assuming that templates/ are in the GIT_EXEC_PATH

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/test-lib.sh |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 495d55d..54bbb97 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -454,7 +454,7 @@ test_create_repo () {
 	repo="$1"
 	mkdir -p "$repo"
 	cd "$repo" || error "Cannot setup test environment"
-	"$GIT_EXEC_PATH/git" init "--template=$GIT_EXEC_PATH/templates/blt/" >&3 2>&4 ||
+	"$GIT_EXEC_PATH/git" init "--template=$owd/../templates/blt/" >&3 2>&4 ||
 	error "cannot run git init -- have you built things yet?"
 	mv .git/hooks .git/hooks-disabled
 	cd "$owd"
@@ -578,8 +578,6 @@ else
 	PATH=$GIT_VALGRIND/bin:$PATH
 	GIT_EXEC_PATH=$GIT_VALGRIND/bin
 	export GIT_VALGRIND
-
-	make_symlink ../../../templates "$GIT_VALGRIND"/bin/templates || exit
 fi
 GIT_TEMPLATE_DIR=$(pwd)/../templates/blt
 unset GIT_CONFIG
-- 
1.6.1.2.582.g3fdd5

Re: [PATCH 7/8] Tests: let --valgrind imply --verbose and --tee

[Jeff King]

On Wed, Feb 04, 2009 at 12:26:26AM +0100, Johannes Schindelin wrote:

> It does not make much sense to run the (expensive) valgrind tests and
> not look at the output.
> 
> To prevent output from scrolling out of reach, the parameter --tee is
> implied, too.

Might it not make sense to want to send the output to a file, but _not_
show it on the terminal (e.g., running from a cron job)? But if I
understand correctly, that is not actually possible with any combination
of "--verbose" and "--tee".

So perhaps it would make more sense to have, rather than "--tee", simply
"--verbose" to output to the terminal and "--save-output" (or similar)
to save the output. If both are requested, it is equivalent to what
"--tee" does now.

On the other hand, I think "--verbose --tee >/dev/null" would probably
accomplish the same thing, so it probably isn't worth too much effort.

Other than that, this series looks pretty reasonable. I am a bit late to
ack, but thank you for following through on it. It looks like it has
caught some bugs already, which is always a good sign. :)

I have access to a decent quad-core box, so I'll try to periodically run
the test suite with valgrind on it. Daily is probably too much, but
maybe whenever we go into rc-freeze is a good time.

-Peff

Re: [PATCH 7/8] Tests: let --valgrind imply --verbose and --tee

[Johannes Schindelin]

Hi,

On Fri, 6 Feb 2009, Jeff King wrote:

> On Wed, Feb 04, 2009 at 12:26:26AM +0100, Johannes Schindelin wrote:
> 
> > It does not make much sense to run the (expensive) valgrind tests and
> > not look at the output.
> > 
> > To prevent output from scrolling out of reach, the parameter --tee is
> > implied, too.
> 
> Might it not make sense to want to send the output to a file, but _not_
> show it on the terminal (e.g., running from a cron job)?

That is sh ... > file 2>&1

> But if I understand correctly, that is not actually possible with any 
> combination of "--verbose" and "--tee".

No, it is not, indeed.

> So perhaps it would make more sense to have, rather than "--tee", simply 
> "--verbose" to output to the terminal and "--save-output" (or similar) 
> to save the output. If both are requested, it is equivalent to what 
> "--tee" does now.

I do not see the point in that.

> On the other hand, I think "--verbose --tee >/dev/null" would probably 
> accomplish the same thing, so it probably isn't worth too much effort.

No.  Why would I want 'make test' to hide the fact from me that something 
did not work, and how it happened to fail?

Ciao,
Dscho

Re: [PATCH 7/8] Tests: let --valgrind imply --verbose and --tee

[Jeff King]

On Fri, Feb 06, 2009 at 08:40:59PM +0100, Johannes Schindelin wrote:

> > On the other hand, I think "--verbose --tee >/dev/null" would probably 
> > accomplish the same thing, so it probably isn't worth too much effort.
> 
> No.  Why would I want 'make test' to hide the fact from me that something 
> did not work, and how it happened to fail?

Because "make test" with "--verbose" generates megabytes of output which
you don't want mailed to you by cron, and that information is already
being saved in a file by "--tee"?

-Peff