Changeset 83614 in webkit

Timestamp:

Apr 12, 2011 11:30:17 AM (13 years ago)

Author:

eric@webkit.org

Message:

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

Reviewed by Adam Barth.

commit-queue should upload failure diffs when tests fail
​https://bugs.webkit.org/show_bug.cgi?id=58348

This change was mostly just plumbing. We were already saving
this information for flaky test reporting. I just made it possible
for normal failures to report archives as well.

I did a little abstraction work to try and share some code between
flakytestreporter.py and this new code. There is still more we could do.

In making this change I also went through and updated the various
places we have urls hard-coded in our python and pointed them at
common.config.urls.

• Scripts/webkitpy/common/checkout/scm.py:
• Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
• Scripts/webkitpy/common/net/buildbot/buildbot.py:
• Scripts/webkitpy/common/net/statusserver.py:
• Scripts/webkitpy/layout_tests/port/base.py:
• Scripts/webkitpy/tool/bot/botinfo.py: Added.
• Scripts/webkitpy/tool/bot/botinfo_unittest.py: Added.
• Scripts/webkitpy/tool/bot/commitqueuetask.py:
• Scripts/webkitpy/tool/bot/flakytestreporter.py:
• Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py:
• Scripts/webkitpy/tool/commands/queues.py:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/common/checkout/scm.py (modified) (1 diff)
• Scripts/webkitpy/common/net/bugzilla/bugzilla.py (modified) (1 diff)
• Scripts/webkitpy/common/net/buildbot/buildbot.py (modified) (1 diff)
• Scripts/webkitpy/common/net/statusserver.py (modified) (2 diffs)
• Scripts/webkitpy/layout_tests/port/base.py (modified) (1 diff)
• Scripts/webkitpy/tool/bot/botinfo.py (added)
• Scripts/webkitpy/tool/bot/botinfo_unittest.py (added)
• Scripts/webkitpy/tool/bot/commitqueuetask.py (modified) (2 diffs)
• Scripts/webkitpy/tool/bot/flakytestreporter.py (modified) (3 diffs)
• Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/queues.py (modified) (3 diffs)
• Scripts/webkitpy/tool/commands/queues_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-04-12  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        commit-queue should upload failure diffs when tests fail
+        https://bugs.webkit.org/show_bug.cgi?id=58348
+
+        This change was mostly just plumbing.  We were already saving
+        this information for flaky test reporting.  I just made it possible
+        for normal failures to report archives as well.
+
+        I did a little abstraction work to try and share some code between
+        flakytestreporter.py and this new code.  There is still more we could do.
+
+        In making this change I also went through and updated the various
+        places we have urls hard-coded in our python and pointed them at
+        common.config.urls.
+
+        * Scripts/webkitpy/common/checkout/scm.py:
+        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
+        * Scripts/webkitpy/common/net/buildbot/buildbot.py:
+        * Scripts/webkitpy/common/net/statusserver.py:
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/tool/bot/botinfo.py: Added.
+        * Scripts/webkitpy/tool/bot/botinfo_unittest.py: Added.
+        * Scripts/webkitpy/tool/bot/commitqueuetask.py:
+        * Scripts/webkitpy/tool/bot/flakytestreporter.py:
+        * Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+
 2011-04-12  Daniel Bates  <dbates@rim.com>
 

trunk/Tools/Scripts/webkitpy/common/checkout/scm.py

@@ -321 +321 @@
 
 class SVN(SCM):
-    # FIXME: We should move these values to a WebKit-specific config file.
+    # FIXME: These belong in common.config.urls
     svn_server_host = "svn.webkit.org"
     svn_server_realm = "<http://svn.webkit.org:80> Mac OS Forge"

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

@@ -221 +221 @@
         self.browser.set_handle_robots(False)
 
-    # FIXME: Much of this should go into some sort of config module:
+    # FIXME: Much of this should go into some sort of config module,
+    # such as common.config.urls.
     bug_server_host = "bugs.webkit.org"
     bug_server_regex = "https?://%s/" % re.sub('\.', '\\.', bug_server_host)

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

@@ -268 +268 @@
 
 class BuildBot(object):
-    # FIXME: This should move into some sort of webkit_config.py
+    # FIXME: This should move into common.config.urls.
     default_host = "build.webkit.org"
 

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

@@ -26 +26 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# This the client designed to talk to Tools/QueueStatusServer.
 
 from webkitpy.common.net.networktransaction import NetworkTransaction
@@ -40 +42 @@
 
 class StatusServer:
+    # FIXME: This should probably move to common.config.urls.
     default_host = "queues.webkit.org"
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py

@@ -130 +130 @@
         self._wdiff_available = True
 
+        # FIXME: prettypatch.py knows this path, why is it copied here?
         self._pretty_patch_path = self.path_from_webkit_base("Websites",
             "bugs.webkit.org", "PrettyPatch", "prettify.rb")

trunk/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py

@@ -60 +60 @@
         self._patch = patch
         self._script_error = None
+        self._results_archive_from_patch_test_run = None
 
     def _validate(self):
@@ -202 +203 @@
 
         if self._build_and_test_without_patch():
-            return self.report_failure()  # The error from the previous ._test() run is real, report it.
+            return self.report_failure(first_results_archive)  # The error from the previous ._test() run is real, report it.
         return False  # Tree must be red, just retry later.
 
-    def report_failure(self):
+    def results_archive_from_patch_test_run(self, patch):
+        assert(self._patch.id() == patch.id())  # CommitQueueTask is not currently re-useable.
+        return self._results_archive_from_patch_test_run
+
+    def report_failure(self, results_archive=None):
         if not self._validate():
             return False
+        self._results_archive_from_patch_test_run = results_archive
         raise self._script_error
 

trunk/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py

@@ -34 +34 @@
 from webkitpy.common.net.layouttestresults import path_for_layout_test, LayoutTestResults
 from webkitpy.common.config import urls
+from webkitpy.tool.bot.botinfo import BotInfo
 from webkitpy.tool.grammar import plural, pluralize, join_with_separators
 
@@ -43 +44 @@
         self._tool = tool
         self._bot_name = bot_name
+        self._bot_info = BotInfo(tool)
 
     def _author_emails_for_test(self, flaky_test):
@@ -122 +124 @@
         return " (%s: %s)" % (heading_string, authors_string)
 
-    def _bot_information(self):
-        bot_id = self._tool.status_server.bot_id
-        bot_id_string = "Bot: %s  " % (bot_id) if bot_id else ""
-        return "%sPort: %s  Platform: %s" % (bot_id_string, self._tool.port().name(), self._tool.platform.display_name())
-
     def _latest_flake_message(self, flaky_result, patch):
         failure_messages = [failure.message() for failure in flaky_result.failures]
         flake_message = "The %s just saw %s flake (%s) while processing attachment %s on bug %s." % (self._bot_name, flaky_result.filename, ", ".join(failure_messages), patch.id(), patch.bug_id())
-        return "%s\n%s" % (flake_message, self._bot_information())
+        return "%s\n%s" % (flake_message, self._bot_info.summary_text())
 
     def _results_diff_path_for_test(self, test_path):

trunk/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py

@@ -98 +98 @@
         self.assertEqual(reporter._follow_duplicate_chain(bug).id(), 76)
 
-    def test_bot_information(self):
-        tool = MockTool()
-        tool.status_server = MockStatusServer("MockBotId")
-        reporter = FlakyTestReporter(tool, 'dummy-queue')
-        self.assertEqual(reporter._bot_information(), "Bot: MockBotId  Port: MockPort  Platform: MockPlatform 1.0")
-
     def test_report_flaky_tests_creating_bug(self):
         tool = MockTool()

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

@@ -45 +45 @@
 from webkitpy.common.system.deprecated_logging import error, log
 from webkitpy.common.system.executive import ScriptError
+from webkitpy.tool.bot.botinfo import BotInfo
 from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate
 from webkitpy.tool.bot.feeders import CommitQueueFeeder, EWSFeeder
@@ -259 +260 @@
         return True
 
+    # FIXME: This is not really specific to the commit-queue and could be shared.
+    def _upload_results_archive_for_patch(self, patch, results_archive_zip):
+        bot_id = self._tool.status_server.bot_id or "bot"
+        description = "Archive of layout-test-results from %s" % bot_id
+        # results_archive is a ZipFile object, grab the File object (.fp) to pass to Mechanize for uploading.
+        results_archive_file = results_archive_zip.fp
+        # Rewind the file object to start (since Mechanize won't do that automatically)
+        # See https://bugs.webkit.org/show_bug.cgi?id=54593
+        results_archive_file.seek(0)
+        comment_text = "The attached test failures were seen while running run-webkit-tests on the %s.\n" % (self.name)
+        # FIXME: We could easily list the test failures from the archive here.
+        comment_text += BotInfo(self._tool).summary_text()
+        self._tool.bugs.add_attachment_to_bug(patch.bug_id(), results_archive_file, description, filename="layout-test-results.zip", comment_text=comment_text)
+
     def process_work_item(self, patch):
         self._cc_watchers(patch.bug_id())
@@ -270 +285 @@
             validator = CommitterValidator(self._tool.bugs)
             validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task.failure_status_id, e))
+            results_archive = task.results_archive_from_patch_test_run(patch)
+            if results_archive:
+                self._upload_results_archive_for_patch(patch, results_archive)
             self._did_fail(patch)
 

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

@@ -389 +389 @@
         queue.bind_to_tool(MockTool())
         patch = queue._tool.bugs.fetch_attachment(128)
+        # This is just to test that the method doesn't raise.
         queue.archive_last_layout_test_results(patch)
+
+    def test_upload_results_archive_for_patch(self):
+        queue = CommitQueue()
+        queue.bind_to_tool(MockTool())
+        patch = queue._tool.bugs.fetch_attachment(128)
+        expected_stderr = """MOCK add_attachment_to_bug: bug_id=42, description=Archive of layout-test-results from bot filename=layout-test-results.zip
+-- Begin comment --
+The attached test failures were seen while running run-webkit-tests on the commit-queue.
+Port: MockPort  Platform: MockPlatform 1.0
+-- End comment --
+"""
+        OutputCapture().assert_outputs(self, queue._upload_results_archive_for_patch, [patch, Mock()], expected_stderr=expected_stderr)