Changeset 87626 in webkit


Ignore:
Timestamp:
May 28, 2011 3:26:15 PM (13 years ago)
Author:
abarth@webkit.org
Message:

2011-05-28 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

EWS builds patches that fail to build twice, which seems useless and slows down the bots
​https://bugs.webkit.org/show_bug.cgi?id=55585

This patch switches all the early warning system bots over to the new
PatchAnalysisTask-based infrastructure. This patch makes these bots
more efficient (in the case where patches fail to build) and paves the
way for running tests on these bots!

  • Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
  • Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:
  • Scripts/webkitpy/tool/bot/patchanalysistask.py:
  • Scripts/webkitpy/tool/commands/earlywarningsystem.py:
  • Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
  • Scripts/webkitpy/tool/commands/queues.py:
Location:
trunk/Tools
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r87607 r87626  
     12011-05-28  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        EWS builds patches that fail to build twice, which seems useless and slows down the bots
     6        https://bugs.webkit.org/show_bug.cgi?id=55585
     7
     8        This patch switches all the early warning system bots over to the new
     9        PatchAnalysisTask-based infrastructure.  This patch makes these bots
     10        more efficient (in the case where patches fail to build) and paves the
     11        way for running tests on these bots!
     12
     13        * Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
     14        * Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:
     15        * Scripts/webkitpy/tool/bot/patchanalysistask.py:
     16        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
     17        * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
     18        * Scripts/webkitpy/tool/commands/queues.py:
     19
    1202011-05-28  Kenichi Ishibashi  <bashi@chromium.org>
    221

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

    r87601 r87626  
    8282        return archive
    8383
     84    def build_style(self):
     85        return "both"
     86
    8487
    8588class FailingTestCommitQueue(MockCommitQueue):

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

    r85504 r87626  
    4141
    4242class EarlyWarningSystemTask(PatchAnalysisTask):
     43    def __init__(self, delegate, patch, run_tests=True):
     44        PatchAnalysisTask.__init__(self, delegate, patch)
     45        self._run_tests = run_tests
     46
    4347    def validate(self):
    4448        self._patch = self._delegate.refetch_patch(self._patch)
    … …  
    6468                return False
    6569            return self.report_failure()
     70        if not self._run_tests:
     71            return True
    6672        return self._test_patch()

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

    r87601 r87626  
    5656        raise NotImplementedError("subclasses must implement")
    5757
     58    def build_style(self):
     59        raise NotImplementedError("subclasses must implement")
     60
    5861    # We could make results_archive optional, but for now it's required.
    5962    def report_flaky_tests(self, patch, flaky_tests, results_archive):
    … …  
    111114            "--no-clean",
    112115            "--no-update",
    113             "--build-style=both",
     116            "--build-style=%s" % self._delegate.build_style(),
    114117        ],
    115118        "Built patch",
    … …  
    121124            "--force-clean",
    122125            "--no-update",
    123             "--build-style=both",
     126            "--build-style=%s" % self._delegate.build_style(),
    124127        ],
    125128        "Able to build without patch",

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

    r86020 r87626  
    3838
    3939
    40 class AbstractEarlyWarningSystem(AbstractReviewQueue):
     40class AbstractEarlyWarningSystem(AbstractReviewQueue, EarlyWarningSystemTaskDelegate):
    4141    _build_style = "release"
     42    # FIXME: Switch _run_tests from opt-in to opt-out once more bots are ready to run tests.
     43    _run_tests = False
    4244
    4345    def __init__(self):
    … …  
    4850        return True
    4951
    50     def _can_build(self):
    51         try:
    52             self.run_webkit_patch([
    53                 "build",
    54                 self.port.flag(),
    55                 "--build-style=%s" % self._build_style,
    56                 "--force-clean",
    57                 "--no-update"])
    58             return True
    59         except ScriptError, e:
    60             failure_log = self._log_from_script_error_for_upload(e)
    61             self._update_status("Unable to perform a build", results_file=failure_log)
    62             return False
    63 
    64     def _build(self, patch, first_run=False):
    65         try:
    66             args = [
    67                 "build-attachment",
    68                 self.port.flag(),
    69                 "--build",
    70                 "--build-style=%s" % self._build_style,
    71                 "--force-clean",
    72                 "--quiet",
    73                 "--non-interactive",
    74                 patch.id()]
    75             if not first_run:
    76                 # See commit-queue for an explanation of what we're doing here.
    77                 args.append("--no-update")
    78                 args.append("--parent-command=%s" % self.name)
    79             self.run_webkit_patch(args)
    80             return True
    81         except ScriptError, e:
    82             if first_run:
    83                 return False
    84             raise
    85 
    86     def review_patch(self, patch):
    87         if patch.is_obsolete():
    88             self._did_error(patch, "%s does not process obsolete patches." % self.name)
    89             return False
    90 
    91         if patch.bug().is_closed():
    92             self._did_error(patch, "%s does not process patches on closed bugs." % self.name)
    93             return False
    94 
    95         if not self._build(patch, first_run=True):
    96             if not self._can_build():
    97                 return False
    98             self._build(patch)
    99         return True
    100 
    101     @classmethod
    102     def _post_reject_message_on_bug(cls, tool, patch, status_id, extra_message_text=None):
    103         results_link = tool.status_server.results_url_for_status(status_id)
    104         message = "Attachment %s did not pass %s (%s):\nOutput: %s" % (patch.id(), cls.name, cls.port_name, results_link)
    105         # FIXME: We might want to add some text about rejecting from the commit-queue in
    106         # the case where patch.commit_queue() isn't already set to '-'.
    107         if cls.watchers:
    108             tool.bugs.add_cc_to_bug(patch.bug_id(), cls.watchers)
    109         tool.bugs.set_flag_on_attachment(patch.id(), "commit-queue", "-", message, extra_message_text)
    110 
    111     @classmethod
    112     def handle_script_error(cls, tool, state, script_error):
    113         is_svn_apply = script_error.command_name() == "svn-apply"
    114         status_id = cls._update_status_for_script_error(tool, state, script_error, is_error=is_svn_apply)
    115         if is_svn_apply:
    116             QueueEngine.exit_after_handled_error(script_error)
    117         cls._post_reject_message_on_bug(tool, state['patch'], status_id)
    118         exit(1)
    119 
    120 
    121 # FIXME: This should merge with AbstractEarlyWarningSystem once all the EWS
    122 # bots run tests.
    123 class AbstractTestingEWS(AbstractEarlyWarningSystem, EarlyWarningSystemTaskDelegate):
    12452    def begin_work_queue(self):
    12553        # FIXME: This violates abstraction
    12654        self._tool._port = self.port
    127         AbstractEarlyWarningSystem.begin_work_queue(self)
     55        AbstractReviewQueue.begin_work_queue(self)
    12856        self._expected_failures = ExpectedFailures()
    12957        self._layout_test_results_reader = LayoutTestResultsReader(self._tool, self._log_directory())
    … …  
    13664        return "New failing tests:\n%s" % "\n".join(unexpected_failures)
    13765
     66    def _post_reject_message_on_bug(self, tool, patch, status_id, extra_message_text=None):
     67        results_link = tool.status_server.results_url_for_status(status_id)
     68        message = "Attachment %s did not pass %s (%s):\nOutput: %s" % (patch.id(), self.name, self.port_name, results_link)
     69        # FIXME: We might want to add some text about rejecting from the commit-queue in
     70        # the case where patch.commit_queue() isn't already set to '-'.
     71        if self.watchers:
     72            tool.bugs.add_cc_to_bug(patch.bug_id(), self.watchers)
     73        tool.bugs.set_flag_on_attachment(patch.id(), "commit-queue", "-", message, extra_message_text)
     74
    13875    def review_patch(self, patch):
    139         task = EarlyWarningSystemTask(self, patch)
     76        task = EarlyWarningSystemTask(self, patch, self._run_tests)
    14077        if not task.validate():
    14178            self._did_error(patch, "%s did not process patch." % self.name)
    … …  
    181118        return self._layout_test_results_reader.archive(patch)
    182119
     120    def build_style(self):
     121        return self._build_style
     122
    183123    def refetch_patch(self, patch):
    184124        return self._tool.bugs.fetch_attachment(patch.id())
    … …  
    235175
    236176
    237 class ChromiumLinuxEWS(AbstractTestingEWS):
     177class ChromiumLinuxEWS(AbstractChromiumEWS):
    238178    # FIXME: We should rename this command to cr-linux-ews, but that requires
    239179    #        a database migration. :(
    240180    name = "chromium-ews"
    241181    port_name = "chromium-xvfb"
    242 
    243     # FIXME: ChromiumLinuxEWS should inherit from AbstractChromiumEWS once
    244     # all the Chromium EWS bots run tests
    245     watchers = AbstractChromiumEWS.watchers
     182    run_tests = True
    246183
    247184

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

    r86020 r87626  
    3838
    3939class AbstractEarlyWarningSystemTest(QueuesTest):
    40     # Needed to define port_name, used in AbstractEarlyWarningSystem.__init__
    41     class TestEWS(AbstractEarlyWarningSystem):
    42         name = "mock-ews"
    43         port_name = "win"  # Needs to be a port which port/factory understands.
    44 
    45     def test_can_build(self):
    46         queue = self.TestEWS()
    47         queue.bind_to_tool(MockTool(log_executive=True))
    48         queue._options = MockOptions(port=None)
    49         expected_stderr = "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--port=win', '--build-style=release', '--force-clean', '--no-update']\n"
    50         OutputCapture().assert_outputs(self, queue._can_build, [], expected_stderr=expected_stderr)
    51 
    52         def mock_run_webkit_patch(args):
    53             raise ScriptError("MOCK script error")
    54 
    55         queue.run_webkit_patch = mock_run_webkit_patch
    56         expected_stderr = "MOCK: update_status: mock-ews Unable to perform a build\n"
    57         OutputCapture().assert_outputs(self, queue._can_build, [], expected_stderr=expected_stderr)
    58 
    59     # FIXME: This belongs on an AbstractReviewQueueTest object in queues_unittest.py
    60     def test_subprocess_handled_error(self):
    61         queue = AbstractReviewQueue()
    62         queue.bind_to_tool(MockTool())
    63 
    64         def mock_review_patch(patch):
    65             raise ScriptError('MOCK script error', exit_code=QueueEngine.handled_error_code)
    66 
    67         queue.review_patch = mock_review_patch
    68         mock_patch = queue._tool.bugs.fetch_attachment(197)
    69         expected_stderr = "MOCK: release_work_item: None 197\n"
    70         OutputCapture().assert_outputs(self, queue.process_work_item, [mock_patch], expected_stderr=expected_stderr, expected_exception=ScriptError)
    71 
    72     def test_post_reject_message_on_bug(self):
    73         tool = MockTool()
    74         patch = tool.bugs.fetch_attachment(197)
    75         expected_stderr = """MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Attachment 197 did not pass mock-ews (win):
    76 Output: http://dummy_url' and additional comment 'EXTRA'
    77 """
    78         OutputCapture().assert_outputs(self,
    79             self.TestEWS._post_reject_message_on_bug,
    80             kwargs={'tool': tool, 'patch': patch, 'status_id': 1, 'extra_message_text': "EXTRA"},
    81             expected_stderr=expected_stderr)
    82 
    83 
    84 class AbstractTestingEWSTest(QueuesTest):
    8540    def test_failing_tests_message(self):
    8641        # Needed to define port_name, used in AbstractEarlyWarningSystem.__init__
    87         class TestEWS(AbstractTestingEWS):
     42        class TestEWS(AbstractEarlyWarningSystem):
    8843            port_name = "win"  # Needs to be a port which port/factory understands.
    8944
    … …  
    9954
    10055class EarlyWarningSytemTest(QueuesTest):
    101     def test_failed_builds(self):
    102         ews = ChromiumWindowsEWS()
    103         ews.bind_to_tool(MockTool())
    104         ews._build = lambda patch, first_run=False: False
    105         ews._can_build = lambda: True
    106         mock_patch = ews._tool.bugs.fetch_attachment(197)
    107         ews.review_patch(mock_patch)
    108 
    10956    def _default_expected_stderr(self, ews):
    11057        string_replacemnts = {
    11158            "name": ews.name,
    11259            "port": ews.port_name,
    113             "watchers": ews.watchers,
    11460        }
    11561        expected_stderr = {
    … …  
    11864            "next_work_item": "",
    11965            "process_work_item": "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s 197\n" % string_replacemnts,
    120             "handle_script_error": """MOCK: update_status: %(name)s ScriptError error message
    121 MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Attachment 197 did not pass %(name)s (%(port)s):
    122 Output: http://dummy_url' and additional comment 'None'
    123 """ % string_replacemnts,
     66            "handle_script_error": "ScriptError error message\n",
    12467        }
    12568        return expected_stderr
    … …  
    12770    def _test_builder_ews(self, ews):
    12871        ews.bind_to_tool(MockTool())
    129         expected_exceptions = {
    130             "handle_script_error": SystemExit,
    131         }
    132         self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews), expected_exceptions=expected_exceptions)
     72        self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews))
    13373
    13474    def _test_committer_only_ews(self, ews):
    … …  
    13777        string_replacemnts = {"name": ews.name}
    13878        expected_stderr["process_work_item"] = "MOCK: update_status: %(name)s Error: %(name)s cannot process patches from non-committers :(\nMOCK: release_work_item: %(name)s 197\n" % string_replacemnts
    139         expected_exceptions = {"handle_script_error": SystemExit}
    140         self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
     79        self.assert_queue_outputs(ews, expected_stderr=expected_stderr)
    14180
    14281    def _test_testing_ews(self, ews):

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

    r85802 r87626  
    326326        return self._layout_test_results_reader.archive(patch)
    327327
     328    def build_style(self):
     329        return "both"
     330
    328331    def refetch_patch(self, patch):
    329332        return self._tool.bugs.fetch_attachment(patch.id())
Note: See TracChangeset for help on using the changeset viewer.