- Revision
- 123845
- Author
- [email protected]
- Date
- 2012-07-27 00:48:04 -0700 (Fri, 27 Jul 2012)
Log Message
Simplify ExpectedFailures
https://bugs.webkit.org/show_bug.cgi?id=92216
Reviewed by Eric Seidel.
This patch simplifies the ExpectedFailures class we use to remember
which tests are currently failing on the bots. When we wrote this code
originally, we weren't entirely sure how it would work. Now that we
understand it more clearly, we can write the code more clearly.
* Scripts/webkitpy/tool/bot/expectedfailures.py:
(_has_failures):
(_is_trustworthy):
(ExpectedFailures.__init__):
(ExpectedFailures.failures_were_expected):
(ExpectedFailures.unexpected_failures_observed):
(ExpectedFailures.update):
* Scripts/webkitpy/tool/bot/expectedfailures_unittest.py:
(ExpectedFailuresTest._assert_can_trust):
(ExpectedFailuresTest.test_failures_were_expected):
(ExpectedFailuresTest.test_unexpected_failures_observed):
(ExpectedFailuresTest.test_unexpected_failures_observed_when_tree_is_hosed):
* Scripts/webkitpy/tool/bot/patchanalysistask.py:
(PatchAnalysisTask._test):
(PatchAnalysisTask._build_and_test_without_patch):
(PatchAnalysisTask._test_patch):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (123844 => 123845)
--- trunk/Tools/ChangeLog 2012-07-27 07:44:19 UTC (rev 123844)
+++ trunk/Tools/ChangeLog 2012-07-27 07:48:04 UTC (rev 123845)
@@ -1,3 +1,32 @@
+2012-07-27 Adam Barth <[email protected]>
+
+ Simplify ExpectedFailures
+ https://bugs.webkit.org/show_bug.cgi?id=92216
+
+ Reviewed by Eric Seidel.
+
+ This patch simplifies the ExpectedFailures class we use to remember
+ which tests are currently failing on the bots. When we wrote this code
+ originally, we weren't entirely sure how it would work. Now that we
+ understand it more clearly, we can write the code more clearly.
+
+ * Scripts/webkitpy/tool/bot/expectedfailures.py:
+ (_has_failures):
+ (_is_trustworthy):
+ (ExpectedFailures.__init__):
+ (ExpectedFailures.failures_were_expected):
+ (ExpectedFailures.unexpected_failures_observed):
+ (ExpectedFailures.update):
+ * Scripts/webkitpy/tool/bot/expectedfailures_unittest.py:
+ (ExpectedFailuresTest._assert_can_trust):
+ (ExpectedFailuresTest.test_failures_were_expected):
+ (ExpectedFailuresTest.test_unexpected_failures_observed):
+ (ExpectedFailuresTest.test_unexpected_failures_observed_when_tree_is_hosed):
+ * Scripts/webkitpy/tool/bot/patchanalysistask.py:
+ (PatchAnalysisTask._test):
+ (PatchAnalysisTask._build_and_test_without_patch):
+ (PatchAnalysisTask._test_patch):
+
2012-07-27 Csaba Osztrogonác <[email protected]>
[Qt][WK2] REGRESSION(r119127): resetting window.internals settings between tests doesn't work properly
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures.py (123844 => 123845)
--- trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures.py 2012-07-27 07:44:19 UTC (rev 123844)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures.py 2012-07-27 07:48:04 UTC (rev 123845)
@@ -30,45 +30,31 @@
class ExpectedFailures(object):
def __init__(self):
self._failures = set()
- # If the set of failures is unbounded, self._failures isn't very
- # meaningful because we can't store an unbounded set in memory.
- self._failures_are_bounded = True
+ self._is_trustworthy = True
- def _has_failures(self, results):
- return bool(results and len(results.failing_tests()) != 0)
+ @classmethod
+ def _has_failures(cls, results):
+ return bool(results and results.failing_tests())
- def has_bounded_failures(self, results):
- assert(results) # You probably want to call _has_failures first!
- return bool(results.failure_limit_count() and len(results.failing_tests()) < results.failure_limit_count())
+ @classmethod
+ def _should_trust(cls, results):
+ return bool(cls._has_failures(results) and results.failure_limit_count() and len(results.failing_tests()) < results.failure_limit_count())
- def _can_trust_results(self, results):
- return self._has_failures(results) and self.has_bounded_failures(results)
-
def failures_were_expected(self, results):
- if not self._can_trust_results(results):
+ if not self._is_trustworthy:
return False
+ if not self._should_trust(results):
+ return False
return set(results.failing_tests()) <= self._failures
def unexpected_failures_observed(self, results):
+ if not self._is_trustworthy:
+ return None
if not self._has_failures(results):
return None
- if not self._failures_are_bounded:
- return None
return set(results.failing_tests()) - self._failures
- def shrink_expected_failures(self, results, run_success):
- if run_success:
- self._failures = set()
- self._failures_are_bounded = True
- elif self._can_trust_results(results):
- # Remove all expected failures which are not in the new failing results.
- self._failures.intersection_update(set(results.failing_tests()))
- self._failures_are_bounded = True
-
- def grow_expected_failures(self, results):
- if not self._can_trust_results(results):
- self._failures_are_bounded = False
- return
- self._failures.update(results.failing_tests())
- self._failures_are_bounded = True
- # FIXME: Should we assert() here that expected_failures never crosses a certain size?
+ def update(self, results):
+ if results:
+ self._failures = set(results.failing_tests())
+ self._is_trustworthy = self._should_trust(results)
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures_unittest.py (123844 => 123845)
--- trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures_unittest.py 2012-07-27 07:44:19 UTC (rev 123844)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures_unittest.py 2012-07-27 07:48:04 UTC (rev 123845)
@@ -45,7 +45,7 @@
class ExpectedFailuresTest(unittest.TestCase):
def _assert_can_trust(self, results, can_trust):
- self.assertEquals(ExpectedFailures()._can_trust_results(results), can_trust)
+ self.assertEquals(ExpectedFailures._should_trust(results), can_trust)
def test_can_trust_results(self):
self._assert_can_trust(None, False)
@@ -61,21 +61,22 @@
def test_failures_were_expected(self):
failures = ExpectedFailures()
- failures.grow_expected_failures(MockResults(['foo.html']))
+ failures.update(MockResults(['foo.html']))
self._assert_expected(failures, ['foo.html'], True)
self._assert_expected(failures, ['bar.html'], False)
- failures.shrink_expected_failures(MockResults(['baz.html']), False)
+ self._assert_expected(failures, ['bar.html', 'foo.html'], False)
+
+ failures.update(MockResults(['baz.html']))
+ self._assert_expected(failures, ['baz.html'], True)
self._assert_expected(failures, ['foo.html'], False)
- self._assert_expected(failures, ['baz.html'], False)
- failures.grow_expected_failures(MockResults(['baz.html']))
- self._assert_expected(failures, ['baz.html'], True)
- failures.shrink_expected_failures(MockResults(), True)
+ failures.update(MockResults([]))
self._assert_expected(failures, ['baz.html'], False)
+ self._assert_expected(failures, ['foo.html'], False)
def test_unexpected_failures_observed(self):
failures = ExpectedFailures()
- failures.grow_expected_failures(MockResults(['foo.html']))
+ failures.update(MockResults(['foo.html']))
self.assertEquals(failures.unexpected_failures_observed(MockResults(['foo.html', 'bar.html'])), set(['bar.html']))
self.assertEquals(failures.unexpected_failures_observed(MockResults(['baz.html'])), set(['baz.html']))
unbounded_results = MockResults(['baz.html', 'qux.html', 'taco.html'], failure_limit=3)
@@ -85,7 +86,7 @@
def test_unexpected_failures_observed_when_tree_is_hosed(self):
failures = ExpectedFailures()
- failures.grow_expected_failures(MockResults(['foo.html', 'banana.html'], failure_limit=2))
+ failures.update(MockResults(['foo.html', 'banana.html'], failure_limit=2))
self.assertEquals(failures.unexpected_failures_observed(MockResults(['foo.html', 'bar.html'])), None)
self.assertEquals(failures.unexpected_failures_observed(MockResults(['baz.html'])), None)
unbounded_results = MockResults(['baz.html', 'qux.html', 'taco.html'], failure_limit=3)
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py (123844 => 123845)
--- trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py 2012-07-27 07:44:19 UTC (rev 123844)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py 2012-07-27 07:48:04 UTC (rev 123845)
@@ -134,7 +134,7 @@
"Unable to build without patch")
def _test(self):
- success = self._run_command([
+ return self._run_command([
"build-and-test",
"--no-clean",
"--no-update",
@@ -145,11 +145,8 @@
"Passed tests",
"Patch does not pass tests")
- self._expected_failures.shrink_expected_failures(self._delegate.test_results(), success)
- return success
-
def _build_and_test_without_patch(self):
- success = self._run_command([
+ return self._run_command([
"build-and-test",
"--force-clean",
"--no-update",
@@ -160,9 +157,6 @@
"Able to pass tests without patch",
"Unable to pass tests without patch (tree is red?)")
- self._expected_failures.shrink_expected_failures(self._delegate.test_results(), success)
- return success
-
def _land(self):
# Unclear if this should pass --quiet or not. If --parent-command always does the reporting, then it should.
return self._run_command([
@@ -220,7 +214,7 @@
return self.report_failure(first_results_archive, first_results, first_script_error)
clean_tree_results = self._delegate.test_results()
- self._expected_failures.grow_expected_failures(clean_tree_results)
+ self._expected_failures.update(clean_tree_results)
# Re-check if the original results are now to be expected to avoid a full re-try.
if self._expected_failures.failures_were_expected(first_results):
