Changeset 53079 in webkit

Timestamp:

Jan 11, 2010 8:50:07 AM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-01-10 Adam Barth <​abarth@webkit.org>

Reviewed by David Kilzer.

bugzilla-tool submit-patch mistakenly picks up bug URLs in non-ChangeLog files
​https://bugs.webkit.org/show_bug.cgi?id=33197

We should just search for bug numbers in the ChangeLogs instead of in
the whole diff.

• Scripts/test-webkitpy:
• Scripts/webkitpy/commands/abstractdiffcommand.py: Added.
• Scripts/webkitpy/commands/abstractdiffcommand_unittest.py: Added.
• Scripts/webkitpy/commands/download.py:
• Scripts/webkitpy/commands/upload.py:
• Scripts/webkitpy/mock_bugzillatool.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/test-webkitpy (modified) (1 diff)
• Scripts/webkitpy/commands/abstractdiffcommand.py (copied) (copied from trunk/WebKitTools/Scripts/test-webkitpy) (2 diffs, 1 prop)
• Scripts/webkitpy/commands/abstractdiffcommand_unittest.py (added)
• Scripts/webkitpy/commands/download.py (modified) (4 diffs)
• Scripts/webkitpy/commands/upload.py (modified) (5 diffs)
• Scripts/webkitpy/mock_bugzillatool.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-01-10  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by David Kilzer.
+
+        bugzilla-tool submit-patch mistakenly picks up bug URLs in non-ChangeLog files
+        https://bugs.webkit.org/show_bug.cgi?id=33197
+
+        We should just search for bug numbers in the ChangeLogs instead of in
+        the whole diff.
+ 
+        * Scripts/test-webkitpy:
+        * Scripts/webkitpy/commands/abstractdiffcommand.py: Added.
+        * Scripts/webkitpy/commands/abstractdiffcommand_unittest.py: Added.
+        * Scripts/webkitpy/commands/download.py:
+        * Scripts/webkitpy/commands/upload.py:
+        * Scripts/webkitpy/mock_bugzillatool.py:
+
 2010-01-11  Chris Jerdonek  <chris.jerdonek@gmail.com>
 

trunk/WebKitTools/Scripts/test-webkitpy

@@ -34 +34 @@
 from webkitpy.buildbot_unittest import *
 from webkitpy.changelogs_unittest import *
+from webkitpy.commands.abstractdiffcommand_unittest import *
 from webkitpy.commands.download_unittest import *
 from webkitpy.commands.early_warning_system_unittest import *

trunk/WebKitTools/Scripts/webkitpy/commands/abstractdiffcommand.py

@@ -1 @@
-#!/usr/bin/env python
-# Copyright (c) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -28 +27 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import sys
-import unittest
+from webkitpy.bugzilla import parse_bug_id
+from webkitpy.commands.abstractsequencedcommand import AbstractSequencedCommand
 
-from webkitpy.bugzilla_unittest import *
-from webkitpy.buildbot_unittest import *
-from webkitpy.changelogs_unittest import *
-from webkitpy.commands.download_unittest import *
-from webkitpy.commands.early_warning_system_unittest import *
-from webkitpy.commands.upload_unittest import *
-from webkitpy.commands.queries_unittest import *
-from webkitpy.commands.queues_unittest import *
-from webkitpy.committers_unittest import *
-from webkitpy.credentials_unittest import *
-from webkitpy.diff_parser_unittest import *
-from webkitpy.executive_unittest import *
-from webkitpy.multicommandtool_unittest import *
-from webkitpy.networktransaction_unittest import *
-from webkitpy.queueengine_unittest import *
-from webkitpy.steps.steps_unittest import *
-from webkitpy.steps.updatechangelogswithreview_unittests import *
-from webkitpy.style.unittests import * # for check-webkit-style
-from webkitpy.webkit_logging_unittest import *
-from webkitpy.webkitport_unittest import *
 
-if __name__ == "__main__":
-    # FIXME: This is a hack, but I'm tired of commenting out the test.
-    #        See https://bugs.webkit.org/show_bug.cgi?id=31818
-    if len(sys.argv) > 1 and sys.argv[1] == "--all":
-        sys.argv.remove("--all")
-        from webkitpy.scm_unittest import *
-
-    unittest.main()
+class AbstractDiffCommand(AbstractSequencedCommand):
+    def _bug_id(self, args, tool):
+        # Perfer a bug id passed as an argument over a bug URL in the ChangeLogs.
+        bug_id = args and args[0]
+        if not bug_id:
+            bug_id = parse_bug_id(tool.scm().commit_message_for_this_commit().message())
+        return bug_id

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

@@ -37 +37 @@
 # We could instead use from modules import buildsteps and then prefix every buildstep with "buildsteps."
 from webkitpy.changelogs import ChangeLog
+from webkitpy.commands.abstractdiffcommand import AbstractDiffCommand
 from webkitpy.commands.abstractsequencedcommand import AbstractSequencedCommand
 from webkitpy.comments import bug_comment_from_commit_text
@@ -67 +68 @@
 
 
-class Land(AbstractSequencedCommand):
+class Land(AbstractDiffCommand):
     name = "land"
     help_text = "Land the current working directory diff and updates the associated bug if any"
@@ -89 +90 @@
             "patch" : {
                 "id" : None,
-                "bug_id" : (args and args[0]) or parse_bug_id(tool.scm().create_patch()),
+                "bug_id" : self._bug_id(args, tool),
             }
         }
@@ -269 +270 @@
     def _parse_bug_id_from_revision_diff(tool, revision):
         original_diff = tool.scm().diff_for_revision(revision)
+        # FIXME: This is wrong because it picks up bug numbers outside of ChangeLogs
         return parse_bug_id(original_diff)
 

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

@@ -39 +39 @@
 
 from webkitpy.bugzilla import parse_bug_id
+from webkitpy.commands.abstractdiffcommand import AbstractDiffCommand
 from webkitpy.commands.abstractsequencedcommand import AbstractSequencedCommand
 from webkitpy.comments import bug_comment_from_svn_revision
@@ -100 +101 @@
 
 
-class AbstractPatchUploadingCommand(AbstractSequencedCommand):
-    def _bug_id(self, args, tool, state):
-        # Perfer a bug id passed as an argument over a bug url in the diff (i.e. ChangeLogs).
-        bug_id = args and args[0]
-        if not bug_id:
-            state["diff"] = tool.scm().create_patch()
-            bug_id = parse_bug_id(state["diff"])
-        return bug_id
-
-
-class Post(AbstractPatchUploadingCommand):
+class Post(AbstractDiffCommand):
     name = "post"
     help_text = "Attach the current working directory diff to a bug as a patch file"
@@ -123 +114 @@
 
     def _prepare_state(self, options, args, tool):
-        state = {}
-        state["bug_id"] = self._bug_id(args, tool, state)
+        state = { "bug_id" : self._bug_id(args, tool) }
         if not state["bug_id"]:
             error("No bug id passed and no bug url found in diff, can't post.")
@@ -146 +136 @@
 
 
-class Upload(AbstractPatchUploadingCommand):
+class Upload(AbstractDiffCommand):
     name = "upload"
     help_text = "Automates the process of uploading a patch for review"
@@ -170 +160 @@
 
     def _prepare_state(self, options, args, tool):
-        state = {}
-        state["bug_id"] = self._bug_id(args, tool, state)
+        state = { "bug_id" : self._bug_id(args, tool) }
         return state
 

trunk/WebKitTools/Scripts/webkitpy/mock_bugzillatool.py

@@ -157 +157 @@
         return ["Commitish1", "Commitish2"]
 
+    def commit_message_for_this_commit(self):
+        return CommitMessage(["CommitMessage1", "https://bugs.webkit.org/show_bug.cgi?id=93"])
+
     def commit_message_for_local_commit(self, commit_id):
         if commit_id == "Commitish1":
-            return CommitMessage("CommitMessage1\nhttps://bugs.example.org/show_bug.cgi?id=42\n")
+            return CommitMessage(["CommitMessage1", "https://bugs.webkit.org/show_bug.cgi?id=42"])
         if commit_id == "Commitish2":
-            return CommitMessage("CommitMessage2\nhttps://bugs.example.org/show_bug.cgi?id=75\n")
+            return CommitMessage(["CommitMessage2", "https://bugs.webkit.org/show_bug.cgi?id=75"])
         raise Exception("Bogus commit_id in commit_message_for_local_commit.")