Rename ModifierMatcher to SpecificityCalculator.
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Jul 2011 07:38:01 +0000 (07:38 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 17 Jul 2011 07:38:01 +0000 (07:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=64660

It's a little longer than before, but it is much clearer.

Reviewed by Adam Barth.

* Scripts/webkitpy/layout_tests/models/test_expectations.py: Renamed and adjusted
    all callsites, also rewrote the comments.
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py: Adjusted callsites
    and renamed tests.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@91166 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 488d16c..0909558 100644 (file)
@@ -1,3 +1,17 @@
+2011-07-17  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Rename ModifierMatcher to SpecificityCalculator.
+        https://bugs.webkit.org/show_bug.cgi?id=64660
+
+        It's a little longer than before, but it is much clearer.
+
+        Reviewed by Adam Barth.
+
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py: Renamed and adjusted
+            all callsites, also rewrote the comments.
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py: Adjusted callsites
+            and renamed tests.
+
 2011-07-15  Adam Barth  <abarth@webkit.org>
 
         gardening server should proxy buildbot status for garden-o-matic
index f4d7c2a..76be89b 100644 (file)
@@ -176,15 +176,15 @@ class TestExpectationParser:
 
     def __init__(self, port, test_config, full_test_list, allow_rebaseline_modifier):
         self._port = port
-        self._matcher = ModifierMatcher(test_config)
+        self._specificity_calculator = SpecificityCalculator(test_config)
         self._full_test_list = full_test_list
         self._allow_rebaseline_modifier = allow_rebaseline_modifier
 
     def parse(self, expectation_line):
-        self._matcher.match(expectation_line)
+        self._specificity_calculator.calculate(expectation_line)
         self._check_semantics(expectation_line)
 
-        if expectation_line.num_matches == ModifierMatcher.NO_MATCH:
+        if expectation_line.specificity == SpecificityCalculator.INVALID:
             return
 
         self._check_modifiers_against_expectations(expectation_line)
@@ -329,7 +329,7 @@ class TestExpectationLine:
         self.expectations = []
         self.parsed_expectations = set()
         self.comment = None
-        self.num_matches = ModifierMatcher.NO_MATCH
+        self.specificity = SpecificityCalculator.INVALID
         self.matching_tests = []
         self.errors = []
         self.warnings = []
@@ -497,7 +497,7 @@ class TestExpectationsModel:
         Returns True if we've already seen a expectation_line.name that matches more of the test
             than this path does
         """
-        # FIXME: See comment below about matching test configs and num_matches.
+        # FIXME: See comment below about matching test configs and specificity.
         if not self.has_test(test):
             # We've never seen this test before.
             return False
@@ -530,16 +530,16 @@ class TestExpectationsModel:
         # more modifiers to override lines that matched fewer modifiers.
         # However, we currently view these as errors. If we decide to make
         # this policy permanent, we can probably simplify this code
-        # and the ModifierMatcher code a fair amount.
+        # and the SpecificityCalculator code a fair amount.
         #
         # To use the "more modifiers wins" policy, change the errors for overrides
         # to be warnings and return False".
 
-        if prev_expectation_line.num_matches == expectation_line.num_matches:
+        if prev_expectation_line.specificity == expectation_line.specificity:
             expectation_line.errors.append('Duplicate or ambiguous %s.' % expectation_source)
             return True
 
-        if prev_expectation_line.num_matches < expectation_line.num_matches:
+        if prev_expectation_line.specificity < expectation_line.specificity:
             expectation_line.errors.append('More specific entry on line %d overrides line %d' % (lineno, prev_lineno))
             # FIXME: return False if we want more specific to win.
             return True
@@ -806,31 +806,31 @@ class TestExpectations:
             self._model.add_expectation_line(lineno, expectation_line, overrides_allowed)
 
 
-class ModifierMatchResult(object):
+class SpecificityCalculation(object):
     def __init__(self, modifiers):
-        self.num_matches = ModifierMatcher.NO_MATCH
+        self.specificity = SpecificityCalculator.INVALID
         self._modifiers = modifiers
         self._matched_modifiers = []
         self._matched_regexes = set()
         self._matched_macros = set()
 
 
-class ModifierMatcher(object):
+class SpecificityCalculator(object):
 
     """
-    This class manages the interpretation of the "modifiers" for a given
-    line in the expectations file. Modifiers are the tokens that appear to the
-    left of the colon on a line. For example, "BUG1234", "DEBUG", and "WIN" are
-    all modifiers. This class gets what the valid modifiers are, and which
-    modifiers are allowed to exist together on a line, from the
-    TestConfiguration object that is passed in to the call.
-
-    This class detects *intra*-line errors like unknown modifiers, but
-    does not detect *inter*-line modifiers like duplicate expectations.
-
-    More importantly, this class is also used to determine if a given line
-    matches the port in question. Matches are ranked according to the number
-    of modifiers that match on a line. A line with no modifiers matches
+    This class determines how specific are the modifiers for a given
+    TestExpectationLine. Some modifiers describe a test configuration for which this
+    test expectation is applicable. There is a degree of specificity for these modifiers.
+
+    For example, 'XP RELEASE CPU' is very specific, because it limits applicable test configuration to
+    Windows XP system in Release mode, with CPU-backed graphics.
+
+    On the other hand, '' (empty modifier) makes the test applicable to any test configuration.
+
+    This class finds such modifiers, interprets their meaning and determines specificity of
+    a given test expectation.
+
+    The specificity is determined as a count of modifiers that match. A line with no modifiers matches
     everything and has a score of zero. A line with one modifier matches only
     ports that have that modifier and gets a score of 1, and so one. Ports
     that don't match at all get a score of -1.
@@ -853,6 +853,9 @@ class ModifierMatcher(object):
     In addition to the definitions of all of the modifiers, the class
     supports "macros" that are expanded prior to interpretation, and "ignore
     regexes" that can be used to skip over modifiers like the BUG* modifiers.
+
+    This class also detects errors in this test expectation, like unknown modifiers,
+    invalid modifier combinations, and duplicate modifiers.
     """
     MACROS = {
         'mac': ['leopard', 'snowleopard'],
@@ -865,16 +868,16 @@ class ModifierMatcher(object):
                          TestExpectations.MODIFIERS.keys()[:-1])
     DUPLICATE_REGEXES_ALLOWED = ['bug\w+']
 
-    # Magic value returned when the modifiers don't match.
-    NO_MATCH = -1
+    # Magic value returned when the modifiers don't match the configuration at all.
+    INVALID = -1
 
     # FIXME: The code currently doesn't detect combinations of modifiers
     # that are syntactically valid but semantically invalid, like
-    # 'MAC XP'. See ModifierMatchTest.test_invalid_combinations() in the
+    # 'MAC XP'. See SpecificityCalculatorTest.test_invalid_combinations() in the
     # _unittest.py file.
 
     def __init__(self, test_config):
-        """Initialize a ModifierMatcher argument with the TestConfiguration it
+        """Initialize a SpecificityCalculator argument with the TestConfiguration it
         should be matched against."""
         self.test_config = test_config
         self.allowed_configurations = test_config.all_test_configurations()
@@ -894,23 +897,23 @@ class ModifierMatcher(object):
                 self._categories_for_modifiers[modifier] = category
                 self._all_modifiers.add(modifier)
 
-    def match(self, expectation_line):
+    def calculate(self, expectation_line):
         """Checks a expectation.modifiers against the config set in the constructor.
         Options may be either actual modifier strings, "macro" strings
         that get expanded to a list of modifiers, or strings that are allowed
         to be ignored. All of the modifiers must be passed in in lower case.
 
-        Returns the number of matching categories, or NO_MATCH (-1) if it
+        Returns specificity relative to the test configuration, or INVALID (-1) if it
         doesn't match or there were errors found. Matches are prioritized
         by the number of matching categories, because the more specific
         the modifier list, the more categories will match.
         """
         old_error_count = len(expectation_line.errors)
-        result = ModifierMatchResult(expectation_line.modifiers)
+        result = SpecificityCalculation(expectation_line.modifiers)
         self._parse(expectation_line, result)
         if old_error_count == len(expectation_line.errors):
             self._count_matches(result)
-        expectation_line.num_matches = result.num_matches
+        expectation_line.specificity = result.specificity
 
     def _parse(self, expectation_line, result):
         # FIXME: Should we warn about lines having every value in a category?
@@ -963,13 +966,13 @@ class ModifierMatcher(object):
     def _count_matches(self, result):
         """Returns the number of modifiers that match the test config."""
         categorized_modifiers = self._group_by_category(result._matched_modifiers)
-        result.num_matches = 0
+        result.specificity = 0
         for category, modifier in self.test_config.items():
             if category in categorized_modifiers:
                 if modifier in categorized_modifiers[category]:
-                    result.num_matches += 1
+                    result.specificity += 1
                 else:
-                    result.num_matches = self.NO_MATCH
+                    result.specificity = self.INVALID
                     return
 
     def _group_by_category(self, modifiers):
index e033670..53d90e4 100644 (file)
@@ -384,82 +384,79 @@ class RebaseliningTest(Base):
         self.assertEqual(len(self._exp.get_rebaselining_failures()), 0)
 
 
-class ModifierTests(unittest.TestCase):
+class SpecificityCalculatorTests(unittest.TestCase):
     def setUp(self):
         port_obj = port.get('test-win-xp', None)
         self.config = port_obj.test_configuration()
-        self.matcher = ModifierMatcher(self.config)
+        self.calculator = SpecificityCalculator(self.config)
 
-    def match(self, modifiers, expected_num_matches=-1, values=None, num_errors=0):
-        matcher = self.matcher
-        if values:
-            matcher = ModifierMatcher(self.FakeTestConfiguration(values))
+    def assert_specificity(self, modifiers, expected_specificity=-1, num_errors=0):
         expectation = TestExpectationLine()
         expectation.modifiers = modifiers
-        matcher.match(expectation)
+        self.calculator.calculate(expectation)
         self.assertEqual(len(expectation.warnings), 0)
         self.assertEqual(len(expectation.errors), num_errors)
-        self.assertEqual(expectation.num_matches, expected_num_matches,
+        self.assertEqual(expectation.specificity, expected_specificity,
              'match(%s, %s) returned -> %d, expected %d' %
              (modifiers, str(self.config.values()),
-              expectation.num_matches, expected_num_matches))
+              expectation.specificity, expected_specificity))
 
     def test_bad_match_modifier(self):
-        self.match(['foo'], num_errors=1)
+        self.assert_specificity(['foo'], num_errors=1)
 
     def test_none(self):
-        self.match([], 0)
+        self.assert_specificity([], 0)
 
     def test_one(self):
-        self.match(['xp'], 1)
-        self.match(['win'], 1)
-        self.match(['release'], 1)
-        self.match(['cpu'], 1)
-        self.match(['x86'], 1)
-        self.match(['leopard'], -1)
-        self.match(['gpu'], -1)
-        self.match(['debug'], -1)
+        self.assert_specificity(['xp'], 1)
+        self.assert_specificity(['win'], 1)
+        self.assert_specificity(['release'], 1)
+        self.assert_specificity(['cpu'], 1)
+        self.assert_specificity(['x86'], 1)
+        self.assert_specificity(['leopard'], -1)
+        self.assert_specificity(['gpu'], -1)
+        self.assert_specificity(['debug'], -1)
 
     def test_two(self):
-        self.match(['xp', 'release'], 2)
-        self.match(['win7', 'release'], -1)
-        self.match(['win7', 'xp'], 1)
+        self.assert_specificity(['xp', 'release'], 2)
+        self.assert_specificity(['win7', 'release'], -1)
+        self.assert_specificity(['win7', 'xp'], 1)
 
     def test_three(self):
-        self.match(['win7', 'xp', 'release'], 2)
-        self.match(['xp', 'debug', 'x86'], -1)
-        self.match(['xp', 'release', 'x86'], 3)
-        self.match(['xp', 'cpu', 'release'], 3)
+        self.assert_specificity(['win7', 'xp', 'release'], 2)
+        self.assert_specificity(['xp', 'debug', 'x86'], -1)
+        self.assert_specificity(['xp', 'release', 'x86'], 3)
+        self.assert_specificity(['xp', 'cpu', 'release'], 3)
 
     def test_four(self):
-        self.match(['xp', 'release', 'cpu', 'x86'], 4)
-        self.match(['win7', 'xp', 'release', 'cpu'], 3)
-        self.match(['win7', 'xp', 'debug', 'cpu'], -1)
+        self.assert_specificity(['xp', 'release', 'cpu', 'x86'], 4)
+        self.assert_specificity(['win7', 'xp', 'release', 'cpu'], 3)
+        self.assert_specificity(['win7', 'xp', 'debug', 'cpu'], -1)
 
     def test_case_insensitivity(self):
-        self.match(['Win'], num_errors=1)
-        self.match(['WIN'], num_errors=1)
-        self.match(['win'], 1)
+        self.assert_specificity(['Win'], num_errors=1)
+        self.assert_specificity(['WIN'], num_errors=1)
+        self.assert_specificity(['win'], 1)
 
     def test_duplicates(self):
-        self.match(['release', 'release'], num_errors=1)
-        self.match(['win', 'xp'], num_errors=1)
-        self.match(['xp', 'xp'], num_errors=1)
-        self.match(['xp', 'release', 'xp', 'release'], num_errors=2)
-        self.match(['rebaseline', 'rebaseline'], num_errors=1)
+        self.assert_specificity(['release', 'release'], num_errors=1)
+        self.assert_specificity(['win', 'xp'], num_errors=1)
+        self.assert_specificity(['xp', 'xp'], num_errors=1)
+        self.assert_specificity(['xp', 'release', 'xp', 'release'], num_errors=2)
+        self.assert_specificity(['rebaseline', 'rebaseline'], num_errors=1)
 
     def test_unknown_modifier(self):
-        self.match(['vms'], num_errors=1)
+        self.assert_specificity(['vms'], num_errors=1)
 
     def test_duplicate_bugs(self):
         # BUG* regexes can appear multiple times.
-        self.match(['bugfoo', 'bugbar'], 0)
+        self.assert_specificity(['bugfoo', 'bugbar'], 0)
 
     def test_regexes_are_ignored(self):
-        self.match(['bug123xy', 'rebaseline', 'wontfix', 'slow', 'skip'], 0)
+        self.assert_specificity(['bug123xy', 'rebaseline', 'wontfix', 'slow', 'skip'], 0)
 
     def test_none_is_invalid(self):
-        self.match(['none'], num_errors=1)
+        self.assert_specificity(['none'], num_errors=1)
 
 
 class TestExpectationParserTests(unittest.TestCase):