Code review comment for lp:~bac/launchpad/retest

Revision history for this message
Brad Crittenden (bac) wrote :

On Sep 25, 2009, at 11:39 , Barry Warsaw wrote:

> Review: Approve
> On Sep 25, 2009, at 02:40 PM, Brad Crittenden wrote:
>
>> Adding a new utility to make re-running a failed test set easy.
>
> Okay, you are my new best friend! Thanks for writing this awesome
> utility.
> I have some minor comments about style, but merge-conditional r=me
> with their
> consideration.
>
> review approve
> status approve

Thanks!

>
> === added file 'utilities/retest.py'
> --- utilities/retest.py 1970-01-01 00:00:00 +0000
> +++ utilities/retest.py 2009-09-25 14:40:29 +0000
>> +def extractTests(fd):
>> + """Get the set of tests to be run.
>> +
>> + Given an open file descriptor pointing to a test summary
>> report, find all
>> + of the tests to be run.
>> + """
>
> Isn't fd really a file object, not a file descriptor? Also, to avoid
> abbreviations maybe 'input_file' is a better name?

Right. Changed.

>
>> + failed_tests = set()
>> + reading_tests = False
>> + line = True
>> + while (line):
>> + line = fd.readline()
>
> You can do this more efficiently like so
>
> for line in input_file:

Fixed.

« Back to merge proposal