Re: [PATCH] test-lib: write test results to test-results/<basename>-<pid>

["SZEDER Gábor" <szeder@ira.uka.de>]

Hi,


On Fri, Mar 13, 2009 at 05:36:13PM +0100, Johannes Schindelin wrote:
> On Fri, 13 Mar 2009, Johannes Schindelin wrote:
> 
> > The earlier code meant to attempt to strip everything except the test
> > number, but only stripped the part starting with the last dash.
> > 
> > However, there is no reason why we should not use the whole basename.

I agree.

> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > 
> > 	Even if this is not strictly necessary after Hannes' test cleanup, 
> > 	it would still be nice.
>
> Just to clarify: it fixes the issue that these two tests share the same 
> file in test-results/: t5521-pull-options.sh  t5521-pull-symlink.sh
> 
> As a consequence, one's results overwrite the other one's.

The pid of the test process makes the name of the test result file
unique for each test, even in the mentioned case, e.g. it would be
something like t5521-pull-12345 and t5521-pull-23456.  However, after
Hannes' patch there is no need for keeping that pid around because the
test result file names would be unique for each test anyway.

Moreover, if we would remove the pif from the test result file name,
we could also remove the 'pre-clean' target from 't/Makefile'.  With
the pid appended, we need that 'pre-clean' target to clean up all
leftovers from the previous run.  Without the pid each test will
always write to the same test result file, so we could actually just
overwrite the cruft from the last run.

Something like the patch below.  Thoughts?


Best,
Gábor

---
 t/Makefile    |    7 ++-----
 t/test-lib.sh |    4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index 0d65ced..2e6e205 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -14,14 +14,11 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
 TSVN = $(wildcard t91[0-9][0-9]-*.sh)
 
-all: pre-clean $(T) aggregate-results clean
+all: $(T) aggregate-results clean
 
 $(T):
 	@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
 
-pre-clean:
-	$(RM) -r test-results
-
 clean:
 	$(RM) -r 'trash directory' test-results
 
@@ -33,5 +30,5 @@ 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
+.PHONY: $(T) aggregate-results clean
 .NOTPARALLEL:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0bd24d5..d82c784 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -418,9 +418,9 @@ test_done () {
 	trap - exit
 	test_results_dir="$TEST_DIRECTORY/test-results"
 	mkdir -p "$test_results_dir"
-	test_results_path="$test_results_dir/${0%.sh}-$$"
+	test_results_path="$test_results_dir/${0%.sh}"
 
-	echo "total $test_count" >> $test_results_path
+	echo "total $test_count" > $test_results_path
 	echo "success $test_success" >> $test_results_path
 	echo "fixed $test_fixed" >> $test_results_path
 	echo "broken $test_broken" >> $test_results_path