threads / patch / 13489

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

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

replies: 20people: 6as markdown or json

Sverre Rabbelier· May 12, 2008, 09:33 UTC · lore

[PATCH 0/3] Aggregate testcase results

Heya,

The following patch series provides a summary of the test results at the end of each 'full run'. The reason I wrote this series is that I noticed when running 'make' in '/t/' that there is no way for me to find out if all testcases still pass without scrolling through the output. Since almost all testcases -do- pass anyway, and it takes quite a long time to run them all I wanted a way to see at a glance if a change I made breaks something (that is covered by the testcases).

Cheers,
Sverre Rabbelier
Sverre Rabbelier· May 12, 2008, 09:33 UTC · re: Sverre Rabbelier · lore

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

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 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
Vegard Nossum· May 12, 2008, 14:55 UTC · re: Sverre Rabbelier · lore

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

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
Sverre Rabbelier· May 12, 2008, 09:33 UTC · re: Sverre Rabbelier · lore

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

This is a simple script that aggregates key:value pairs in a file. I am sure this can be done better with a 'grep | sed | awk' combination, my skills with awk / your program of choice is not as profound. This script serves more as a demonstration on how to use the testcase output.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
 t/key_value_parser.py |   78 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)
 create mode 100755 t/key_value_parser.py
Show changes to t/key_value_parser.py +78 −0
diff --git a/t/key_value_parser.py b/t/key_value_parser.py
new file mode 100755
index 0000000..e172df1
--- /dev/null
+++ b/t/key_value_parser.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python
+
+"""Module for parsing files with 'key value' pairs.
+
+Usage summary: key_value_parser.py "path/to/file"
+"""
+
+def parseFile(path):
+	"""Parses a file containing pair value couples.
+	
+	The values are expected to be integers.
+	If more than one value for a pair is found, these values are aggregated.
+	The size of the longest key is stored in '__maxsize'.
+	A dictionairy with only '__maxsize:0' is returned if an error occured.
+	"""
+
+	dict = {"__maxsize" : 0}
+
+	try:
+		file = open(path)
+		for line in file:
+			if line == '\n':
+				continue
+
+			pos = line.index(' ')
+			key = line[:pos]
+			
+			# Skip the space and the trailing newline
+			value = line[pos+1:-1] 
+			intvalue = int(value)
+
+			if key in dict:
+				dict[key] = intvalue + dict[key]
+			else:
+				dict[key] = intvalue
+
+			if len(key) > dict["__maxsize"]:
+				dict["__maxsize"] = len(key)
+
+	except IOError:
+		print("Cannot open or read from file " + path)
+	except ValueError:
+		print("Malformed line in file ")
+
+	return dict
+
+def main(argv):
+	""" Invokes parseFile on the path specified as argument.
+	
+	If no argument is specified, a default of 
+	'/tmp/git-test-results' is used.
+
+	The resulting dictionary is printed, ignoring keys starting with '__'.
+	All output is left justified to the size of the largest key.
+	"""
+	path = "/tmp/git-test-results"
+	
+	# If a path was specified as argument, use that
+	if len(argv) > 1:
+		path = argv[1]
+
+	# Parse the file
+	dict = parseFile(path)
+
+	# Retreive the max size and add one space for readability
+	maxsize = dict["__maxsize"] + 1
+
+	# Print the result
+	for key,value in dict.iteritems():
+		# Don't print meta-data
+		if key.startswith("__"):
+			continue
+
+		print(key.ljust(maxsize) + str(value))
+
+if __name__ == '__main__':
+	import sys
+	main(sys.argv)

-- 
1.5.5.1.178.g1f811
Jakub Narebski· May 12, 2008, 10:14 UTC · re: Sverre Rabbelier · lore

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

Sverre Rabbelier <srabbelier@gmail.com> writes:
> This is a simple script that aggregates key:value pairs in a file.
> I am sure this can be done better with a 'grep | sed | awk' combination,
> my skills with awk / your program of choice is not as profound.

Or Perl, as Perl was created for that. I'd rather don't reintroduce Python dependency...

-- 
Jakub Narebski
Poland
ShadeHawk on #git
Sverre Rabbelier· May 12, 2008, 10:16 UTC · re: Jakub Narebski · lore

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

On Mon, May 12, 2008 at 12:14 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Or Perl, as Perl was created for that.  I'd rather don't reintroduce
> Python dependency...

I'm sure Perl would work just fine, except that I'm not good with Perl either. If anyone feels like writing up something in Perl I'd be happy to test it and send in a new patch with the Perl script.

-- 
Cheers,

Sverre Rabbelier
Johannes Schindelin· May 12, 2008, 13:00 UTC · re: Sverre Rabbelier · lore

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

Hi,
On Mon, 12 May 2008, Sverre Rabbelier wrote:
Show 7 quoted lines
> On Mon, May 12, 2008 at 12:14 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> > Or Perl, as Perl was created for that.  I'd rather don't reintroduce 
> > Python dependency...
> 
> I'm sure Perl would work just fine, except that I'm not good with Perl 
> either. If anyone feels like writing up something in Perl I'd be happy 
> to test it and send in a new patch with the Perl script.
Umm.  Perl cannot be so difficult as to reintroduce the dependency.  
Remember: msysGit comes _without_ Python.

Ciao, Dscho

Sverre Rabbelier· May 12, 2008, 13:05 UTC · re: Johannes Schindelin · lore

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

On Mon, May 12, 2008 at 3:00 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

>  Umm.  Perl cannot be so difficult as to reintroduce the dependency.
>  Remember: msysGit comes _without_ Python.

Agreed, as said in the commit message, it "serves more as a demonstration on how to use the testcase output".

-- 
Cheers,

Sverre Rabbelier
Johannes Schindelin· May 12, 2008, 13:24 UTC · re: Sverre Rabbelier · lore

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

Hi,
On Mon, 12 May 2008, Sverre Rabbelier wrote:
Show 7 quoted lines
> On Mon, May 12, 2008 at 3:00 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >  Umm.  Perl cannot be so difficult as to reintroduce the dependency.
> >  Remember: msysGit comes _without_ Python.
> 
> Agreed, as said in the commit message, it "serves more as a 
> demonstration on how to use the testcase output". -- Cheers,
Ah, but then it is rather a "RFTTP" instead of a "PATCH".
:-)

Ciao, Dscho

Miklos Vajna· Jun 8, 2008, 00:18 UTC · re: Sverre Rabbelier · lore

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

This is a simple script that aggregates key:value pairs in a file.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Mon, May 12, 2008 at 11:33:51AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> This is a simple script that aggregates key:value pairs in a file.
Here is a shell version. Just to avoid python.
 t/key_value_parser.sh |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
 create mode 100755 t/key_value_parser.sh
Show changes to t/key_value_parser.sh +33 −0
diff --git a/t/key_value_parser.sh b/t/key_value_parser.sh
new file mode 100755
index 0000000..db568fe
--- /dev/null
+++ b/t/key_value_parser.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+input="/tmp/git-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.6.rc0.dirty
Sverre Rabbelier· Jun 8, 2008, 00:34 UTC · re: Miklos Vajna · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 8, 2008 at 2:18 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
Show 55 quoted lines
> This is a simple script that aggregates key:value pairs in a file.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> On Mon, May 12, 2008 at 11:33:51AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> This is a simple script that aggregates key:value pairs in a file.
>
> Here is a shell version. Just to avoid python.
>
>  t/key_value_parser.sh |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
>  create mode 100755 t/key_value_parser.sh
>
> diff --git a/t/key_value_parser.sh b/t/key_value_parser.sh
> new file mode 100755
> index 0000000..db568fe
> --- /dev/null
> +++ b/t/key_value_parser.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +
> +input="/tmp/git-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.6.rc0.dirty

Awesome, what do you want to do with the other patches? I mean, this patch on it's own doesn't make a lot of sense, but with [1/3] and [3/3] I think it deserves some proper reviewing by the list.

-- 
Cheers,

Sverre Rabbelier
Miklos Vajna· Jun 8, 2008, 00:49 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 08, 2008 at 02:34:25AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Awesome, what do you want to do with the other patches?
Nothing? It's your series. :-)
> I mean, this patch on it's own doesn't make a lot of sense, but with
> [1/3] and [3/3] I think it deserves some proper reviewing by the list.
Sure. I would suggest:
1) Remove that ugly /tmp/git-test-results, place it under t/.
2) Resend a series indicating this is no longer a demonstration but a
real series which you want to be included. ;-)

Ah and it's bikesheding, but probably key_value_parser.sh is not the best name for such a script. Maybe aggregate-results.sh or something like that.

Sverre Rabbelier· Jun 8, 2008, 00:56 UTC · re: Miklos Vajna · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 8, 2008 at 2:49 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Sun, Jun 08, 2008 at 02:34:25AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> Awesome, what do you want to do with the other patches?
>
> Nothing? It's your series. :-)

Heh, I'm not sure what the protocol is here :P. I could send in the series with your patch as second... that is, if I can figure out how to apply it from gmail (maybe you can send me the patch as attachment? :D).

Show 6 quoted lines
>> I mean, this patch on it's own doesn't make a lot of sense, but with
>> [1/3] and [3/3] I think it deserves some proper reviewing by the list.
>
> Sure. I would suggest:
>
> 1) Remove that ugly /tmp/git-test-results, place it under t/.

I remember trying that but it not working, which was why I put it there in the first place. I'll give it a shot again tomorrow though.

> 2) Resend a series indicating this is no longer a demonstration but a
> real series which you want to be included. ;-)
ACK on that one ;).
> Ah and it's bikesheding, but probably key_value_parser.sh is not the
> best name for such a script. Maybe aggregate-results.sh or something
> like that.

Sure, but that's what it was though, a simple key_value_parser, your version is actually a result aggregator.

-- 
Cheers,

Sverre Rabbelier
Miklos Vajna· Jun 8, 2008, 02:26 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 08, 2008 at 02:56:09AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heh, I'm not sure what the protocol is here :P. I could send in the
> series with your patch as second... that is, if I can figure out how
> to apply it from gmail (maybe you can send me the patch as attachment?
> :D).
Or:
$ git pull git://repo.or.cz/git/vmiklos.git stat
Sverre Rabbelier· Jun 8, 2008, 11:43 UTC · re: Miklos Vajna · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 8, 2008 at 4:26 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
Show 9 quoted lines
> On Sun, Jun 08, 2008 at 02:56:09AM +0200, Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> Heh, I'm not sure what the protocol is here :P. I could send in the
>> series with your patch as second... that is, if I can figure out how
>> to apply it from gmail (maybe you can send me the patch as attachment?
>> :D).
>
> Or:
>
> $ git pull git://repo.or.cz/git/vmiklos.git stat

I used the 'show original message' feature, so that I could edit it before applying. I changed the name to aggregate-results.sh as suggested and changed the path to /t/test-results. Now let's see if I can make a new patch series out of this...

-- 
Cheers,

Sverre Rabbelier
Johannes Schindelin· Jun 8, 2008, 17:27 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] A simple script to parse the results from the testcases

Hi,
On Sun, 8 Jun 2008, Sverre Rabbelier wrote:
> I used the 'show original message' feature, so that I could edit it 
> before applying.
FWIW that's what "commit --amend" and "rebase --interactive" are for.

Ciao, Dscho

Sverre Rabbelier· Jun 8, 2008, 17:30 UTC · re: Johannes Schindelin · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 8, 2008 at 7:27 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 8 quoted lines
> Hi,
>
> On Sun, 8 Jun 2008, Sverre Rabbelier wrote:
>
>> I used the 'show original message' feature, so that I could edit it
>> before applying.
>
> FWIW that's what "commit --amend" and "rebase --interactive" are for.

Hmmm, yeah, but can you change the filename easily with commit --ammend / rebase --interactive?

-- 
Cheers,

Sverre Rabbelier
Mikael Magnusson· Jun 8, 2008, 18:06 UTC · re: Sverre Rabbelier · lore

Re: [PATCH] A simple script to parse the results from the testcases

2008/6/8 Sverre Rabbelier <srabbelier@gmail.com>:
Show 13 quoted lines
> On Sun, Jun 8, 2008 at 7:27 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi,
>>
>> On Sun, 8 Jun 2008, Sverre Rabbelier wrote:
>>
>>> I used the 'show original message' feature, so that I could edit it
>>> before applying.
>>
>> FWIW that's what "commit --amend" and "rebase --interactive" are for.
>
> Hmmm, yeah, but can you change the filename easily with commit
> --ammend / rebase --interactive?

git mv oldname newname git commit --amend ;: not --ammend

-- 
Mikael Magnusson
Sverre Rabbelier· Jun 8, 2008, 18:09 UTC · re: Mikael Magnusson · lore

Re: [PATCH] A simple script to parse the results from the testcases

On Sun, Jun 8, 2008 at 8:06 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> git mv oldname newname
> git commit --amend ;: not --ammend

Details, details. Ah well, the way I did it seemed the most easy at the time, maybe next time I'll use amend (my favorite feature in git that I usually spell incorrectly at least twice).

-- 
Cheers,

Sverre Rabbelier
Sverre Rabbelier· May 12, 2008, 09:33 UTC · re: Sverre Rabbelier · lore

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 |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Show changes to t/Makefile +8 −1
diff --git a/t/Makefile b/t/Makefile
index 72d7884..3955ee8 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -14,13 +14,20 @@ 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 /tmp/git-test-results
+
 clean:
 	$(RM) -r trash
+	$(RM) -f /tmp/git-test-results
+
+aggregate-results:
+	./key_value_parser.py
 
 # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
 full-svn-test:

-- 
1.5.5.1.178.g1f811
Vegard Nossum· May 12, 2008, 15:03 UTC · re: Sverre Rabbelier · lore

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

Hi,
On Mon, May 12, 2008 at 11:33 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 36 quoted lines
> 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 |    9 ++++++++-
>   1 files changed, 8 insertions(+), 1 deletions(-)
>
>  diff --git a/t/Makefile b/t/Makefile
>  index 72d7884..3955ee8 100644
>  --- a/t/Makefile
>  +++ b/t/Makefile
>  @@ -14,13 +14,20 @@ 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 /tmp/git-test-results
>  +
>   clean:
>         $(RM) -r trash
>  +       $(RM) -f /tmp/git-test-results
>  +
>  +aggregate-results:
>  +       ./key_value_parser.py
>
>   # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL
>   full-svn-test:
>  --
>  1.5.5.1.178.g1f811

I am not really familiar with the git makefile in particular, but usually it's a good idea to put

.PHONY: pre-clean aggregate-results

as well if these targets are not output files. (Rationale can be found in section 4.6 "Phony Targets" of the GNU make manual.)

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

← back to recent threads