[SCM] WebKit Debian packaging branch, debian/experimental, updated. debian/1.3.8-1-142-g786665c
eric at webkit.org
eric at webkit.org
Mon Dec 27 16:27:05 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 73d4f991f1cebfe6ef0943cda462747b90f6edcd
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Dec 21 12:55:45 2010 +0000
2010-12-21 Eric Seidel <eric at 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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@74408 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 2f3b843..c949b21 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,31 @@
+2010-12-21 Eric Seidel <eric at 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:
+
2010-12-20 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py
index 6a838e8..1d82ea8 100644
--- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py
+++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py
@@ -184,7 +184,11 @@ class CommitQueueTask(object):
second_failing_tests = self._failing_tests_from_last_run()
if first_failing_tests != second_failing_tests:
- self._report_flaky_tests(first_failing_tests + second_failing_tests)
+ # We could report flaky tests here, but since run-webkit-tests
+ # is run with --exit-after-N-failures=1, we would need to
+ # be careful not to report constant failures as flaky due to earlier
+ # flaky test making them not fail (no results) in one of the runs.
+ # See https://bugs.webkit.org/show_bug.cgi?id=51272
return False
if self._build_and_test_without_patch():
@@ -210,6 +214,7 @@ class CommitQueueTask(object):
# no one has set commit-queue- since we started working on the patch.)
if not self._validate():
return False
+ # FIXME: We should understand why the land failure occured and retry if possible.
if not self._land():
raise self._script_error
return True
diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
index fed7fa9..376f407 100644
--- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
+++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
@@ -67,11 +67,13 @@ class MockCommitQueue(CommitQueueTaskDelegate):
class CommitQueueTaskTest(unittest.TestCase):
- def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None):
+ def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None, expect_retry=False):
tool = MockTool(log_executive=True)
patch = tool.bugs.fetch_attachment(197)
task = CommitQueueTask(commit_queue, patch)
- OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
+ success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
+ if not expected_exception:
+ self.assertEqual(success, not expect_retry)
def test_success_case(self):
commit_queue = MockCommitQueue([])
@@ -97,7 +99,7 @@ command_passed: success_message='Landed patch' patch='197'
expected_stderr = """run_webkit_patch: ['clean']
command_failed: failure_message='Unable to clean working directory' script_error='MOCK clean failure' patch='197'
"""
- self._run_through_task(commit_queue, expected_stderr)
+ self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
def test_update_failure(self):
commit_queue = MockCommitQueue([
@@ -109,7 +111,7 @@ command_passed: success_message='Cleaned working directory' patch='197'
run_webkit_patch: ['update']
command_failed: failure_message='Unable to update working directory' script_error='MOCK update failure' patch='197'
"""
- self._run_through_task(commit_queue, expected_stderr)
+ self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
def test_apply_failure(self):
commit_queue = MockCommitQueue([
@@ -165,7 +167,7 @@ command_failed: failure_message='Patch does not build' script_error='MOCK build
run_webkit_patch: ['build', '--force-clean', '--no-update', '--build-style=both']
command_failed: failure_message='Unable to build without patch' script_error='MOCK clean build failure' patch='197'
"""
- self._run_through_task(commit_queue, expected_stderr)
+ self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
def test_flaky_test_failure(self):
commit_queue = MockCommitQueue([
@@ -193,6 +195,48 @@ command_passed: success_message='Landed patch' patch='197'
"""
self._run_through_task(commit_queue, expected_stderr)
+ _double_flaky_test_counter = 0
+
+ def test_double_flaky_test_failure(self):
+ commit_queue = MockCommitQueue([
+ None,
+ None,
+ None,
+ None,
+ ScriptError("MOCK test failure"),
+ ScriptError("MOCK test failure again"),
+ ])
+ # The (subtle) point of this test is that report_flaky_tests does not appear
+ # in the expected_stderr for this run.
+ # Note also that there is no attempt to run the tests w/o the patch.
+ expected_stderr = """run_webkit_patch: ['clean']
+command_passed: success_message='Cleaned working directory' patch='197'
+run_webkit_patch: ['update']
+command_passed: success_message='Updated working directory' patch='197'
+run_webkit_patch: ['apply-attachment', '--no-update', '--non-interactive', 197]
+command_passed: success_message='Applied patch' patch='197'
+run_webkit_patch: ['build', '--no-clean', '--no-update', '--build-style=both']
+command_passed: success_message='Built patch' patch='197'
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure' patch='197'
+run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']
+command_failed: failure_message='Patch does not pass tests' script_error='MOCK test failure again' patch='197'
+"""
+ tool = MockTool(log_executive=True)
+ patch = tool.bugs.fetch_attachment(197)
+ task = CommitQueueTask(commit_queue, patch)
+ self._double_flaky_test_counter = 0
+
+ def mock_failing_tests_from_last_run():
+ CommitQueueTaskTest._double_flaky_test_counter += 1
+ if CommitQueueTaskTest._double_flaky_test_counter % 2:
+ return ['foo.html']
+ return ['bar.html']
+
+ task._failing_tests_from_last_run = mock_failing_tests_from_last_run
+ success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr)
+ self.assertEqual(success, False)
+
def test_test_failure(self):
commit_queue = MockCommitQueue([
None,
@@ -244,7 +288,7 @@ command_failed: failure_message='Patch does not pass tests' script_error='MOCK t
run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive']
command_failed: failure_message='Unable to pass tests without patch (tree is red?)' script_error='MOCK clean test failure' patch='197'
"""
- self._run_through_task(commit_queue, expected_stderr)
+ self._run_through_task(commit_queue, expected_stderr, expect_retry=True)
def test_land_failure(self):
commit_queue = MockCommitQueue([
@@ -268,4 +312,5 @@ command_passed: success_message='Passed tests' patch='197'
run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 197]
command_failed: failure_message='Unable to land patch' script_error='MOCK land failure' patch='197'
"""
+ # FIXME: This should really be expect_retry=True for a better user experiance.
self._run_through_task(commit_queue, expected_stderr, ScriptError)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list