2010-04-20 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Apr 2010 19:54:46 +0000 (19:54 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Apr 2010 19:54:46 +0000 (19:54 +0000)
        Reviewed by Adam Barth.

        REGRESSION(57531): the commit-queue still hates Tor Arne Vestbø
        https://bugs.webkit.org/show_bug.cgi?id=37765

        I fixed the queue to not ignore Tor as a reviwer in r57531,
        but instead it throws an exception every time his name is in a patch.

        This fixes our Executive.run_command code to work around a Popen
        bug http://bugs.python.org/issue5290 whereby python versions before 2.6
        do not correctly handle unicode objects as input or output to
        Popen.communicate.

        Following the advice of:
        http://farmdev.com/talks/unicode/
        I'm attempting to take the python unicode plunge and use unicode()
        objects as strings instead of str() objects everywhere in webkitpy.

        We do not have to use u"" instead of "" because u"a" == "a" as expected
        in Python.  Python will generate a warning to the console in cases where
        a unicode() == str() operation cannot be performed.

        I also cleaned up the input handling in run_command a little by adding
        a new _compute_input() method which can return early instead of having
        such a long/cluttered if-block.

        Executive.run* now correctly accept and return unicode() objects.
        I attempted to fix all the places that we call .write() to make sure we
        encode any unicode() objects into utf-8.

        All places which use StringIO need to be sure to pass StringIO a
        pre-encoded byte-array (str object) instead of unicode so that
        clients which read from the StringIO don't have encoding exceptions.
        To make this easier, I removed the patch_file_object support from
        add_patch_to_bug, and changed the 4 places which previously used
        StringIO to create a fake patch file.

        I attempted to document any places where we are not correctly converting
        to/from bytes (str() objects) to strings (unicode() objects).

        * Scripts/webkitpy/common/checkout/api_unittest.py:
         - Read/write utf-8 files instead of ascii.
         - Update the tests to use test for proper unicode() handling.
        * Scripts/webkitpy/common/checkout/changelog_unittest.py:
         - Use unicode() strings instead of str() byte arrays.
        * Scripts/webkitpy/common/checkout/scm.py:
         - Remove use of str().
        * Scripts/webkitpy/common/checkout/scm_unittest.py:
         - Read/write utf-8 files and use unicode() strings in testing.
        * Scripts/webkitpy/common/config/committers.py:
         - Use \u instead of \x to make slightly clearer what we're doing.
        * Scripts/webkitpy/common/net/bugzilla.py:
         - Add a new _string_contents() method and explain why
           we have to call unicode() on the result of soup.string
           and why it's safe to do so w/o needing to pass a codec name.
         - Remove the (unused) support for passing a file object to add_patch_to_bug().
        * Scripts/webkitpy/common/net/buildbot.py:
         - Use unicode() instead of str() when needing to coax a
           NavigableString object into a unicode() object.
        * Scripts/webkitpy/common/net/statusserver.py:
         - Remove use of str()
        * Scripts/webkitpy/common/prettypatch.py:
         - Write out the patch file as utf-8.
        * Scripts/webkitpy/common/system/autoinstall.py:
         - Add a FIXME about encoding.
        * Scripts/webkitpy/common/system/deprecated_logging.py:
         - Document that tee() works on bytes, not strings.
        * Scripts/webkitpy/common/system/executive.py:
         - Make run* properly take and return unicode() objects.
        * Scripts/webkitpy/common/system/executive_unittest.py:
         - Added a unit test to make sure we don't break Tor again!
        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
         - Write out the test list as utf-8.
        * Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:
         - Write out json files as utf-8.
        * Scripts/webkitpy/layout_tests/layout_package/metered_stream.py:
         - Add FIXME about encoding handling.
        * Scripts/webkitpy/tool/commands/upload.py:
         - Pass the diff directly to add_patch_to_bug instead of creating a StringIO file wrapper.
        * Scripts/webkitpy/tool/mocktool.py:
         - Rename add_patch_to_bug argument to match bugzilla.py
        * Scripts/webkitpy/tool/steps/postdiff.py:
         - Pass the diff directly to add_patch_to_bug instead of creating a StringIO file wrapper.
        * Scripts/webkitpy/tool/steps/postdiffforcommit.py: ditto.
        * Scripts/webkitpy/tool/steps/postdiffforrevert.py: ditto.
        * Scripts/webkitpy/tool/steps/steps_unittest.py:
         - Fixed spurious logging seen when running test-webkitpy

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@57907 268f45cc-cd09-0410-ab3c-d52691b4dbfc

32 files changed:
WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/common/checkout/api.py
WebKitTools/Scripts/webkitpy/common/checkout/api_unittest.py
WebKitTools/Scripts/webkitpy/common/checkout/changelog.py
WebKitTools/Scripts/webkitpy/common/checkout/changelog_unittest.py
WebKitTools/Scripts/webkitpy/common/checkout/commitinfo.py
WebKitTools/Scripts/webkitpy/common/checkout/scm.py
WebKitTools/Scripts/webkitpy/common/checkout/scm_unittest.py
WebKitTools/Scripts/webkitpy/common/config/committers.py
WebKitTools/Scripts/webkitpy/common/net/bugzilla.py
WebKitTools/Scripts/webkitpy/common/net/buildbot.py
WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py
WebKitTools/Scripts/webkitpy/common/net/statusserver.py
WebKitTools/Scripts/webkitpy/common/prettypatch.py
WebKitTools/Scripts/webkitpy/common/system/autoinstall.py
WebKitTools/Scripts/webkitpy/common/system/deprecated_logging.py
WebKitTools/Scripts/webkitpy/common/system/executive.py
WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/metered_stream.py
WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
WebKitTools/Scripts/webkitpy/tool/commands/queues.py
WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
WebKitTools/Scripts/webkitpy/tool/commands/upload.py
WebKitTools/Scripts/webkitpy/tool/mocktool.py
WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py
WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py
WebKitTools/Scripts/webkitpy/tool/steps/postdiffforcommit.py
WebKitTools/Scripts/webkitpy/tool/steps/postdiffforrevert.py
WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py

index a63c5be..9536812 100644 (file)
@@ -1,3 +1,93 @@
+2010-04-20  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        REGRESSION(57531): the commit-queue still hates Tor Arne Vestbø
+        https://bugs.webkit.org/show_bug.cgi?id=37765
+
+        I fixed the queue to not ignore Tor as a reviwer in r57531,
+        but instead it throws an exception every time his name is in a patch.
+
+        This fixes our Executive.run_command code to work around a Popen
+        bug http://bugs.python.org/issue5290 whereby python versions before 2.6
+        do not correctly handle unicode objects as input or output to
+        Popen.communicate.
+
+        Following the advice of:
+        http://farmdev.com/talks/unicode/
+        I'm attempting to take the python unicode plunge and use unicode()
+        objects as strings instead of str() objects everywhere in webkitpy.
+
+        We do not have to use u"" instead of "" because u"a" == "a" as expected
+        in Python.  Python will generate a warning to the console in cases where
+        a unicode() == str() operation cannot be performed.
+
+        I also cleaned up the input handling in run_command a little by adding
+        a new _compute_input() method which can return early instead of having
+        such a long/cluttered if-block.
+
+        Executive.run* now correctly accept and return unicode() objects.
+        I attempted to fix all the places that we call .write() to make sure we
+        encode any unicode() objects into utf-8.
+
+        All places which use StringIO need to be sure to pass StringIO a
+        pre-encoded byte-array (str object) instead of unicode so that
+        clients which read from the StringIO don't have encoding exceptions.
+        To make this easier, I removed the patch_file_object support from
+        add_patch_to_bug, and changed the 4 places which previously used
+        StringIO to create a fake patch file.
+
+        I attempted to document any places where we are not correctly converting
+        to/from bytes (str() objects) to strings (unicode() objects).
+
+        * Scripts/webkitpy/common/checkout/api_unittest.py:
+         - Read/write utf-8 files instead of ascii.
+         - Update the tests to use test for proper unicode() handling.
+        * Scripts/webkitpy/common/checkout/changelog_unittest.py:
+         - Use unicode() strings instead of str() byte arrays.
+        * Scripts/webkitpy/common/checkout/scm.py:
+         - Remove use of str().
+        * Scripts/webkitpy/common/checkout/scm_unittest.py:
+         - Read/write utf-8 files and use unicode() strings in testing.
+        * Scripts/webkitpy/common/config/committers.py:
+         - Use \u instead of \x to make slightly clearer what we're doing.
+        * Scripts/webkitpy/common/net/bugzilla.py:
+         - Add a new _string_contents() method and explain why
+           we have to call unicode() on the result of soup.string
+           and why it's safe to do so w/o needing to pass a codec name.
+         - Remove the (unused) support for passing a file object to add_patch_to_bug().
+        * Scripts/webkitpy/common/net/buildbot.py:
+         - Use unicode() instead of str() when needing to coax a
+           NavigableString object into a unicode() object.
+        * Scripts/webkitpy/common/net/statusserver.py:
+         - Remove use of str()
+        * Scripts/webkitpy/common/prettypatch.py:
+         - Write out the patch file as utf-8.
+        * Scripts/webkitpy/common/system/autoinstall.py:
+         - Add a FIXME about encoding.
+        * Scripts/webkitpy/common/system/deprecated_logging.py:
+         - Document that tee() works on bytes, not strings.
+        * Scripts/webkitpy/common/system/executive.py:
+         - Make run* properly take and return unicode() objects.
+        * Scripts/webkitpy/common/system/executive_unittest.py:
+         - Added a unit test to make sure we don't break Tor again!
+        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
+         - Write out the test list as utf-8.
+        * Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:
+         - Write out json files as utf-8.
+        * Scripts/webkitpy/layout_tests/layout_package/metered_stream.py:
+         - Add FIXME about encoding handling.
+        * Scripts/webkitpy/tool/commands/upload.py:
+         - Pass the diff directly to add_patch_to_bug instead of creating a StringIO file wrapper.
+        * Scripts/webkitpy/tool/mocktool.py:
+         - Rename add_patch_to_bug argument to match bugzilla.py
+        * Scripts/webkitpy/tool/steps/postdiff.py:
+         - Pass the diff directly to add_patch_to_bug instead of creating a StringIO file wrapper.
+        * Scripts/webkitpy/tool/steps/postdiffforcommit.py: ditto.
+        * Scripts/webkitpy/tool/steps/postdiffforrevert.py: ditto.
+        * Scripts/webkitpy/tool/steps/steps_unittest.py:
+         - Fixed spurious logging seen when running test-webkitpy
+
 2010-04-20  Chris Jerdonek  <cjerdonek@webkit.org>
 
         Reviewed by Shinichiro Hamaji.
index c4e2b69..e6dd3a1 100644 (file)
@@ -50,7 +50,8 @@ class Checkout(object):
 
     def _latest_entry_for_changelog_at_revision(self, changelog_path, revision):
         changelog_contents = self._scm.contents_at_revision(changelog_path, revision)
-        return ChangeLog.parse_latest_entry_from_file(StringIO.StringIO(changelog_contents))
+        fake_file = StringIO.StringIO(changelog_contents.encode("utf-8"))
+        return ChangeLog.parse_latest_entry_from_file(fake_file)
 
     def changelog_entries_for_revision(self, revision):
         changed_files = self._scm.changed_files_for_revision(revision)
index e99caee..7f7ad83 100644 (file)
@@ -26,6 +26,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import codecs
 import os
 import shutil
 import tempfile
@@ -39,12 +40,12 @@ from webkitpy.thirdparty.mock import Mock
 
 # FIXME: Copied from scm_unittest.py
 def write_into_file_at_path(file_path, contents):
-    new_file = open(file_path, 'w')
+    new_file = codecs.open(file_path, "w", "utf-8")
     new_file.write(contents)
     new_file.close()
 
 
-_changelog1entry1 = """2010-03-25  Eric Seidel  <eric@webkit.org>
+_changelog1entry1 = u"""2010-03-25  Tor Arne Vestb\u00f8  <vestbo@webkit.org>
 
         Unreviewed build fix to un-break webkit-patch land.
 
@@ -53,7 +54,7 @@ _changelog1entry1 = """2010-03-25  Eric Seidel  <eric@webkit.org>
 
         * Scripts/webkitpy/common/checkout/api.py: import scm.CommitMessage
 """
-_changelog1entry2 = """2010-03-25  Adam Barth  <abarth@webkit.org>
+_changelog1entry2 = u"""2010-03-25  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
 
@@ -62,8 +63,8 @@ _changelog1entry2 = """2010-03-25  Adam Barth  <abarth@webkit.org>
 
         * Scripts/webkitpy/common/checkout/api.py:
 """
-_changelog1 = "\n".join([_changelog1entry1, _changelog1entry2])
-_changelog2 = """2010-03-25  Eric Seidel  <eric@webkit.org>
+_changelog1 = u"\n".join([_changelog1entry1, _changelog1entry2])
+_changelog2 = u"""2010-03-25  Tor Arne Vestb\u00f8  <vestbo@webkit.org>
 
         Unreviewed build fix to un-break webkit-patch land.
 
@@ -79,7 +80,7 @@ _changelog2 = """2010-03-25  Eric Seidel  <eric@webkit.org>
 """
 
 class CommitMessageForThisCommitTest(unittest.TestCase):
-    expected_commit_message = """2010-03-25  Eric Seidel  <eric@webkit.org>
+    expected_commit_message = u"""2010-03-25  Tor Arne Vestb\u00f8  <vestbo@webkit.org>
 
         Unreviewed build fix to un-break webkit-patch land.
 
@@ -87,7 +88,7 @@ class CommitMessageForThisCommitTest(unittest.TestCase):
         https://bugs.webkit.org/show_bug.cgi?id=36629
 
         * Scripts/webkitpy/common/checkout/api.py: import scm.CommitMessage
-2010-03-25  Eric Seidel  <eric@webkit.org>
+2010-03-25  Tor Arne Vestb\u00f8  <vestbo@webkit.org>
 
         Unreviewed build fix to un-break webkit-patch land.
 
@@ -137,8 +138,8 @@ class CheckoutTest(unittest.TestCase):
         checkout.changelog_entries_for_revision = lambda revision: [ChangeLogEntry(_changelog1entry1)]
         commitinfo = checkout.commit_info_for_revision(4)
         self.assertEqual(commitinfo.bug_id(), 36629)
-        self.assertEqual(commitinfo.author_name(), "Eric Seidel")
-        self.assertEqual(commitinfo.author_email(), "eric@webkit.org")
+        self.assertEqual(commitinfo.author_name(), u"Tor Arne Vestb\u00f8")
+        self.assertEqual(commitinfo.author_email(), "vestbo@webkit.org")
         self.assertEqual(commitinfo.reviewer_text(), None)
         self.assertEqual(commitinfo.reviewer(), None)
         self.assertEqual(commitinfo.committer_email(), "committer@example.com")
index e93896f..862dc48 100644 (file)
@@ -102,12 +102,13 @@ class ChangeLog(object):
         date_line_regexp = re.compile(ChangeLogEntry.date_line_regexp)
         entry_lines = []
         # The first line should be a date line.
-        first_line = changelog_file.readline()
+        first_line = unicode(changelog_file.readline(), "utf-8")
         if not date_line_regexp.match(first_line):
             return None
         entry_lines.append(first_line)
 
         for line in changelog_file:
+            line = unicode(line, "utf-8")
             # If we've hit the next entry, return.
             if date_line_regexp.match(line):
                 # Remove the extra newline at the end
@@ -117,7 +118,9 @@ class ChangeLog(object):
 
     def latest_entry(self):
         # ChangeLog files are always UTF-8, we read them in as such to support Reviewers with unicode in their names.
-        changelog_file = codecs.open(self.path, "r", "utf-8")
+        # We don't use codecs.open here to make the api for parse_latest_entry_from_file clearer.
+        # If we did, then it would be unclear as to whos reponsibility decoding of the file should be.
+        changelog_file = open(self.path, "r")
         try:
             return self.parse_latest_entry_from_file(changelog_file)
         finally:
index 9210c9c..9e7ab2c 100644 (file)
@@ -87,8 +87,8 @@ class ChangeLogsTest(unittest.TestCase):
 '''
 
     def test_latest_entry_parse(self):
-        changelog_contents = "%s\n%s" % (self._example_entry, self._example_changelog)
-        changelog_file = StringIO(changelog_contents)
+        changelog_contents = u"%s\n%s" % (self._example_entry, self._example_changelog)
+        changelog_file = StringIO(changelog_contents.encode("utf-8"))
         latest_entry = ChangeLog.parse_latest_entry_from_file(changelog_file)
         self.assertEquals(latest_entry.contents(), self._example_entry)
         self.assertEquals(latest_entry.author_name(), "Peter Kasting")
@@ -121,7 +121,7 @@ class ChangeLogsTest(unittest.TestCase):
 '''
 
     def test_set_reviewer(self):
-        changelog_contents = "%s\n%s" % (self._new_entry_boilerplate, self._example_changelog)
+        changelog_contents = u"%s\n%s" % (self._new_entry_boilerplate, self._example_changelog)
         changelog_path = self._write_tmp_file_with_contents(changelog_contents)
         reviewer_name = 'Test Reviewer'
         ChangeLog(changelog_path).set_reviewer(reviewer_name)
@@ -169,7 +169,7 @@ class ChangeLogsTest(unittest.TestCase):
 '''
 
     def _assert_update_for_revert_output(self, args, expected_entry):
-        changelog_contents = "%s\n%s" % (self._new_entry_boilerplate, self._example_changelog)
+        changelog_contents = u"%s\n%s" % (self._new_entry_boilerplate, self._example_changelog)
         changelog_path = self._write_tmp_file_with_contents(changelog_contents)
         changelog = ChangeLog(changelog_path)
         changelog.update_for_revert(*args)
index 7c3315f..448d530 100644 (file)
@@ -28,8 +28,6 @@
 #
 # WebKit's python module for holding information on a commit
 
-import StringIO
-
 from webkitpy.common.checkout.changelog import view_source_url
 from webkitpy.common.config.committers import CommitterList
 
index 2704f07..c4f48ce 100644 (file)
@@ -145,7 +145,7 @@ class SCM:
         return filenames
 
     def strip_r_from_svn_revision(self, svn_revision):
-        match = re.match("^r(?P<svn_revision>\d+)", svn_revision)
+        match = re.match("^r(?P<svn_revision>\d+)", unicode(svn_revision))
         if (match):
             return match.group('svn_revision')
         return svn_revision
@@ -344,7 +344,7 @@ class SVN(SCM):
 
     def changed_files_for_revision(self, revision):
         # As far as I can tell svn diff --summarize output looks just like svn status output.
-        status_command = ["svn", "diff", "--summarize", "-c", str(revision)]
+        status_command = ["svn", "diff", "--summarize", "-c", revision]
         return self.run_status_and_extract_filenames(status_command, self._status_regexp("ACDMR"))
 
     def conflicted_files(self):
@@ -364,15 +364,15 @@ class SVN(SCM):
         return run_command(self.script_path("svn-create-patch"), cwd=self.checkout_root, return_stderr=False)
 
     def committer_email_for_revision(self, revision):
-        return run_command(["svn", "propget", "svn:author", "--revprop", "-r", str(revision)]).rstrip()
+        return run_command(["svn", "propget", "svn:author", "--revprop", "-r", revision]).rstrip()
 
     def contents_at_revision(self, path, revision):
         remote_path = "%s/%s" % (self._repository_url(), path)
-        return run_command(["svn", "cat", "-r", str(revision), remote_path])
+        return run_command(["svn", "cat", "-r", revision, remote_path])
 
     def diff_for_revision(self, revision):
         # FIXME: This should probably use cwd=self.checkout_root
-        return run_command(['svn', 'diff', '-c', str(revision)])
+        return run_command(['svn', 'diff', '-c', revision])
 
     def _repository_url(self):
         return self.value_from_svn_info(self.checkout_root, 'URL')
@@ -405,7 +405,7 @@ class SVN(SCM):
         return run_command(svn_commit_args, error_handler=commit_error_handler)
 
     def svn_commit_log(self, svn_revision):
-        svn_revision = self.strip_r_from_svn_revision(str(svn_revision))
+        svn_revision = self.strip_r_from_svn_revision(svn_revision)
         return run_command(['svn', 'log', '--non-interactive', '--revision', svn_revision]);
 
     def last_svn_commit_log(self):
index c0a64d4..9727fb7 100644 (file)
@@ -28,6 +28,7 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import base64
+import codecs
 import getpass
 import os
 import os.path
@@ -57,12 +58,12 @@ def run_silent(args, cwd=None):
         raise ScriptError('Failed to run "%s"  exit_code: %d  cwd: %s' % (args, exit_code, cwd))
 
 def write_into_file_at_path(file_path, contents):
-    file = open(file_path, 'w')
+    file = codecs.open(file_path, "w", "utf-8")
     file.write(contents)
     file.close()
 
 def read_from_path(file_path):
-    file = open(file_path, 'r')
+    file = codecs.open(file_path, "r", "utf-8")
     contents = file.read()
     file.close()
     return contents
index 58b2885..655f23c 100644 (file)
@@ -239,7 +239,7 @@ reviewers_list = [
     Reviewer("Steve Falkenburg", "sfalken@apple.com", "sfalken"),
     Reviewer("Tim Omernick", "timo@apple.com"),
     Reviewer("Timothy Hatcher", ["timothy@hatcher.name", "timothy@apple.com"], "xenon"),
-    Reviewer(u'Tor Arne Vestb\xf8', "vestbo@webkit.org", "torarne"),
+    Reviewer(u"Tor Arne Vestb\u00f8", "vestbo@webkit.org", "torarne"),
     Reviewer("Vicki Murley", "vicki@apple.com"),
     Reviewer("Xan Lopez", ["xan.lopez@gmail.com", "xan@gnome.org", "xan@webkit.org"], "xan"),
     Reviewer("Yury Semikhatsky", "yurys@chromium.org", "yurys"),
index bb22d14..2dd4d20 100644 (file)
@@ -32,6 +32,7 @@
 
 import os.path
 import re
+import StringIO
 import subprocess
 
 from datetime import datetime # used in timestamp()
@@ -411,7 +412,16 @@ class Bugzilla(object):
             if flag['status'] == '+':
                 attachment[result_key] = flag['setter']
 
+    def _string_contents(self, soup):
+        # WebKit's bugzilla instance uses UTF-8.
+        # BeautifulSoup always returns Unicode strings, however
+        # the .string method returns a (unicode) NavigableString.
+        # NavigableString can confuse other parts of the code, so we
+        # convert from NavigableString to a real unicode() object using unicode().
+        return unicode(soup.string)
+
     def _parse_attachment_element(self, element, bug_id):
+
         attachment = {}
         attachment['bug_id'] = bug_id
         attachment['is_obsolete'] = (element.has_key('isobsolete') and element['isobsolete'] == "1")
@@ -419,9 +429,9 @@ class Bugzilla(object):
         attachment['id'] = int(element.find('attachid').string)
         # FIXME: No need to parse out the url here.
         attachment['url'] = self.attachment_url_for_id(attachment['id'])
-        attachment['name'] = unicode(element.find('desc').string)
-        attachment['attacher_email'] = str(element.find('attacher').string)
-        attachment['type'] = str(element.find('type').string)
+        attachment['name'] = self._string_contents(element.find('desc'))
+        attachment['attacher_email'] = self._string_contents(element.find('attacher'))
+        attachment['type'] = self._string_contents(element.find('type'))
         self._parse_attachment_flag(
                 element, 'review', attachment, 'reviewer_email')
         self._parse_attachment_flag(
@@ -432,10 +442,10 @@ class Bugzilla(object):
         soup = BeautifulSoup(page)
         bug = {}
         bug["id"] = int(soup.find("bug_id").string)
-        bug["title"] = unicode(soup.find("short_desc").string)
-        bug["reporter_email"] = str(soup.find("reporter").string)
-        bug["assigned_to_email"] = str(soup.find("assigned_to").string)
-        bug["cc_emails"] = [str(element.string)
+        bug["title"] = self._string_contents(soup.find("short_desc"))
+        bug["reporter_email"] = self._string_contents(soup.find("reporter"))
+        bug["assigned_to_email"] = self._string_contents(soup.find("assigned_to"))
+        bug["cc_emails"] = [self._string_contents(element)
                             for element in soup.findAll('cc')]
         bug["attachments"] = [self._parse_attachment_element(element, bug["id"]) for element in soup.findAll('attachment')]
         return bug
@@ -532,7 +542,7 @@ class Bugzilla(object):
 
     def _fill_attachment_form(self,
                               description,
-                              patch_file_object,
+                              diff,
                               comment_text=None,
                               mark_for_review=False,
                               mark_for_commit_queue=False,
@@ -552,6 +562,9 @@ class Bugzilla(object):
             patch_name = "bug-%s-%s.patch" % (bug_id, timestamp())
         else:
             patch_name ="%s.patch" % timestamp()
+
+        # ClientForm expects a file-like object
+        patch_file_object = StringIO.StringIO(diff.encode("utf-8"))
         self.browser.add_file(patch_file_object,
                               "text/plain",
                               patch_name,
@@ -559,7 +572,7 @@ class Bugzilla(object):
 
     def add_patch_to_bug(self,
                          bug_id,
-                         patch_file_object,
+                         diff,
                          description,
                          comment_text=None,
                          mark_for_review=False,
@@ -578,8 +591,9 @@ class Bugzilla(object):
         self.browser.open("%sattachment.cgi?action=enter&bugid=%s" % (
                           self.bug_server_url, bug_id))
         self.browser.select_form(name="entryform")
+
         self._fill_attachment_form(description,
-                                   patch_file_object,
+                                   diff,
                                    mark_for_review=mark_for_review,
                                    mark_for_commit_queue=mark_for_commit_queue,
                                    mark_for_landing=mark_for_landing,
@@ -612,7 +626,7 @@ class Bugzilla(object):
                    bug_title,
                    bug_description,
                    component=None,
-                   patch_file_object=None,
+                   diff=None,
                    patch_description=None,
                    cc=None,
                    blocked=None,
@@ -637,14 +651,14 @@ class Bugzilla(object):
         if cc:
             self.browser["cc"] = cc
         if blocked:
-            self.browser["blocked"] = str(blocked)
+            self.browser["blocked"] = unicode(blocked)
         self.browser["short_desc"] = bug_title
         self.browser["comment"] = bug_description
 
-        if patch_file_object:
+        if diff:
             self._fill_attachment_form(
                     patch_description,
-                    patch_file_object,
+                    diff,
                     mark_for_review=mark_for_review,
                     mark_for_commit_queue=mark_for_commit_queue)
 
index 753e909..e4e6273 100644 (file)
@@ -44,7 +44,7 @@ _log = get_logger(__file__)
 
 class Builder(object):
     def __init__(self, name, buildbot):
-        self._name = unicode(name)
+        self._name = name
         self._buildbot = buildbot
         self._builds_cache = {}
         self._revision_to_build_number = None
@@ -223,12 +223,12 @@ class LayoutTestResults(object):
         parsed_results = {}
         tables = BeautifulSoup(page).findAll("table")
         for table in tables:
-            table_title = table.findPreviousSibling("p").string
+            table_title = unicode(table.findPreviousSibling("p").string)
             if table_title not in cls.expected_keys:
                 # This Exception should only ever be hit if run-webkit-tests changes its results.html format.
-                raise Exception("Unhandled title: %s" % str(table_title))
+                raise Exception("Unhandled title: %s" % table_title)
             # We might want to translate table titles into identifiers before storing.
-            parsed_results[table_title] = [row.find("a").string for row in table.findAll("tr")]
+            parsed_results[table_title] = [unicode(row.find("a").string) for row in table.findAll("tr")]
 
         return parsed_results
 
@@ -361,7 +361,7 @@ class BuildBot(object):
 
         # First cell is the name
         name_link = status_cells[0].find('a')
-        builder["name"] = name_link.string
+        builder["name"] = unicode(name_link.string)
 
         self._parse_last_build_cell(builder, status_cells[1])
         self._parse_current_build_cell(builder, status_cells[2])
@@ -410,13 +410,13 @@ class BuildBot(object):
         return urllib2.urlopen(build_status_url)
 
     def _parse_twisted_file_row(self, file_row):
-        string_or_empty = lambda string: str(string) if string else ""
+        string_or_empty = lambda soup: unicode(soup.string) if soup.string else u""
         file_cells = file_row.findAll('td')
         return {
-            "filename" : string_or_empty(file_cells[0].find("a").string),
-            "size" : string_or_empty(file_cells[1].string),
-            "type" : string_or_empty(file_cells[2].string),
-            "encoding" : string_or_empty(file_cells[3].string),
+            "filename": string_or_empty(file_cells[0].find("a")),
+            "size": string_or_empty(file_cells[1]),
+            "type": string_or_empty(file_cells[2]),
+            "encoding": string_or_empty(file_cells[3]),
         }
 
     def _parse_twisted_directory_listing(self, page):
index f765f6e..67c46ed 100644 (file)
@@ -51,7 +51,7 @@ class BuilderTest(unittest.TestCase):
 
     def setUp(self):
         self.buildbot = BuildBot()
-        self.builder = Builder("Test Builder", self.buildbot)
+        self.builder = Builder(u"Test Builder \u2661", self.buildbot)
         self._install_fetch_build(lambda build_number: ["test1", "test2"])
 
     def test_find_failure_transition(self):
index e8987a9..d9b52a2 100644 (file)
@@ -52,9 +52,9 @@ class StatusServer:
         if not patch:
             return
         if patch.bug_id():
-            self.browser["bug_id"] = str(patch.bug_id())
+            self.browser["bug_id"] = unicode(patch.bug_id())
         if patch.id():
-            self.browser["patch_id"] = str(patch.id())
+            self.browser["patch_id"] = unicode(patch.id())
 
     def _add_results_file(self, results_file):
         if not results_file:
@@ -79,7 +79,7 @@ class StatusServer:
         update_svn_revision_url = "%s/update-svn-revision" % self.url
         self.browser.open(update_svn_revision_url)
         self.browser.select_form(name="update_svn_revision")
-        self.browser["number"] = str(svn_revision_number)
+        self.browser["number"] = unicode(svn_revision_number)
         self.browser["broken_bot"] = broken_bot
         return self.browser.submit().read()
 
index 8157f9c..e7c876a 100644 (file)
@@ -38,7 +38,7 @@ class PrettyPatch(object):
     def pretty_diff_file(self, diff):
         pretty_diff = self.pretty_diff(diff)
         diff_file = tempfile.NamedTemporaryFile(suffix=".html")
-        diff_file.write(pretty_diff)
+        diff_file.write(pretty_diff.encode("utf-8"))
         diff_file.flush()
         return diff_file
 
index 32fd2cf..35006b1 100755 (executable)
@@ -124,7 +124,7 @@ class AutoInstaller(object):
         _log.debug('    "%s"' % path)
         file = open(path, "w")
         try:
-            file.write(text)
+            file.write(text)  # FIXME: What's the encoding of this file?
         finally:
             file.close()
 
index ba1c5eb..f7e59a3 100644 (file)
@@ -45,9 +45,10 @@ class tee:
     def __init__(self, *files):
         self.files = files
 
-    def write(self, string):
+    # Callers should pass an already encoded string for writing.
+    def write(self, bytes):
         for file in self.files:
-            file.write(string)
+            file.write(bytes)
 
 class OutputTee:
     def __init__(self):
index b6126e4..cbe0e1f 100644 (file)
@@ -88,6 +88,7 @@ def run_command(*args, **kwargs):
 class Executive(object):
 
     def _run_command_with_teed_output(self, args, teed_output):
+        args = map(unicode, args)  # Popen will throw an exception if args are non-strings (like int())
         child_process = subprocess.Popen(args,
                                          stdout=subprocess.PIPE,
                                          stderr=subprocess.STDOUT)
@@ -99,9 +100,16 @@ class Executive(object):
             output_line = child_process.stdout.readline()
             if output_line == "" and child_process.poll() != None:
                 return child_process.poll()
+            # We assume that the child process wrote to us in utf-8,
+            # so no re-encoding is necessary before writing here.
             teed_output.write(output_line)
 
-    def run_and_throw_if_fail(self, args, quiet=False):
+    # FIXME: Remove this deprecated method and move callers to run_command.
+    # FIXME: This method is a hack to allow running command which both
+    # capture their output and print out to stdin.  Useful for things
+    # like "build-webkit" where we want to display to the user that we're building
+    # but still have the output to stuff into a log file.
+    def run_and_throw_if_fail(self, args, quiet=False, decode_output=True):
         # Cache the child's output locally so it can be used for error reports.
         child_out_file = StringIO.StringIO()
         tee_stdout = sys.stdout
@@ -116,6 +124,10 @@ class Executive(object):
         child_output = child_out_file.getvalue()
         child_out_file.close()
 
+        # We assume the child process output utf-8
+        if decode_output:
+            child_output = child_output.decode("utf-8")
+
         if exit_code:
             raise ScriptError(script_args=args,
                               exit_code=exit_code,
@@ -145,7 +157,7 @@ class Executive(object):
             # os.kill isn't available on Windows.  However, when I tried it
             # using Cygwin, it worked fine.  We should investigate whether
             # we need this platform specific code here.
-            subprocess.call(('taskkill.exe', '/f', '/pid', str(pid)),
+            subprocess.call(('taskkill.exe', '/f', '/pid', unicode(pid)),
                             stdin=open(os.devnull, 'r'),
                             stdout=subprocess.PIPE,
                             stderr=subprocess.PIPE)
@@ -163,31 +175,34 @@ class Executive(object):
     def ignore_error(error):
         pass
 
-    # FIXME: This should be merged with run_and_throw_if_fail
-
+    def _compute_stdin(self, input):
+        """Returns (stdin, string_to_communicate)"""
+        if not input:
+            return (None, None)
+        if hasattr(input, "read"):  # Check if the input is a file.
+            return (input, None)  # Assume the file is in the right encoding.
+
+        # Popen in Python 2.5 and before does not automatically encode unicode objects.
+        # http://bugs.python.org/issue5290
+        # See https://bugs.webkit.org/show_bug.cgi?id=37528
+        # for an example of a regresion caused by passing a unicode string directly.
+        # FIXME: We may need to encode differently on different platforms.
+        if isinstance(input, unicode):
+            input = input.encode("utf-8")
+        return (subprocess.PIPE, input)
+
+    # FIXME: run_and_throw_if_fail should be merged into this method.
     def run_command(self,
                     args,
                     cwd=None,
                     input=None,
                     error_handler=None,
                     return_exit_code=False,
-                    return_stderr=True):
-        if hasattr(input, 'read'): # Check if the input is a file.
-            stdin = input
-            string_to_communicate = None
-        else:
-            stdin = None
-            if input:
-                stdin = subprocess.PIPE
-            # string_to_communicate seems to need to be a str for proper
-            # communication with shell commands.
-            # See https://bugs.webkit.org/show_bug.cgi?id=37528
-            # For an example of a regresion caused by passing a unicode string through.
-            string_to_communicate = str(input)
-        if return_stderr:
-            stderr = subprocess.STDOUT
-        else:
-            stderr = None
+                    return_stderr=True,
+                    decode_output=True):
+        args = map(unicode, args)  # Popen will throw an exception if args are non-strings (like int())
+        stdin, string_to_communicate = self._compute_stdin(input)
+        stderr = subprocess.STDOUT if return_stderr else None
 
         process = subprocess.Popen(args,
                                    stdin=stdin,
@@ -195,6 +210,9 @@ class Executive(object):
                                    stderr=stderr,
                                    cwd=cwd)
         output = process.communicate(string_to_communicate)[0]
+        # run_command automatically decodes to unicode() unless explicitly told not to.
+        if decode_output:
+            output = output.decode("utf-8")
         exit_code = process.wait()
 
         if return_exit_code:
index ac380f8..64c552b 100644 (file)
@@ -1,4 +1,4 @@
-# Copyright (C) 2009 Google Inc. All rights reserved.
+# Copyright (C) 2010 Google Inc. All rights reserved.
 # Copyright (C) 2009 Daniel Bates (dbates@intudata.com). All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -31,6 +31,7 @@ import unittest
 
 from webkitpy.common.system.executive import Executive, run_command
 
+
 class ExecutiveTest(unittest.TestCase):
 
     def test_run_command_with_bad_command(self):
@@ -38,5 +39,26 @@ class ExecutiveTest(unittest.TestCase):
             run_command(["foo_bar_command_blah"], error_handler=Executive.ignore_error, return_exit_code=True)
         self.failUnlessRaises(OSError, run_bad_command)
 
-if __name__ == '__main__':
-    unittest.main()
+    def test_run_command_with_unicode(self):
+        """Validate that it is safe to pass unicode() objects
+        to Executive.run* methods, and they will return unicode()
+        objects by default unless decode_output=False"""
+        executive = Executive()
+        unicode_tor = u"WebKit \u2661 Tor Arne Vestb\u00F8!"
+        utf8_tor = unicode_tor.encode("utf-8")
+
+        output = executive.run_command(["cat"], input=unicode_tor)
+        self.assertEquals(output, unicode_tor)
+
+        output = executive.run_command(["echo", "-n", unicode_tor])
+        self.assertEquals(output, unicode_tor)
+
+        output = executive.run_command(["echo", "-n", unicode_tor], decode_output=False)
+        self.assertEquals(output, utf8_tor)
+
+        # FIXME: We should only have one run* method to test
+        output = executive.run_and_throw_if_fail(["echo", "-n", unicode_tor], quiet=True)
+        self.assertEquals(output, unicode_tor)
+
+        output = executive.run_and_throw_if_fail(["echo", "-n", unicode_tor], quiet=True, decode_output=False)
+        self.assertEquals(output, utf8_tor)
index 93b4c79..6548ef1 100644 (file)
@@ -35,6 +35,7 @@ the output.  When there are no more URLs to process in the shared queue, the
 thread exits.
 """
 
+import codecs
 import copy
 import logging
 import os
@@ -286,7 +287,7 @@ class TestShellThread(threading.Thread):
         # This is created in run_webkit_tests.py:_PrepareListsAndPrintOutput.
         tests_run_filename = os.path.join(self._options.results_directory,
                                           "tests_run.txt")
-        tests_run_file = open(tests_run_filename, "a")
+        tests_run_file = codecs.open(tests_run_filename, "a", "utf-8")
 
         while True:
             if self._canceled:
index 6263540..a0592d3 100644 (file)
@@ -27,6 +27,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import codecs
 import logging
 import os
 import subprocess
@@ -118,7 +119,7 @@ class JSONResultsGenerator(object):
         """Generates the JSON output file."""
         json = self._get_json()
         if json:
-            results_file = open(self._results_file_path, "w")
+            results_file = codecs.open(self._results_file_path, "w", "utf-8")
             results_file.write(json)
             results_file.close()
 
index 930b9e4..6de5d49 100644 (file)
@@ -60,6 +60,8 @@ class MeteredStream:
         self._stream = stream
         self._last_update = ""
 
+    # FIXME: Does this take a string (unicode) or an array of bytes (str)?
+    # If it takes a string, it needs to call txt.encode("utf-8")
     def write(self, txt):
         """Write to the stream, overwriting and resetting the meter."""
         if self._dirty:
@@ -86,6 +88,8 @@ class MeteredStream:
             return
         self._write(str)
 
+    # FIXME: Does this take a string (unicode) or an array of bytes (str)?
+    # If it takes a string, it needs to call txt.encode("utf-8")
     def update(self, str):
         """
         Write a message that is also included when logging verbosely.
index e47a4a4..698c6d0 100644 (file)
@@ -40,7 +40,7 @@ class MacTest(unittest.TestCase):
         relative_paths = [path[len(port.path_from_webkit_base()):] for path in skipped_paths]
         self.assertEqual(relative_paths, ['LayoutTests/platform/mac-leopard/Skipped', 'LayoutTests/platform/mac/Skipped'])
 
-    example_skipped_file = """
+    example_skipped_file = u"""
 # <rdar://problem/5647952> fast/events/mouseout-on-window.html needs mac DRT to issue mouse out events
 fast/events/mouseout-on-window.html
 
@@ -58,7 +58,7 @@ svg/batik/text/smallFonts.svg
 
     def test_skipped_file_paths(self):
         port = mac.MacPort()
-        skipped_file = StringIO.StringIO(self.example_skipped_file)
+        skipped_file = StringIO.StringIO(self.example_skipped_file.encode("utf-8"))
         self.assertEqual(port._tests_from_skipped_file(skipped_file), self.example_skipped_tests)
 
 
index 9ea34c0..7505c62 100644 (file)
@@ -26,8 +26,6 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-from StringIO import StringIO
-
 from webkitpy.tool.commands.queues import AbstractReviewQueue
 from webkitpy.common.config.committers import CommitterList
 from webkitpy.common.config.ports import WebKitPort
index f0da379..3318531 100644 (file)
@@ -71,8 +71,9 @@ class AbstractQueue(Command, QueueEngineDelegate):
     def run_webkit_patch(self, args):
         webkit_patch_args = [self.tool.path()]
         # FIXME: This is a hack, we should have a more general way to pass global options.
-        webkit_patch_args += ["--status-host=%s" % self.tool.status_server.host]
-        webkit_patch_args += map(str, args)
+        webkit_patch_args.append("--status-host")
+        webkit_patch_args.append(self.tool.status_server.host)
+        webkit_patch_args.extend(args)
         return self.tool.executive.run_and_throw_if_fail(webkit_patch_args)
 
     def _log_directory(self):
@@ -123,7 +124,7 @@ class AbstractQueue(Command, QueueEngineDelegate):
         if is_error:
             message = "Error: %s" % message
         output = script_error.message_with_output(output_limit=1024*1024) # 1MB
-        return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(output))
+        return tool.status_server.update_status(cls.name, message, state["patch"], StringIO(output.encode("utf-8")))
 
 
 class AbstractPatchQueue(AbstractQueue):
index f0f7c86..ee55128 100644 (file)
@@ -75,7 +75,7 @@ class AbstractQueueTest(CommandsTest):
         queue.bind_to_tool(tool)
 
         queue.run_webkit_patch(run_args)
-        expected_run_args = ["echo", "--status-host=example.com"] + map(str, run_args)
+        expected_run_args = ["echo", "--status-host", "example.com"] + run_args
         tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args)
 
     def test_run_webkit_patch(self):
@@ -150,7 +150,7 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c
 Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
 1 patch in commit-queue [106]
 """,
-            "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '76543']\n",
+            "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host', 'example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\n",
         }
         self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr)
 
index bdf060a..33bca7a 100644 (file)
@@ -30,7 +30,6 @@
 
 import os
 import re
-import StringIO
 import sys
 
 from optparse import make_option
@@ -260,10 +259,6 @@ class PostCommits(AbstractDeclarativeCommand):
             comment_text += tool.scm().files_changed_summary_for_commit(commit_id)
         return comment_text
 
-    def _diff_file_for_commit(self, tool, commit_id):
-        diff = tool.scm().create_patch_from_local_commit(commit_id)
-        return StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
-
     def execute(self, options, args, tool):
         commit_ids = tool.scm().commit_ids_from_commitish_arguments(args)
         if len(commit_ids) > 10: # We could lower this limit, 10 is too many for one bug as-is.
@@ -284,10 +279,10 @@ class PostCommits(AbstractDeclarativeCommand):
                 steps.ObsoletePatches(tool, options).run(state)
                 have_obsoleted_patches.add(bug_id)
 
-            diff_file = self._diff_file_for_commit(tool, commit_id)
+            diff = tool.scm().create_patch_from_local_commit(commit_id)
             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_file, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+            tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
 
 
 # FIXME: This command needs to be brought into the modern age with steps and CommitInfo.
@@ -404,8 +399,7 @@ class CreateBug(AbstractDeclarativeCommand):
             comment_text += tool.scm().files_changed_summary_for_commit(commit_id)
 
         diff = tool.scm().create_patch_from_local_commit(commit_id)
-        diff_file = StringIO.StringIO(diff) # create_bug expects a file-like object
-        bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff_file, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+        bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
 
         if bug_id and len(commit_ids) > 1:
             options.bug_id = bug_id
@@ -424,8 +418,7 @@ class CreateBug(AbstractDeclarativeCommand):
             comment_text = commit_message.body(lstrip=True)
 
         diff = tool.scm().create_patch()
-        diff_file = StringIO.StringIO(diff) # create_bug expects a file-like object
-        bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff_file, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
+        bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit)
 
     def prompt_for_bug_title_and_comment(self):
         bug_title = User.prompt("Bug title: ")
index c56c53e..7d314d1 100644 (file)
@@ -260,7 +260,7 @@ class MockBugzilla(Mock):
                    bug_title,
                    bug_description,
                    component=None,
-                   patch_file_object=None,
+                   diff=None,
                    patch_description=None,
                    cc=None,
                    blocked=None,
@@ -299,7 +299,7 @@ class MockBugzilla(Mock):
 
     def add_patch_to_bug(self,
                          bug_id,
-                         patch_file_object,
+                         diff,
                          description,
                          comment_text=None,
                          mark_for_review=False,
index 1ad343d..2187fc0 100644 (file)
@@ -39,7 +39,7 @@ class AbstractStep(object):
     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)
+        self._tool.executive.run_and_throw_if_fail([port.script_path(script_name)], quiet)
 
     # FIXME: The port should live on the tool.
     def port(self):
index 6a3dee4..80dacca 100644 (file)
@@ -26,8 +26,6 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import StringIO
-
 from webkitpy.tool.steps.abstractstep import AbstractStep
 from webkitpy.tool.steps.options import Options
 
@@ -44,7 +42,6 @@ class PostDiff(AbstractStep):
 
     def run(self, state):
         diff = self.cached_lookup(state, "diff")
-        diff_file = StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
         description = self._options.description or "Patch"
         comment_text = None
         codereview_issue = state.get("codereview_issue")
@@ -52,6 +49,6 @@ class PostDiff(AbstractStep):
         # but it makes doing the rietveld integration a lot easier.
         if codereview_issue:
             description += "-%s" % state["codereview_issue"]
-        self._tool.bugs.add_patch_to_bug(state["bug_id"], diff_file, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit)
+        self._tool.bugs.add_patch_to_bug(state["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(state["bug_id"]))
index 03b9e78..13bc00c 100644 (file)
@@ -26,8 +26,6 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import StringIO
-
 from webkitpy.tool.steps.abstractstep import AbstractStep
 
 
@@ -35,7 +33,7 @@ class PostDiffForCommit(AbstractStep):
     def run(self, state):
         self._tool.bugs.add_patch_to_bug(
             state["bug_id"],
-            StringIO.StringIO(self.cached_lookup(state, "diff")),
+            self.cached_lookup(state, "diff"),
             "Patch for landing",
             mark_for_review=False,
             mark_for_landing=True)
index 3b9da04..bfa631f 100644 (file)
@@ -26,8 +26,6 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import StringIO
-
 from webkitpy.common.net.bugzilla import Attachment
 from webkitpy.tool.steps.abstractstep import AbstractStep
 
@@ -44,7 +42,7 @@ following command:\n\n\
 where ATTACHMENT_ID is the ID of this attachment."
         self._tool.bugs.add_patch_to_bug(
             state["bug_id"],
-            StringIO.StringIO(self.cached_lookup(state, "diff")),
+            self.cached_lookup(state, "diff"),
             "%s%s" % (Attachment.rollout_preamble, state["revision"]),
             comment_text=comment_text,
             mark_for_review=False,
index 40bee90..5abfc6d 100644 (file)
@@ -48,7 +48,8 @@ class StepsTest(unittest.TestCase):
     def test_update_step(self):
         options = Mock()
         options.update = True
-        self._run_step(Update, options)
+        expected_stderr = "Updating working directory\n"
+        OutputCapture().assert_outputs(self, self._run_step, [Update, options], expected_stderr=expected_stderr)
 
     def test_prompt_for_bug_or_title_step(self):
         tool = MockTool()