Changeset 86020 in webkit

Timestamp:

May 7, 2011 7:50:09 PM (13 years ago)

Author:

abarth@webkit.org

Message:

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

Reviewed by Eric Seidel.

Testing EWS spins on patches with a large number of failures
​https://bugs.webkit.org/show_bug.cgi?id=60441

In cases where the tree has a small number of persistent failures but a
patch has a large number of failures, we weren't believing any results.
That lead to us retrying the patch forever while we waited for
trustworthy results that would never come.

This patch loosens the semantics of unexpected_failures to return a
subset (rather than the exact set) of unexpected failures. In the case
where the tree has a bounded number of failures and the patch has an
unbounded number of failures, we can accurately compute such a subset
and reject the patch.

• Scripts/webkitpy/tool/bot/expectedfailures.py:
• Scripts/webkitpy/tool/bot/expectedfailures_unittest.py:
• Scripts/webkitpy/tool/bot/patchanalysistask.py:
• Scripts/webkitpy/tool/commands/earlywarningsystem.py:
• Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/tool/bot/expectedfailures.py (modified) (3 diffs)
• Scripts/webkitpy/tool/bot/expectedfailures_unittest.py (modified) (1 diff)
• Scripts/webkitpy/tool/bot/patchanalysistask.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/earlywarningsystem.py (modified) (1 diff)
• Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-05-07  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Testing EWS spins on patches with a large number of failures
+        https://bugs.webkit.org/show_bug.cgi?id=60441
+
+        In cases where the tree has a small number of persistent failures but a
+        patch has a large number of failures, we weren't believing any results.
+        That lead to us retrying the patch forever while we waited for
+        trustworthy results that would never come.
+
+        This patch loosens the semantics of unexpected_failures to return a
+        subset (rather than the exact set) of unexpected failures.  In the case
+        where the tree has a bounded number of failures and the patch has an
+        unbounded number of failures, we can accurately compute such a subset
+        and reject the patch.
+
+        * Scripts/webkitpy/tool/bot/expectedfailures.py:
+        * Scripts/webkitpy/tool/bot/expectedfailures_unittest.py:
+        * Scripts/webkitpy/tool/bot/patchanalysistask.py:
+        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+        * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
+
 2011-05-07  Joe Mason  <jmason@rim.com>
 

trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures.py

@@ -31 +31 @@
     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
+
+    def _has_failures(self, results):
+        return bool(results and len(results.failing_tests()) != 0)
+
+    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())
 
     def _can_trust_results(self, results):
-        if not results or not results.failure_limit_count():
-            return False
-        return len(results.failing_tests()) != 0 and len(results.failing_tests()) < results.failure_limit_count()
+        return self._has_failures(results) and self.has_bounded_failures(results)
 
     def failures_were_expected(self, results):
@@ -42 +50 @@
         return set(results.failing_tests()) <= self._failures
 
-    def unexpected_failures(self, results):
-        if not self._can_trust_results(results):
+    def unexpected_failures_observed(self, results):
+        if not self._has_failures(results):
+            return None
+        if not self._failures_are_bounded:
             return None
         return set(results.failing_tests()) - self._failures
@@ -50 +60 @@
         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?

trunk/Tools/Scripts/webkitpy/tool/bot/expectedfailures_unittest.py

@@ -74 +74 @@
         self._assert_expected(failures, ['baz.html'], False)
 
-    def test_unexpected_failures(self):
+    def test_unexpected_failures_observed(self):
         failures = ExpectedFailures()
         failures.grow_expected_failures(MockResults(['foo.html']))
-        self.assertEquals(failures.unexpected_failures(MockResults(['foo.html', 'bar.html'])), set(['bar.html']))
-        self.assertEquals(failures.unexpected_failures(MockResults(['baz.html'])), set(['baz.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)
+        self.assertEquals(failures.unexpected_failures_observed(unbounded_results), set(['baz.html', 'qux.html', 'taco.html']))
+        unbounded_results_with_existing_failure = MockResults(['foo.html', 'baz.html', 'qux.html', 'taco.html'], failure_limit=4)
+        self.assertEquals(failures.unexpected_failures_observed(unbounded_results_with_existing_failure), set(['baz.html', 'qux.html', 'taco.html']))
+
+    def test_unexpected_failures_observed_when_tree_is_hosed(self):
+        failures = ExpectedFailures()
+        failures.grow_expected_failures(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)
+        self.assertEquals(failures.unexpected_failures_observed(unbounded_results), None)
+        unbounded_results_with_existing_failure = MockResults(['foo.html', 'baz.html', 'qux.html', 'taco.html'], failure_limit=4)
+        self.assertEquals(failures.unexpected_failures_observed(unbounded_results_with_existing_failure), None)

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

@@ -221 +221 @@
         # Now that we have updated information about failing tests with a clean checkout, we can
         # tell if our original failures were unexpected and fail the patch if necessary.
-        if self._expected_failures.unexpected_failures(first_results):
+        if self._expected_failures.unexpected_failures_observed(first_results):
             return self.report_failure(first_results_archive, first_results)
 

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

@@ -131 +131 @@
     def _failing_tests_message(self, task, patch):
         results = task.results_from_patch_test_run(patch)
-        unexpected_failures = self._expected_failures.unexpected_failures(results)
+        unexpected_failures = self._expected_failures.unexpected_failures_observed(results)
         if not unexpected_failures:
             return None

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

@@ -92 +92 @@
         ews._options = MockOptions(port=None, confirm=False)
         OutputCapture().assert_outputs(self, ews.begin_work_queue, expected_stderr=self._default_begin_work_queue_stderr(ews.name, ews._tool.scm().checkout_root))
-        ews._expected_failures.unexpected_failures = lambda results: set(["foo.html", "bar.html"])
+        ews._expected_failures.unexpected_failures_observed = lambda results: set(["foo.html", "bar.html"])
         task = Mock()
         patch = ews._tool.bugs.fetch_attachment(197)