threads / patch / 13861

patch, 3 partsHook up the result aggregation in the test makefile.

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

## tl;dr

9 messages between Jun 8, 2008 and Jun 9, 2008. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Sverre Rabbelier· Jun 8, 2008, 14:04 UTC · lore

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

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(-)
Show changes to t/test-lib.sh +10 −0
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
Sverre Rabbelier· Jun 8, 2008, 14:04 UTC · re: Sverre Rabbelier · lore

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

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
Show changes to t/aggregate-results.sh +33 −0
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
Sverre Rabbelier· Jun 8, 2008, 14:04 UTC · re: Sverre Rabbelier · lore
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(-)
Show changes to t/Makefile +9 −2
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
Jakub Narebski· Jun 8, 2008, 14:42 UTC · re: Sverre Rabbelier · lore

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

"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
Sverre Rabbelier· Jun 8, 2008, 14:45 UTC · re: Jakub Narebski · lore

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

On Sun, Jun 8, 2008 at 4:42 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Show 8 quoted lines
> "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
Junio C Hamano· Jun 8, 2008, 18:53 UTC · re: Sverre Rabbelier · lore

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

"Sverre Rabbelier" <srabbelier@gmail.com> writes:
Show 14 quoted lines
> 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.

Sverre Rabbelier· Jun 8, 2008, 19:02 UTC · re: Junio C Hamano · lore

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

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.)

Show 7 quoted lines
> 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
Junio C Hamano· Jun 8, 2008, 20:59 UTC · re: Sverre Rabbelier · lore

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

"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.

Show 5 quoted lines
> 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".

Sverre Rabbelier· Jun 9, 2008, 00:44 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to t/test-lib.sh +7 −1
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

← back to recent threads