* [PATCH 1/2] t/Makefile: fix result handling with TEST_OUTPUT_DIRECTORY @ 2013-04-26 18:55 John Keeping 2013-04-26 18:55 ` [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY John Keeping 0 siblings, 1 reply; 7+ messages in thread From: John Keeping @ 2013-04-26 18:55 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Thomas Rast, Jeff King, John Keeping When TEST_OUTPUT_DIRECTORY is set, the test results will be generated in "$TEST_OUTPUT_DIRECTORY/test-results", which may not be the same as "test-results" in t/Makefile. This causes the aggregate-results target to fail as it finds no count files. Fix this by introducing TEST_RESULTS_DIRECTORY and using it wherever test-results is referenced. Signed-off-by: John Keeping <john@keeping.me.uk> --- t/Makefile | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/t/Makefile b/t/Makefile index 44ca7d3..0e1408d 100644 --- a/t/Makefile +++ b/t/Makefile @@ -15,9 +15,16 @@ PROVE ?= prove DEFAULT_TEST_TARGET ?= test TEST_LINT ?= test-lint-duplicates test-lint-executable +ifdef TEST_OUTPUT_DIRECTORY +TEST_RESULTS_DIRECTORY = $(TEST_RESULTS_DIRECTORY)/test-results +else +TEST_RESULTS_DIRECTORY = test-results +endif + # Shell quote; SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) +TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY)) T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)) TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh)) @@ -36,10 +43,10 @@ $(T): @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS) pre-clean: - $(RM) -r test-results + $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)' clean-except-prove-cache: - $(RM) -r 'trash directory'.* test-results + $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)' $(RM) -r valgrind/bin clean: clean-except-prove-cache @@ -65,7 +72,7 @@ aggregate-results-and-cleanup: $(T) $(MAKE) clean aggregate-results: - for f in test-results/t*-*.counts; do \ + for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \ echo "$$f"; \ done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh -- 1.8.2.1.715.gb260f47 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY 2013-04-26 18:55 [PATCH 1/2] t/Makefile: fix result handling with TEST_OUTPUT_DIRECTORY John Keeping @ 2013-04-26 18:55 ` John Keeping 2013-04-29 18:00 ` Thomas Rast 0 siblings, 1 reply; 7+ messages in thread From: John Keeping @ 2013-04-26 18:55 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Thomas Rast, Jeff King, John Keeping 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> --- 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 90661a2..d4d95fa 100644 --- a/Makefile +++ b/Makefile @@ -2166,6 +2166,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 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 diff --git a/t/valgrind/analyze.sh b/t/valgrind/analyze.sh index d8105d9..7b58f01 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.2.1.715.gb260f47 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY 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 ` [PATCH 2/2 v2] " John Keeping 2013-04-29 18:17 ` [PATCH 2/2] " Junio C Hamano 0 siblings, 2 replies; 7+ messages in thread From: Thomas Rast @ 2013-04-29 18:00 UTC (permalink / raw) To: John Keeping; +Cc: git, Johannes Schindelin, Jeff King 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. -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] test output: respect $TEST_OUTPUT_DIRECTORY 2013-04-29 18:00 ` Thomas Rast @ 2013-04-29 18:16 ` John Keeping 2013-04-29 18:17 ` [PATCH 2/2] " Junio C Hamano 1 sibling, 0 replies; 7+ messages in thread From: John Keeping @ 2013-04-29 18:16 UTC (permalink / raw) To: Thomas Rast; +Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY 2013-04-29 18:00 ` Thomas Rast 2013-04-29 18:16 ` [PATCH 2/2 v2] " John Keeping @ 2013-04-29 18:17 ` Junio C Hamano 2013-04-29 18:19 ` John Keeping 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2013-04-29 18:17 UTC (permalink / raw) To: Thomas Rast; +Cc: John Keeping, git, Johannes Schindelin, Jeff King Thomas Rast <trast@inf.ethz.ch> writes: > 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. Heh, when I let my eyes coast over it I didn't notice them either. Everything in that patch is bad. This squashed in? t/test-lib.sh | 4 ++-- t/valgrind/analyze.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 1b1e843..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_OUTPUT_DIRECTORY)/test-results" - BASE="$(TEST_OUTPUT_DIRECTORY)/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 7b58f01..2ffc80f 100755 --- a/t/valgrind/analyze.sh +++ b/t/valgrind/analyze.sh @@ -119,7 +119,7 @@ handle_one () { finish_output } -for test_script in "$(TEST_OUTPUT_DIRECTORY)"/test-results/*.out +for test_script in "$TEST_OUTPUT_DIRECTORY"/test-results/*.out do handle_one $test_script done ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY 2013-04-29 18:17 ` [PATCH 2/2] " Junio C Hamano @ 2013-04-29 18:19 ` John Keeping 2013-04-29 18:27 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: John Keeping @ 2013-04-29 18:19 UTC (permalink / raw) To: Junio C Hamano; +Cc: Thomas Rast, git, Johannes Schindelin, Jeff King On Mon, Apr 29, 2013 at 11:17:00AM -0700, Junio C Hamano wrote: > Thomas Rast <trast@inf.ethz.ch> writes: > > > 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. > > Heh, when I let my eyes coast over it I didn't notice them either. > Everything in that patch is bad. > > This squashed in? This is identical to the interdiff of what I posted at the same time, so it obviously looks good to me. > t/test-lib.sh | 4 ++-- > t/valgrind/analyze.sh | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 1b1e843..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_OUTPUT_DIRECTORY)/test-results" > - BASE="$(TEST_OUTPUT_DIRECTORY)/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 7b58f01..2ffc80f 100755 > --- a/t/valgrind/analyze.sh > +++ b/t/valgrind/analyze.sh > @@ -119,7 +119,7 @@ handle_one () { > finish_output > } > > -for test_script in "$(TEST_OUTPUT_DIRECTORY)"/test-results/*.out > +for test_script in "$TEST_OUTPUT_DIRECTORY"/test-results/*.out > do > handle_one $test_script > done ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test output: respect $TEST_OUTPUT_DIRECTORY 2013-04-29 18:19 ` John Keeping @ 2013-04-29 18:27 ` Junio C Hamano 0 siblings, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2013-04-29 18:27 UTC (permalink / raw) To: John Keeping; +Cc: Thomas Rast, git, Johannes Schindelin, Jeff King John Keeping <john@keeping.me.uk> writes: > This is identical to the interdiff of what I posted at the same time, so > it obviously looks good to me. Thanks. I've replaced the old tip with your version. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-29 18:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 ` [PATCH 2/2 v2] " John Keeping 2013-04-29 18:17 ` [PATCH 2/2] " Junio C Hamano 2013-04-29 18:19 ` John Keeping 2013-04-29 18:27 ` Junio C Hamano
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).