All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.