Changeset 75480 in webkit


Ignore:
Timestamp:
Jan 11, 2011 2:36:36 AM (13 years ago)
Author:
eric@webkit.org
Message:

2011-01-11 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

commit-queue should know how to upload archived results (for test flakes or general failures)
​https://bugs.webkit.org/show_bug.cgi?id=52048

Now the queue will always upload results. Either the entire zip, or just
the diffs.txt in the case of text failures.

This should make understanding flakes much easier, and paves the way
for having the EWS run layout tests (and upload failures).

In order to upload .zip files I had to teach bugzilla.py to autodetect
mime types from the filename. Since mimetypes.py doesn't include a mapping
for .patch files, I have it add one before calling guess_type.

We may find that always uploading the whole zip instead of just the -diffs.txt
file is preferable, but for now I'm keeping the old behavior because it makes
quickly understanding text failures easy.

  • Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
  • Scripts/webkitpy/common/system/workspace.py: Added.
  • Scripts/webkitpy/common/system/workspace_unittest.py: Added.
  • Scripts/webkitpy/tool/bot/commitqueuetask.py:
  • Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
  • Scripts/webkitpy/tool/bot/flakytestreporter.py:
  • Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py:
  • Scripts/webkitpy/tool/commands/queues.py:
  • Scripts/webkitpy/tool/commands/queues_unittest.py:
Location:
trunk/Tools
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r75475 r75480  
     12011-01-11  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        commit-queue should know how to upload archived results (for test flakes or general failures)
     6        https://bugs.webkit.org/show_bug.cgi?id=52048
     7
     8        Now the queue will always upload results.  Either the entire zip, or just
     9        the diffs.txt in the case of text failures.
     10
     11        This should make understanding flakes much easier, and paves the way
     12        for having the EWS run layout tests (and upload failures).
     13
     14        In order to upload .zip files I had to teach bugzilla.py to autodetect
     15        mime types from the filename.  Since mimetypes.py doesn't include a mapping
     16        for .patch files, I have it add one before calling guess_type.
     17
     18        We may find that always uploading the whole zip instead of just the -diffs.txt
     19        file is preferable, but for now I'm keeping the old behavior because it makes
     20        quickly understanding text failures easy.
     21
     22        * Scripts/webkitpy/common/net/bugzilla/bugzilla.py:
     23        * Scripts/webkitpy/common/system/workspace.py: Added.
     24        * Scripts/webkitpy/common/system/workspace_unittest.py: Added.
     25        * Scripts/webkitpy/tool/bot/commitqueuetask.py:
     26        * Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
     27        * Scripts/webkitpy/tool/bot/flakytestreporter.py:
     28        * Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py:
     29        * Scripts/webkitpy/tool/commands/queues.py:
     30        * Scripts/webkitpy/tool/commands/queues_unittest.py:
     31
    1322011-01-10  Sheriff Bot  <webkit.review.bot@gmail.com>
    233

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

    r74138 r75480  
    3131# WebKit's Python module for interacting with Bugzilla
    3232
     33import mimetypes
    3334import os.path
    3435import re
    … …  
    442443
    443444        filename = filename or "%s.patch" % timestamp()
    444         mimetype = mimetype or "text/plain"
     445        if not mimetype:
     446            mimetypes.add_type('text/plain', '.patch')  # Make sure mimetypes knows about .patch
     447            mimetype, _ = mimetypes.guess_type(filename)
     448        if not mimetype:
     449            mimetype = "text/plain"  # Bugzilla might auto-guess for us and we might not need this?
    445450        self.browser.add_file(file_object, mimetype, filename, 'data')
    446451

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

    r75366 r75480  
    4747        raise NotImplementedError("subclasses must implement")
    4848
    49     def report_flaky_tests(self, patch, flaky_tests):
     49    def archive_last_layout_test_results(self, patch):
     50        raise NotImplementedError("subclasses must implement")
     51
     52    # We could make results_archive optional, but for now it's required.
     53    def report_flaky_tests(self, patch, flaky_tests, results_archive):
    5054        raise NotImplementedError("subclasses must implement")
    5155
    … …  
    171175        "Unable to land patch")
    172176
    173     def _report_flaky_tests(self, flaky_test_results):
    174         self._delegate.report_flaky_tests(self._patch, flaky_test_results)
     177    def _report_flaky_tests(self, flaky_test_results, results_archive):
     178        self._delegate.report_flaky_tests(self._patch, flaky_test_results, results_archive)
    175179
    176180    def _test_patch(self):
    … …  
    180184            return True
    181185
    182         first_failing_results = self._failing_results_from_last_run()
    183         first_failing_tests = [result.filename for result in first_failing_results]
     186        first_results = self._failing_results_from_last_run()
     187        first_failing_tests = [result.filename for result in first_results]
     188        first_results_archive = self._delegate.archive_last_layout_test_results(self._patch)
    184189        if self._test():
    185             self._report_flaky_tests(first_failing_results)
    186             return True
    187 
    188         second_failing_results = self._failing_results_from_last_run()
    189         second_failing_tests = [result.filename for result in second_failing_results]
     190            self._report_flaky_tests(first_results, first_results_archive)
     191            return True
     192
     193        second_results = self._failing_results_from_last_run()
     194        second_failing_tests = [result.filename for result in second_results]
    190195        if first_failing_tests != second_failing_tests:
    191196            # We could report flaky tests here, but since run-webkit-tests

  • trunk/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py

    r75366 r75480  
    6666        return None
    6767
    68     def report_flaky_tests(self, patch, flaky_results):
     68    def report_flaky_tests(self, patch, flaky_results, results_archive):
    6969        flaky_tests = [result.filename for result in flaky_results]
    70         log("report_flaky_tests: patch='%s' flaky_tests='%s'" % (patch.id(), flaky_tests))
     70        log("report_flaky_tests: patch='%s' flaky_tests='%s' archive='%s'" % (patch.id(), flaky_tests, results_archive.filename))
     71
     72    def archive_last_layout_test_results(self, patch):
     73        log("archive_last_layout_test_results: patch='%s'" % patch.id())
     74        archive = Mock()
     75        archive.filename = "mock-archive-%s.zip" % patch.id()
     76        return archive
    7177
    7278
    … …  
    195201run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    196202command_failed: failure_message='Patch does not pass tests' script_error='MOCK tests failure' patch='197'
     203archive_last_layout_test_results: patch='197'
    197204run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    198205command_passed: success_message='Passed tests' patch='197'
    199 report_flaky_tests: patch='197' flaky_tests='[]'
     206report_flaky_tests: patch='197' flaky_tests='[]' archive='mock-archive-197.zip'
    200207run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 197]
    201208command_passed: success_message='Landed patch' patch='197'
    … …  
    227234run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    228235command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
     236archive_last_layout_test_results: patch='197'
    229237run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    230238command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
    … …  
    264272run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    265273command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
     274archive_last_layout_test_results: patch='197'
    266275run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    267276command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
    … …  
    291300run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    292301command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
     302archive_last_layout_test_results: patch='197'
    293303run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
    294304command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'

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

    r75114 r75480  
    132132        return "%s\n%s" % (flake_message, self._bot_information())
    133133
    134     def _results_diff_path_for_test(self, flaky_test):
     134    def _results_diff_path_for_test(self, test_path):
    135135        # FIXME: This is a big hack.  We should get this path from results.json
    136136        # except that old-run-webkit-tests doesn't produce a results.json
    137137        # so we just guess at the file path.
    138         results_path = self._tool.port().layout_tests_results_path()
    139         results_directory = os.path.dirname(results_path)
    140         test_path = os.path.join(results_directory, flaky_test)
    141138        (test_path_root, _) = os.path.splitext(test_path)
    142139        return "%s-diffs.txt" % test_path_root
    … …  
    154151            self._tool.bugs.post_comment_to_bug(bug.id(), latest_flake_message)
    155152
    156     def report_flaky_tests(self, flaky_test_results, patch):
     153    def _attach_failure_diff(self, flake_bug_id, flaky_test, results_archive):
     154        results_diff_path = self._results_diff_path_for_test(flaky_test)
     155        # Check to make sure that the path makes sense.
     156        # Since we're not actually getting this path from the results.html
     157        # there is a chance it's wrong.
     158        bot_id = self._tool.status_server.bot_id or "bot"
     159        if results_diff_path in results_archive.namelist():
     160            results_diff = results_archive.read(results_diff_path)
     161            description = "Failure diff from %s" % bot_id
     162            self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_diff, description, filename="failure.diff")
     163        else:
     164            _log.warn("%s does not exist in results archive, uploading entire archive." % results_diff_path)
     165            description = "Archive of layout-test-results from %s" % bot_id
     166            # results_archive is a ZipFile object, grab the File object (.fp) to pass to Mechanize for uploading.
     167            self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_archive.fp, description, filename="layout-test-results.zip")
     168
     169    def report_flaky_tests(self, patch, flaky_test_results, results_archive):
    157170        message = "The %s encountered the following flaky tests while processing attachment %s:\n\n" % (self._bot_name, patch.id())
    158171        for flaky_result in flaky_test_results:
    … …  
    166179            else:
    167180                bug = self._follow_duplicate_chain(bug)
     181                # FIXME: Ideally we'd only make one comment per flake, not two.  But that's not possible
     182                # in all cases (e.g. when reopening), so for now file attachment and comment are separate.
    168183                self._update_bug_for_flaky_test(bug, latest_flake_message)
    169184                flake_bug_id = bug.id()
    170             # FIXME: Ideally we'd only make one comment per flake, not two.  But that's not possible
    171             # in all cases (e.g. when reopening), so for now we do the attachment in a second step.
    172             results_diff_path = self._results_diff_path_for_test(flaky_test)
    173             # Check to make sure that the path makes sense.
    174             # Since we're not actually getting this path from the results.html
    175             # there is a high probaility it's totally wrong.
    176             if self._tool.filesystem.exists(results_diff_path):
    177                 results_diff = self._tool.filesystem.read_binary_file(results_diff_path)
    178                 bot_id = self._tool.status_server.bot_id or "bot"
    179                 self._tool.bugs.add_attachment_to_bug(flake_bug_id, results_diff, "Failure diff from %s" % bot_id, filename="failure.diff")
    180             else:
    181                 _log.error("%s does not exist as expected, not uploading." % results_diff_path)
     185
     186            self._attach_failure_diff(flake_bug_id, flaky_test, results_archive)
    182187            message += "%s bug %s%s\n" % (flaky_test, flake_bug_id, self._optional_author_string(author_emails))
    183188

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

    r75114 r75480  
    3434from webkitpy.layout_tests.layout_package import test_results
    3535from webkitpy.layout_tests.layout_package import test_failures
     36from webkitpy.thirdparty.mock import Mock
    3637from webkitpy.tool.bot.flakytestreporter import FlakyTestReporter
    3738from webkitpy.tool.mocktool import MockTool, MockStatusServer
    … …  
    141142"""
    142143        test_results = [self._mock_test_result('foo/bar.html')]
    143         OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [test_results, patch], expected_stderr=expected_stderr)
     144
     145        class MockZipFile(object):
     146            def read(self, path):
     147                return ""
     148
     149            def namelist(self):
     150                return ['foo/bar-diffs.txt']
     151
     152        OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [patch, test_results, MockZipFile()], expected_stderr=expected_stderr)
    144153
    145154    def test_optional_author_string(self):
    … …  
    151160    def test_results_diff_path_for_test(self):
    152161        reporter = FlakyTestReporter(MockTool(), 'dummy-queue')
    153         self.assertEqual(reporter._results_diff_path_for_test("test.html"), "/mock/test-diffs.txt")
     162        self.assertEqual(reporter._results_diff_path_for_test("test.html"), "test-diffs.txt")
    154163
    155164    # report_flaky_tests is also tested by queues_unittest

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

    r75149 r75480  
    310310        return LayoutTestResults.results_from_string(results_html)
    311311
     312    def _results_directory(self):
     313        results_path = self._tool.port().layout_tests_results_path()
     314        # FIXME: This is wrong in two ways:
     315        # 1. It assumes that results.html is at the top level of the results tree.
     316        # 2. This uses the "old" ports.py infrastructure instead of the new layout_tests/port
     317        # which will not support Chromium.  However the new arch doesn't work with old-run-webkit-tests
     318        # so we have to use this for now.
     319        return os.path.dirname(results_path)
     320
     321    def archive_last_layout_test_results(self, patch):
     322        results_directory = self._results_directory()
     323        results_name, _ = os.path.splitext(os.path.basename(results_directory))
     324        # Note: We name the zip with the bug_id instead of patch_id to match work_item_log_path().
     325        zip_path = self._tool.workspace.find_unused_filename(self._log_directory(), "%s-%s" % (patch.bug_id(), results_name), "zip")
     326        self._tool.workspace.create_zip(zip_path, results_directory)
     327        return zip_path
     328
    312329    def refetch_patch(self, patch):
    313330        return self._tool.bugs.fetch_attachment(patch.id())
    314331
    315     def report_flaky_tests(self, patch, flaky_test_results):
     332    def report_flaky_tests(self, patch, flaky_test_results, results_archive=None):
    316333        reporter = FlakyTestReporter(self._tool, self.name)
    317         reporter.report_flaky_tests(flaky_test_results, patch)
     334        reporter.report_flaky_tests(patch, flaky_test_results, results_archive)
    318335
    319336    # StepSequenceErrorHandler methods

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

    r75149 r75480  
    2828
    2929import os
     30import StringIO
    3031
    3132from webkitpy.common.checkout.scm import CheckoutNeedsUpdate
    3233from webkitpy.common.net.bugzilla import Attachment
     34from webkitpy.common.system.filesystem_mock import MockFileSystem
    3335from webkitpy.common.system.outputcapture import OutputCapture
    3436from webkitpy.layout_tests.layout_package import test_results
    … …  
    343345--- End comment ---
    344346
     347MOCK add_attachment_to_bug: bug_id=76, description=Failure diff from bot filename=failure.diff
    345348MOCK bug comment: bug_id=76, cc=None
    346349--- Begin comment ---
    … …  
    349352--- End comment ---
    350353
     354MOCK add_attachment_to_bug: bug_id=76, description=Archive of layout-test-results from bot filename=layout-test-results.zip
    351355MOCK bug comment: bug_id=42, cc=None
    352356--- Begin comment ---
    … …  
    361365        test_names = ["foo/bar.html", "bar/baz.html"]
    362366        test_results = [self._mock_test_result(name) for name in test_names]
    363         OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results], expected_stderr=expected_stderr)
     367
     368        class MockZipFile(object):
     369            def __init__(self):
     370                self.fp = StringIO()
     371
     372            def read(self, path):
     373                return ""
     374
     375            def namelist(self):
     376                # This is intentionally missing one diffs.txt to exercise the "upload the whole zip" codepath.
     377                return ['foo/bar-diffs.txt']
     378
     379        OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results, MockZipFile()], expected_stderr=expected_stderr)
    364380
    365381    def test_layout_test_results(self):
    … …  
    371387        self.assertEquals(queue.layout_test_results(), None)
    372388
     389    def test_archive_last_layout_test_results(self):
     390        queue = CommitQueue()
     391        queue.bind_to_tool(MockTool())
     392        patch = queue._tool.bugs.fetch_attachment(128)
     393        queue.archive_last_layout_test_results(patch)
     394
    373395
    374396class StyleQueueTest(QueuesTest):

  • trunk/Tools/Scripts/webkitpy/tool/main.py

    r74138 r75480  
    4141from webkitpy.common.net.irc.ircproxy import IRCProxy
    4242from webkitpy.common.net.statusserver import StatusServer
    43 from webkitpy.common.system.executive import Executive
    44 from webkitpy.common.system.filesystem import FileSystem
    45 from webkitpy.common.system.platforminfo import PlatformInfo
    46 from webkitpy.common.system.user import User
     43from webkitpy.common.system import executive, filesystem, platforminfo, user, workspace
    4744from webkitpy.layout_tests import port
    4845from webkitpy.tool.multicommandtool import MultiCommandTool
    … …  
    7168        self.bugs = Bugzilla()
    7269        self.buildbot = BuildBot()
    73         self.executive = Executive()
     70        self.executive = executive.Executive()
    7471        self._irc = None
    75         self.filesystem = FileSystem()
     72        self.filesystem = filesystem.FileSystem()
     73        self.workspace = workspace.Workspace(self.filesystem, self.executive)
    7674        self._port = None
    77         self.user = User()
     75        self.user = user.User()
    7876        self._scm = None
    7977        self._checkout = None
    8078        self.status_server = StatusServer()
    8179        self.port_factory = port.factory
    82         self.platform = PlatformInfo()
     80        self.platform = platforminfo.PlatformInfo()
    8381
    8482    def scm(self):

  • trunk/Tools/Scripts/webkitpy/tool/mocktool.py

    r74927 r75480  
    687687
    688688
     689class MockWorkspace(object):
     690    def find_unused_filename(self, directory, name, extension, search_limit=10):
     691        return "%s/%s.%s" % (directory, name, extension)
     692
     693    def create_zip(self, zip_path, source_path):
     694        pass
     695
     696
    689697class MockTool(object):
    690698
    … …  
    695703        self.executive = MockExecutive(should_log=log_executive)
    696704        self.filesystem = MockFileSystem()
     705        self.workspace = MockWorkspace()
    697706        self._irc = None
    698707        self.user = MockUser()
Note: See TracChangeset for help on using the changeset viewer.