← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~ursinha/launchpad/auto-approve into lp:launchpad

 

Ursula Junque has proposed merging lp:~ursinha/launchpad/auto-approve into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ursinha/launchpad/auto-approve/+merge/45850

These changes implement ec2 land capability to work with the new Launchpad Tarmac merge workflow.

= Summary =

This branch adds the -T (--use-tarmac) option to ec2 land, so when the tests finish, one "tests" vote is added to the merge proposal, and it's queue status is set to approved or rejected. 
For this to work, I had to add a --mergeproposal-address to ec2 test, considering ec2 land is just a wrapper that generates an ec2 test command line and runs it as a subprocess.

== Tests ==

I've tested the newly created method by adding more tests to test_autoland.py.

./bin/test -vvt devscripts.*

== Demo and Q/A ==

I've tested by running the command in a real merge proposal in Launchpad, using the --dry-run option, to check if the command line is correctly generated by ec2 land.

$ ec2 land <merge_proposal> --force --dry-run --use-tarmac

Should show the command line with the ec2 test --mergeproposal-address I've added so it can generate the signed message to send launchpad after the tests.

-- 
https://code.launchpad.net/~ursinha/launchpad/auto-approve/+merge/45850
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~ursinha/launchpad/auto-approve into lp:launchpad.
=== modified file 'lib/devscripts/autoland.py'
--- lib/devscripts/autoland.py	2011-01-04 20:22:39 +0000
+++ lib/devscripts/autoland.py	2011-01-11 13:08:59 +0000
@@ -6,6 +6,8 @@
     STAGING_SERVICE_ROOT)
 from lazr.uri import URI
 from bzrlib.errors import BzrCommandError
+from bzrlib.email_message import EmailMessage
+from bzrlib.config import GlobalConfig
 
 
 class MissingReviewError(Exception):
@@ -191,6 +193,63 @@
         self._mp.lp_save()
 
 
+class MergeProposalMessage(EmailMessage):
+
+    def __init__(self, mail_to, mail_from, successful=None,
+        source_branch=None, gpg_strategy=None):
+
+        if successful:
+            self.subject = "Test results: SUCCESS"
+            self.body_text = """
+Full test results were mailed to the submitter.
+
+ review approve tests
+ merge approved
+"""
+        else:
+            self.subject = "Test results: FAILURE"
+            self.body_text = """
+Full test results were mailed to the submitter.
+
+ review needsfixing tests
+ merge rejected
+"""
+        self.successful = successful
+        self.source_branch = source_branch
+        self.mail_from = mail_from
+        self.mail_to = mail_to
+        self.gpg_strategy = gpg_strategy
+
+    def to_signed(self, unsigned_text):
+        """Serialize as a signed string."""
+
+        if self.source_branch:
+            config = self.source_branch.get_config()
+        else:
+            config = GlobalConfig()
+        if self.gpg_strategy is None:
+            strategy = GPGStrategy(config)
+        else:
+            strategy = self.gpg_strategy
+        return strategy.sign(unsigned_text)
+
+    def to_email(self, mail_from=None, mail_to=None):
+        """Serialize as an email message.
+
+        :param mail_from: The from address for the message
+        :param mail_to: The address to send the message to
+        :return: an email message
+        """
+        if mail_from is None:
+            mail_from = self.mail_from
+        if mail_to is None:
+            mail_to = self.mail_to
+        body = self.to_signed(self.body_text)
+        message = EmailMessage(mail_from, mail_to, self.subject, body)
+        return message
+
+
+
 def get_testfix_clause(testfix=False):
     """Get the testfix clause."""
     if testfix:

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2010-11-29 21:32:06 +0000
+++ lib/devscripts/ec2test/builtins.py	2011-01-11 13:08:59 +0000
@@ -223,6 +223,12 @@
                   "to ``-o '-vv'``.  For instance, to run specific tests, "
                   "you might use ``-o '-vvt my_test_pattern'``.")),
         Option(
+            'mergeproposal-address', type=str,
+            help=('Address of the merge proposal to be updated with test'
+                'results. Should be used only by ec2 land. If provided, you '
+                'will be asked for your GPG passphrase before the test run '
+                'begins.')),
+        Option(
             'submit-pqm-message', short_name='s', type=str, argname="MSG",
             help=(
                 'A pqm message to submit if the test run is successful.  If '
@@ -271,7 +277,7 @@
             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,
-            include_download_cache_changes=False):
+            include_download_cache_changes=False, mergeproposal_address=None):
         if debug:
             pdb.set_trace()
         if branch is None:
@@ -312,7 +318,7 @@
             open_browser=open_browser, pqm_email=pqm_email,
             include_download_cache_changes=include_download_cache_changes,
             instance=instance, launchpad_login=instance._launchpad_login,
-            timeout=480)
+            timeout=480, mergeproposal_address=mergeproposal_address)
 
         instance.set_up_and_run(postmortem, attached, runner.run_tests)
 
@@ -324,6 +330,8 @@
         debug_option,
         Option('dry-run', help="Just print the equivalent ec2 test command."),
         Option('print-commit', help="Print the full commit message."),
+        Option('use-tarmac', short_name='T', help="This should be prepared "
+            "for Tarmac landing instead of PQM."),
         Option(
             'testfix',
             help="This is a testfix (tags commit with [testfix])."),
@@ -352,7 +360,7 @@
     takes_args = ['merge_proposal?']
 
     def _get_landing_command(self, source_url, target_url, commit_message,
-                             emails, attached):
+                             emails, attached, mergeproposal_address=None):
         """Return the command that would need to be run to submit with ec2."""
         ec2_path = os.path.join(get_launchpad_root(), 'utilities', 'ec2')
         command = [ec2_path, 'test']
@@ -366,13 +374,16 @@
         command.extend(
             ['-b', 'launchpad=%s' % (target_branch_name), '-s',
              commit_message, str(source_url)])
+        if mergeproposal_address:
+            command.extend(
+                ['--mergeproposal-address=%s' % mergeproposal_address])
         return command
 
     def run(self, merge_proposal=None, machine=None,
             instance_type=DEFAULT_INSTANCE_TYPE, postmortem=False,
             debug=False, commit_text=None, dry_run=False, testfix=False,
             no_qa=False, incremental=False, rollback=None, print_commit=False,
-            force=False, attached=False):
+            force=False, attached=False, use_tarmac=False):
         try:
             from devscripts.autoland import (
                 LaunchpadBranchLander, MissingReviewError, MissingBugsError,
@@ -447,9 +458,14 @@
             print commit_message
             return
 
+        if use_tarmac:
+            mergeproposal_address= mp._mp.address
+        else:
+            mergeproposal_address = None
+
         landing_command = self._get_landing_command(
             mp.source_branch, mp.target_branch, commit_message,
-            mp.get_stakeholder_emails(), attached)
+            mp.get_stakeholder_emails(), attached, mergeproposal_address)
         if dry_run:
             print landing_command
         else:

=== modified file 'lib/devscripts/ec2test/remote.py'
--- lib/devscripts/ec2test/remote.py	2010-10-26 15:47:24 +0000
+++ lib/devscripts/ec2test/remote.py	2011-01-11 13:08:59 +0000
@@ -306,7 +306,8 @@
     """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, smtp_connection=None):
+                 emails=None, success_message=None, failure_message=None,
+                 pqm_message=None, smtp_connection=None):
         """Construct a `Request`.
 
         :param branch_url: The public URL to the Launchpad branch we are
@@ -320,6 +321,12 @@
             dependencies.
         :param emails: A list of emails to send the results to. If not
             provided, no emails are sent.
+        :param success_message: The message to update merge proposal in
+            Launchpad with successful results. If not provided, we don't
+            change the merge proposal.
+        :param failure_message: The message to update merge proposal in
+            Launchpad with failure results. If not provided, we don't
+            change the merge proposal.
         :param pqm_message: The message to submit to PQM. If not provided, we
             don't submit to PQM.
         :param smtp_connection: The `SMTPConnection` to use to send email.
@@ -330,6 +337,8 @@
         self._sourcecode_path = sourcecode_path
         self._emails = emails
         self._pqm_message = pqm_message
+        self._success_message = success_message
+        self._failure_message = failure_message
         # Used for figuring out how to send emails.
         self._bzr_config = bzrlib.config.GlobalConfig()
         if smtp_connection is None:
@@ -502,6 +511,18 @@
         message = self._build_report_email(successful, body_text, full_log_gz)
         self._send_email(message)
 
+    def send_mergeproposal_email(self, successful):
+        """Send an email to the merge proposal adding a tests vote, and
+        approving or disapproving it, according to test results.
+
+        :param successful: True for pass, False for failure.
+        """
+        if successful:
+            message = self._success_message
+        else:
+            message = self._failure_message
+        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)):
@@ -596,6 +617,8 @@
             summary = self.get_summary_contents()
             full_log_gz = gzip_data(self.get_full_log_contents())
             self._request.send_report_email(False, summary, full_log_gz)
+        if (self._request._failure_message is not None):
+            self._request.send_mergeproposal_email(False)
 
     def get_index_contents(self):
         """Return the contents of the index.html page."""
@@ -632,7 +655,11 @@
         """The tests are done and the results are known."""
         self._end_time = datetime.datetime.utcnow()
         successful = result.wasSuccessful()
-        self._handle_pqm_submission(successful)
+        if (self._request._success_message is None and
+            self._request._failure_message is None):
+            self._handle_pqm_submission(successful)
+        else:
+            self._request.send_mergeproposal_email(successful)
         if self._request.wants_email:
             email_text = self._request.format_result(
                 result, self._start_time, self._end_time)
@@ -846,6 +873,22 @@
               'the email address from `bzr whoami`. May be supplied multiple '
               'times. `bzr whoami` will be used as the From: address.'))
     parser.add_option(
+        '--success-message', dest='success_message', default=None,
+        help=('A base64-encoded pickle (string) of a email message to be '
+            'sent to Launchpad, updating the merge proposal by adding a '
+            'tests vote with an approval, and setting the merge proposal '
+            'status to approved. Should be used only by ec2 land. If '
+            'provided, you will be asked for your GPG passphrase before '
+            'the test run begins.'))
+    parser.add_option(
+        '--failure-message', dest='failure_message', default=None,
+        help=('A base64-encoded pickle (string) of a email message to be '
+            'sent to Launchpad, updating the merge proposal by adding a '
+            'tests vote with a needsfixing, and setting the merge proposal '
+            'status to rejected. Should be used only by ec2 land. If '
+            'provided, you will be asked for your GPG passphrase before '
+            'the test run begins.'))
+    parser.add_option(
         '-s', '--submit-pqm-message', dest='pqm_message', default=None,
         help=('A base64-encoded pickle (string) of a pqm message '
               '(bzrib.plugins.pqm.pqm_submit.PQMEmailMessage) to submit if '
@@ -877,11 +920,20 @@
 
     if options.debug:
         import pdb; pdb.set_trace()
-    if options.pqm_message is not None:
+    if (options.success_message is not None and
+        options.failure_message is not None):
+        pqm_message = None
+        success_message = pickle.loads(
+            options.success_message.decode('string-escape').decode('base64'))
+        failure_message = pickle.loads(
+            options.failure_message.decode('string-escape').decode('base64'))
+    elif options.pqm_message is not None:
         pqm_message = pickle.loads(
             options.pqm_message.decode('string-escape').decode('base64'))
+        success_message = failure_message = None
     else:
         pqm_message = None
+        success_message = failure_message = None
 
     # Locations for Launchpad. These are actually defined by the configuration
     # of the EC2 image that we use.
@@ -895,7 +947,8 @@
 
     request = Request(
         options.public_branch, options.public_branch_revno, TEST_DIR,
-        SOURCECODE_DIR, options.email, pqm_message, smtp_connection)
+        SOURCECODE_DIR, options.email, success_message, failure_message,
+        pqm_message, smtp_connection)
     # Only write to stdout if we are running as the foreground process.
     echo_to_stdout = not options.daemon
     logger = WebTestLogger.make_in_directory(

=== modified file 'lib/devscripts/ec2test/testrunner.py'
--- lib/devscripts/ec2test/testrunner.py	2010-08-18 17:40:29 +0000
+++ lib/devscripts/ec2test/testrunner.py	2011-01-11 13:08:59 +0000
@@ -18,6 +18,9 @@
 from bzrlib.bzrdir import BzrDir
 from bzrlib.config import GlobalConfig
 from bzrlib.errors import UncommittedChanges
+from bzrlib.gpg import GPGStrategy
+from bzrlib.email_message import EmailMessage
+from devscripts.autoland import MergeProposalMessage
 from bzrlib.plugins.pqm.pqm_submit import (
     NoPQMSubmissionAddress, PQMSubmission)
 
@@ -102,6 +105,57 @@
     return dict(map(normalize_branch_input, branches))
 
 
+class MergeProposalMessage(EmailMessage):
+
+    def __init__(self, mail_to, mail_from, successful=None,
+        source_branch=None):
+
+        if successful:
+            self.subject = "Test results: SUCCESS"
+            self.body_text = """
+Full test results were mailed to the submitter.
+
+ review approve tests
+ merge approved
+"""
+        else:
+            self.subject = "Test results: FAILURE"
+            self.body_text = """
+Full test results were mailed to the submitter.
+
+ review needsfixing tests
+ merge rejected
+"""
+        self.source_branch = source_branch
+        self.mail_from = mail_from
+        self.mail_to = mail_to
+
+    def to_signed(self, unsigned_text):
+        """Serialize as a signed string."""
+
+        if self.source_branch:
+            config = self.source_branch.get_config()
+        else:
+            config = GlobalConfig()
+        strategy = GPGStrategy(config)
+        return strategy.sign(unsigned_text)
+
+    def to_email(self, mail_from=None, mail_to=None):
+        """Serialize as an email message.
+
+        :param mail_from: The from address for the message
+        :param mail_to: The address to send the message to
+        :return: an email message
+        """
+        if mail_from is None:
+            mail_from = self.mail_from
+        if mail_to is None:
+            mail_to = self.mail_to
+        body = self.to_signed(self.body_text)
+        message = EmailMessage(mail_from, mail_to, self.subject, body)
+        return message
+
+
 class EC2TestRunner:
 
     name = 'ec2-test-runner'
@@ -116,7 +170,7 @@
                  open_browser=False, pqm_email=None,
                  include_download_cache_changes=None, instance=None,
                  launchpad_login=None,
-                 timeout=None):
+                 timeout=None, mergeproposal_address=None):
         """Create a new EC2TestRunner.
 
         :param timeout: Number of minutes before we force a shutdown. This is
@@ -143,6 +197,7 @@
         self.file = file
         self._launchpad_login = launchpad_login
         self.timeout = timeout
+        self.mergeproposal_address = mergeproposal_address
 
         trunk_specified = False
         trunk_branch = TRUNK_BRANCH
@@ -199,6 +254,13 @@
                         pqm_submit_location = trunk_branch
                 elif pqm_submit_location is None and trunk_specified:
                     pqm_submit_location = trunk_branch
+
+                # Explicitly ignoring pqm submission if the merge proposal
+                # address is provided. That means that we should use tarmac
+                # instead of pqm.
+                if self.mergeproposal_address is not None:
+                    pqm_message = None
+
                 # modified from pqm_submit.py
                 submission = PQMSubmission(
                     source_branch=bzrbranch,
@@ -236,7 +298,20 @@
                                 cache_basis_tree.unlock()
                         finally:
                             cache_tree.unlock()
-                if pqm_message is not None:
+
+                if mergeproposal_address is not None:
+                    self.message = None
+                    self.success_message = MergeProposalMessage(
+                            mergeproposal_address,
+                            config.username(),
+                            successful=True,
+                            source_branch=bzrbranch).to_email()
+                    self.failure_message = MergeProposalMessage(
+                            mergeproposal_address,
+                            config.username(),
+                            successful=False,
+                            source_branch=bzrbranch).to_email()
+                elif pqm_message is not None:
                     if self.download_cache_additions:
                         raise UncommittedChanges(cache_tree)
                     # get the submission message
@@ -255,6 +330,7 @@
                         raise NoPQMSubmissionAddress(bzrbranch)
                     mail_to = pqm_email.encode('utf8') # same here
                     self.message = submission.to_email(mail_from, mail_to)
+                    self.success_message = self.failure_message = None
                 elif (self.download_cache_additions and
                       self.include_download_cache_changes is None):
                     raise UncommittedChanges(
@@ -264,6 +340,8 @@
                         '--ignore-download-cache-changes, -c and -g '
                         'respectively), or '
                         'commit or remove the files in the download-cache.')
+
+
         self._branch = branch
 
         if email is not False:
@@ -287,7 +365,8 @@
         self.email = email
 
         # Email configuration.
-        if email is not None or pqm_message is not None:
+        if (email is not None or pqm_message is not None or
+            mergeproposal_address is not None):
             self._smtp_server = config.get_user_option('smtp_server')
             # Refuse localhost, because there's no SMTP server _on the actual
             # EC2 instance._

=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
--- lib/devscripts/ec2test/tests/test_remote.py	2010-10-06 11:46:51 +0000
+++ lib/devscripts/ec2test/tests/test_remote.py	2011-01-11 13:08:59 +0000
@@ -74,7 +74,8 @@
 
     def make_request(self, branch_url=None, revno=None,
                      trunk=None, sourcecode_path=None,
-                     emails=None, pqm_message=None, emails_sent=None):
+                     emails=None, success_message=None, failure_message=None,
+                     pqm_message=None, emails_sent=None):
         """Make a request to test, specifying only things we care about.
 
         Note that the returned request object will not ever send email, but
@@ -93,7 +94,7 @@
         smtp_connection = LoggingSMTPConnection(emails_sent)
         request = Request(
             branch_url, revno, trunk.basedir, sourcecode_path, emails,
-            pqm_message, smtp_connection)
+            success_message, failure_message, pqm_message, smtp_connection)
         return request
 
     def make_sourcecode(self, branches):

=== modified file 'lib/devscripts/tests/test_autoland.py'
--- lib/devscripts/tests/test_autoland.py	2010-11-05 18:48:39 +0000
+++ lib/devscripts/tests/test_autoland.py	2011-01-11 13:08:59 +0000
@@ -16,7 +16,7 @@
     get_bazaar_host, get_bugs_clause, get_reviewer_clause,
     get_reviewer_handle, get_qa_clause, get_testfix_clause,
     MissingReviewError, MissingBugsError, MissingBugsIncrementalError,
-    MergeProposal)
+    MergeProposal, MergeProposalMessage)
 
 
 class FakeBug:
@@ -65,6 +65,43 @@
         pass
 
 
+class FakeGPGStrategy:
+
+    def __init__(self, text_to_return=""):
+        self.sign = FakeMethod(text_to_return)
+
+
+class TestMergeProposalMessage(unittest.TestCase):
+
+    def setUp(self):
+        self.mail_to = "mp+1234@xxxxxxxxxxx"
+        self.mail_from = "me@xxxxxxxxxxx"
+        self.gpg_strategy = FakeGPGStrategy()
+
+    def test_merge_proposal_message_is_successful(self):
+        successful = True
+        message = MergeProposalMessage(self.mail_to, self.mail_from,
+            successful=successful, gpg_strategy=self.gpg_strategy)
+        self.assertTrue(message.successful)
+        self.assertTrue("SUCCESS" in message.subject)
+
+    def test_merge_proposal_message_not_successful(self):
+        successful = False
+        message = MergeProposalMessage(self.mail_to, self.mail_from,
+            successful=successful, gpg_strategy=self.gpg_strategy)
+        self.assertFalse(message.successful)
+        self.assertTrue("FAILURE" in message.subject)
+
+    def test_to_email(self):
+        fake_signed_text = "signed msg body"
+        gpg_strategy = FakeGPGStrategy(fake_signed_text)
+
+        message = MergeProposalMessage(self.mail_to, self.mail_from,
+            successful=True, gpg_strategy=gpg_strategy)
+        message = message.to_email()
+        self.assertEqual(message._body, fake_signed_text)
+
+
 class TestPQMRegexAcceptance(unittest.TestCase):
     """Tests if the generated commit message is accepted by PQM regexes."""
 


Follow ups