Changeset 85802 in webkit


Ignore:
Timestamp:
May 4, 2011 4:22:04 PM (13 years ago)
Author:
eric@webkit.org
Message:

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

Reviewed by Adam Barth.

The testing EWS bots should upload layout-test-results.zip to bugs after failures
​https://bugs.webkit.org/show_bug.cgi?id=60223

This required sharing a bit of code between the commit-queue
(which already knew how to do this) and the new EWS testing bots.

In the process I also cleaned up EWS testing a little and
removed some dead code from the commit-queue.

  • Scripts/webkitpy/common/config/committervalidator.py:
  • Scripts/webkitpy/tool/commands/earlywarningsystem.py:
  • Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
  • Scripts/webkitpy/tool/commands/queues.py:
  • Scripts/webkitpy/tool/commands/queues_unittest.py:
Location:
trunk/Tools
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r85797 r85802  
     12011-05-04  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        The testing EWS bots should upload layout-test-results.zip to bugs after failures
     6        https://bugs.webkit.org/show_bug.cgi?id=60223
     7
     8        This required sharing a bit of code between the commit-queue
     9        (which already knew how to do this) and the new EWS testing bots.
     10
     11        In the process I also cleaned up EWS testing a little and
     12        removed some dead code from the commit-queue.
     13
     14        * Scripts/webkitpy/common/config/committervalidator.py:
     15        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
     16        * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
     17        * Scripts/webkitpy/tool/commands/queues.py:
     18        * Scripts/webkitpy/tool/commands/queues_unittest.py:
     19
    1202011-05-04  James Kozianski  <koz@chromium.org>
    221

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

    r85449 r85802  
    2929
    3030import os
     31
     32from webkitpy.common.system.deprecated_logging import error, log
    3133
    3234from .svn import SVN

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

    r85672 r85802  
    105105
    106106    @classmethod
    107     def read_git_config(cls, key):
     107    def read_git_config(cls, key, cwd=None):
    108108        # FIXME: This should probably use cwd=self.checkout_root.
    109109        # Pass --get-all for cases where the config has multiple values
    110         return run_command(["git", "config", "--get-all", key],
    111             error_handler=Executive.ignore_error).rstrip('\n')
     110        # Pass the cwd if provided so that we can handle the case of running webkit-patch outside of the working directory.
     111        return run_command(["git", "config", "--get-all", key], error_handler=Executive.ignore_error, cwd=cwd).rstrip('\n')
    112112
    113113    @staticmethod
    … …  
    120120
    121121    def local_commits(self):
    122         # FIXME: This should probably use cwd=self.checkout_root
    123         return self.run(['git', 'log', '--pretty=oneline', 'HEAD...' + self.remote_branch_ref()]).splitlines()
     122        return self.run(['git', 'log', '--pretty=oneline', 'HEAD...' + self.remote_branch_ref()], cwd=self.checkout_root).splitlines()
    124123
    125124    def rebase_in_progress(self):
    … …  
    127126
    128127    def working_directory_is_clean(self):
    129         # FIXME: This should probably use cwd=self.checkout_root
    130         return self.run(['git', 'diff', 'HEAD', '--name-only']) == ""
     128        return self.run(['git', 'diff', 'HEAD', '--name-only'], cwd=self.checkout_root) == ""
    131129
    132130    def clean_working_directory(self):
    … …  
    265263
    266264    def diff_for_file(self, path, log=None):
    267         return self.run(['git', 'diff', 'HEAD', '--', path])
     265        return self.run(['git', 'diff', 'HEAD', '--', path], cwd=self.checkout_root)
    268266
    269267    def show_head(self, path):
    … …  
    286284
    287285    def _assert_can_squash(self, working_directory_is_clean):
    288         squash = Git.read_git_config('webkit-patch.commit-should-always-squash')
     286        squash = Git.read_git_config('webkit-patch.commit-should-always-squash', cwd=self.checkout_root)
    289287        should_squash = squash and squash.lower() == "true"
    290288
    … …  
    317315        if not force_squash:
    318316            self._assert_can_squash(working_directory_is_clean)
    319         self.run(['git', 'reset', '--soft', self.remote_merge_base()])
     317        self.run(['git', 'reset', '--soft', self.remote_merge_base()], cwd=self.checkout_root)
    320318        self.commit_locally_with_message(message)
    321319        return self.push_local_commits_to_server(username=username, password=password)
    … …  
    379377
    380378    def remote_merge_base(self):
    381         return self.run(['git', 'merge-base', self.remote_branch_ref(), 'HEAD']).strip()
     379        return self.run(['git', 'merge-base', self.remote_branch_ref(), 'HEAD'], cwd=self.checkout_root).strip()
    382380
    383381    def remote_branch_ref(self):
    384382        # Use references so that we can avoid collisions, e.g. we don't want to operate on refs/heads/trunk if it exists.
    385         remote_branch_refs = Git.read_git_config('svn-remote.svn.fetch')
     383        remote_branch_refs = Git.read_git_config('svn-remote.svn.fetch', cwd=self.checkout_root)
    386384        if not remote_branch_refs:
    387385            remote_master_ref = 'refs/remotes/origin/master'
    … …  
    396394
    397395    def commit_locally_with_message(self, message):
    398         self.run(['git', 'commit', '--all', '-F', '-'], input=message)
     396        self.run(['git', 'commit', '--all', '-F', '-'], input=message, cwd=self.checkout_root)
    399397
    400398    def push_local_commits_to_server(self, username=None, password=None):
    … …  
    406404        if username:
    407405            dcommit_command.extend(["--username", username])
    408         output = self.run(dcommit_command, error_handler=commit_error_handler, input=password)
     406        output = self.run(dcommit_command, error_handler=commit_error_handler, input=password, cwd=self.checkout_root)
    409407        # Return a string which looks like a commit so that things which parse this output will succeed.
    410408        if self.dryrun:

  • trunk/Tools/Scripts/webkitpy/common/config/committervalidator.py

    r74301 r85802  
    5656
    5757    def _flag_permission_rejection_message(self, setter_email, flag_name):
    58         # This could be queried from the status_server.
    59         queue_administrator = "eseidel@chromium.org"
    6058        # This could be queried from the tool.
    6159        queue_name = "commit-queue"

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

    r85537 r85802  
    120120        self._layout_test_results_reader = LayoutTestResultsReader(self._tool, self._log_directory())
    121121
     122    def _post_reject_message_on_bug(self, task, patch):
     123        results_link = self._tool.status_server.results_url_for_status(task.failure_status_id)
     124        message = "Attachment %s did not pass %s:\nOutput: %s" % (patch.id(), self.name, results_link)
     125        results = task.results_from_patch_test_run(patch)
     126        unexpected_failures = self._expected_failures.unexpected_failures(results)
     127        if unexpected_failures:
     128            message += "\nNew failing tests:\n%s" % "\n".join(unexpected_failures)
     129        self._tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=self.watchers)
     130
    122131    def review_patch(self, patch):
    123132        task = EarlyWarningSystemTask(self, patch)
    … …  
    131140            return False
    132141        except ScriptError, e:
    133             # FIXME: We should upload the results archive on failure.
    134             results_link = self._tool.status_server.results_url_for_status(task.failure_status_id)
    135             message = "Attachment %s did not pass %s:\nOutput: %s" % (patch.id(), self.name, results_link)
    136             results = task.results_from_patch_test_run(patch)
    137             unexpected_failures = self._expected_failures.unexpected_failures(results)
    138             if unexpected_failures:
    139                 message += "\nNew failing tests:\n%s" % "\n".join(unexpected_failures)
    140             self._tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=self.watchers)
     142            # FIXME: This should just use CommitterValidator.reject_patch_from_commit_queue
     143            self._post_reject_message_on_bug(patch)
     144            results_archive = task.results_archive_from_patch_test_run(patch)
     145            if results_archive:
     146                self._upload_results_archive_for_patch(patch, results_archive)
    141147            self._did_fail(patch)
    142148            # FIXME: We're supposed to be able to raise e again here and have
    … …  
    179185    @classmethod
    180186    def handle_script_error(cls, tool, state, script_error):
     187        # FIXME: Why does this not exit(1) like the superclass does?
    181188        log(script_error.message_with_output())
    182189

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

    r85524 r85802  
    9494        return expected_stderr
    9595
    96     def _test_ews(self, ews):
     96    def _test_builder_ews(self, ews):
    9797        ews.bind_to_tool(MockTool())
    9898        expected_exceptions = {
    … …  
    110110
    111111    def _test_testing_ews(self, ews):
    112         expected_stderr = {
    113             "begin_work_queue": self._default_begin_work_queue_stderr(ews.name, MockTool().scm().checkout_root),
    114             "handle_unexpected_error": """Mock error message
    115 """,
    116             "next_work_item": "",
    117             "process_work_item": """MOCK: update_status: %s Pass
    118 MOCK: release_work_item: %s 197
    119 """ % (ews.name, ews.name),
    120             "handle_script_error": """ScriptError error message
    121 """,
    122         }
    123         tool = MockTool()
    124         tool.filesystem.write_text_file('/tmp/layout-test-results/unexpected_results.json', "")
    125         self.assert_queue_outputs(ews, expected_stderr=expected_stderr, tool=tool)
     112        ews.layout_test_results = lambda: None
     113        ews.bind_to_tool(MockTool())
     114        expected_stderr = self._default_expected_stderr(ews)
     115        expected_stderr["handle_script_error"] = "ScriptError error message\n"
     116        self.assert_queue_outputs(ews, expected_stderr=expected_stderr)
    126117
    127     # FIXME: If all EWSes are going to output the same text, we
    128     # could test them all in one method with a for loop over an array.
    129     def test_chromium_linux_ews(self):
     118    def test_committer_only_ewses(self):
     119        self._test_committer_only_ews(MacEWS())
     120        self._test_committer_only_ews(ChromiumMacEWS())
     121
     122    def test_builder_ewses(self):
     123        self._test_builder_ews(ChromiumWindowsEWS())
     124        self._test_builder_ews(QtEWS())
     125        self._test_builder_ews(GtkEWS())
     126        self._test_builder_ews(EflEWS())
     127
     128    def test_testing_ewses(self):
    130129        self._test_testing_ews(ChromiumLinuxEWS())
    131 
    132     def test_chromium_windows_ews(self):
    133         self._test_ews(ChromiumWindowsEWS())
    134 
    135     def test_qt_ews(self):
    136         self._test_ews(QtEWS())
    137 
    138     def test_gtk_ews(self):
    139         self._test_ews(GtkEWS())
    140 
    141     def test_efl_ews(self):
    142         self._test_ews(EflEWS())
    143 
    144     def test_mac_ews(self):
    145         self._test_committer_only_ews(MacEWS())
    146 
    147     def test_chromium_mac_ews(self):
    148         self._test_committer_only_ews(ChromiumMacEWS())

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

    r84704 r85802  
    240240        self._release_work_item(patch)
    241241
    242     def work_item_log_path(self, patch):
    243         return os.path.join(self._log_directory(), "%s.log" % patch.bug_id())
    244 
    245 
    246 class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
    247     name = "commit-queue"
    248 
    249     # AbstractPatchQueue methods
    250 
    251     def begin_work_queue(self):
    252         AbstractPatchQueue.begin_work_queue(self)
    253         self.committer_validator = CommitterValidator(self._tool.bugs)
    254         self._expected_failures = ExpectedFailures()
    255         self._layout_test_results_reader = LayoutTestResultsReader(self._tool, self._log_directory())
    256 
    257     def next_work_item(self):
    258         return self._next_patch()
    259 
    260     def should_proceed_with_work_item(self, patch):
    261         patch_text = "rollout patch" if patch.is_rollout() else "patch"
    262         self._update_status("Processing %s" % patch_text, patch)
    263         return True
    264 
    265     # FIXME: This is not really specific to the commit-queue and could be shared.
     242    # FIXME: This probably belongs at a layer below AbstractPatchQueue, but shared by CommitQueue and the EarlyWarningSystem.
    266243    def _upload_results_archive_for_patch(self, patch, results_archive_zip):
    267244        bot_id = self._tool.status_server.bot_id or "bot"
    … …  
    272249        # See https://bugs.webkit.org/show_bug.cgi?id=54593
    273250        results_archive_file.seek(0)
     251        # FIXME: This is a small lie to always say run-webkit-tests since Chromium uses new-run-webkit-tests.
     252        # We could make this code look up the test script name off the port.
    274253        comment_text = "The attached test failures were seen while running run-webkit-tests on the %s.\n" % (self.name)
    275         # FIXME: We could easily list the test failures from the archive here.
     254        # FIXME: We could easily list the test failures from the archive here,
     255        # currently callers do that separately.
    276256        comment_text += BotInfo(self._tool).summary_text()
    277257        self._tool.bugs.add_attachment_to_bug(patch.bug_id(), results_archive_file, description, filename="layout-test-results.zip", comment_text=comment_text)
     258
     259    def work_item_log_path(self, patch):
     260        return os.path.join(self._log_directory(), "%s.log" % patch.bug_id())
     261
     262
     263class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
     264    name = "commit-queue"
     265
     266    # AbstractPatchQueue methods
     267
     268    def begin_work_queue(self):
     269        AbstractPatchQueue.begin_work_queue(self)
     270        self.committer_validator = CommitterValidator(self._tool.bugs)
     271        self._expected_failures = ExpectedFailures()
     272        self._layout_test_results_reader = LayoutTestResultsReader(self._tool, self._log_directory())
     273
     274    def next_work_item(self):
     275        return self._next_patch()
     276
     277    def should_proceed_with_work_item(self, patch):
     278        patch_text = "rollout patch" if patch.is_rollout() else "patch"
     279        self._update_status("Processing %s" % patch_text, patch)
     280        return True
    278281
    279282    def process_work_item(self, patch):

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

    r84689 r85802  
    171171        self.assertEquals(queue._next_patch().id(), 197)
    172172
     173    def test_upload_results_archive_for_patch(self):
     174        queue = AbstractPatchQueue()
     175        queue.name = "mock-queue"
     176        tool = MockTool()
     177        queue.bind_to_tool(tool)
     178        queue._options = Mock()
     179        queue._options.port = None
     180        patch = queue._tool.bugs.fetch_attachment(128)
     181        expected_stderr = """MOCK add_attachment_to_bug: bug_id=42, description=Archive of layout-test-results from bot filename=layout-test-results.zip
     182-- Begin comment --
     183The attached test failures were seen while running run-webkit-tests on the mock-queue.
     184Port: MockPort  Platform: MockPlatform 1.0
     185-- End comment --
     186"""
     187        OutputCapture().assert_outputs(self, queue._upload_results_archive_for_patch, [patch, Mock()], expected_stderr=expected_stderr)
     188
    173189
    174190class NeedsUpdateSequence(StepSequence):
    … …  
    391407        OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results, MockZipFile()], expected_stderr=expected_stderr)
    392408
    393     def test_upload_results_archive_for_patch(self):
    394         queue = TestCommitQueue(MockTool())
    395         queue.begin_work_queue()
    396         patch = queue._tool.bugs.fetch_attachment(128)
    397         expected_stderr = """MOCK add_attachment_to_bug: bug_id=42, description=Archive of layout-test-results from bot filename=layout-test-results.zip
    398 -- Begin comment --
    399 The attached test failures were seen while running run-webkit-tests on the commit-queue.
    400 Port: MockPort  Platform: MockPlatform 1.0
    401 -- End comment --
    402 """
    403         OutputCapture().assert_outputs(self, queue._upload_results_archive_for_patch, [patch, Mock()], expected_stderr=expected_stderr)
    404 
    405409
    406410class StyleQueueTest(QueuesTest):
Note: See TracChangeset for help on using the changeset viewer.