Merge lp:~jml/launchpad/subunit-by-default into lp:launchpad

= Purpose =

This branch is intended to make debugging of test failures on EC2 easier by providing subunit output and by providing better error messages.

It also changes the default behaviour of the 'ec2 test' command to run in headless mode, deletes the '--headless' option and creates a new '--attached' option that is the inverse of the old '--headless' option.

= Implementation =

Most of the command changes are fairly boring. The interesting changes are in ec2test-remote and testrunner.py

In ec2test-remote, I deleted the JS checker, which is no longer used. I also removed the regex-based mechanic for producing output summary.

Instead, the full output is produced via subunit and interpreted using a component from subunit that drives a Python standard library TestResult object. I made my own TestResult object that prints out errors and failures as they occur.

I also rejiggered the subprocess code to have less nesting. I think this has made the behaviour clearer.

Finally, the subunit output is attached to the failure email. The attachment should have the correct mime type and a useful filename.

= Testing =

I've done several test runs through ec2 at various stages of development, including the final stage.

I forgot to mention. There is an evil hack that makes sure that subunit is available to ec2test-remote.py. I'd love to be able to get rid of it, but I don't know how.

Looks good. I have a couple of questions/suggestions. I'm going to
borrow SummaryResult if that's okay, for my very-long-in-the-tooth-
but-still-going parallelization branch.

> === modified file 'lib/devscripts/ec2test/ec2test-remote.py'
> --- lib/devscripts/ec2test/ec2test-remote.py 2009-10-09 15:04:24 +0000
> +++ lib/devscripts/ec2test/ec2test-remote.py 2010-02-02 13:45:04 +0000
> @@ -106,80 +129,82 @@
> call = self.build_test_command()
>
> try:
> + popen = subprocess.Popen(
> + call, bufsize=-1,
> + stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
> + cwd=self.test_dir)
> +
> + self._gather_test_output(popen.stdout, summary_file, out_file)
> +
> + # Grab the testrunner exit status
> + result = popen.wait()
> +
> + if self.pqm_message is not None:
> + subject = self.pqm_message.get('Subject')
> + if result:
> + # failure
> + summary_file.write(
> + '\n\n**NOT** submitted to PQM:\n%s\n' % (subject,))
> + else:
> + # success
> + conn = bzrlib.smtp_connection.SMTPConnection(config)
> + conn.send_email(self.pqm_message)
> + summary_file.write(
> + '\n\nSUBMITTED TO PQM:\n%s\n' % (subject,))
> + except:
> + summary_file.write('\n\nERROR IN TESTRUNNER\n\n')
> + traceback.print_exc(file=summary_file)
> + result = 1
> + raise
> + finally:
> + # It probably isn't safe to close the log files ourselves,
> + # since someone else might try to write to them later.
> try:
> - try:
> - popen = subprocess.Popen(
> - call, bufsize=-1,
> - stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
> - cwd=self.test_dir)
> -
> - self._gather_test_output(popen, summary_file, out_file)
> -
> - # Grab the testrunner exit status
> - result = popen.wait()
> -
> - if self.pqm_message is not None:
> - subject = self.pqm_message.get('Subject')
> - if result:
> - # failure
> - summary_file.write(
> - '\n\n**NOT** submitted to PQM:\n%s\n' %
> - (subject,))
> - else:
> - # success
> - conn = bzrlib.smtp_connection.SMTPConnection(
> - config)
> - conn.send_email(self.pqm_message)
> - summary_file.write('\n\nSUBMITTED TO PQM:\n%s\n' %
> - (subject,))
> - except:
> - summary_file.write('\n\nERROR IN TESTRUNNER\n\n')
> - traceback.print_exc(file=summary_file)
> - ...

Maybe it's worth adding the subunit PPA <https://launchpad.net/~subunit/+archive/ppa> to the EC2 host and installing from there, instead of doing the sys.path hack? It's not clear that it's a lot cleaner though.

On Tue, Feb 2, 2010 at 2:01 PM, Gavin Panella
<email address hidden> wrote:
> Review: Approve
> Looks good. I have a couple of questions/suggestions. I'm going to
> borrow SummaryResult if that's okay, for my very-long-in-the-tooth-
> but-still-going parallelization branch.
>

It's fine. It'll be in trunk, of course, by the time you get there :)

>
>> === modified file 'lib/devscripts/ec2test/ec2test-remote.py'
>> --- lib/devscripts/ec2test/ec2test-remote.py  2009-10-09 15:04:24 +0000
>> +++ lib/devscripts/ec2test/ec2test-remote.py  2010-02-02 13:45:04 +0000
>> @@ -106,80 +129,82 @@
>>          call = self.build_test_command()
>>
>>          try:
>> +            popen = subprocess.Popen(
>> +                call, bufsize=-1,
>> +                stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
>> +                cwd=self.test_dir)
>> +
>> +            self._gather_test_output(popen.stdout, summary_file, out_file)
>> +
>> +            # Grab the testrunner exit status
>> +            result = popen.wait()
>> +
>> +            if self.pqm_message is not None:
>> +                subject = self.pqm_message.get('Subject')
>> +                if result:
>> +                    # failure
>> +                    summary_file.write(
>> +                        '\n\n**NOT** submitted to PQM:\n%s\n' % (subject,))
>> +                else:
>> +                    # success
>> +                    conn = bzrlib.smtp_connection.SMTPConnection(config)
>> +                    conn.send_email(self.pqm_message)
>> +                    summary_file.write(
>> +                        '\n\nSUBMITTED TO PQM:\n%s\n' % (subject,))
>> +        except:
>> +            summary_file.write('\n\nERROR IN TESTRUNNER\n\n')
>> +            traceback.print_exc(file=summary_file)
>> +            result = 1
>> +            raise
>> +        finally:
>> +            # It probably isn't safe to close the log files ourselves,
>> +            # since someone else might try to write to them later.
>>              try:
>> -                try:
>> -                    popen = subprocess.Popen(
>> -                        call, bufsize=-1,
>> -                        stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
>> -                        cwd=self.test_dir)
>> -
>> -                    self._gather_test_output(popen, summary_file, out_file)
>> -
>> -                    # Grab the testrunner exit status
>> -                    result = popen.wait()
>> -
>> -                    if self.pqm_message is not None:
>> -                        subject = self.pqm_message.get('Subject')
>> -                        if result:
>> -                            # failure
>> -                            summary_file.write(
>> -                                '\n\n**NOT** submitted to PQM:\n%s\n' %
>> -                                (subject,))
>> -                        else:
>> -                            # success
>> -                            conn = bzrlib.smtp_connection.SMTPConnection(
>> -                                config)
>> -                            conn.send_email(self.pqm_message)
>> -                            summary_file.write('\n\nSUBMITTED TO PQM:\n%s\n' %
>...

> > An alternative for all the logs is to open them line buffered,
> > assuming that works as advertised.
>
> How do you do that?
>
> jml

For stdout I think you can open a new file object with line buffering
enabled (untested):

  my_stdout = os.fdopen(sys.stdout.fileno(), 'w', 1)

For regular files, it's similar:

  fd = open('fred.txt', 'w', 1)

Gavin,

Thanks for the reviews & for the line buffering tip.

I don't think the output of this branch is really where I want it to be for landing, and I don't even think it's an incremental improvement. Also, with further integration testing, I've noticed odd behaviours (although these may be the xvfb bugs people keep mentioning).

In any case, it's not ready to land yet.

jml

Hello,

Now that the new zope.testing with improved subunit support has landed, I feel comfortable and confident that this branch is ready to land also.

Leaving it up for review, since there has been some change since it was last reviewed.

jml

Revs 10207, 10208, 10210 and 10211 all look good.