Changeset 84676 in webkit

Timestamp:

Apr 22, 2011 1:44:46 PM (13 years ago)

Author:

eric@webkit.org

Message:

2011-04-22 Eric Seidel <​eric@webkit.org>

Reviewed by Dimitri Glazkov.

webkit-patch rebaseline is broken
​https://bugs.webkit.org/show_bug.cgi?id=59207

My recent changes to how LayoutTestResults.results_from_string
uncovered a bug in our use of urllib2.
We now are treating the return value from _fetch_results_html()
(which returns urllib2.openurl()) as a string. That's incorrect,
the return value is a file-like object (which auto-converts to a
string in most cases).

I've updated our urllib2-using code to correctly call .read() on the
resulting object before treating it like a string.
Unfortunately there is no easy way to test this mis-understanding
of the openurl() semantics.

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

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/net/buildbot/buildbot.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-04-22  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        webkit-patch rebaseline is broken
+        https://bugs.webkit.org/show_bug.cgi?id=59207
+
+        My recent changes to how LayoutTestResults.results_from_string
+        uncovered a bug in our use of urllib2.
+        We now are treating the return value from _fetch_results_html()
+        (which returns urllib2.openurl()) as a string.  That's incorrect,
+        the return value is a file-like object (which auto-converts to a
+        string in most cases).
+
+        I've updated our urllib2-using code to correctly call .read() on the
+        resulting object before treating it like a string.
+        Unfortunately there is no easy way to test this mis-understanding
+        of the openurl() semantics.
+
+        * Scripts/webkitpy/common/net/buildbot/buildbot.py:
+
 2011-04-22  Adam Barth  <abarth@webkit.org>
 

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

@@ -238 +238 @@
     def _fetch_results_html(self):
         results_html = "%s/results.html" % (self.results_url())
-        # FIXME: This should use NetworkTransaction's 404 handling instead.
+        # FIXME: This should use NetworkTransaction's 404 handling or at least move
+        # to mechanize's to be more consistent with the rest of our code.
         try:
             # It seems this can return None if the url redirects and then returns 404.
-            return urllib2.urlopen(results_html)
+            result = urllib2.urlopen(results_html)
+            if not result:
+                return None
+            # urlopen returns a file-like object which sometimes works fine with str()
+            # but sometimes is a addinfourl object.  In either case calling read() is correct.
+            return result.read()
         except urllib2.HTTPError, error:
             if error.code != 404: