Changeset 74408 in webkit


Ignore:
Timestamp:
Dec 21, 2010 4:55:45 AM (13 years ago)
Author:
eric@webkit.org
Message:

2010-12-21 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

commit-queue will report constant failures as flaky if other tests flake
​https://bugs.webkit.org/show_bug.cgi?id=51272

This patch just removes functionality and adds testing.
Previously we attempted to report flaky tests when we had
two different tests fail in a row. However, since we stop
running the tests at the first failure, our code was wrong in
trying to determine flakiness from the incomplete runs.

Originally I posted an alternate patch:
​https://bug-51272-attachments.webkit.org/attachment.cgi?id=77078
which fixed our flaky logic in this case, however it was decided
that that patch would be too difficult to maintain, so now
I'm just removing the broken logic.

This will dramatically cut-down on our flaky-test false positives
at the (small) cost of the queues being unable to report
any flakiness if the tree is very flaky. (With at least one test
flaking on every run, we'll never report failures anymore.) I think
this is a tradeoff worth making.

  • Scripts/webkitpy/tool/bot/commitqueuetask.py:
  • Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
Location:
trunk/Tools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r74403 r74408  
     12010-12-21  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        commit-queue will report constant failures as flaky if other tests flake
     6        https://bugs.webkit.org/show_bug.cgi?id=51272
     7
     8        This patch just removes functionality and adds testing.
     9        Previously we attempted to report flaky tests when we had
     10        two different tests fail in a row.  However, since we stop
     11        running the tests at the first failure, our code was wrong in
     12        trying to determine flakiness from the incomplete runs.
     13
     14        Originally I posted an alternate patch:
     15        https://bug-51272-attachments.webkit.org/attachment.cgi?id=77078
     16        which fixed our flaky logic in this case, however it was decided
     17        that that patch would be too difficult to maintain, so now
     18        I'm just removing the broken logic.
     19
     20        This will dramatically cut-down on our flaky-test false positives
     21        at the (small) cost of the queues being unable to report
     22        any flakiness if the tree is very flaky.  (With at least one test
     23        flaking on every run, we'll never report failures anymore.)  I think
     24        this is a tradeoff worth making.
     25
     26        * Scripts/webkitpy/tool/bot/commitqueuetask.py:
     27        * Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
     28
    1292010-12-20  Eric Seidel  <eric@webkit.org>
    230

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

    r74403 r74408  
    185185        second_failing_tests = self._failing_tests_from_last_run()
    186186        if first_failing_tests != second_failing_tests:
    187             self._report_flaky_tests(first_failing_tests + second_failing_tests)
     187            # We could report flaky tests here, but since run-webkit-tests
     188            # is run with --exit-after-N-failures=1, we would need to
     189            # be careful not to report constant failures as flaky due to earlier
     190            # flaky test making them not fail (no results) in one of the runs.
     191            # See https://bugs.webkit.org/show_bug.cgi?id=51272
    188192            return False
    189193
    … …  
    211215        if not self._validate():
    212216            return False
     217        # FIXME: We should understand why the land failure occured and retry if possible.
    213218        if not self._land():
    214219            raise self._script_error

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

    r74403 r74408  
    6868
    6969class CommitQueueTaskTest(unittest.TestCase):
    70     def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None):
     70    def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None, expect_retry=False):
    7171        tool = MockTool(log_executive=True)
    7272        patch = tool.bugs.fetch_attachment(197)
    7373        task = CommitQueueTask(commit_queue, patch)
    74         OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
     74        success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
     75        if not expected_exception:
     76            self.assertEqual(success, not expect_retry)
    7577
    7678    def test_success_case(self):
    … …  
    98100command_failed: failure_message='Unable to clean working directory' script_error='MOCK clean failure' patch='197'
    99101"""
    100         self._run_through_task(commit_queue, expected_stderr)
     102        self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
    101103
    102104    def test_update_failure(self):
    … …  
    110112command_failed: failure_message='Unable to update working directory' script_error='MOCK update failure' patch='197'
    111113"""
    112         self._run_through_task(commit_queue, expected_stderr)
     114        self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
    113115
    114116    def test_apply_failure(self):
    … …  
    166168command_failed: failure_message='Unable to build without patch' script_error='MOCK clean build failure' patch='197'
    167169"""
    168         self._run_through_task(commit_queue, expected_stderr)
     170        self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
    169171
    170172    def test_flaky_test_failure(self):
    … …  
    194196        self._run_through_task(commit_queue, expected_stderr)
    195197
     198    _double_flaky_test_counter = 0
     199
     200    def test_double_flaky_test_failure(self):
     201        commit_queue = MockCommitQueue([
     202            None,
     203            None,
     204            None,
     205            None,
     206            ScriptError("MOCK test failure"),
     207            ScriptError("MOCK test failure again"),
     208        ])
     209        # The (subtle) point of this test is that report_flaky_tests does not appear
     210        # in the expected_stderr for this run.
     211        # Note also that there is no attempt to run the tests w/o the patch.
     212        expected_stderr = """run_webkit_patch: ['clean']
     213command_passed: success_message='Cleaned working directory' patch='197'
     214run_webkit_patch: ['update']
     215command_passed: success_message='Updated working directory' patch='197'
     216run_webkit_patch: ['apply-attachment', '--no-update', '--non-interactive', 197]
     217command_passed: success_message='Applied patch' patch='197'
     218run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']
     219command_passed: success_message='Built patch' patch='197'
     220run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
     221command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
     222run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
     223command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
     224"""
     225        tool = MockTool(log_executive=True)
     226        patch = tool.bugs.fetch_attachment(197)
     227        task = CommitQueueTask(commit_queue, patch)
     228        self._double_flaky_test_counter = 0
     229
     230        def mock_failing_tests_from_last_run():
     231            CommitQueueTaskTest._double_flaky_test_counter += 1
     232            if CommitQueueTaskTest._double_flaky_test_counter % 2:
     233                return ['foo.html']
     234            return ['bar.html']
     235
     236        task._failing_tests_from_last_run = mock_failing_tests_from_last_run
     237        success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr)
     238        self.assertEqual(success, False)
     239
    196240    def test_test_failure(self):
    197241        commit_queue = MockCommitQueue([
    … …  
    245289command_failed: failure_message='Unable to pass tests without patch (tree is red?)' script_error='MOCK clean test failure' patch='197'
    246290"""
    247         self._run_through_task(commit_queue, expected_stderr)
     291        self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
    248292
    249293    def test_land_failure(self):
    … …  
    269313command_failed: failure_message='Unable to land patch' script_error='MOCK land failure' patch='197'
    270314"""
     315        # FIXME: This should really be expect_retry=True for a better user experiance.
    271316        self._run_through_task(commit_queue, expected_stderr, ScriptError)
Note: See TracChangeset for help on using the changeset viewer.