← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jml/launchpad/different-ec2-mail into lp:launchpad/devel

 

Jonathan Lange has proposed merging lp:~jml/launchpad/different-ec2-mail into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch took a really long time and doesn't do what I wanted it to originally. 

It busts up ec2test/remote.py, the script that runs on EC2 instances which runs tests, gathers output and sends emails to PQM and developers. It's also responsible for shutting down the instance when running in detached mode.

The refactoring is pretty severe, so you might be better off comparing the old file and the new file side by side, rather than looking at the diff.

I've avoided behavioral changes, even when they would be simple, since mixing them with refactoring generally only causes woe. However, I may have made the occasional slip.

Once I refactored the script and roughly demonstrated that it works by doing a couple of test runs, I added a whole bunch of tests. I wanted to try to reach full test coverage, but I ran out of steam. The untested bits are flagged as such.

To summarize the refactoring:
 * The large chunk of code in an 'if __name__ == "__main__"' block has been 
   split into  parse_options() and main() functions.

 * TestOnMergeRunner has merged into BaseTestRunner and then split up into 
   three classes.
   1. EC2Runner, which knows how to run arbitrary stuff and do 
      daemonization & shutdown around it.
   2. LaunchpadTester, which knows how to run Launchpad tests and gather 
      results.
   3. Request, which knows all about the test run we've been asked to do,
      and how to send information back to the end user.
 
 * WebTestLogger has been beefed up so that its clients don't have to 
   fiddle directly with the files it manages.  It is now also responsible
   for deciding what to do with the test result once it gets it.

 * SummaryResult has been tweaked to be way more testable.

 * A general renaming of internal attributes and methods to have 
   underscore prefixes.

I'm not wedded to any of the names of the classes, but I do think the breakdown of responsibilities makes sense.

In addition to the refactoring, I added a --not-really option to "ec2 test", because it was very helpful for me when debugging.  Since the ec2 commands are already laboring under a heavy burden of options and flags that are rarely (if ever) used, I'd be happy to drop it.

Let me know if you have any questions, and thanks for reviewing such a large branch.
-- 
https://code.launchpad.net/~jml/launchpad/different-ec2-mail/+merge/32905
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/different-ec2-mail into lp:launchpad/devel.
=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2010-08-13 21:41:52 +0000
+++ lib/devscripts/ec2test/builtins.py	2010-08-17 17:29:51 +0000
@@ -249,6 +249,7 @@
                 'If the branch is local, then the bzr configuration is '
                 'consulted; for remote branches "Launchpad PQM '
                 '<launchpad@xxxxxxxxxxxxxxxxx>" is used by default.')),
+        Option('not-really', help=("Do not run the tests.")),
         postmortem_option,
         attached_option,
         debug_option,
@@ -264,8 +265,8 @@
             instance_type=DEFAULT_INSTANCE_TYPE,
             file=None, email=None, test_options=DEFAULT_TEST_OPTIONS,
             noemail=False, submit_pqm_message=None, pqm_public_location=None,
-            pqm_submit_location=None, pqm_email=None, postmortem=False,
-            attached=False, debug=False, open_browser=False,
+            pqm_submit_location=None, pqm_email=None, not_really=False,
+            postmortem=False, attached=False, debug=False, open_browser=False,
             include_download_cache_changes=False):
         if debug:
             pdb.set_trace()
@@ -309,7 +310,11 @@
             instance=instance, launchpad_login=instance._launchpad_login,
             timeout=300)
 
-        instance.set_up_and_run(postmortem, attached, runner.run_tests)
+        if not_really:
+            run = runner.dont_run_tests
+        else:
+            run = runner.run_tests
+        instance.set_up_and_run(postmortem, attached, run)
 
 
 class cmd_land(EC2Command):

=== modified file 'lib/devscripts/ec2test/remote.py'
--- lib/devscripts/ec2test/remote.py	2010-06-08 03:01:14 +0000
+++ lib/devscripts/ec2test/remote.py	2010-08-17 17:29:51 +0000
@@ -1,9 +1,21 @@
 #!/usr/bin/env python
-# Run tests in a daemon.
-#
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+"""Run tests in a daemon.
+
+ * `EC2Runner` handles the daemonization and instance shutdown.
+
+ * `Request` knows everything about the test request we're handling (e.g.
+   "test merging foo-bar-bug-12345 into db-devel").
+
+ * `LaunchpadTester` knows how to actually run the tests and gather the
+   results. It uses `SummaryResult` and `FlagFallStream` to do so.
+
+ * `WebTestLogger` knows how to display the results to the user, and is given
+   the responsibility of handling the results that `LaunchpadTester` gathers.
+"""
+
 from __future__ import with_statement
 
 __metatype__ = type
@@ -38,29 +50,32 @@
 class SummaryResult(unittest.TestResult):
     """Test result object used to generate the summary."""
 
-    double_line = '=' * 70 + '\n'
-    single_line = '-' * 70 + '\n'
+    double_line = '=' * 70
+    single_line = '-' * 70
 
     def __init__(self, output_stream):
         super(SummaryResult, self).__init__()
         self.stream = output_stream
 
-    def printError(self, flavor, test, error):
-        """Print an error to the output stream."""
-        self.stream.write(self.double_line)
-        self.stream.write('%s: %s\n' % (flavor, test))
-        self.stream.write(self.single_line)
-        self.stream.write('%s\n' % (error,))
-        self.stream.flush()
+    def _formatError(self, flavor, test, error):
+        return '\n'.join(
+            [self.double_line,
+             '%s: %s' % (flavor, test),
+             self.single_line,
+             error,
+             ''])
 
     def addError(self, test, error):
         super(SummaryResult, self).addError(test, error)
-        self.printError('ERROR', test, self._exc_info_to_string(error, test))
+        self.stream.write(
+            self._formatError(
+                'ERROR', test, self._exc_info_to_string(error, test)))
 
     def addFailure(self, test, error):
         super(SummaryResult, self).addFailure(test, error)
-        self.printError(
-            'FAILURE', test, self._exc_info_to_string(error, test))
+        self.stream.write(
+            self._formatError(
+                'FAILURE', test, self._exc_info_to_string(error, test)))
 
 
 class FlagFallStream:
@@ -93,52 +108,118 @@
         self._stream.flush()
 
 
-class BaseTestRunner:
-
-    def __init__(self, email=None, pqm_message=None, public_branch=None,
-                 public_branch_revno=None, test_options=None):
-        self.email = email
-        self.pqm_message = pqm_message
-        self.public_branch = public_branch
-        self.public_branch_revno = public_branch_revno
-        self.test_options = test_options
-
-        # Configure paths.
-        self.lp_dir = os.path.join(os.path.sep, 'var', 'launchpad')
-        self.tmp_dir = os.path.join(self.lp_dir, 'tmp')
-        self.test_dir = os.path.join(self.lp_dir, 'test')
-        self.sourcecode_dir = os.path.join(self.test_dir, 'sourcecode')
-
-        # Set up logging.
-        self.logger = WebTestLogger(
-            self.test_dir,
-            self.public_branch,
-            self.public_branch_revno,
-            self.sourcecode_dir
-        )
-
-        # Daemonization options.
-        self.pid_filename = os.path.join(self.lp_dir, 'ec2test-remote.pid')
-        self.daemonized = False
-
-    def daemonize(self):
+class EC2Runner:
+    """Runs generic code in an EC2 instance.
+
+    Handles daemonization, instance shutdown, and email in the case of
+    catastrophic failure.
+    """
+
+    # XXX: JonathanLange 2010-08-17: EC2Runner needs tests.
+
+    # The number of seconds we give this script to clean itself up, and for
+    # 'ec2 test --postmortem' to grab control if needed.  If we don't give
+    # --postmortem enough time to log in via SSH and take control, then this
+    # server will begin to shutdown on its own.
+    #
+    # (FWIW, "grab control" means sending SIGTERM to this script's process id,
+    # thus preventing fail-safe shutdown.)
+    SHUTDOWN_DELAY = 60
+
+    def __init__(self, daemonize, pid_filename, shutdown_when_done,
+                 emails=None):
+        """Make an EC2Runner.
+
+        :param daemonize: Whether or not we will daemonize.
+        :param pid_filename: The filename to store the pid in.
+        :param shutdown_when_done: Whether or not to shut down when the tests
+            are done.
+        :param emails: The email address(es) to send catastrophic failure
+            messages to. If not provided, the error disappears into the ether.
+        """
+        self._should_daemonize = daemonize
+        self._pid_filename = pid_filename
+        self._shutdown_when_done = shutdown_when_done
+        self._emails = emails
+        self._daemonized = False
+
+    def _daemonize(self):
         """Turn the testrunner into a forked daemon process."""
         # This also writes our pidfile to disk to a specific location.  The
         # ec2test.py --postmortem command will look there to find our PID,
         # in order to control this process.
-        daemonize(self.pid_filename)
-        self.daemonized = True
-
-    def remove_pidfile(self):
-        if os.path.exists(self.pid_filename):
-            os.remove(self.pid_filename)
-
-    def ignore_line(self, line):
-        """Return True if the line should be excluded from the summary log.
-
-        Defaults to False.
+        daemonize(self._pid_filename)
+        self._daemonized = True
+
+    def _shutdown_instance(self):
+        """Shut down this EC2 instance."""
+        # Make sure our process is daemonized, and has therefore disconnected
+        # the controlling terminal.  This also disconnects the ec2test.py SSH
+        # connection, thus signalling ec2test.py that it may now try to take
+        # control of the server.
+        if not self._daemonized:
+            # We only want to do this if we haven't already been daemonized.
+            # Nesting daemons is bad.
+            self._daemonize()
+
+        time.sleep(self.SHUTDOWN_DELAY)
+
+        # We'll only get here if --postmortem didn't kill us.  This is our
+        # fail-safe shutdown, in case the user got disconnected or suffered
+        # some other mishap that would prevent them from shutting down this
+        # server on their own.
+        subprocess.call(['sudo', 'shutdown', '-P', 'now'])
+
+    def run(self, name, function, *args, **kwargs):
+        try:
+            if self._should_daemonize:
+                print 'Starting %s daemon...' % (name,)
+                self._daemonize()
+
+            return function(*args, **kwargs)
+        except:
+            config = bzrlib.config.GlobalConfig()
+            # Handle exceptions thrown by the test() or daemonize() methods.
+            if self._emails:
+                bzrlib.email_message.EmailMessage.send(
+                    config, config.username(),
+                    self._emails, '%s FAILED' % (name,),
+                    traceback.format_exc())
+            raise
+        finally:
+            # When everything is over, if we've been ask to shut down, then
+            # make sure we're daemonized, then shutdown.  Otherwise, if we're
+            # daemonized, just clean up the pidfile.
+            if self._shutdown_when_done:
+                self._shutdown_instance()
+            elif self._daemonized:
+                # It would be nice to clean up after ourselves, since we won't
+                # be shutting down.
+                remove_pidfile(self._pid_filename)
+            else:
+                # We're not a daemon, and we're not shutting down.  The user
+                # most likely started this script manually, from a shell
+                # running on the instance itself.
+                pass
+
+
+class LaunchpadTester:
+    """Runs Launchpad tests and gathers their results in a useful way."""
+
+    # XXX: JonathanLange 2010-08-17: LaunchpadTester needs tests.
+
+    def __init__(self, logger, test_directory, test_options=()):
+        """Construct a TestOnMergeRunner.
+
+        :param logger: The WebTestLogger to log to.
+        :param test_directory: The directory to run the tests in. We expect
+            this directory to have a fully-functional checkout of Launchpad
+            and its dependent branches.
+        :param test_options: A sequence of options to pass to the test runner.
         """
-        return False
+        self._logger = logger
+        self._test_directory = test_directory
+        self._test_options = ' '.join(test_options)
 
     def build_test_command(self):
         """Return the command that will execute the test suite.
@@ -148,7 +229,8 @@
 
         Subclasses must provide their own implementation of this method.
         """
-        raise NotImplementedError
+        command = ['make', 'check', 'TESTOPTS="%s"' % self._test_options]
+        return command
 
     def test(self):
         """Run the tests, log the results.
@@ -157,170 +239,335 @@
         have completed.  If necessary, submits the branch to PQM, and mails
         the user the test results.
         """
-        # We need to open the log files here because earlier calls to
-        # os.fork() may have tried to close them.
-        self.logger.prepare()
-
-        out_file     = self.logger.out_file
-        summary_file = self.logger.summary_file
-        config       = bzrlib.config.GlobalConfig()
+        self._logger.prepare()
 
         call = self.build_test_command()
 
         try:
+            self._logger.write_line("Running %s" % (call,))
             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,))
-
-            summary_file.write(
-                '\n(See the attached file for the complete log)\n')
+                cwd=self._test_directory)
+
+            self._gather_test_output(popen.stdout, self._logger)
+
+            exit_status = popen.wait()
         except:
-            summary_file.write('\n\nERROR IN TESTRUNNER\n\n')
-            traceback.print_exc(file=summary_file)
-            result = 1
+            self._logger.error_in_testrunner(sys.exc_info())
+            exit_status = 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:
-                if self.email is not None:
-                    self.send_email(
-                        result, self.logger.summary_filename,
-                        self.logger.out_filename, config)
-            finally:
-                summary_file.close()
-                # we do this at the end because this is a trigger to
-                # ec2test.py back at home that it is OK to kill the process
-                # and take control itself, if it wants to.
-                self.logger.close_logs()
-
-    def send_email(self, result, summary_filename, out_filename, config):
-        """Send an email summarizing the test results.
-
-        :param result: True for pass, False for failure.
-        :param summary_filename: The path to the file where the summary
-            information lives. This will be the body of the email.
-        :param out_filename: The path to the file where the full output
-            lives. This will be zipped and attached.
-        :param config: A Bazaar configuration object with SMTP details.
+            self._logger.got_result(not exit_status)
+
+    def _gather_test_output(self, input_stream, logger):
+        """Write the testrunner output to the logs."""
+        summary_stream = logger.get_summary_stream()
+        result = SummaryResult(summary_stream)
+        subunit_server = subunit.TestProtocolServer(result, summary_stream)
+        for line in input_stream:
+            subunit_server.lineReceived(line)
+            logger.got_line(line)
+
+
+class Request:
+    """A request to have a branch tested and maybe landed."""
+
+    def __init__(self, branch_url, revno, local_branch_path, sourcecode_path,
+                 emails=None, pqm_message=None):
+        """Construct a `Request`.
+
+        :param branch_url: The public URL to the Launchpad branch we are
+            testing.
+        :param revno: The revision number of the branch we are testing.
+        :param local_branch_path: A local path to the Launchpad branch we are
+            testing.  This must be a branch of Launchpad with a working tree.
+        :param sourcecode_path: A local path to the sourcecode dependencies
+            directory (normally '$local_branch_path/sourcecode'). This must
+            contain up-to-date copies of all of Launchpad's sourcecode
+            dependencies.
+        :param emails: A list of emails to send the results to. If not
+            provided, no emails are sent.
+        :param pqm_message: The message to submit to PQM. If not provided, we
+            don't submit to PQM.
+        """
+        self._branch_url = branch_url
+        self._revno = revno
+        self._local_branch_path = local_branch_path
+        self._sourcecode_path = sourcecode_path
+        self._emails = emails
+        self._pqm_message = pqm_message
+        # Used for figuring out how to send emails.
+        self._bzr_config = bzrlib.config.GlobalConfig()
+
+    def _send_email(self, message):
+        """Actually send 'message'."""
+        conn = bzrlib.smtp_connection.SMTPConnection(self._bzr_config)
+        conn.send_email(message)
+
+    def get_trunk_details(self):
+        """Return (branch_url, revno) for trunk."""
+        branch = bzrlib.branch.Branch.open_containing(
+            self._local_branch_path)[0]
+        return branch.get_parent().encode('utf-8'), branch.revno()
+
+    def get_branch_details(self):
+        """Return (branch_url, revno) for the branch we're merging in.
+
+        If we're not merging in a branch, but instead just testing a trunk,
+        then return None.
+        """
+        tree = bzrlib.workingtree.WorkingTree.open(self._local_branch_path)
+        parent_ids = tree.get_parent_ids()
+        if len(parent_ids) < 2:
+            return None
+        return self._branch_url.encode('utf-8'), self._revno
+
+    def _last_segment(self, url):
+        """Return the last segment of a URL."""
+        return url.strip('/').split('/')[-1]
+
+    def get_nick(self):
+        """Get the nick of the branch we are testing."""
+        details = self.get_branch_details()
+        if not details:
+            details = self.get_trunk_details()
+        url, revno = details
+        return self._last_segment(url)
+
+    def get_merge_description(self):
+        """Get a description of the merge request.
+
+        If we're merging a branch, return '$SOURCE_NICK => $TARGET_NICK', if
+        we're just running tests for a trunk branch without merging return
+        '$TRUNK_NICK'.
+        """
+        # XXX: JonathanLange 2010-08-17: Not actually used yet. I think it
+        # would be a great thing to have in the subject of the emails we
+        # receive.
+        source = self.get_branch_details()
+        if not source:
+            return self.get_nick()
+        target = self.get_trunk_details()
+        return '%s => %s' % (
+            self._last_segment(source[0]), self._last_segment(target[0]))
+
+    def get_summary_commit(self):
+        """Get a message summarizing the change from the commit log.
+
+        Returns the last commit message of the merged branch, or None.
+        """
+        # XXX: JonathanLange 2010-08-17: I don't actually know why we are
+        # using this commit message as a summary message. It's used in the
+        # test logs and the EC2 hosted web page.
+        branch = bzrlib.branch.Branch.open_containing(
+            self._local_branch_path)[0]
+        tree = bzrlib.workingtree.WorkingTree.open(self._local_branch_path)
+        parent_ids = tree.get_parent_ids()
+        if len(parent_ids) == 1:
+            return None
+        summary = (
+            branch.repository.get_revision(parent_ids[1]).get_summary())
+        return summary.encode('utf-8')
+
+    def _build_report_email(self, successful, body_text, full_log_gz):
+        """Build a MIME email summarizing the test results.
+
+        :param successful: True for pass, False for failure.
+        :param body_text: The body of the email to send to the requesters.
+        :param full_log_gz: A gzip of the full log.
         """
         message = MIMEMultipart.MIMEMultipart()
-        message['To'] = ', '.join(self.email)
-        message['From'] = config.username()
-        subject = 'Test results: %s' % (result and 'FAILURE' or 'SUCCESS')
+        message['To'] = ', '.join(self._emails)
+        message['From'] = self._bzr_config.username()
+        if successful:
+            status = 'SUCCESS'
+        else:
+            status = 'FAILURE'
+        subject = 'Test results: %s' % (status,)
         message['Subject'] = subject
 
         # Make the body.
-        with open(summary_filename, 'r') as summary_fd:
-            summary = summary_fd.read()
-        body = MIMEText.MIMEText(summary, 'plain', 'utf8')
+        body = MIMEText.MIMEText(body_text, 'plain', 'utf8')
         body['Content-Disposition'] = 'inline'
         message.attach(body)
 
-        # gzip up the full log.
-        fd, path = tempfile.mkstemp()
-        os.close(fd)
-        gz = gzip.open(path, 'wb')
-        full_log = open(out_filename, 'rb')
-        gz.writelines(full_log)
-        gz.close()
-
         # Attach the gzipped log.
-        zipped_log = MIMEApplication(open(path, 'rb').read(), 'x-gzip')
+        zipped_log = MIMEApplication(full_log_gz, 'x-gzip')
         zipped_log.add_header(
             'Content-Disposition', 'attachment',
             filename='%s.log.gz' % self.get_nick())
         message.attach(zipped_log)
-
-        bzrlib.smtp_connection.SMTPConnection(config).send_email(message)
-
-    def get_nick(self):
-        """Return the nick name of the branch that we are testing."""
-        return self.public_branch.strip('/').split('/')[-1]
-
-    def _gather_test_output(self, input_stream, summary_file, out_file):
-        """Write the testrunner output to the logs."""
-        # Only write to stdout if we are running as the foreground process.
-        echo_to_stdout = not self.daemonized
-        result = SummaryResult(summary_file)
-        subunit_server = subunit.TestProtocolServer(result, summary_file)
-        for line in input_stream:
-            subunit_server.lineReceived(line)
-            out_file.write(line)
-            out_file.flush()
-            if echo_to_stdout:
-                sys.stdout.write(line)
-                sys.stdout.flush()
-
-
-class TestOnMergeRunner(BaseTestRunner):
-    """Executes the Launchpad test_on_merge.py test suite."""
-
-    def build_test_command(self):
-        """See BaseTestRunner.build_test_command()."""
-        command = ['make', 'check', 'TESTOPTS=' + self.test_options]
-        return command
+        return message
+
+    def send_report_email(self, successful, body_text, full_log_gz):
+        """Send an email summarizing the test results.
+
+        :param successful: True for pass, False for failure.
+        :param body_text: The body of the email to send to the requesters.
+        :param full_log_gz: A gzip of the full log.
+        """
+        message = self._build_report_email(successful, body_text, full_log_gz)
+        self._send_email(message)
+
+    def iter_dependency_branches(self):
+        """Iterate through the Bazaar branches we depend on."""
+        for name in sorted(os.listdir(self._sourcecode_path)):
+            path = os.path.join(self._sourcecode_path, name)
+            if os.path.isdir(path):
+                try:
+                    branch = bzrlib.branch.Branch.open(path)
+                except bzrlib.errors.NotBranchError:
+                    continue
+                yield name, branch.get_parent(), branch.revno()
+
+    def submit_to_pqm(self, successful):
+        """Submit this request to PQM, if successful & configured to do so."""
+        if not self._pqm_message:
+            return
+        subject = self._pqm_message.get('Subject')
+        if successful:
+            self._send_email(self._pqm_message)
+        return subject
+
+    @property
+    def wants_email(self):
+        """Do the requesters want emails sent to them?"""
+        return bool(self._emails)
 
 
 class WebTestLogger:
     """Logs test output to disk and a simple web page."""
 
-    def __init__(self, test_dir, public_branch, public_branch_revno,
-                 sourcecode_dir):
-        """ Class initialiser """
-        self.test_dir = test_dir
-        self.public_branch = public_branch
-        self.public_branch_revno = public_branch_revno
-        self.sourcecode_dir = sourcecode_dir
-
-        self.www_dir = os.path.join(os.path.sep, 'var', 'www')
-        self.out_filename = os.path.join(self.www_dir, 'current_test.log')
-        self.summary_filename = os.path.join(self.www_dir, 'summary.log')
-        self.index_filename = os.path.join(self.www_dir, 'index.html')
-
-        # We will set up the preliminary bits of the web-accessible log
-        # files. "out" holds all stdout and stderr; "summary" holds filtered
-        # output; and "index" holds an index page.
-        self.out_file = None
-        self.summary_file = None
-        self.index_file = None
-
-    def open_logs(self):
-        """Open all of our log files for writing."""
-        self.out_file = open(self.out_filename, 'w')
-        self.summary_file = open(self.summary_filename, 'w')
-        self.index_file = open(self.index_filename, 'w')
-
-    def flush_logs(self):
+    def __init__(self, full_log_file, summary_file, index_file, request,
+                 echo_to_stdout):
+        """Construct a WebTestLogger.
+
+        Because this writes an HTML file with links to the summary and full
+        logs, you should construct this object with
+        `WebTestLogger.make_in_directory`, which will guarantee that the files
+        are available in the correct locations.
+
+        :param full_log_file: A file-like object that will have the full log
+            output written to it.
+        :param summary_file: A file-like object that will have a
+            human-readable summary written to it.
+        :param index_file: A file-like object that will have an HTML page
+            written to it.
+        :param request: A `Request` object representing the thing that's being
+            tested.
+        :param echo_to_stdout: Whether or not we should echo output to stdout.
+        """
+        self._full_log_file = full_log_file
+        self._summary_file = summary_file
+        self._index_file = index_file
+        self._request = request
+        self._echo_to_stdout = echo_to_stdout
+
+    @classmethod
+    def make_in_directory(cls, www_dir, request, echo_to_stdout):
+        """Make a logger that logs to specific files in `www_dir`.
+
+        :param www_dir: The directory in which to log the files:
+            current_test.log, summary.log and index.html. These files
+            will be overwritten.
+        :param request: A `Request` object representing the thing that's being
+            tested.
+        :param echo_to_stdout: Whether or not we should echo output to stdout.
+        """
+        files = [
+            open(filename, 'w+') for filename in [
+                os.path.join(www_dir, 'current_test.log'),
+                os.path.join(www_dir, 'summary.log'),
+                os.path.join(www_dir, 'index.html')]]
+        files.extend([request, echo_to_stdout])
+        return cls(*files)
+
+    def error_in_testrunner(self, exc_info):
+        """Called when there is a catastrophic error in the test runner."""
+        exc_type, exc_value, exc_tb = exc_info
+        # XXX: JonathanLange 2010-08-17: This should probably log to the full
+        # log as well.
+        self._summary_file.write('\n\nERROR IN TESTRUNNER\n\n')
+        traceback.print_exception(
+            exc_type, exc_value, exc_tb, file=self._summary_file)
+
+    def _flush_logs(self):
         """Flush all of our log file buffers."""
-        self.out_file.flush()
-        self.summary_file.flush()
-        self.index_file.flush()
+        self._full_log_file.flush()
+        self._summary_file.flush()
+        self._index_file.flush()
 
-    def close_logs(self):
+    def _close_logs(self):
         """Closes all of the open log file handles."""
-        self.out_file.close()
-        self.summary_file.close()
-        self.index_file.close()
+        self._full_log_file.close()
+        self._summary_file.close()
+        self._index_file.close()
+
+    def get_index_contents(self):
+        """Return the contents of the index.html page."""
+        return self._get_contents(self._index_file)
+
+    def get_full_log_contents(self):
+        """Return the contents of the complete log."""
+        return self._get_contents(self._full_log_file)
+
+    def get_summary_contents(self):
+        """Return the contents of the summary log."""
+        return self._get_contents(self._summary_file)
+
+    def get_summary_stream(self):
+        """Return a stream that, when written to, writes to the summary."""
+        return self._summary_file
+
+    def got_line(self, line):
+        """Called when we get a line of output from our child processes."""
+        self._full_log_file.write(line)
+        self._full_log_file.flush()
+        if self._echo_to_stdout:
+            sys.stdout.write(line)
+            sys.stdout.flush()
+
+    def _get_contents(self, stream):
+        """Get the full contents of 'stream'."""
+        current_position = stream.tell()
+        stream.seek(0)
+        contents = stream.read()
+        stream.seek(current_position)
+        return contents
+
+    def got_result(self, successful):
+        """The tests are done and the results are known."""
+        self._handle_pqm_submission(successful)
+        if self._request.wants_email:
+            self._summary_file.write(
+                '\n(See the attached file for the complete log)\n')
+            summary = self.get_summary_contents()
+            full_log_gz = gzip_data(self.get_full_log_contents())
+            self._request.send_report_email(successful, summary, full_log_gz)
+        self._close_logs()
+
+    def _handle_pqm_submission(self, successful):
+        subject = self._request.submit_to_pqm(successful)
+        if not subject:
+            return
+        self.write_line('')
+        self.write_line('')
+        if successful:
+            self.write_line('SUBMITTED TO PQM:')
+        else:
+            self.write_line('**NOT** submitted to PQM:')
+        self.write_line(subject)
+
+    def _write(self, msg):
+        """Write to the summary and full log file."""
+        for fd in [self._full_log_file, self._summary_file]:
+            fd.write(msg)
+            fd.flush()
+
+    def write_line(self, msg):
+        """Write to the summary and full log file with a newline."""
+        self._write(msg + '\n')
 
     def prepare(self):
         """Prepares the log files on disk.
@@ -328,20 +575,11 @@
         Writes three log files: the raw output log, the filtered "summary"
         log file, and a HTML index page summarizing the test run paramters.
         """
-        self.open_logs()
-
-        out_file     = self.out_file
-        summary_file = self.summary_file
-        index_file   = self.index_file
-
-        def write(msg):
-            msg += '\n'
-            summary_file.write(msg)
-            out_file.write(msg)
+        # XXX: JonathanLange 2010-07-18: Mostly untested.
         msg = 'Tests started at approximately %(now)s UTC' % {
             'now': datetime.datetime.utcnow().strftime(
                 '%a, %d %b %Y %H:%M:%S')}
-        index_file.write(textwrap.dedent('''\
+        self._index_file.write(textwrap.dedent('''\
             <html>
               <head>
                 <title>Testing</title>
@@ -354,70 +592,57 @@
                   <li><a href="current_test.log">Full results</a></li>
                 </ul>
             ''' % (msg,)))
-        write(msg)
+        self.write_line(msg)
 
-        index_file.write(textwrap.dedent('''\
+        self._index_file.write(textwrap.dedent('''\
             <h2>Branches Tested</h2>
             '''))
 
         # Describe the trunk branch.
-        branch = bzrlib.branch.Branch.open_containing(self.test_dir)[0]
-        msg = '%(trunk)s, revision %(trunk_revno)d\n' % {
-            'trunk': branch.get_parent().encode('utf-8'),
-            'trunk_revno': branch.revno()}
-        index_file.write(textwrap.dedent('''\
+        trunk, trunk_revno = self._request.get_trunk_details()
+        msg = '%s, revision %d\n' % (trunk, trunk_revno)
+        self._index_file.write(textwrap.dedent('''\
             <p><strong>%s</strong></p>
             ''' % (escape(msg),)))
-        write(msg)
-        tree = bzrlib.workingtree.WorkingTree.open(self.test_dir)
-        parent_ids = tree.get_parent_ids()
+        self.write_line(msg)
 
-        # Describe the merged branch.
-        if len(parent_ids) == 1:
-            index_file.write('<p>(no merged branch)</p>\n')
-            write('(no merged branch)')
+        branch_details = self._request.get_branch_details()
+        if not branch_details:
+            self._index_file.write('<p>(no merged branch)</p>\n')
+            self.write_line('(no merged branch)')
         else:
-            summary = (
-                branch.repository.get_revision(parent_ids[1]).get_summary())
-            data = {'name': self.public_branch.encode('utf-8'),
-                    'revno': self.public_branch_revno,
-                    'commit': summary.encode('utf-8')}
+            branch_name, branch_revno = branch_details
+            data = {'name': branch_name,
+                    'revno': branch_revno,
+                    'commit': self._request.get_summary_commit()}
             msg = ('%(name)s, revision %(revno)d '
                    '(commit message: %(commit)s)\n' % data)
-            index_file.write(textwrap.dedent('''\
+            self._index_file.write(textwrap.dedent('''\
                <p>Merged with<br />%(msg)s</p>
                ''' % {'msg': escape(msg)}))
-            write("Merged with")
-            write(msg)
+            self.write_line("Merged with")
+            self.write_line(msg)
 
-        index_file.write('<dl>\n')
-        write('\nDEPENDENCY BRANCHES USED\n')
-        for name in os.listdir(self.sourcecode_dir):
-            path = os.path.join(self.sourcecode_dir, name)
-            if os.path.isdir(path):
-                try:
-                    branch = bzrlib.branch.Branch.open_containing(path)[0]
-                except bzrlib.errors.NotBranchError:
-                    continue
-                data = {'name': name,
-                        'branch': branch.get_parent(),
-                        'revno': branch.revno()}
-                write(
-                    '- %(name)s\n    %(branch)s\n    %(revno)d\n' % data)
-                escaped_data = {'name': escape(name),
-                                'branch': escape(branch.get_parent()),
-                                'revno': branch.revno()}
-                index_file.write(textwrap.dedent('''\
-                    <dt>%(name)s</dt>
-                      <dd>%(branch)s</dd>
-                      <dd>%(revno)s</dd>
-                    ''' % escaped_data))
-        index_file.write(textwrap.dedent('''\
+        self._index_file.write('<dl>\n')
+        self.write_line('\nDEPENDENCY BRANCHES USED\n')
+        for name, branch, revno in self._request.iter_dependency_branches():
+            data = {'name': name, 'branch': branch, 'revno': revno}
+            self.write_line(
+                '- %(name)s\n    %(branch)s\n    %(revno)d\n' % data)
+            escaped_data = {'name': escape(name),
+                            'branch': escape(branch),
+                            'revno': revno}
+            self._index_file.write(textwrap.dedent('''\
+                <dt>%(name)s</dt>
+                  <dd>%(branch)s</dd>
+                  <dd>%(revno)s</dd>
+                ''' % escaped_data))
+        self._index_file.write(textwrap.dedent('''\
                 </dl>
               </body>
             </html>'''))
-        write('\n\nTEST RESULTS FOLLOW\n\n')
-        self.flush_logs()
+        self.write_line('\n\nTEST RESULTS FOLLOW\n\n')
+        self._flush_logs()
 
 
 def daemonize(pid_filename):
@@ -454,6 +679,38 @@
     os.dup2(0, 2)
 
 
+def gunzip_data(data):
+    """Decompress 'data'.
+
+    :param data: The gzip data to decompress.
+    :return: The decompressed data.
+    """
+    fd, path = tempfile.mkstemp()
+    os.write(fd, data)
+    os.close(fd)
+    try:
+        return gzip.open(path, 'r').read()
+    finally:
+        os.unlink(path)
+
+
+def gzip_data(data):
+    """Compress 'data'.
+
+    :param data: The data to compress.
+    :return: The gzip-compressed data.
+    """
+    fd, path = tempfile.mkstemp()
+    os.close(fd)
+    gz = gzip.open(path, 'wb')
+    gz.writelines(data)
+    gz.close()
+    try:
+        return open(path).read()
+    finally:
+        os.unlink(path)
+
+
 def write_pidfile(pid_filename):
     """Write a pidfile for the current process."""
     pid_file = open(pid_filename, "w")
@@ -461,7 +718,14 @@
     pid_file.close()
 
 
-if __name__ == '__main__':
+def remove_pidfile(pid_filename):
+    if os.path.exists(pid_filename):
+        os.remove(pid_filename)
+
+
+def parse_options(argv):
+    """Make an `optparse.OptionParser` for running the tests remotely.
+    """
     parser = optparse.OptionParser(
         usage="%prog [options] [-- test options]",
         description=("Build and run tests for an instance."))
@@ -494,7 +758,11 @@
         type="int", default=None,
         help=('The revision number of the public branch being tested.'))
 
-    options, args = parser.parse_args()
+    return parser.parse_args(argv)
+
+
+def main(argv):
+    options, args = parse_options(argv)
 
     if options.debug:
         import pdb; pdb.set_trace()
@@ -504,65 +772,28 @@
     else:
         pqm_message = None
 
-    runner = TestOnMergeRunner(
-       options.email,
-       pqm_message,
-       options.public_branch,
-       options.public_branch_revno,
-       ' '.join(args))
-
-    try:
-        try:
-            if options.daemon:
-                print 'Starting testrunner daemon...'
-                runner.daemonize()
-
-            runner.test()
-        except:
-            config = bzrlib.config.GlobalConfig()
-            # Handle exceptions thrown by the test() or daemonize() methods.
-            if options.email:
-                bzrlib.email_message.EmailMessage.send(
-                    config, config.username(),
-                    options.email,
-                    'Test Runner FAILED', traceback.format_exc())
-            raise
-    finally:
-
-        # When everything is over, if we've been ask to shut down, then
-        # make sure we're daemonized, then shutdown.  Otherwise, if we're
-        # daemonized, just clean up the pidfile.
-        if options.shutdown:
-            # Make sure our process is daemonized, and has therefore
-            # disconnected the controlling terminal.  This also disconnects
-            # the ec2test.py SSH connection, thus signalling ec2test.py
-            # that it may now try to take control of the server.
-            if not runner.daemonized:
-                # We only want to do this if we haven't already been
-                # daemonized.  Nesting daemons is bad.
-                runner.daemonize()
-
-            # Give the script 60 seconds to clean itself up, and 60 seconds
-            # for the ec2test.py --postmortem option to grab control if
-            # needed.  If we don't give --postmortem enough time to log
-            # in via SSH and take control, then this server will begin to
-            # shutdown on it's own.
-            #
-            # (FWIW, "grab control" means sending SIGTERM to this script's
-            # process id, thus preventing fail-safe shutdown.)
-            time.sleep(60)
-
-            # We'll only get here if --postmortem didn't kill us.  This is
-            # our fail-safe shutdown, in case the user got disconnected
-            # or suffered some other mishap that would prevent them from
-            # shutting down this server on their own.
-            subprocess.call(['sudo', 'shutdown', '-P', 'now'])
-        elif runner.daemonized:
-            # It would be nice to clean up after ourselves, since we won't
-            # be shutting down.
-            runner.remove_pidfile()
-        else:
-            # We're not a daemon, and we're not shutting down.  The user most
-            # likely started this script manually, from a shell running on the
-            # instance itself.
-            pass
+    # Locations for Launchpad. These are actually defined by the configuration
+    # of the EC2 image that we use.
+    LAUNCHPAD_DIR = '/var/launchpad'
+    TEST_DIR = os.path.join(LAUNCHPAD_DIR, 'test')
+    SOURCECODE_DIR = os.path.join(TEST_DIR, 'sourcecode')
+
+    pid_filename = os.path.join(LAUNCHPAD_DIR, 'ec2test-remote.pid')
+
+    request = Request(
+        options.public_branch, options.public_branch_revno, TEST_DIR,
+        SOURCECODE_DIR, options.email, pqm_message)
+    # Only write to stdout if we are running as the foreground process.
+    echo_to_stdout = not options.daemon
+    logger = WebTestLogger.make_in_directory(
+        '/var/www', request, echo_to_stdout)
+
+    runner = EC2Runner(
+        options.daemon, pid_filename, options.shutdown, options.email)
+
+    tester = LaunchpadTester(logger, TEST_DIR, test_options=args[1:])
+    runner.run("Test runner", tester.test)
+
+
+if __name__ == '__main__':
+    main(sys.argv)

=== modified file 'lib/devscripts/ec2test/testrunner.py'
--- lib/devscripts/ec2test/testrunner.py	2010-08-13 22:04:58 +0000
+++ lib/devscripts/ec2test/testrunner.py	2010-08-17 17:29:51 +0000
@@ -454,11 +454,8 @@
         # close ssh connection
         user_connection.close()
 
-    def run_tests(self):
-        self.configure_system()
-        self.prepare_tests()
-        user_connection = self._instance.connect()
-
+    def _build_command(self):
+        """Build the command that we'll use to run the tests."""
         # Make sure we activate the failsafe --shutdown feature.  This will
         # make the server shut itself down after the test run completes, or
         # if the test harness suffers a critical failure.
@@ -496,6 +493,20 @@
 
         # Add any additional options for ec2test-remote.py
         cmd.extend(['--', self.test_options])
+        return ' '.join(cmd)
+
+    def dont_run_tests(self):
+        """Reach the point where we are able to run tests and stop there."""
+        self.configure_system()
+        self.prepare_tests()
+        cmd = self._build_command()
+        self.log('COMMAND: %s\n' % cmd)
+        self.log('ssh ec2test@%s\n' % (self._instance.hostname,))
+
+    def run_tests(self):
+        self.configure_system()
+        self.prepare_tests()
+
         self.log(
             'Running tests... (output is available on '
             'http://%s/)\n' % self._instance.hostname)
@@ -513,7 +524,9 @@
 
         # Run the remote script!  Our execution will block here until the
         # remote side disconnects from the terminal.
-        user_connection.perform(' '.join(cmd))
+        cmd = self._build_command()
+        user_connection = self._instance.connect()
+        user_connection.perform(cmd)
         self._running = True
 
         if not self.headless:

=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
--- lib/devscripts/ec2test/tests/test_remote.py	2010-04-29 09:23:08 +0000
+++ lib/devscripts/ec2test/tests/test_remote.py	2010-08-17 17:29:51 +0000
@@ -5,14 +5,68 @@
 
 __metaclass__ = type
 
+from email.mime.application import MIMEApplication
+from email.mime.text import MIMEText
+import gzip
+from itertools import izip
+import os
 from StringIO import StringIO
 import sys
+import tempfile
+import traceback
 import unittest
 
-from devscripts.ec2test.remote import SummaryResult
-
-
-class TestSummaryResult(unittest.TestCase):
+from bzrlib.config import GlobalConfig
+from bzrlib.tests import TestCaseWithTransport
+
+from testtools import TestCase
+
+from devscripts.ec2test.remote import (
+    FlagFallStream,
+    gunzip_data,
+    gzip_data,
+    remove_pidfile,
+    Request,
+    SummaryResult,
+    WebTestLogger,
+    write_pidfile,
+    )
+
+
+class TestFlagFallStream(TestCase):
+    """Tests for `FlagFallStream`."""
+
+    def test_doesnt_write_before_flag(self):
+        # A FlagFallStream does not forward any writes before it sees the
+        # 'flag'.
+        stream = StringIO()
+        flag = self.getUniqueString('flag')
+        flagfall = FlagFallStream(stream, flag)
+        flagfall.write('foo')
+        flagfall.flush()
+        self.assertEqual('', stream.getvalue())
+
+    def test_writes_after_flag(self):
+        # After a FlagFallStream sees the flag, it forwards all writes.
+        stream = StringIO()
+        flag = self.getUniqueString('flag')
+        flagfall = FlagFallStream(stream, flag)
+        flagfall.write('foo')
+        flagfall.write(flag)
+        flagfall.write('bar')
+        self.assertEqual('%sbar' % (flag,), stream.getvalue())
+
+    def test_mixed_write(self):
+        # If a single call to write has pre-flagfall and post-flagfall data in
+        # it, then only the post-flagfall data is forwarded to the stream.
+        stream = StringIO()
+        flag = self.getUniqueString('flag')
+        flagfall = FlagFallStream(stream, flag)
+        flagfall.write('foo%sbar' % (flag,))
+        self.assertEqual('%sbar' % (flag,), stream.getvalue())
+
+
+class TestSummaryResult(TestCase):
     """Tests for `SummaryResult`."""
 
     def makeException(self, factory=None, *args, **kwargs):
@@ -23,50 +77,506 @@
         except:
             return sys.exc_info()
 
-    def test_printError(self):
-        # SummaryResult.printError() prints out the name of the test, the kind
+    def test_formatError(self):
+        # SummaryResult._formatError() combines the name of the test, the kind
         # of error and the details of the error in a nicely-formatted way.
-        stream = StringIO()
-        result = SummaryResult(stream)
-        result.printError('FOO', 'test', 'error')
-        expected = '%sFOO: test\n%serror\n' % (
+        result = SummaryResult(None)
+        output = result._formatError('FOO', 'test', 'error')
+        expected = '%s\nFOO: test\n%s\nerror\n' % (
             result.double_line, result.single_line)
-        self.assertEqual(expected, stream.getvalue())
+        self.assertEqual(expected, output)
 
     def test_addError(self):
-        # SummaryResult.addError() prints a nicely-formatted error.
-        #
-        # First, use printError to build the error text we expect.
+        # SummaryResult.addError doesn't write immediately.
+        stream = StringIO()
         test = self
-        stream = StringIO()
+        error = self.makeException()
         result = SummaryResult(stream)
-        error = self.makeException()
-        result.printError(
+        expected = result._formatError(
             'ERROR', test, result._exc_info_to_string(error, test))
-        expected = stream.getvalue()
-        # Now, call addError and check that it matches.
-        stream = StringIO()
-        result = SummaryResult(stream)
         result.addError(test, error)
         self.assertEqual(expected, stream.getvalue())
 
-    def test_addFailure(self):
-        # SummaryResult.addFailure() prints a nicely-formatted error.
-        #
-        # First, use printError to build the error text we expect.
+    def test_addFailure_does_not_write_immediately(self):
+        # SummaryResult.addFailure doesn't write immediately.
+        stream = StringIO()
         test = self
-        stream = StringIO()
+        error = self.makeException()
         result = SummaryResult(stream)
-        error = self.makeException(test.failureException)
-        result.printError(
+        expected = result._formatError(
             'FAILURE', test, result._exc_info_to_string(error, test))
-        expected = stream.getvalue()
-        # Now, call addFailure and check that it matches.
-        stream = StringIO()
-        result = SummaryResult(stream)
         result.addFailure(test, error)
         self.assertEqual(expected, stream.getvalue())
 
 
+class TestPidfileHelpers(TestCase):
+    """Tests for `write_pidfile` and `remove_pidfile`."""
+
+    def test_write_pidfile(self):
+        fd, path = tempfile.mkstemp()
+        self.addCleanup(os.unlink, path)
+        os.close(fd)
+        write_pidfile(path)
+        self.assertEqual(os.getpid(), int(open(path, 'r').read()))
+
+    def test_remove_pidfile(self):
+        fd, path = tempfile.mkstemp()
+        os.close(fd)
+        write_pidfile(path)
+        remove_pidfile(path)
+        self.assertEqual(False, os.path.exists(path))
+
+    def test_remove_nonexistent_pidfile(self):
+        directory = tempfile.mkdtemp()
+        path = os.path.join(directory, 'doesntexist')
+        remove_pidfile(path)
+        self.assertEqual(False, os.path.exists(path))
+
+
+class TestGzip(TestCase):
+    """Tests for gzip helpers."""
+
+    def test_gzip_data(self):
+        data = 'foobarbaz\n'
+        compressed = gzip_data(data)
+        fd, path = tempfile.mkstemp()
+        os.write(fd, compressed)
+        os.close(fd)
+        self.assertEqual(data, gzip.open(path, 'r').read())
+
+    def test_gunzip_data(self):
+        data = 'foobarbaz\n'
+        compressed = gzip_data(data)
+        self.assertEqual(data, gunzip_data(compressed))
+
+
+class RequestHelpers:
+
+    def make_trunk(self, parent_url='http://example.com/bzr/trunk'):
+        """Make a trunk branch suitable for use with `Request`.
+
+        `Request` expects to be given a path to a working tree that has a
+        branch with a configured parent URL, so this helper returns such a
+        working tree.
+        """
+        nick = parent_url.strip('/').split('/')[-1]
+        tree = self.make_branch_and_tree(nick)
+        tree.branch.set_parent(parent_url)
+        return tree
+
+    def make_request(self, branch_url=None, revno=None,
+                     trunk=None, sourcecode_path=None,
+                     emails=None, pqm_message=None):
+        """Make a request to test, specifying only things we care about.
+
+        Note that the returned request object will not ever send email, but
+        will instead log "sent" emails to `request.emails_sent`.
+        """
+        if trunk is None:
+            trunk = self.make_trunk()
+        if sourcecode_path is None:
+            sourcecode_path = self.make_sourcecode(
+                [('a', 'http://example.com/bzr/a', 2),
+                 ('b', 'http://example.com/bzr/b', 3),
+                 ('c', 'http://example.com/bzr/c', 5)])
+        request = Request(
+            branch_url, revno, trunk.basedir, sourcecode_path, emails,
+            pqm_message)
+        request.emails_sent = []
+        request._send_email = request.emails_sent.append
+        return request
+
+    def make_sourcecode(self, branches):
+        """Make a sourcecode directory with sample branches.
+
+        :param branches: A list of (name, parent_url, revno) tuples.
+        :return: The path to the sourcecode directory.
+        """
+        self.build_tree(['sourcecode/'])
+        for name, parent_url, revno in branches:
+            tree = self.make_branch_and_tree('sourcecode/%s' % (name,))
+            tree.branch.set_parent(parent_url)
+            for i in range(revno):
+                tree.commit(message=str(i))
+        return 'sourcecode/'
+
+    def make_logger(self, request=None, echo_to_stdout=False):
+        full_log = StringIO()
+        summary = StringIO()
+        index = StringIO()
+        if request is None:
+            request = self.make_request()
+        return WebTestLogger(
+            full_log, summary, index, request, echo_to_stdout)
+
+
+class TestRequest(TestCaseWithTransport, RequestHelpers):
+    """Tests for `Request`."""
+
+    def test_doesnt_want_email(self):
+        # If no email addresses were provided, then the user does not want to
+        # receive email.
+        req = self.make_request()
+        self.assertEqual(False, req.wants_email)
+
+    def test_wants_email(self):
+        # If some email addresses were provided, then the user wants to
+        # receive email.
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        self.assertEqual(True, req.wants_email)
+
+    def test_get_trunk_details(self):
+        parent = 'http://example.com/bzr/branch'
+        tree = self.make_trunk(parent)
+        req = self.make_request(trunk=tree)
+        self.assertEqual(
+            (parent, tree.branch.revno()), req.get_trunk_details())
+
+    def test_get_branch_details_no_commits(self):
+        req = self.make_request(trunk=self.make_trunk())
+        self.assertEqual(None, req.get_branch_details())
+
+    def test_get_branch_details_no_merge(self):
+        tree = self.make_trunk()
+        tree.commit(message='foo')
+        req = self.make_request(trunk=tree)
+        self.assertEqual(None, req.get_branch_details())
+
+    def test_get_branch_details_merge(self):
+        tree = self.make_trunk()
+        # Fake a merge, giving silly revision ids.
+        tree.add_pending_merge('foo', 'bar')
+        req = self.make_request(
+            branch_url='https://example.com/bzr/thing', revno=42, trunk=tree)
+        self.assertEqual(
+            ('https://example.com/bzr/thing', 42), req.get_branch_details())
+
+    def test_get_nick_trunk_only(self):
+        tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel')
+        req = self.make_request(trunk=tree)
+        self.assertEqual('db-devel', req.get_nick())
+
+    def test_get_nick_merge(self):
+        tree = self.make_trunk()
+        # Fake a merge, giving silly revision ids.
+        tree.add_pending_merge('foo', 'bar')
+        req = self.make_request(
+            branch_url='https://example.com/bzr/thing', revno=42, trunk=tree)
+        self.assertEqual('thing', req.get_nick())
+
+    def test_get_merge_description_trunk_only(self):
+        tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel')
+        req = self.make_request(trunk=tree)
+        self.assertEqual('db-devel', req.get_merge_description())
+
+    def test_get_merge_description_merge(self):
+        tree = self.make_trunk(parent_url='http://example.com/bzr/db-devel/')
+        tree.add_pending_merge('foo', 'bar')
+        req = self.make_request(
+            branch_url='https://example.com/bzr/thing', revno=42, trunk=tree)
+        self.assertEqual('thing => db-devel', req.get_merge_description())
+
+    def test_get_summary_commit(self):
+        # The summary commit message is the last commit message of the branch
+        # we're merging in.
+        trunk = self.make_trunk()
+        trunk.commit(message="a starting point")
+        thing_bzrdir = trunk.branch.bzrdir.sprout('thing')
+        thing = thing_bzrdir.open_workingtree()
+        thing.commit(message="a new thing")
+        trunk.merge_from_branch(thing.branch)
+        req = self.make_request(
+            branch_url='https://example.com/bzr/thing',
+            revno=thing.branch.revno(),
+            trunk=trunk)
+        self.assertEqual("a new thing", req.get_summary_commit())
+
+    def test_iter_dependency_branches(self):
+        # iter_dependency_branches yields a list of branches in the sourcecode
+        # directory, along with their parent URLs and their revnos.
+        sourcecode_branches = [
+            ('b', 'http://example.com/parent-b', 3),
+            ('a', 'http://example.com/parent-a', 2),
+            ('c', 'http://example.com/parent-c', 5),
+            ]
+        sourcecode_path = self.make_sourcecode(sourcecode_branches)
+        self.build_tree(
+            ['%s/not-a-branch/' % sourcecode_path,
+             '%s/just-a-file' % sourcecode_path])
+        req = self.make_request(sourcecode_path=sourcecode_path)
+        branches = list(req.iter_dependency_branches())
+        self.assertEqual(sorted(sourcecode_branches), branches)
+
+    def test_submit_to_pqm_no_message(self):
+        # If there's no PQM message, then 'submit_to_pqm' returns None.
+        req = self.make_request(pqm_message=None)
+        subject = req.submit_to_pqm(successful=True)
+        self.assertIs(None, subject)
+
+    def test_submit_to_pqm_no_message_doesnt_send(self):
+        # If there's no PQM message, then 'submit_to_pqm' returns None.
+        req = self.make_request(pqm_message=None)
+        req.submit_to_pqm(successful=True)
+        self.assertEqual([], req.emails_sent)
+
+    def test_submit_to_pqm_unsuccessful(self):
+        # submit_to_pqm returns the subject of the PQM mail even if it's
+        # handling a failed test run.
+        message = {'Subject:': 'My PQM message'}
+        req = self.make_request(pqm_message=message)
+        subject = req.submit_to_pqm(successful=False)
+        self.assertIs(message.get('Subject'), subject)
+
+    def test_submit_to_pqm_unsuccessful_no_email(self):
+        # submit_to_pqm doesn't send any email if the run was unsuccessful.
+        message = {'Subject:': 'My PQM message'}
+        req = self.make_request(pqm_message=message)
+        req.submit_to_pqm(successful=False)
+        self.assertEqual([], req.emails_sent)
+
+    def test_submit_to_pqm_successful(self):
+        # submit_to_pqm returns the subject of the PQM mail.
+        message = {'Subject:': 'My PQM message'}
+        req = self.make_request(pqm_message=message)
+        subject = req.submit_to_pqm(successful=True)
+        self.assertIs(message.get('Subject'), subject)
+        self.assertEqual([message], req.emails_sent)
+
+    def test_report_email_subject_success(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        email = req._build_report_email(True, 'foo', 'gobbledygook')
+        self.assertEqual('Test results: SUCCESS', email['Subject'])
+
+    def test_report_email_subject_failure(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        email = req._build_report_email(False, 'foo', 'gobbledygook')
+        self.assertEqual('Test results: FAILURE', email['Subject'])
+
+    def test_report_email_recipients(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx', 'bar@xxxxxxxxxxx'])
+        email = req._build_report_email(False, 'foo', 'gobbledygook')
+        self.assertEqual('foo@xxxxxxxxxxx, bar@xxxxxxxxxxx', email['To'])
+
+    def test_report_email_sender(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        email = req._build_report_email(False, 'foo', 'gobbledygook')
+        self.assertEqual(GlobalConfig().username(), email['From'])
+
+    def test_report_email_body(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        email = req._build_report_email(False, 'foo', 'gobbledygook')
+        [body, attachment] = email.get_payload()
+        self.assertIsInstance(body, MIMEText)
+        self.assertEqual('inline', body['Content-Disposition'])
+        self.assertEqual('text/plain; charset="utf-8"', body['Content-Type'])
+        self.assertEqual("foo", body.get_payload())
+
+    def test_report_email_attachment(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        email = req._build_report_email(False, "foo", "gobbledygook")
+        [body, attachment] = email.get_payload()
+        self.assertIsInstance(attachment, MIMEApplication)
+        self.assertEqual('application/x-gzip', attachment['Content-Type'])
+        self.assertEqual(
+            'attachment; filename="%s.log.gz"' % req.get_nick(),
+            attachment['Content-Disposition'])
+        self.assertEqual(
+            "gobbledygook", attachment.get_payload().decode('base64'))
+
+    def test_send_report_email_sends_email(self):
+        req = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        expected = req._build_report_email(False, "foo", "gobbledygook")
+        req.send_report_email(False, "foo", "gobbledygook")
+        [observed] = req.emails_sent
+        # The standard library sucks. None of the MIME objects have __eq__
+        # implementations.
+        for expected_part, observed_part in izip(
+            expected.walk(), observed.walk()):
+            self.assertEqual(type(expected_part), type(observed_part))
+            self.assertEqual(expected_part.items(), observed_part.items())
+            self.assertEqual(
+                expected_part.is_multipart(), observed_part.is_multipart())
+            if not expected_part.is_multipart():
+                self.assertEqual(
+                    expected_part.get_payload(), observed_part.get_payload())
+
+
+class TestWebTestLogger(TestCaseWithTransport, RequestHelpers):
+
+    def patch(self, obj, name, value):
+        orig = getattr(obj, name)
+        setattr(obj, name, value)
+        self.addCleanup(setattr, obj, name, orig)
+        return orig
+
+    def test_make_in_directory(self):
+        # WebTestLogger.make_in_directory constructs a logger that writes to a
+        # bunch of specific files in a directory.
+        self.build_tree(['www/'])
+        request = self.make_request()
+        logger = WebTestLogger.make_in_directory('www', request, False)
+        # A method on logger that writes to _everything_.
+        logger.prepare()
+        self.assertEqual(
+            logger.get_summary_contents(), open('www/summary.log').read())
+        self.assertEqual(
+            logger.get_full_log_contents(),
+            open('www/current_test.log').read())
+        self.assertEqual(
+            logger.get_index_contents(), open('www/index.html').read())
+
+    def test_initial_full_log(self):
+        # Initially, the full log has nothing in it.
+        logger = self.make_logger()
+        self.assertEqual('', logger.get_full_log_contents())
+
+    def test_initial_summary_contents(self):
+        # Initially, the summary log has nothing in it.
+        logger = self.make_logger()
+        self.assertEqual('', logger.get_summary_contents())
+
+    def test_got_line_no_echo(self):
+        # got_line forwards the line to the full log, but does not forward to
+        # stdout if echo_to_stdout is False.
+        stdout = StringIO()
+        self.patch(sys, 'stdout', stdout)
+        logger = self.make_logger(echo_to_stdout=False)
+        logger.got_line("output from script\n")
+        self.assertEqual(
+            "output from script\n", logger.get_full_log_contents())
+        self.assertEqual("", stdout.getvalue())
+
+    def test_got_line_echo(self):
+        # got_line forwards the line to the full log, and to stdout if
+        # echo_to_stdout is True.
+        stdout = StringIO()
+        self.patch(sys, 'stdout', stdout)
+        logger = self.make_logger(echo_to_stdout=True)
+        logger.got_line("output from script\n")
+        self.assertEqual(
+            "output from script\n", logger.get_full_log_contents())
+        self.assertEqual("output from script\n", stdout.getvalue())
+
+    def test_write_line(self):
+        # write_line writes a line to both the full log and the summary log.
+        logger = self.make_logger()
+        logger.write_line('foo')
+        self.assertEqual('foo\n', logger.get_full_log_contents())
+        self.assertEqual('foo\n', logger.get_summary_contents())
+
+    def test_error_in_testrunner(self):
+        # error_in_testrunner logs the traceback to the summary log in a very
+        # prominent way.
+        try:
+            1/0
+        except ZeroDivisionError:
+            exc_info = sys.exc_info()
+        stack = ''.join(traceback.format_exception(*exc_info))
+        logger = self.make_logger()
+        logger.error_in_testrunner(exc_info)
+        self.assertEqual(
+            "\n\nERROR IN TESTRUNNER\n\n%s" % (stack,),
+            logger.get_summary_contents())
+
+
+class TestResultHandling(TestCaseWithTransport, RequestHelpers):
+    """Tests for how we handle the result at the end of the test suite."""
+
+    def get_body_text(self, email):
+        return email.get_payload()[0].get_payload()
+
+    def test_success_no_emails(self):
+        request = self.make_request(emails=[])
+        logger = self.make_logger(request=request)
+        logger.got_result(True)
+        self.assertEqual([], request.emails_sent)
+
+    def test_failure_no_emails(self):
+        request = self.make_request(emails=[])
+        logger = self.make_logger(request=request)
+        logger.got_result(False)
+        self.assertEqual([], request.emails_sent)
+
+    def test_submits_to_pqm_on_success(self):
+        message = {'Subject': 'foo'}
+        request = self.make_request(emails=[], pqm_message=message)
+        logger = self.make_logger(request=request)
+        logger.got_result(True)
+        self.assertEqual([message], request.emails_sent)
+
+    def test_records_pqm_submission_in_email(self):
+        message = {'Subject': 'foo'}
+        request = self.make_request(
+            emails=['foo@xxxxxxxxxxx'], pqm_message=message)
+        logger = self.make_logger(request=request)
+        logger.got_result(True)
+        [pqm_message, user_message] = request.emails_sent
+        self.assertEqual(message, pqm_message)
+        self.assertIn(
+            'SUBMITTED TO PQM:\n%s' % (message['Subject'],),
+            self.get_body_text(user_message))
+
+    def test_doesnt_submit_to_pqm_no_failure(self):
+        message = {'Subject': 'foo'}
+        request = self.make_request(emails=[], pqm_message=message)
+        logger = self.make_logger(request=request)
+        logger.got_result(False)
+        self.assertEqual([], request.emails_sent)
+
+    def test_records_non_pqm_submission_in_email(self):
+        message = {'Subject': 'foo'}
+        request = self.make_request(
+            emails=['foo@xxxxxxxxxxx'], pqm_message=message)
+        logger = self.make_logger(request=request)
+        logger.got_result(False)
+        [user_message] = request.emails_sent
+        self.assertIn(
+            '**NOT** submitted to PQM:\n%s' % (message['Subject'],),
+            self.get_body_text(user_message))
+
+    def test_email_refers_to_attached_log(self):
+        request = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        logger = self.make_logger(request=request)
+        logger.got_result(False)
+        [user_message] = request.emails_sent
+        self.assertIn(
+            '(See the attached file for the complete log)\n',
+            self.get_body_text(user_message))
+
+    def test_email_body_is_summary(self):
+        # The body of the email sent to the user is the summary file.
+        # XXX: JonathanLange 2010-08-17: We actually want to change this
+        # behaviour so that the summary log stays roughly the same as it is
+        # now and can vary independently from the contents of the sent
+        # email. We probably just want the contents of the email to be a list
+        # of failing tests.
+        request = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        logger = self.make_logger(request=request)
+        logger.get_summary_stream().write('bar\nbaz\nqux\n')
+        logger.got_result(False)
+        [user_message] = request.emails_sent
+        self.assertEqual(
+            'bar\nbaz\nqux\n\n(See the attached file for the complete log)\n',
+            self.get_body_text(user_message))
+
+    def test_gzip_of_full_log_attached(self):
+        # The full log is attached to the email.
+        request = self.make_request(emails=['foo@xxxxxxxxxxx'])
+        logger = self.make_logger(request=request)
+        logger.got_line("output from test process\n")
+        logger.got_line("more output\n")
+        logger.got_result(False)
+        [user_message] = request.emails_sent
+        [body, attachment] = user_message.get_payload()
+        self.assertEqual('application/x-gzip', attachment['Content-Type'])
+        self.assertEqual(
+            'attachment; filename="%s.log.gz"' % request.get_nick(),
+            attachment['Content-Disposition'])
+        attachment_contents = attachment.get_payload().decode('base64')
+        uncompressed = gunzip_data(attachment_contents)
+        self.assertEqual(
+            "output from test process\nmore output\n", uncompressed)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)