git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Thomas Rast <trast@inf.ethz.ch>
Cc: git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 2/2 v2] test output: respect $TEST_OUTPUT_DIRECTORY
Date: Mon, 29 Apr 2013 19:16:21 +0100	[thread overview]
Message-ID: <20130429181621.GL472@serenity.lan> (raw)
In-Reply-To: <87fvy9dxok.fsf@hexa.v.cablecom.net>

Most test results go in $TEST_OUTPUT_DIRECTORY, but the output files for
tests run with --tee or --valgrind just use bare "test-results".
Changes these so that they do respect $TEST_OUTPUT_DIRECTORY.

As a result of this, the valgrind/analyze.sh script may no longer
inspect the correct files so it is also updated to respect
$TEST_OUTPUT_DIRECTORY by adding it to GIT-BUILD-OPTIONS.  This may be a
regression for people who have TEST_OUTPUT_DIRECTORY in their config.mak
but want to override it in the environment, but this change merely
brings it into line with GIT_TEST_OPTS which already cannot be
overridden if it is specified in config.mak.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Mon, Apr 29, 2013 at 08:00:27PM +0200, Thomas Rast wrote:
> John Keeping <john@keeping.me.uk> writes:
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index ca6bdef..70ad085 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -54,8 +54,8 @@ done,*)
> >  	# do not redirect again
> >  	;;
> >  *' --tee '*|*' --va'*)
> > -	mkdir -p test-results
> > -	BASE=test-results/$(basename "$0" .sh)
> > +	mkdir -p "$(TEST_OUTPUT_DIRECTORY)/test-results"
> > +	BASE="$(TEST_OUTPUT_DIRECTORY)/test-results/$(basename "$0" .sh)"
> >  	(GIT_TEST_TEE_STARTED=done ${SHELL_PATH} "$0" "$@" 2>&1;
> >  	 echo $? > $BASE.exit) | tee $BASE.out
> >  	test "$(cat $BASE.exit)" = 0
> 
> Hmm, I initially was too lazy to review this change, and now it's biting
> me.  The above is Makefile-quoted, which to the shell reads like a
> command substitution.

Yeah, that's completely wrong - clearly it was too late on Friday
evening when I wrote this.  There was another case of the same in
t/valgrind/analyze.sh.

All fixed in this version.

 Makefile              | 3 +++
 t/test-lib.sh         | 4 ++--
 t/valgrind/analyze.sh | 8 ++++++--
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 598d631..ef5be0f 100644
--- a/Makefile
+++ b/Makefile
@@ -2153,6 +2153,9 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
 	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
+ifdef TEST_OUTPUT_DIRECTORY
+	@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@
+endif
 ifdef GIT_TEST_OPTS
 	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
 endif
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 657b0bd..e7d169c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -54,8 +54,8 @@ done,*)
 	# do not redirect again
 	;;
 *' --tee '*|*' --va'*)
-	mkdir -p test-results
-	BASE=test-results/$(basename "$0" .sh)
+	mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results"
+	BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)"
 	(GIT_TEST_TEE_STARTED=done ${SHELL_PATH} "$0" "$@" 2>&1;
 	 echo $? > $BASE.exit) | tee $BASE.out
 	test "$(cat $BASE.exit)" = 0
diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh
index d8105d9..2ffc80f 100755
--- a/t/valgrind/analyze.sh
+++ b/t/valgrind/analyze.sh
@@ -1,6 +1,10 @@
 #!/bin/sh
 
-out_prefix=$(dirname "$0")/../test-results/valgrind.out
+# Get TEST_OUTPUT_DIRECTORY from GIT-BUILD-OPTIONS if it's there...
+. "$(dirname "$0")/../../GIT-BUILD-OPTIONS"
+# ... otherwise set it to the default value.
+: ${TEST_OUTPUT_DIRECTORY=$(dirname "$0")/..}
+
 output=
 count=0
 total_count=0
@@ -115,7 +119,7 @@ handle_one () {
 	finish_output
 }
 
-for test_script in "$(dirname "$0")"/../test-results/*.out
+for test_script in "$TEST_OUTPUT_DIRECTORY"/test-results/*.out
 do
 	handle_one $test_script
 done
-- 
1.8.3.rc0.149.g98a72f2.dirty

  reply	other threads:[~2013-04-29 18:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-26 18:55 [PATCH 1/2] t/Makefile: fix result handling with TEST_OUTPUT_DIRECTORY John Keeping
2013-04-26 18:55 ` [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY John Keeping
2013-04-29 18:00   ` Thomas Rast
2013-04-29 18:16     ` John Keeping [this message]
2013-04-29 18:17     ` Junio C Hamano
2013-04-29 18:19       ` John Keeping
2013-04-29 18:27         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130429181621.GL472@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=trast@inf.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).