You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bhuvaneswaran A <bh...@collab.net> on 2009/10/08 09:19:08 UTC

[PATCH] Time taken to execute the test suite

Hello,
I'm in the process of generating junit file from Subversion build
process. The junit file is generated by parsing the log file, tests.log
created during "make check" process. The junit file can be used to
integrate the Subversion build with continuous integration tools like
Hudson. This patch is to follow ...

Meanwhile, please find attached a patch to print the time taken by each
test suite in the log file, if specified. The time may be mentioned in
the junit file associated to the test suite.

Can you please review this patch?

[[[
This fix adds the ability to print the time taken to execute each test.
The time taken is printed in the log file, if specified; otherwise in
the stdout interface. 
  
* build/run_tests.py
  import time
  (TestHarness._run_test): Print the time taken by each test in
  "HH:MM:SShrs" format.
]]]
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2404858

Re: [PATCH] Time taken to execute the test suite

Posted by Bhuvaneswaran A <bh...@collab.net>.
On Fri, 2009-10-09 at 11:45 +0200, Branko Cibej wrote:
> Bhuvaneswaran A wrote:
> >
> > I was using http:// before. Changed the url and committed the patch in
> > r39887.
> >
> > May I nominate this fix for 1.6.x release and include your vote?
> >   
> 
> Yes, please do that.

Done, as in r39896. Thanks!
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405572

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Bhuvaneswaran A wrote:
> On Fri, 2009-10-09 at 07:15 +0200, Branko Čibej wrote:
>   
>> You *are* using https:// to commit, right? AFAIK Mike changed the
>> access
>> controls on svn.collab.net to only allow writes over secure
>> connections.
>>     
>
> I was using http:// before. Changed the url and committed the patch in
> r39887.
>
> May I nominate this fix for 1.6.x release and include your vote?
>   

Yes, please do that.

-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405463

Re: [PATCH] Time taken to execute the test suite

Posted by Bhuvaneswaran A <bh...@collab.net>.
On Fri, 2009-10-09 at 07:15 +0200, Branko Čibej wrote:
> 
> You *are* using https:// to commit, right? AFAIK Mike changed the
> access
> controls on svn.collab.net to only allow writes over secure
> connections.

I was using http:// before. Changed the url and committed the patch in
r39887.

May I nominate this fix for 1.6.x release and include your vote?
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405420

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Bhuvaneswaran A wrote:
> Thank you, Branko and Julian.
>
> On Thu, 2009-10-08 at 20:32 +0200, Branko Čibej wrote:
>   
>> Yes, I was going to say ... to make parsing the log file easier, the
>> name of the test should be in the ELAPSED: line -- as per my
>> suggestion
>> in an earlier mail.
>>     
>
> Yeah, i've included the test name in the ELAPSED: line.
>
>   
>> But +1, please go ahead and commit this. It looks like a nice change
>> to
>> back-port to 1.6, too.
>>     
>
> hmm, i'm unable to commit. Is commit restricted for partial committers
> to commit outside their area? I see this error:
>
> bhuvan@rainbow:~/projects/subversion/build$ svn ci run_tests.py
> --username bhuvan
> svn: Commit failed (details follow):
> svn: Server sent unexpected return value (403 Forbidden) in response to
> MKACTIVITY request for '/repos/svn/!
> svn/act/f251f847-ce9d-46f8-8020-0c35f977b0fb'
> svn: Your commit message was left in a temporary file:
> svn:    '/home/bhuvan/projects/subversion/build/svn-commit.tmp'
>   

You *are* using https:// to commit, right? AFAIK Mike changed the access
controls on svn.collab.net to only allow writes over secure connections.
-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405415

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Bhuvaneswaran A wrote:
> Thank you, Branko and Julian.
>
> On Thu, 2009-10-08 at 20:32 +0200, Branko Čibej wrote:
>   
>> Yes, I was going to say ... to make parsing the log file easier, the
>> name of the test should be in the ELAPSED: line -- as per my
>> suggestion
>> in an earlier mail.
>>     
>
> Yeah, i've included the test name in the ELAPSED: line.
>
>   
>> But +1, please go ahead and commit this. It looks like a nice change
>> to
>> back-port to 1.6, too.
>>     
>
> hmm, i'm unable to commit. Is commit restricted for partial committers
> to commit outside their area? I see this error:
>
> bhuvan@rainbow:~/projects/subversion/build$ svn ci run_tests.py
> --username bhuvan
> svn: Commit failed (details follow):
> svn: Server sent unexpected return value (403 Forbidden) in response to
> MKACTIVITY request for '/repos/svn/!
> svn/act/f251f847-ce9d-46f8-8020-0c35f977b0fb'
> svn: Your commit message was left in a temporary file:
> svn:    '/home/bhuvan/projects/subversion/build/svn-commit.tmp'
>   

You *are* using https:// to commit, right? AFAIK Mike changed the access
controls on svn.collab.net to only allow writes over secure connections.
-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405415

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405666

Re: [PATCH] Time taken to execute the test suite

Posted by Bhuvaneswaran A <bh...@collab.net>.
Thank you, Branko and Julian.

On Thu, 2009-10-08 at 20:32 +0200, Branko Čibej wrote:
> 
> Yes, I was going to say ... to make parsing the log file easier, the
> name of the test should be in the ELAPSED: line -- as per my
> suggestion
> in an earlier mail.

Yeah, i've included the test name in the ELAPSED: line.

> But +1, please go ahead and commit this. It looks like a nice change
> to
> back-port to 1.6, too.

hmm, i'm unable to commit. Is commit restricted for partial committers
to commit outside their area? I see this error:

bhuvan@rainbow:~/projects/subversion/build$ svn ci run_tests.py
--username bhuvan
svn: Commit failed (details follow):
svn: Server sent unexpected return value (403 Forbidden) in response to
MKACTIVITY request for '/repos/svn/!
svn/act/f251f847-ce9d-46f8-8020-0c35f977b0fb'
svn: Your commit message was left in a temporary file:
svn:    '/home/bhuvan/projects/subversion/build/svn-commit.tmp'

If yes, can someone commit the attached patch and propose to back-port
it to 1.6? If no, can someone who has access to svn.collab.net (Mike?)
fix this issue, please?

[[[
This fix adds the ability to print the time taken to execute each test.
The time taken is printed in the log file, if specified; otherwise in
the stdout interface. 
  
* build/run_tests.py
  import time
  (TestHarness._run_test): Print the time taken by each test using
  following format:
  ELAPSED: test_name HH:MM:SS

Reviewed by: julianfoad
             brane
]]]
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405407

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Julian Foad wrote:
> Bhuvaneswaran A wrote:
>   
>> Thank you once again for the review comments, Branko. 
>>
>> Please find attached the revised patch. The log message remains the
>> same.
>>     
>
> I'm not sure Branko is still on line now,

'Course I'm online. It's not even midnight yet. :)

> but it looks good to me. +1.
>
> [[[
> PASS:  autoprop_tests.py 15: fail to add a file with mixed EOL style
> CLEANUP: svn-test-work/working_copies/autoprop_tests-15
> CLEANUP: svn-test-work/repositories/autoprop_tests-15
> END: autoprop_tests.py
> ELAPSED: 00:00:14
>
> START: basic_tests.py
> [...]
> ]]]
>
> (You can always tweak it later if, for example, you decide it would be
> better to include the name of the test in the ELAPSED line.)
>   

Yes, I was going to say ... to make parsing the log file easier, the
name of the test should be in the ELAPSED: line -- as per my suggestion
in an earlier mail.

But +1, please go ahead and commit this. It looks like a nice change to
back-port to 1.6, too.

-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405271

Re: [PATCH] Time taken to execute the test suite

Posted by Julian Foad <ju...@btopenworld.com>.
Bhuvaneswaran A wrote:
> Thank you once again for the review comments, Branko. 
> 
> Please find attached the revised patch. The log message remains the
> same.

I'm not sure Branko is still on line now, but it looks good to me. +1.

[[[
PASS:  autoprop_tests.py 15: fail to add a file with mixed EOL style
CLEANUP: svn-test-work/working_copies/autoprop_tests-15
CLEANUP: svn-test-work/repositories/autoprop_tests-15
END: autoprop_tests.py
ELAPSED: 00:00:14

START: basic_tests.py
[...]
]]]

(You can always tweak it later if, for example, you decide it would be
better to include the name of the test in the ELAPSED line.)

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405207

Re: [PATCH] Time taken to execute the test suite

Posted by Bhuvaneswaran A <bh...@collab.net>.
Thank you once again for the review comments, Branko. 

Please find attached the revised patch. The log message remains the
same.

On Thu, 2009-10-08 at 12:25 +0200, Branko Čibej wrote:
> Bhuvaneswaran A wrote:
> > Thank you for the review comments, Branko.
> >
> > On Thu, 2009-10-08 at 11:41 +0200, Branko Čibej wrote:
> >   
> >>> if progbase[-3:] == '.py':
> >>> progname = sys.executable
> >>> cmdline = [quote(progname),
> >>> @@ -255,10 +257,14 @@
> >>> print(TextColors.FAILURE + 'FAILURE' + TextColors.ENDC)
> >>> else:
> >>> print(TextColors.SUCCESS + 'success' + TextColors.ENDC)
> >>> + endtime = int(time.time())
> >>> + epoch = endtime - starttime
> >>> + (hrs, mins, secs) = ((epoch/(60*60))%24, (epoch/60)%60, epoch%60)
> >>> + total_time = "%02d:%02d:%02d" % (hrs, mins, secs)
> >>>       
> >> elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)
> >>     
> >
> > The argument to time.strftime() method should be of type "list", not
> > "float". The above change complains:
> >   File "./build/run_tests.py", line 260, in _run_test
> >     elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)
> > TypeError: argument must be 9-item sequence, not float
> > make: *** [check] Error 1
> >   
> 
> Duh, of course I meant:
> 
> elapsed_time = time.strftime('%H:%M:%S', time.gmtime(time.time() -
> start_time))
> 
> > May I incorporate rest of your review comments and attach the patch?
> >   
> 
> 
> Of course.
> 
> -- Brane
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405162

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Bhuvaneswaran A wrote:
> Thank you for the review comments, Branko.
>
> On Thu, 2009-10-08 at 11:41 +0200, Branko Čibej wrote:
>   
>>> if progbase[-3:] == '.py':
>>> progname = sys.executable
>>> cmdline = [quote(progname),
>>> @@ -255,10 +257,14 @@
>>> print(TextColors.FAILURE + 'FAILURE' + TextColors.ENDC)
>>> else:
>>> print(TextColors.SUCCESS + 'success' + TextColors.ENDC)
>>> + endtime = int(time.time())
>>> + epoch = endtime - starttime
>>> + (hrs, mins, secs) = ((epoch/(60*60))%24, (epoch/60)%60, epoch%60)
>>> + total_time = "%02d:%02d:%02d" % (hrs, mins, secs)
>>>       
>> elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)
>>     
>
> The argument to time.strftime() method should be of type "list", not
> "float". The above change complains:
>   File "./build/run_tests.py", line 260, in _run_test
>     elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)
> TypeError: argument must be 9-item sequence, not float
> make: *** [check] Error 1
>   

Duh, of course I meant:

elapsed_time = time.strftime('%H:%M:%S', time.gmtime(time.time() -
start_time))

> May I incorporate rest of your review comments and attach the patch?
>   


Of course.

-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2404894

Re: [PATCH] Time taken to execute the test suite

Posted by Bhuvaneswaran A <bh...@collab.net>.
Thank you for the review comments, Branko.

On Thu, 2009-10-08 at 11:41 +0200, Branko Čibej wrote:
> 
> > if progbase[-3:] == '.py':
> > progname = sys.executable
> > cmdline = [quote(progname),
> > @@ -255,10 +257,14 @@
> > print(TextColors.FAILURE + 'FAILURE' + TextColors.ENDC)
> > else:
> > print(TextColors.SUCCESS + 'success' + TextColors.ENDC)
> > + endtime = int(time.time())
> > + epoch = endtime - starttime
> > + (hrs, mins, secs) = ((epoch/(60*60))%24, (epoch/60)%60, epoch%60)
> > + total_time = "%02d:%02d:%02d" % (hrs, mins, secs)
> 
> elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)

The argument to time.strftime() method should be of type "list", not
"float". The above change complains:
  File "./build/run_tests.py", line 260, in _run_test
    elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)
TypeError: argument must be 9-item sequence, not float
make: *** [check] Error 1

May I incorporate rest of your review comments and attach the patch?
-- 
Bhuvaneswaran A    
CollabNet Software P Ltd.  |  www.collab.net

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2404875

Re: [PATCH] Time taken to execute the test suite

Posted by Branko Cibej <br...@xbc.nu>.
Bhuvaneswaran A wrote:
> Hello,
> I'm in the process of generating junit file from Subversion build
> process. The junit file is generated by parsing the log file, tests.log
> created during "make check" process. The junit file can be used to
> integrate the Subversion build with continuous integration tools like
> Hudson. This patch is to follow ...
>
> Meanwhile, please find attached a patch to print the time taken by each
> test suite in the log file, if specified. The time may be mentioned in
> the junit file associated to the test suite.
>
> Can you please review this patch?
>
> [[[
> This fix adds the ability to print the time taken to execute each test.
> The time taken is printed in the log file, if specified; otherwise in
> the stdout interface. 
>   
> * build/run_tests.py
>   import time
>   (TestHarness._run_test): Print the time taken by each test in
>   "HH:MM:SShrs" format.
> ]]]
>   
Surely you didn't mean to put that "import time" into the log message. :)
Why the "hrs" suffix on the time? That makes the timestamp format
non-standard.

> Index: build/run_tests.py
> =======================================
> --- build/run_tests.py	(revision 39857)
> +++ build/run_tests.py	(working copy)
> @@ -16,6 +16,7 @@
> '''
> import os, sys
> +import time
> import getopt
> try:
> @@ -193,6 +194,7 @@
> else:
> print('START: %s' % progbase)
> + starttime = int(time.time())

start_time = time.time()

> if progbase[-3:] == '.py':
> progname = sys.executable
> cmdline = [quote(progname),
> @@ -255,10 +257,14 @@
> print(TextColors.FAILURE + 'FAILURE' + TextColors.ENDC)
> else:
> print(TextColors.SUCCESS + 'success' + TextColors.ENDC)
> + endtime = int(time.time())
> + epoch = endtime - starttime
> + (hrs, mins, secs) = ((epoch/(60*60))%24, (epoch/60)%60, epoch%60)
> + total_time = "%02d:%02d:%02d" % (hrs, mins, secs)

elapsed_time = time.strftime('%H:%M:%S', time.time() - start_time)

> if self.log:
> - self.log.write('END: %s\n\n' % progbase)
> + self.log.write('END: %s; TIME TAKEN: %shrs\n\n' % (progbase,
> total_time))
> else:
> - print('END: %s\n' % progbase)
> + print('END: %s; TIME TAKEN: %shrs\n' % (progbase, total_time))
> return failed

I object to appending the elapsed time onto the END: line. It makes
parsing the log file harder. Instead, just put the elapsed time on its
own line, e.g.:

    self.log.write('ELAPSED: %s %s\n\n' % (progbase, elapsed_time))

-- Brane

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2404867