[PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Sverre Rabbelier]

This change is needed order to aggregate data on the test run later on.
Because writing to the current directory is not possible, we write to /tmp/.
Suggestions for a better location are welcome.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 t/test-lib.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7c2a8ba..68b6555 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -152,6 +152,7 @@ test_failure=0
 test_count=0
 test_fixed=0
 test_broken=0
+test_success=0
 
 die () {
 	echo >&5 "FATAL: Unexpected exit with code $?"
@@ -177,6 +178,7 @@ test_tick () {
 
 test_ok_ () {
 	test_count=$(expr "$test_count" + 1)
+	test_success=$(expr "$test_success" + 1)
 	say_color "" "  ok $test_count: $@"
 }
 
@@ -337,6 +339,14 @@ test_create_repo () {
 
 test_done () {
 	trap - exit
+	test_results_path="/tmp/git-test-results"
+
+	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
+	echo "failed $test_failure" >> $test_results_path
+	echo "" >> $test_results_path
 
 	if test "$test_fixed" != 0
 	then
-- 
1.5.5.1.178.g1f811

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Vegard Nossum]

Hi!

On Mon, May 12, 2008 at 11:33 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> This change is needed order to aggregate data on the test run later on.
>  Because writing to the current directory is not possible, we write to /tmp/.
>  Suggestions for a better location are welcome.

I have seen this in another (sh) script:

die() {
        echo "$@"
        exit 1
}

T=`mktemp` || die "cannot create temp file"
...
rm $T


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036

[PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Sverre Rabbelier]

From: Sverre Rabbelier <srabbelier@gmail.com>

This change is needed order to aggregate data on the test run later on.
Because writing to the current directory is not possible, we write to /tmp/.
Suggestions for a better location are welcome.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 t/test-lib.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7a8bd27..4585fde 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -152,6 +152,7 @@ test_failure=0
 test_count=0
 test_fixed=0
 test_broken=0
+test_success=0
 
 die () {
 	echo >&5 "FATAL: Unexpected exit with code $?"
@@ -193,6 +194,7 @@ test_tick () {
 
 test_ok_ () {
 	test_count=$(expr "$test_count" + 1)
+	test_success=$(expr "$test_success" + 1)
 	say_color "" "  ok $test_count: $@"
 }
 
@@ -353,6 +355,14 @@ test_create_repo () {
 
 test_done () {
 	trap - exit
+	test_results_path="../test-results"
+
+	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
+	echo "failed $test_failure" >> $test_results_path
+	echo "" >> $test_results_path
 
 	if test "$test_fixed" != 0
 	then
-- 
1.5.5.1.214.g82ce2.dirty

[PATCH 2/3] A simple script to parse the results from the testcases

[Sverre Rabbelier]

From: Miklos Vajna <vmiklos@frugalware.org>

This is a simple script that aggregates key:value pairs in a file.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 t/aggregate-results.sh |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
 create mode 100755 t/aggregate-results.sh

diff --git a/t/aggregate-results.sh b/t/aggregate-results.sh
new file mode 100755
index 0000000..9e4322c
--- /dev/null
+++ b/t/aggregate-results.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+input="test-results"
+
+fixed=0
+success=0
+failed=0
+broken=0
+total=0
+
+while read type value
+do
+	case $type in
+	'')
+		continue ;;
+	fixed)	
+		fixed=$(($fixed + $value)) ;;
+	success)	
+		success=$(($success + $value)) ;;
+	failed)	
+		failed=$(($failed + $value)) ;;
+	broken)	
+		broken=$(( $broken + $value)) ;;
+	total)	
+		total=$(( $total + $value)) ;;
+	esac
+done < $input
+
+printf "%-8s%d\n" fixed $fixed
+printf "%-8s%d\n" success $success
+printf "%-8s%d\n" failed $failed
+printf "%-8s%d\n" broken $broken
+printf "%-8s%d\n" total $total
-- 
1.5.5.1.214.g82ce2.dirty

[PATCH 3/3] Hook up the result aggregation in the test makefile.

[Sverre Rabbelier]

From: Sverre Rabbelier <srabbelier@gmail.com>

This patch makes 'make' output the aggregated results at the end of each build.
The 'git-test-result' file is removed both before and after each build.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 t/Makefile |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index c6a60ab..f9ff933 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -14,18 +14,25 @@ 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: $(T) clean
+all: pre-clean $(T) aggregate-results clean
 
 $(T):
 	@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
 
+pre-clean:
+	$(RM) -f test-results
+
 clean:
 	$(RM) -r 'trash directory'
+	$(RM) -f test-results
+
+aggregate-results:
+	./aggregate-results.sh
 
 # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
 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: $(T) clean
+.PHONY: pre-clean $(T) aggregate-results clean
 .NOTPARALLEL:
-- 
1.5.5.1.214.g82ce2.dirty

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Jakub Narebski]

"Sverre Rabbelier" <sverre@rabbelier.nl> writes:

> Because writing to the current directory is not possible, we write to /tmp/.
> Suggestions for a better location are welcome.

> +	test_results_path="../test-results"

Errr... it looks like you forgot to update commit message.

But that aside, I quite like this series.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Sverre Rabbelier]

On Sun, Jun 8, 2008 at 4:42 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> "Sverre Rabbelier" <sverre@rabbelier.nl> writes:
>
>> Because writing to the current directory is not possible, we write to /tmp/.
>> Suggestions for a better location are welcome.
>
>> +     test_results_path="../test-results"
>
> Errr... it looks like you forgot to update commit message.

Yeah, I intended to fix that when amending the original commit, but I
forgot. I thought I couldn't write to the current dir, but instead I
was writing to /t/trash, which of course gets deleted after every
test. Obviously outputting to ../test-results fixed that.

> But that aside, I quite like this series.

Thanks!

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Junio C Hamano]

"Sverre Rabbelier" <srabbelier@gmail.com> writes:

> On Sun, Jun 8, 2008 at 4:42 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> "Sverre Rabbelier" <sverre@rabbelier.nl> writes:
>>
>>> Because writing to the current directory is not possible, we write to /tmp/.
>>> Suggestions for a better location are welcome.
>>
>>> +     test_results_path="../test-results"
>>
>> Errr... it looks like you forgot to update commit message.
>
> Yeah, I intended to fix that when amending the original commit, but I
> forgot. I thought I couldn't write to the current dir, but instead I
> was writing to /t/trash, which of course gets deleted after every
> test. Obviously outputting to ../test-results fixed that.

Some tests may chdir around and when you hit test_done they may not be in
the directory directly under t/ anymore.  I'd say that such a test is
ill-mannered, but we would want to protect ourselves against such mischief
somehow.

Also, this won't be an immediate issue, but we may want to lift the
".NOTPARALLEL" limitation in t/Makefile sometime in the future.  A handful
tests need to be adjusted for this change, as they currently refer their
test vectors in t/tXXXX/ as ../tXXXX, and with such a change, we would run
each test in "t/trash directory/t$$" where $$ is the pid, or something
like that.  A single "test-results" file everybody races to append would
become unwieldy at that point.  Something to keep in mind.

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Sverre Rabbelier]

On Sun, Jun 8, 2008 at 8:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Some tests may chdir around and when you hit test_done they may not be in
> the directory directly under t/ anymore.  I'd say that such a test is
> ill-mannered, but we would want to protect ourselves against such mischief
> somehow.

Ah, I guess logging to /tmp/git-test-result was more reliable then.
Any suggestions as to what is the best solution here? (We could for
example save $PWD at the beginning of test-lib.sh.)

> Also, this won't be an immediate issue, but we may want to lift the
> ".NOTPARALLEL" limitation in t/Makefile sometime in the future.  A handful
> tests need to be adjusted for this change, as they currently refer their
> test vectors in t/tXXXX/ as ../tXXXX, and with such a change, we would run
> each test in "t/trash directory/t$$" where $$ is the pid, or something
> like that.  A single "test-results" file everybody races to append would
> become unwieldy at that point.  Something to keep in mind.

Ah, yes, I was going to have each test log to their own file
(test-results-$$) and then cat the result together, but I figured that
since we are .NOTPARALLEL anyway it would be more efficient not to. I
reckon that whenever we decide to make such a change it will be then
that we modify this script, if it is included?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH 1/3] Modified test-lib.sh to output stats to /tmp/git-test-results

[Junio C Hamano]

"Sverre Rabbelier" <srabbelier@gmail.com> writes:

> Ah, I guess logging to /tmp/git-test-result was more reliable then.
> Any suggestions as to what is the best solution here? (We could for
> example save $PWD at the beginning of test-lib.sh.)

Saving at the beginning to use in test_done, and optionally detecting 
that cwd was changed and warn, would be sensible, I think.

> Ah, yes, I was going to have each test log to their own file
> (test-results-$$) and then cat the result together, but I figured that
> since we are .NOTPARALLEL anyway it would be more efficient not to. I
> reckon that whenever we decide to make such a change it will be then
> that we modify this script, if it is included?

Yup.  That is what I meant by "not immediate, but something to keep in
mind".

[PATCH 4/4] Make test-lib warn if the directory changes.

[Sverre Rabbelier]

From: Sverre Rabbelier <srabbelier@gmail.com>

If a script moves out of it's directory, but thereafter
does not move back, the test results would not get written
to the proper file, as such, remember where we start at
and then later on, write to that place.
Also, if it is noticed that the directory was changed,
issue a warning.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 t/test-lib.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4585fde..d9f2f4e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -5,6 +5,7 @@
 
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
+ORIGINAL_PATH=$PWD
 
 # For repeatability, reset the environment to known value.
 LANG=C
@@ -355,7 +356,12 @@ test_create_repo () {
 
 test_done () {
 	trap - exit
-	test_results_path="../test-results"
+	test_results_path="$ORIGINAL_PATH/test-results"
+
+  if test "$ORIGINAL_PATH" != "$PWD"
+  then
+    say_color error "Script changed directory from '$ORIGINAL_PATH' to '$PWD'!"
+  fi
 
 	echo "total $test_count" >> $test_results_path
 	echo "success $test_success" >> $test_results_path
-- 
1.5.6.rc2