[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

[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

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