Changeset 220715 in webkit

Timestamp:

Aug 14, 2017 2:14:10 PM (7 years ago)

Author:

dbates@webkit.org

Message:

webkit-patch: Passing --no-review should submit patch to EWS by default
​https://bugs.webkit.org/show_bug.cgi?id=148899

Reviewed by David Kilzer.

Make "webkit-patch upload --no-review" and "webkit-patch post-commits --no-review" submit
the patch(es) for EWS analysis by default. Add a new optional command line argument, --no-ews,
to these commands to not submit a non-reviewed patch(es) for EWS analysis.

Frequently I want to post a draft of a patch without review to have the EWS bots process
it to catch build and test breakage before I clean it up and post it for formal review.
Currently this requires using "webkit-patch upload --no-review --open-bug" to upload the
patch and open the bug in Safari so that I can click the Submit for EWS analysis button.
We should teach "webkit-patch upload" and "webkit-patch post-commits" how to do this.

• Scripts/webkitpy/common/net/bugzilla/bugzilla.py:

(Bugzilla._parse_attachment_id_from_add_patch_to_bug_response): Add helper function to
parse the attachment id from the response page after adding an attachment to the bug.
(Bugzilla.add_patch_to_bug): Modified to parse and return the attachment id from the
HTTP response after uploading the patch.

• Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py:

(MockBugzilla.add_patch_to_bug): Return a dummy attachment id for testing purposes.

• Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py:

(testparse_attachment_id_from_add_patch_to_bug_response): Added.

• Scripts/webkitpy/tool/commands/upload.py:

(Upload): Add step SubmitToEWS to the list of steps when uploading a patch.
(PostCommits.init): Add --no-ews option to the list of applicable option flags for
this command.
(PostCommits.execute): Submit the patch to EWS, if applicable.

• Scripts/webkitpy/tool/commands/upload_unittest.py:

(test_upload): Set options.ews to False as the upload command expects this option
to be specified.
(test_upload_with_no_review_and_ews): Added.

• Scripts/webkitpy/tool/steps/init.py: Import module SubmitToEWS.
• Scripts/webkitpy/tool/steps/options.py:

(Options): Added command line option --no-ews (defaults: False - submit to EWS).

• Scripts/webkitpy/tool/steps/postdiff.py:

(PostDiff.options): Add --no-ews option to the list of applicable option flags for
this command.
(PostDiff.run): Submit the patch to EWS, if applicable.

• Scripts/webkitpy/tool/steps/submittoews.py: Added.

(SubmitToEWS):
(SubmitToEWS.options): Return an empty array as we this step does not have any command line options.
(SubmitToEWS.run): Submit the specified attachment ids for EWS analysis.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/net/bugzilla/bugzilla.py (modified) (2 diffs)
• Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py (modified) (1 diff)
• Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/upload.py (modified) (3 diffs)
• Scripts/webkitpy/tool/commands/upload_unittest.py (modified) (2 diffs)
• Scripts/webkitpy/tool/steps/__init__.py (modified) (1 diff)
• Scripts/webkitpy/tool/steps/options.py (modified) (1 diff)
• Scripts/webkitpy/tool/steps/postdiff.py (modified) (2 diffs)
• Scripts/webkitpy/tool/steps/submittoews.py (added)

trunk/Tools/ChangeLog

@@ +1 @@
+2017-08-14  Daniel Bates  <dabates@apple.com>
+
+        webkit-patch: Passing --no-review should submit patch to EWS by default
+        https://bugs.webkit.org/show_bug.cgi?id=148899
+
+        Reviewed by David Kilzer.
+
+        Make "webkit-patch upload --no-review" and "webkit-patch post-commits --no-review" submit
+        the patch(es) for EWS analysis by default. Add a new optional command line argument, --no-ews,
+        to these commands to not submit a non-reviewed patch(es) for EWS analysis.
+
+        Frequently I want to post a draft of a patch without review to have the EWS bots process
+        it to catch build and test breakage before I clean it up and post it for formal review.
+        Currently this requires using "webkit-patch upload --no-review --open-bug" to upload the
+        patch and open the bug in Safari so that I can click the Submit for EWS analysis button.
+        We should teach "webkit-patch upload" and "webkit-patch post-commits" how to do this.
+
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+        (Bugzilla._parse_attachment_id_from_add_patch_to_bug_response): Add helper function to
+        parse the attachment id from the response page after adding an attachment to the bug.
+        (Bugzilla.add_patch_to_bug): Modified to parse and return the attachment id from the
+        HTTP response after uploading the patch.
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py:
+        (MockBugzilla.add_patch_to_bug): Return a dummy attachment id for testing purposes.
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py:
+        (test__parse_attachment_id_from_add_patch_to_bug_response): Added.
+        * Scripts/webkitpy/tool/commands/upload.py:
+        (Upload): Add step SubmitToEWS to the list of steps when uploading a patch. 
+        (PostCommits.__init__): Add --no-ews option to the list of applicable option flags for
+        this command.
+        (PostCommits.execute): Submit the patch to EWS, if applicable.
+        * Scripts/webkitpy/tool/commands/upload_unittest.py:
+        (test_upload): Set options.ews to False as the upload command expects this option
+        to be specified.
+        (test_upload_with_no_review_and_ews): Added.
+        * Scripts/webkitpy/tool/steps/__init__.py: Import module SubmitToEWS.
+        * Scripts/webkitpy/tool/steps/options.py:
+        (Options): Added command line option --no-ews (defaults: False - submit to EWS).
+        * Scripts/webkitpy/tool/steps/postdiff.py:
+        (PostDiff.options): Add --no-ews option to the list of applicable option flags for
+        this command.
+        (PostDiff.run): Submit the patch to EWS, if applicable.
+        * Scripts/webkitpy/tool/steps/submittoews.py: Added.
+        (SubmitToEWS):
+        (SubmitToEWS.options): Return an empty array as we this step does not have any command line options.
+        (SubmitToEWS.run): Submit the specified attachment ids for EWS analysis.
+
 2017-08-14  Ryan Haddad  <ryanhaddad@apple.com>
 

trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py

@@ -621 +621 @@
         self.browser.submit()
 
+    @staticmethod
+    def _parse_attachment_id_from_add_patch_to_bug_response(response_html):
+        match = re.search('<title>Attachment (?P<attachment_id>\d+) added to Bug \d+</title>', response_html)
+        if match:
+            return match.group('attachment_id')
+        _log.warning('Unable to parse attachment id')
+        return None
+
     # FIXME: The arguments to this function should be simplified and then
     # this should be merged into add_attachment_to_bug
@@ -653 +661 @@
             _log.info(comment_text)
             self.browser['comment'] = comment_text
-        self.browser.submit()
+        response = self.browser.submit()
+        return self._parse_attachment_id_from_add_patch_to_bug_response(response.read())
 
     # FIXME: There has to be a more concise way to write this method.

trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py

@@ -462 +462 @@
             _log.info(comment_text)
             _log.info("-- End comment --")
+        return '10001'
 
     def add_cc_to_bug(self, bug_id, ccs):

trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py

@@ -342 +342 @@
         title_html_bugzilla_425 = "<title>Bug 101640 Submitted &ndash; Testing webkit-patch again</title>"
         self.assertEqual(bugzilla._check_create_bug_response(title_html_bugzilla_425), '101640')
+
+    def test__parse_attachment_id_from_add_patch_to_bug_response(self):
+        bugzilla = Bugzilla()
+
+        title_html = '<title>Attachment 317591 added to Bug 175247</title>'
+        self.assertEqual(bugzilla._parse_attachment_id_from_add_patch_to_bug_response(title_html), '317591')
+
+        title_html = '<title>Attachment 317591; malformed</title>'
+        self.assertEqual(bugzilla._parse_attachment_id_from_add_patch_to_bug_response(title_html), None)
+
+        title_html = '<title>Attachment A added to Bug 175247</title>'
+        self.assertEqual(bugzilla._parse_attachment_id_from_add_patch_to_bug_response(title_html), None)
 
 

trunk/Tools/Scripts/webkitpy/tool/commands/upload.py

@@ -285 +285 @@
         steps.EnsureBugIsOpenAndAssigned,
         steps.PostDiff,
+        steps.SubmitToEWS,
     ]
     long_help = """upload uploads the current diff to bugs.webkit.org.
@@ -322 +323 @@
             steps.Options.review,
             steps.Options.request_commit,
+            steps.Options.ews,
         ]
         Command.__init__(self, options=options, requires_local_commits=True)
@@ -357 +359 @@
             description = options.description or commit_message.description(lstrip=True, strip_url=True)
             comment_text = self._comment_text_for_commit(options, commit_message, tool, commit_id)
-            tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+            attachment_id = tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+
+            # We only need to submit --no-review patches to EWS as patches posted for review are
+            # automatically submitted to EWS by EWSFeeder.
+            if not options.review and options.ews:
+                state = {'attachment_ids': [attachment_id]}
+                steps.SubmitToEWS(tool, options).run(state)
 
 

trunk/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py

@@ -124 +124 @@
         options.request_commit = False
         options.review = True
+        options.submit_to_ews = False
         options.sort_xcode_project = False
         options.suggest_reviewers = False
@@ -132 +133 @@
 MOCK add_patch_to_bug: bug_id=50000, description=MOCK description, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False
 MOCK: user.open_url: http://example.com/50000
+"""
+        self.assert_execute_outputs(Upload(), [50000], options=options, expected_logs=expected_logs)
+
+    def test_upload_with_no_review_and_ews(self):
+        options = MockOptions()
+        options.cc = None
+        options.check_style = True
+        options.check_style_filter = None
+        options.comment = None
+        options.description = 'MOCK description'
+        options.non_interactive = False
+        options.request_commit = False
+        options.review = False
+        options.ews = True
+        options.sort_xcode_project = False
+        options.suggest_reviewers = False
+        expected_logs = """MOCK: user.open_url: file://...
+Was that diff correct?
+Obsoleting 2 old patches on bug 50000
+MOCK reassign_bug: bug_id=50000, assignee=None
+MOCK add_patch_to_bug: bug_id=50000, description=MOCK description, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=False
+MOCK: user.open_url: http://example.com/50000
+MOCK: submit_to_ews: 10001
 """
         self.assert_execute_outputs(Upload(), [50000], options=options, expected_logs=expected_logs)

trunk/Tools/Scripts/webkitpy/tool/steps/__init__.py

@@ -61 +61 @@
 from webkitpy.tool.steps.runtests import RunTests
 from webkitpy.tool.steps.sortxcodeprojectfiles import SortXcodeProjectFiles
+from webkitpy.tool.steps.submittoews import SubmitToEWS
 from webkitpy.tool.steps.suggestreviewers import SuggestReviewers
 from webkitpy.tool.steps.update import Update

trunk/Tools/Scripts/webkitpy/tool/steps/options.py

@@ -46 +46 @@
     description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment")
     email = make_option("--email", action="store", type="string", dest="email", help="Email address to use in ChangeLogs.")
+    ews = make_option('--no-ews', action='store_false', dest='ews', default=True, help='Do not submit the patch to EWS for analysis (only applies when using --no-review).')
     force_clean = make_option("--force-clean", action="store_true", dest="force_clean", default=False, help="Clean working directory before applying patches (removes local changes and commits)")
     git_commit = make_option("-g", "--git-commit", action="store", dest="git_commit", help="Operate on a local commit. If a range, the commits are squashed into one. <ref>.... includes the working copy changes. UPSTREAM can be used for the upstream/tracking branch.")

trunk/Tools/Scripts/webkitpy/tool/steps/postdiff.py

@@ -40 +40 @@
             Options.request_commit,
             Options.open_bug,
+            Options.ews,
         ]
 
@@ -48 +49 @@
         bug_id = state["bug_id"]
 
-        self._tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
+        attachment_id = self._tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
         if self._options.open_bug:
             self._tool.user.open_url(self._tool.bugs.bug_url_for_bug_id(bug_id))
+
+        # We only need to submit --no-review patches to EWS as patches posted for review are
+        # automatically submitted to EWS by EWSFeeder.
+        if not self._options.review and self._options.ews:
+            state['attachment_ids'] = [attachment_id]