* [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 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.