nrwt: implement the actual cascade of TestExpectations
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2012 20:43:13 +0000 (20:43 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2012 20:43:13 +0000 (20:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88947

Reviewed by Ojan Vafai.

This change implements the actual cascade by removing the
concept of 'overrides' from the TestExpectations object and
parsing each file separately.

There is an actual semantic change in this patch, in that
setting an expectation on a directory in one file will override
the expectations on any individual tests set in prior files. The
test_overrides__directory() unit test verifies this.

Otherwise, this patch mostly consists of deleting code :).

* Scripts/webkitpy/layout_tests/models/test_expectations.py:
(TestExpectationsModel.__init__):
(TestExpectationsModel.add_expectation_line):
(TestExpectationsModel._add_test):
(TestExpectationsModel._already_seen_better_match):
(TestExpectations.__init__):
(TestExpectations._add_expectations):
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
(test_overrides__directory):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@120243 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py

index 50d9aeb..d5557c2 100644 (file)
@@ -1,5 +1,33 @@
 2012-06-13  Dirk Pranke  <dpranke@chromium.org>
 
+        nrwt: implement the actual cascade of TestExpectations
+        https://bugs.webkit.org/show_bug.cgi?id=88947
+
+        Reviewed by Ojan Vafai.
+
+        This change implements the actual cascade by removing the
+        concept of 'overrides' from the TestExpectations object and
+        parsing each file separately.
+
+        There is an actual semantic change in this patch, in that
+        setting an expectation on a directory in one file will override
+        the expectations on any individual tests set in prior files. The
+        test_overrides__directory() unit test verifies this.
+
+        Otherwise, this patch mostly consists of deleting code :).
+
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (TestExpectationsModel.__init__):
+        (TestExpectationsModel.add_expectation_line):
+        (TestExpectationsModel._add_test):
+        (TestExpectationsModel._already_seen_better_match):
+        (TestExpectations.__init__):
+        (TestExpectations._add_expectations):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (test_overrides__directory):
+
+2012-06-13  Dirk Pranke  <dpranke@chromium.org>
+
         webkitpy: update callers to use port.expectation_dict() instead of test_expectations() and test_expectations_overrides()
         https://bugs.webkit.org/show_bug.cgi?id=88946
 
index 0bbb8c7..906401d 100644 (file)
@@ -446,13 +446,6 @@ class TestExpectationsModel(object):
         # Maps a test to a TestExpectationLine instance.
         self._test_to_expectation_line = {}
 
-        # List of tests that are in the overrides file (used for checking for
-        # duplicates inside the overrides file itself). Note that just because
-        # a test is in this set doesn't mean it's necessarily overridding a
-        # expectation in the regular expectations; the test might not be
-        # mentioned in the regular expectations file at all.
-        self._overridding_tests = set()
-
         self._modifier_to_tests = self._dict_of_sets(TestExpectations.MODIFIERS)
         self._expectation_to_tests = self._dict_of_sets(TestExpectations.EXPECTATIONS)
         self._timeline_to_tests = self._dict_of_sets(TestExpectations.TIMELINES)
@@ -524,31 +517,26 @@ class TestExpectationsModel(object):
     def get_expectations(self, test):
         return self._test_to_expectations[test]
 
-    def add_expectation_line(self, expectation_line, in_overrides=False, in_skipped=False):
+    def add_expectation_line(self, expectation_line, in_skipped=False):
         """Returns a list of warnings encountered while matching modifiers."""
 
         if expectation_line.is_invalid():
             return
 
         for test in expectation_line.matching_tests:
-            if not in_skipped and self._already_seen_better_match(test, expectation_line, in_overrides):
+            if not in_skipped and self._already_seen_better_match(test, expectation_line):
                 continue
 
             self._clear_expectations_for_test(test, expectation_line)
             self._test_to_expectation_line[test] = expectation_line
-            self._add_test(test, expectation_line, in_overrides)
+            self._add_test(test, expectation_line)
 
-    def _add_test(self, test, expectation_line, in_overrides):
+    def _add_test(self, test, expectation_line):
         """Sets the expected state for a given test.
 
         This routine assumes the test has not been added before. If it has,
         use _clear_expectations_for_test() to reset the state prior to
-        calling this.
-
-        Args:
-          test: test to add
-          expectation_line: expectation to add
-          in_overrides: whether we're parsing the regular expectations or the overridding expectations"""
+        calling this."""
         self._test_to_expectations[test] = expectation_line.parsed_expectations
         for expectation in expectation_line.parsed_expectations:
             self._expectation_to_tests[expectation].add(test)
@@ -572,9 +560,6 @@ class TestExpectationsModel(object):
         else:
             self._result_type_to_tests[FAIL].add(test)
 
-        if in_overrides:
-            self._overridding_tests.add(test)
-
     def _clear_expectations_for_test(self, test, expectation_line):
         """Remove prexisting expectations for this test.
         This happens if we are seeing a more precise path
@@ -597,7 +582,7 @@ class TestExpectationsModel(object):
             if test in set_of_tests:
                 set_of_tests.remove(test)
 
-    def _already_seen_better_match(self, test, expectation_line, in_overrides):
+    def _already_seen_better_match(self, test, expectation_line):
         """Returns whether we've seen a better match already in the file.
 
         Returns True if we've already seen a expectation_line.name that matches more of the test
@@ -610,6 +595,10 @@ class TestExpectationsModel(object):
 
         prev_expectation_line = self._test_to_expectation_line[test]
 
+        if prev_expectation_line.filename != expectation_line.filename:
+            # We've moved on to a new expectation file, which overrides older ones.
+            return False
+
         if len(prev_expectation_line.path) > len(expectation_line.path):
             # The previous path matched more of the test.
             return True
@@ -618,20 +607,9 @@ class TestExpectationsModel(object):
             # This path matches more of the test.
             return False
 
-        if in_overrides and test not in self._overridding_tests:
-            # We have seen this path, but that's okay because it is
-            # in the overrides and the earlier path was in the
-            # expectations (not the overrides).
-            return False
-
         # At this point we know we have seen a previous exact match on this
         # base path, so we need to check the two sets of modifiers.
 
-        if in_overrides:
-            expectation_source = "override"
-        else:
-            expectation_source = "expectation"
-
         # FIXME: This code was originally designed to allow lines that matched
         # more modifiers to override lines that matched fewer modifiers.
         # However, we currently view these as errors.
@@ -760,13 +738,13 @@ class TestExpectations(object):
 
         expectations_dict = port.expectations_dict()
         self._expectations = self._parser.parse(expectations_dict.keys()[0], expectations_dict.values()[0])
-        self._add_expectations(self._expectations, in_overrides=False)
+        self._add_expectations(self._expectations)
 
         if len(expectations_dict) > 1 and include_overrides:
-            overrides = ''.join(expectations_dict.values()[1:])
-            overrides_expectations = self._parser.parse('overrides', overrides)
-            self._add_expectations(overrides_expectations, in_overrides=True)
-            self._expectations += overrides_expectations
+            for name in expectations_dict.keys()[1:]:
+                expectations = self._parser.parse(name, expectations_dict[name])
+                self._add_expectations(expectations)
+                self._expectations += expectations
 
         # FIXME: move ignore_tests into port.skipped_layout_tests()
         self._add_skipped_tests(port.skipped_layout_tests(tests).union(set(port.get_option('ignore_tests', []))))
@@ -896,13 +874,13 @@ class TestExpectations(object):
 
         return TestExpectationSerializer.list_to_string(filter(without_rebaseline_modifier, self._expectations))
 
-    def _add_expectations(self, expectation_list, in_overrides):
+    def _add_expectations(self, expectation_list):
         for expectation_line in expectation_list:
             if not expectation_line.expectations:
                 continue
 
             if self._is_lint_mode or self._test_config in expectation_line.matching_configurations:
-                self._model.add_expectation_line(expectation_line, in_overrides)
+                self._model.add_expectation_line(expectation_line)
 
     def _add_skipped_tests(self, tests_to_skip):
         if not tests_to_skip:
index 5fcf156..a51b99e 100644 (file)
@@ -244,6 +244,12 @@ SKIP : failures/expected/image.html""", is_lint_mode=True)
                        "BUG_OVERRIDE : failures/expected/text.html = IMAGE")
         self.assert_exp('failures/expected/text.html', IMAGE)
 
+    def test_overrides__directory(self):
+        self.parse_exp("BUG_EXP: failures/expected/text.html = TEXT",
+                       "BUG_OVERRIDE: failures/expected = CRASH")
+        self.assert_exp('failures/expected/text.html', CRASH)
+        self.assert_exp('failures/expected/image.html', CRASH)
+
     def test_overrides__duplicate(self):
         self.assert_bad_expectations("BUG_EXP: failures/expected/text.html = TEXT",
                                      "BUG_OVERRIDE : failures/expected/text.html = IMAGE\n"