Changeset 52480 in webkit

Timestamp:

Dec 21, 2009 10:46:57 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2009-12-21 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

[bzt] Automate the process of calling prepare-ChangeLog
​https://bugs.webkit.org/show_bug.cgi?id=32816

This patch automates the process of creating a bug and patch and
uploading it to bugzilla. The first cut just calls
prepare-ChangeLog.

This patch required some refactoring of upload.py to the Step
model, but that's worth doing anyway.

• Scripts/bugzilla-tool:
• Scripts/modules/buildsteps.py:
• Scripts/modules/commands/download.py:
• Scripts/modules/commands/upload.py:
• Scripts/modules/commands/upload_unittest.py:
• Scripts/modules/mock_bugzillatool.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/bugzilla-tool (modified) (2 diffs)
• Scripts/modules/buildsteps.py (modified) (4 diffs)
• Scripts/modules/commands/download.py (modified) (1 diff)
• Scripts/modules/commands/upload.py (modified) (4 diffs)
• Scripts/modules/commands/upload_unittest.py (modified) (1 diff)
• Scripts/modules/mock_bugzillatool.py (modified) (3 diffs)
• Scripts/modules/user.py (copied) (copied from trunk/WebKitTools/Scripts/modules/commands/upload_unittest.py) (2 diffs)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2009-12-21  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        [bzt] Automate the process of calling prepare-ChangeLog
+        https://bugs.webkit.org/show_bug.cgi?id=32816
+
+        This patch automates the process of creating a bug and patch and
+        uploading it to bugzilla.  The first cut just calls
+        prepare-ChangeLog.
+
+        This patch required some refactoring of upload.py to the Step
+        model, but that's worth doing anyway.
+
+        * Scripts/bugzilla-tool:
+        * Scripts/modules/buildsteps.py:
+        * Scripts/modules/commands/download.py:
+        * Scripts/modules/commands/upload.py:
+        * Scripts/modules/commands/upload_unittest.py:
+        * Scripts/modules/mock_bugzillatool.py:
+
 2009-12-21  Darin Adler  <darin@apple.com>
 

trunk/WebKitTools/Scripts/bugzilla-tool

@@ -44 +44 @@
 from modules.multicommandtool import MultiCommandTool
 from modules.scm import detect_scm_system
+from modules.user import User
+
 
 class BugzillaTool(MultiCommandTool):
@@ -54 +56 @@
         self.buildbot = BuildBot()
         self.executive = Executive()
+        self.user = User()
         self._scm = None
         self.status_bot = StatusBot()

trunk/WebKitTools/Scripts/modules/buildsteps.py

@@ -28 +28 @@
 
 import os
+import StringIO
 
 from optparse import make_option
@@ -53 +54 @@
     reviewer = make_option("-r", "--reviewer", action="store", type="string", dest="reviewer", help="Update ChangeLogs to say Reviewed by REVIEWER.")
     complete_rollout = make_option("--complete-rollout", action="store_true", dest="complete_rollout", help="Commit the revert and re-open the original bug.")
+    obsolete_patches = make_option("--no-obsolete", action="store_false", dest="obsolete_patches", default=True, help="Do not obsolete old patches before posting this one.")
+    review = make_option("--no-review", action="store_false", dest="review", default=True, help="Do not mark the patch for review.")
+    request_commit = make_option("--request-commit", action="store_true", dest="request_commit", default=False, help="Mark the patch as needing auto-commit after review.")
+    description = make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")")
 
 
@@ -63 +68 @@
     def _run_script(self, script_name, quiet=False, port=WebKitPort):
         log("Running %s" % script_name)
+        # FIXME: This should use self.port()
         self._tool.executive.run_and_throw_if_fail(port.script_path(script_name), quiet)
 
@@ -106 +112 @@
 
 class PrepareChangelogStep(AbstractStep):
-    def run(self, state):
-        self._run_script("prepare-ChangeLog")
+    @classmethod
+    def options(cls):
+        return [
+            CommandOptions.port,
+            CommandOptions.quiet,
+            CommandOptions.non_interactive,
+        ]
+
+    def run(self, state):
+        os.chdir(self._tool.scm().checkout_root)
+        args = [self.port().script_path("prepare-ChangeLog")]
+        if not self._options.non_interactive:
+            args.append("-o")
+        if state["bug_id"]:
+            args.append("--bug=%s" % state["bug_id"])
+        self._tool.executive.run_and_throw_if_fail(args, self._options.quiet)
+        if not self._options.non_interactive:
+            self._tool.user.prompt("Press enter when ready to continue.")
+
+
+class ObsoletePatchesOnBugStep(AbstractStep):
+    @classmethod
+    def options(cls):
+        return [
+            CommandOptions.obsolete_patches,
+        ]
+
+    def run(self, state):
+        if not self._options.obsolete_patches:
+            return
+        bug_id = state["bug_id"]
+        patches = self._tool.bugs.fetch_patches_from_bug(bug_id)
+        if not patches:
+            return
+        log("Obsoleting %s on bug %s" % (pluralize("old patch", len(patches)), bug_id))
+        for patch in patches:
+            self._tool.bugs.obsolete_attachment(patch["id"])
+
+
+class PostDiffToBugStep(AbstractStep):
+    @classmethod
+    def options(cls):
+        return [
+            CommandOptions.description,
+            CommandOptions.review,
+            CommandOptions.request_commit,
+        ]
+
+    def run(self, state):
+        diff = self._tool.scm().create_patch()
+        diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
+        description = self._options.description or "Patch"
+        self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
 
 

trunk/WebKitTools/Scripts/modules/commands/download.py

@@ -53 +53 @@
 
 
+# FIXME: Move this to a more general location.
 class AbstractSequencedCommmand(AbstractDeclarativeCommmand):
     steps = None

trunk/WebKitTools/Scripts/modules/commands/upload.py

@@ -37 +37 @@
 
 from modules.bugzilla import parse_bug_id
+from modules.buildsteps import PrepareChangelogStep, CommandOptions, ObsoletePatchesOnBugStep, PostDiffToBugStep
+from modules.commands.download import AbstractSequencedCommmand
 from modules.comments import bug_comment_from_svn_revision
 from modules.grammar import pluralize
@@ -53 +55 @@
 
 
-class ObsoleteAttachments(Command):
+class ObsoleteAttachments(AbstractSequencedCommmand):
     name = "obsolete-attachments"
-    def __init__(self):
-        Command.__init__(self, "Mark all attachments on a bug as obsolete", "BUGID")
-
-    def execute(self, options, args, tool):
-        bug_id = args[0]
-        attachments = tool.bugs.fetch_attachments_from_bug(bug_id)
-        for attachment in attachments:
-            if not attachment["is_obsolete"]:
-                tool.bugs.obsolete_attachment(attachment["id"])
-
-
-class PostDiff(Command):
+    help_text = "Mark all attachments on a bug as obsolete"
+    argument_names = "BUGID"
+    steps = [
+        ObsoletePatchesOnBugStep,
+    ]
+
+    def _prepare_state(self, options, args, tool):
+        return { "bug_id" : args[0] }
+
+
+class PostDiff(AbstractSequencedCommmand):
     name = "post-diff"
+    help_text = "Attach the current working directory diff to a bug as a patch file"
+    argument_names = "[BUGID]"
     show_in_main_help = True
-    def __init__(self):
-        options = [
-            make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")"),
-        ]
-        options += self.posting_options()
-        Command.__init__(self, "Attach the current working directory diff to a bug as a patch file", "[BUGID]", options=options)
-
-    @staticmethod
-    def posting_options():
-        return [
-            make_option("--no-obsolete", action="store_false", dest="obsolete_patches", default=True, help="Do not obsolete old patches before posting this one."),
-            make_option("--no-review", action="store_false", dest="review", default=True, help="Do not mark the patch for review."),
-            make_option("--request-commit", action="store_true", dest="request_commit", default=False, help="Mark the patch as needing auto-commit after review."),
-        ]
-
-    @staticmethod
-    def obsolete_patches_on_bug(bug_id, bugs):
-        patches = bugs.fetch_patches_from_bug(bug_id)
-        if len(patches):
-            log("Obsoleting %s on bug %s" % (pluralize("old patch", len(patches)), bug_id))
-            for patch in patches:
-                bugs.obsolete_attachment(patch["id"])
-
-    def execute(self, options, args, tool):
+    steps = [
+        ObsoletePatchesOnBugStep,
+        PostDiffToBugStep,
+    ]
+
+    def _prepare_state(self, options, args, tool):
         # Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs).
         bug_id = (args and args[0]) or parse_bug_id(tool.scm().create_patch())
         if not bug_id:
             error("No bug id passed and no bug url found in diff, can't post.")
-
-        if options.obsolete_patches:
-            self.obsolete_patches_on_bug(bug_id, tool.bugs)
-
-        diff = tool.scm().create_patch()
-        diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
-
-        description = options.description or "Patch"
-        tool.bugs.add_patch_to_bug(bug_id, diff_file, description, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+        return { "bug_id" : bug_id }
+
+
+class SubmitPatch(AbstractSequencedCommmand):
+    name = "submit-patch"
+    help_text = "Experimental.  Creates a patch from the current working copy and uploads bugzilla"
+    argument_names = "BUGID"
+    steps = [
+        PrepareChangelogStep,
+        # FIXME: Add a CreateBugStep!
+        ObsoletePatchesOnBugStep,
+        PostDiffToBugStep,
+    ]
+
+    def _prepare_state(self, options, args, tool):
+        bug_id = args[0]
+        return { "bug_id" : bug_id }
 
 
@@ -116 +109 @@
             make_option("--add-log-as-comment", action="store_true", dest="add_log_as_comment", default=False, help="Add commit log message as a comment when uploading the patch."),
             make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: description from commit message)"),
+            CommandOptions.obsolete_patches,
+            CommandOptions.review,
+            CommandOptions.request_commit,
         ]
-        options += PostDiff.posting_options()
         Command.__init__(self, "Attach a range of local commits to bugs as patch files", "COMMITISH", options=options, requires_local_commits=True)
 
@@ -148 +143 @@
 
             if options.obsolete_patches and bug_id not in have_obsoleted_patches:
-                PostDiff.obsolete_patches_on_bug(bug_id, tool.bugs)
+                state = { "bug_id": bug_id }
+                ObsoletePatchesOnBugStep(tool, options).run(state)
                 have_obsoleted_patches.add(bug_id)
 

trunk/WebKitTools/Scripts/modules/commands/upload_unittest.py

@@ -34 +34 @@
 class UploadCommandsTest(CommandsTest):
     def test_obsolete_attachments(self):
-        self.assert_execute_outputs(ObsoleteAttachments(), [42])
+        expected_stderr = "Obsoleting 2 old patches on bug 42\n"
+        self.assert_execute_outputs(ObsoleteAttachments(), [42], expected_stderr=expected_stderr)
 
     def test_post_diff(self):
         expected_stderr = "Obsoleting 2 old patches on bug 42\n"
         self.assert_execute_outputs(PostDiff(), [42], expected_stderr=expected_stderr)
+
+    def test_submit_patch(self):
+        expected_stderr = "Obsoleting 2 old patches on bug 42\n"
+        self.assert_execute_outputs(SubmitPatch(), [42], expected_stderr=expected_stderr)

trunk/WebKitTools/Scripts/modules/mock_bugzillatool.py

@@ -109 +109 @@
         return []
 
+
 class MockSCM(Mock):
     def __init__(self):
@@ -143 +144 @@
 
 
+class MockUser(object):
+    def prompt(self, message):
+        return "Mock user response"
+
+
 class MockStatusBot(object):
     def __init__(self):
@@ -159 +165 @@
         self.buildbot = MockBuildBot()
         self.executive = Mock()
+        self.user = MockUser()
         self._scm = MockSCM()
         self.status_bot = MockStatusBot()

trunk/WebKitTools/Scripts/modules/user.py

@@ -1 @@
-# Copyright (C) 2009 Google Inc. All rights reserved.
-#
+# Copyright (c) 2009, Google Inc. All rights reserved.
+# 
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
-#
-#    * Redistributions of source code must retain the above copyright
+# 
+#     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
-#    * Redistributions in binary form must reproduce the above
+#     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
 # in the documentation and/or other materials provided with the
 # distribution.
-#    * Neither the name of Google Inc. nor the names of its
+#     * Neither the name of Google Inc. nor the names of its
 # contributors may be used to endorse or promote products derived from
 # this software without specific prior written permission.
-#
+# 
 # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
@@ -27 +27 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import unittest
-
-from modules.commands.commandtest import CommandsTest
-from modules.commands.upload import *
-
-class UploadCommandsTest(CommandsTest):
-    def test_obsolete_attachments(self):
-        self.assert_execute_outputs(ObsoleteAttachments(), [42])
-
-    def test_post_diff(self):
-        expected_stderr = "Obsoleting 2 old patches on bug 42\n"
-        self.assert_execute_outputs(PostDiff(), [42], expected_stderr=expected_stderr)
+class User(object):
+    def prompt(self, message):
+        return raw_input(message)