[PATCH 0/3] Aggregate testcase results

[PATCH 0/3] Aggregate testcase results

[Sverre Rabbelier]

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

[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 2/3] A simple python script to parse the results from the testcases

[Sverre Rabbelier]

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

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

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

[Jakub Narebski]

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

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

[Sverre Rabbelier]

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

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

[Johannes Schindelin]

Hi,

On Mon, 12 May 2008, Sverre Rabbelier wrote:

> 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

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

[Sverre Rabbelier]

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

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

[Johannes Schindelin]

Hi,

On Mon, 12 May 2008, Sverre Rabbelier wrote:

> 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

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

[Miklos Vajna]

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

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

[Sverre Rabbelier]

On Sun, Jun 8, 2008 at 2:18 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> 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

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

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.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Sverre Rabbelier]

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

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

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

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Sverre Rabbelier]

On Sun, Jun 8, 2008 at 4:26 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> 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

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

[Johannes Schindelin]

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

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

[Sverre Rabbelier]

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?

-- 
Cheers,

Sverre Rabbelier

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

[Mikael Magnusson]

2008/6/8 Sverre Rabbelier <srabbelier@gmail.com>:
> 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

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

[Sverre Rabbelier]

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

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

[Sverre Rabbelier]

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

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

[Vegard Nossum]

Hi,

On Mon, May 12, 2008 at 11:33 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> 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