Get rid of the distinction between modifiers and expectations.

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

 # Copyright (C) 2010 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 20 matching lines @@
 """
 
 import logging
 import re
 
 from webkitpy.layout_tests.models.test_configuration import TestConfigurationConverter
 
 _log = logging.getLogger(__name__)
 
 
-# Test expectation and modifier constants.
+# Test expectation and specifier constants.
 #
 # FIXME: range() starts with 0 which makes if expectation checks harder
 # as PASS is 0.
 (PASS, FAIL, TEXT, IMAGE, IMAGE_PLUS_TEXT, AUDIO, TIMEOUT, CRASH, SKIP, WONTFIX,
  SLOW, REBASELINE, NEEDS_REBASELINE, NEEDS_MANUAL_REBASELINE, MISSING, FLAKY, NOW, NONE) = range(18)
 
 # FIXME: Perhas these two routines should be part of the Port instead?
 BASELINE_SUFFIX_LIST = ('png', 'wav', 'txt')
 
 WEBKIT_BUG_PREFIX = 'webkit.org/b/'
@@ skipping 13 matching lines @@
     def __str__(self):
         return '\n'.join(map(str, self.warnings))
 
     def __repr__(self):
         return 'ParseError(warnings=%s)' % self.warnings
 
 
 class TestExpectationParser(object):
     """Provides parsing facilities for lines in the test_expectation.txt file."""
 
+    # FIXME: Rename these to *_KEYWORD as in MISSING_KEYWORD above, but make the case studdly-caps to match the actual file contents.

[Dirk Pranke, 2013/07/26 23:07:28]

"studly" ?

     REBASELINE_MODIFIER = 'rebaseline'
     NEEDS_REBASELINE_MODIFIER = 'needsrebaseline'
     NEEDS_MANUAL_REBASELINE_MODIFIER = 'needsmanualrebaseline'
     PASS_EXPECTATION = 'pass'
     SKIP_MODIFIER = 'skip'
     SLOW_MODIFIER = 'slow'
     WONTFIX_MODIFIER = 'wontfix'
 
     TIMEOUT_EXPECTATION = 'timeout'
 
-    MISSING_BUG_WARNING = 'Test lacks BUG modifier.'
+    MISSING_BUG_WARNING = 'Test lacks BUG specifier.'
 
-    def __init__(self, port, full_test_list, allow_rebaseline_modifier):
+    def __init__(self, port, full_test_list, allow_rebaseline):
         self._port = port
         self._test_configuration_converter = TestConfigurationConverter(set(port.all_test_configurations()), port.configuration_specifier_macros())
         self._full_test_list = full_test_list
-        self._allow_rebaseline_modifier = allow_rebaseline_modifier
+        self._allow_rebaseline = allow_rebaseline
 
     def parse(self, filename, expectations_string):
         expectation_lines = []
         line_number = 0
         for line in expectations_string.split("\n"):
             line_number += 1
             test_expectation = self._tokenize_line(filename, line, line_number)
             self._parse_line(test_expectation)
             expectation_lines.append(test_expectation)
         return expectation_lines
@@ skipping 10 matching lines @@
     def expectation_line_for_test(self, test_name, expectations):
         expectation_line = self._create_expectation_line(test_name, expectations, '<Bot TestExpectations>')
         self._parse_line(expectation_line)
         return expectation_line
 
 
     def expectation_for_skipped_test(self, test_name):
         if not self._port.test_exists(test_name):
             _log.warning('The following test %s from the Skipped list doesn\'t exist' % test_name)
         expectation_line = self._create_expectation_line(test_name, [TestExpectationParser.PASS_EXPECTATION], '<Skipped file>')
-        # FIXME: It's not clear what the expectations for a skipped test should be; the expectations
-        # might be different for different entries in a Skipped file, or from the command line, or from
-        # only running parts of the tests. It's also not clear if it matters much.
-        expectation_line.modifiers = [TestExpectationParser.SKIP_MODIFIER, TestExpectationParser.WONTFIX_MODIFIER]
+        expectation_line.expectations = [TestExpectationParser.SKIP_MODIFIER, TestExpectationParser.WONTFIX_MODIFIER]
         expectation_line.is_skipped_outside_expectations_file = True
         self._parse_line(expectation_line)
         return expectation_line
 
     def _parse_line(self, expectation_line):
         if not expectation_line.name:
             return
 
         if not self._check_test_exists(expectation_line):
             return
 
         expectation_line.is_file = self._port.test_isfile(expectation_line.name)
         if expectation_line.is_file:
             expectation_line.path = expectation_line.name
         else:
             expectation_line.path = self._port.normalize_test_name(expectation_line.name)
 
         self._collect_matching_tests(expectation_line)
 
-        self._parse_modifiers(expectation_line)
+        self._parse_specifiers(expectation_line)
         self._parse_expectations(expectation_line)
 
-    def _parse_modifiers(self, expectation_line):
-        has_wontfix = False
-        parsed_specifiers = set()
-
-        modifiers = [modifier.lower() for modifier in expectation_line.modifiers]
+    def _parse_specifiers(self, expectation_line):
+        parsed_specifiers = set([specifier.lower() for specifier in expectation_line.specifiers])
         expectations = [expectation.lower() for expectation in expectation_line.expectations]
 
-        if self.SLOW_MODIFIER in modifiers and self.TIMEOUT_EXPECTATION in expectations:
+        if self.SLOW_MODIFIER in expectations and self.TIMEOUT_EXPECTATION in expectations:
             expectation_line.warnings.append('A test can not be both SLOW and TIMEOUT. If it times out indefinitely, then it should be just TIMEOUT.')
 
-        for modifier in expectation_line.modifiers:
-            # FIXME: Store the unmodified modifier.
-            modifier = modifier.lower()
-            if modifier in TestExpectations.MODIFIERS:
-                expectation_line.parsed_modifiers.append(modifier)
-                if modifier == self.WONTFIX_MODIFIER:
-                    has_wontfix = True
-            else:
-                parsed_specifiers.add(modifier)
-
-        if not expectation_line.bugs and not has_wontfix and self._port.warn_if_bug_missing_in_test_expectations():
+        if not expectation_line.bugs and self.WONTFIX_MODIFIER not in expectations and self._port.warn_if_bug_missing_in_test_expectations():
             expectation_line.warnings.append(self.MISSING_BUG_WARNING)
 
-        if self._allow_rebaseline_modifier and self.REBASELINE_MODIFIER in modifiers:
+        if self._allow_rebaseline and self.REBASELINE_MODIFIER in expectations:
             expectation_line.warnings.append('REBASELINE should only be used for running rebaseline.py. Cannot be checked in.')
 
         expectation_line.matching_configurations = self._test_configuration_converter.to_config_set(parsed_specifiers, expectation_line.warnings)
 
     def _parse_expectations(self, expectation_line):
         result = set()
         for part in expectation_line.expectations:
             expectation = TestExpectations.expectation_from_string(part)
             if expectation is None:  # Careful, PASS is currently 0.
                 expectation_line.warnings.append('Unsupported expectation: %s' % part)
@@ skipping 29 matching lines @@
         if not expectation_line.is_file:
             # this is a test category, return all the tests of the category.
             expectation_line.matching_tests = [test for test in self._full_test_list if test.startswith(expectation_line.path)]
             return
 
         # this is a test file, do a quick check if it's in the
         # full test suite.
         if expectation_line.path in self._full_test_list:
             expectation_line.matching_tests.append(expectation_line.path)
 
-    # FIXME: Update the original modifiers and remove this once the old syntax is gone.
+    # FIXME: Update the original specifiers and remove this once the old syntax is gone.
     _configuration_tokens_list = [
         'Mac', 'SnowLeopard', 'Lion', 'MountainLion',
         'Win', 'XP', 'Win7',
         'Linux',
         'Android',
         'Release',
         'Debug',
     ]
 
     _configuration_tokens = dict((token, token.upper()) for token in _configuration_tokens_list)
     _inverted_configuration_tokens = dict((value, name) for name, value in _configuration_tokens.iteritems())
 
-    # FIXME: Update the original modifiers list and remove this once the old syntax is gone.
+    # FIXME: Update the original specifiers list and remove this once the old syntax is gone.
     _expectation_tokens = {
         'Crash': 'CRASH',
         'Failure': 'FAIL',
         'ImageOnlyFailure': 'IMAGE',
         MISSING_KEYWORD: 'MISSING',
         'Pass': 'PASS',
         'Rebaseline': 'REBASELINE',
         NEEDS_REBASELINE_KEYWORD: NEEDS_REBASELINE_KEYWORD,
         NEEDS_MANUAL_REBASELINE_KEYWORD: NEEDS_MANUAL_REBASELINE_KEYWORD,
         'Skip': 'SKIP',
         'Slow': 'SLOW',
         'Timeout': 'TIMEOUT',
         'WontFix': 'WONTFIX',
     }
 
     _inverted_expectation_tokens = dict([(value, name) for name, value in _expectation_tokens.iteritems()] +
                                         [('TEXT', 'Failure'), ('IMAGE+TEXT', 'Failure'), ('AUDIO', 'Failure')])
 
     # FIXME: Seems like these should be classmethods on TestExpectationLine instead of TestExpectationParser.
     @classmethod
     def _tokenize_line(cls, filename, expectation_string, line_number):
         """Tokenizes a line from TestExpectations and returns an unparsed TestExpectationLine instance using the old format.
 
         The new format for a test expectation line is:
 
-        [[bugs] [ "[" <configuration modifiers> "]" <name> [ "[" <expectations> "]" ["#" <comment>]
+        [[bugs] [ "[" <configuration specifiers> "]" <name> [ "[" <expectations> "]" ["#" <comment>]
 
         Any errant whitespace is not preserved.
 
         """
         expectation_line = TestExpectationLine()
         expectation_line.original_string = expectation_string
         expectation_line.filename = filename
         expectation_line.line_numbers = str(line_number)
 
         comment_index = expectation_string.find("#")
         if comment_index == -1:
             comment_index = len(expectation_string)
         else:
             expectation_line.comment = expectation_string[comment_index + 1:]
 
         remaining_string = re.sub(r"\s+", " ", expectation_string[:comment_index].strip())
         if len(remaining_string) == 0:
             return expectation_line
 
         # special-case parsing this so that we fail immediately instead of treating this as a test name
         if remaining_string.startswith('//'):
             expectation_line.warnings = ['use "#" instead of "//" for comments']
             return expectation_line
 
         bugs = []
-        modifiers = []
+        specifiers = []
         name = None
         expectations = []
         warnings = []
         has_unrecognized_expectation = False
 
         tokens = remaining_string.split()
         state = 'start'
         for token in tokens:
             if (token.startswith(WEBKIT_BUG_PREFIX) or
                 token.startswith(CHROMIUM_BUG_PREFIX) or
@@ skipping 28 matching lines @@
                     state = 'name'
                 elif state == 'expectations':
                     state = 'done'
                 else:
                     warnings.append('unexpected "]"')
                     break
             elif token in ('//', ':', '='):
                 warnings.append('"%s" is not legal in the new TestExpectations syntax.' % token)
                 break
             elif state == 'configuration':
-                modifiers.append(cls._configuration_tokens.get(token, token))
+                specifiers.append(cls._configuration_tokens.get(token, token))
             elif state == 'expectations':
-                if token in ('Rebaseline', 'Skip', 'Slow', 'WontFix'):
-                    modifiers.append(token.upper())
-                elif token not in cls._expectation_tokens:
+                if token not in cls._expectation_tokens:
                     has_unrecognized_expectation = True
                     warnings.append('Unrecognized expectation "%s"' % token)
                 else:
                     expectations.append(cls._expectation_tokens.get(token, token))
             elif state == 'name_found':
                 warnings.append('expecting "[", "#", or end of line instead of "%s"' % token)
                 break
             else:
                 name = token
                 state = 'name_found'
 
         if not warnings:
             if not name:
                 warnings.append('Did not find a test name.')
             elif state not in ('name_found', 'done'):
                 warnings.append('Missing a "]"')
 
-        if 'WONTFIX' in modifiers and 'SKIP' not in modifiers and not expectations:
-            modifiers.append('SKIP')
+        if 'WONTFIX' in expectations and 'SKIP' not in expectations:
+            expectations.append('SKIP')
 
-        if 'SKIP' in modifiers and expectations:
-            # FIXME: This is really a semantic warning and shouldn't be here. Remove when we drop the old syntax.
-            warnings.append('A test marked Skip must not have other expectations.')
-        elif not expectations:
-            if not has_unrecognized_expectation and 'SKIP' not in modifiers and 'REBASELINE' not in modifiers and NEEDS_REBASELINE_KEYWORD not in modifiers and NEEDS_MANUAL_REBASELINE_KEYWORD not in modifiers and 'SLOW' not in modifiers:
-                warnings.append('Missing expectations.')
-            expectations = ['PASS']
+        if ('SKIP' in expectations or 'WONTFIX' in expectations) and len(set(expectations) - set(['SKIP', 'WONTFIX'])):
+            warnings.append('A test marked Skip or WontFix must not have other expectations.')
+
+        if not expectations and not has_unrecognized_expectation:
+            warnings.append('Missing expectations.')
 
         expectation_line.bugs = bugs
-        expectation_line.modifiers = modifiers
+        expectation_line.specifiers = specifiers
         expectation_line.expectations = expectations
         expectation_line.name = name
         expectation_line.warnings = warnings
         return expectation_line
 
     @classmethod
     def _split_space_separated(cls, space_separated_string):
         """Splits a space-separated string into an array."""
         return [part.strip() for part in space_separated_string.strip().split(' ')]
 
 
 class TestExpectationLine(object):
     """Represents a line in test expectations file."""
 
     def __init__(self):
         """Initializes a blank-line equivalent of an expectation."""
         self.original_string = None
         self.filename = None  # this is the path to the expectations file for this line
         self.line_numbers = None
         self.name = None  # this is the path in the line itself
         self.path = None  # this is the normpath of self.name
         self.bugs = []
-        self.modifiers = []
-        self.parsed_modifiers = []
+        self.specifiers = []
+        self.parsed_specifiers = []
         self.matching_configurations = set()
         self.expectations = []
         self.parsed_expectations = set()
         self.comment = None
         self.matching_tests = []
         self.warnings = []
         self.is_skipped_outside_expectations_file = False
 
     def __eq__(self, other):
         return (self.original_string == other.original_string
             and self.filename == other.filename
             and self.line_numbers == other.line_numbers
             and self.name == other.name
             and self.path == other.path
             and self.bugs == other.bugs
-            and self.modifiers == other.modifiers
-            and self.parsed_modifiers == other.parsed_modifiers
+            and self.specifiers == other.specifiers
+            and self.parsed_specifiers == other.parsed_specifiers
             and self.matching_configurations == other.matching_configurations
             and self.expectations == other.expectations
             and self.parsed_expectations == other.parsed_expectations
             and self.comment == other.comment
             and self.matching_tests == other.matching_tests
             and self.warnings == other.warnings
             and self.is_skipped_outside_expectations_file == other.is_skipped_outside_expectations_file)
 
     def is_invalid(self):
         return self.warnings and self.warnings != [TestExpectationParser.MISSING_BUG_WARNING]
@@ skipping 29 matching lines @@
         # We only care about filenames when we're linting, in which case the filenames are the same.
         # Not clear that there's anything better to do when not linting and the filenames are different.
         if model_all_expectations:
             result.filename = line2.filename
         result.line_numbers = line1.line_numbers + "," + line2.line_numbers
         result.name = line1.name
         result.path = line1.path
         result.parsed_expectations = set(line1.parsed_expectations) | set(line2.parsed_expectations)
         result.expectations = list(set(line1.expectations) | set(line2.expectations))
         result.bugs = list(set(line1.bugs) | set(line2.bugs))
-        result.modifiers = list(set(line1.modifiers) | set(line2.modifiers))
-        result.parsed_modifiers = list(set(line1.parsed_modifiers) | set(line2.parsed_modifiers))
+        result.specifiers = list(set(line1.specifiers) | set(line2.specifiers))
+        result.parsed_specifiers = list(set(line1.parsed_specifiers) | set(line2.parsed_specifiers))
         result.matching_configurations = set(line1.matching_configurations) | set(line2.matching_configurations)
         result.matching_tests = list(list(set(line1.matching_tests) | set(line2.matching_tests)))
         result.warnings = list(set(line1.warnings) | set(line2.warnings))
         result.is_skipped_outside_expectations_file = line1.is_skipped_outside_expectations_file or line2.is_skipped_outside_expectations_file
         return result
 
-    def to_string(self, test_configuration_converter, include_modifiers=True, include_expectations=True, include_comment=True):
+    def to_string(self, test_configuration_converter, include_specifiers=True, include_expectations=True, include_comment=True):
         parsed_expectation_to_string = dict([[parsed_expectation, expectation_string] for expectation_string, parsed_expectation in TestExpectations.EXPECTATIONS.items()])
 
         if self.is_invalid():
             return self.original_string or ''
 
         if self.name is None:
             return '' if self.comment is None else "#%s" % self.comment
 
         if test_configuration_converter and self.bugs:
             specifiers_list = test_configuration_converter.to_specifiers_list(self.matching_configurations)
             result = []
             for specifiers in specifiers_list:
-                # FIXME: this is silly that we join the modifiers and then immediately split them.
-                modifiers = self._serialize_parsed_modifiers(test_configuration_converter, specifiers).split()
+                # FIXME: this is silly that we join the specifiers and then immediately split them.
+                specifiers = self._serialize_parsed_specifiers(test_configuration_converter, specifiers).split()
                 expectations = self._serialize_parsed_expectations(parsed_expectation_to_string).split()
-                result.append(self._format_line(self.bugs, modifiers, self.name, expectations, self.comment))
+                result.append(self._format_line(self.bugs, specifiers, self.name, expectations, self.comment))
             return "\n".join(result) if result else None
 
-        return self._format_line(self.bugs, self.modifiers, self.name, self.expectations, self.comment,
-            include_modifiers, include_expectations, include_comment)
+        return self._format_line(self.bugs, self.specifiers, self.name, self.expectations, self.comment,
+            include_specifiers, include_expectations, include_comment)
 
     def to_csv(self):
         # Note that this doesn't include the comments.
-        return '%s,%s,%s,%s' % (self.name, ' '.join(self.bugs), ' '.join(self.modifiers), ' '.join(self.expectations))
+        return '%s,%s,%s,%s' % (self.name, ' '.join(self.bugs), ' '.join(self.specifiers), ' '.join(self.expectations))
 
     def _serialize_parsed_expectations(self, parsed_expectation_to_string):
         result = []
         for index in TestExpectations.EXPECTATION_ORDER:
             if index in self.parsed_expectations:
                 result.append(parsed_expectation_to_string[index])
         return ' '.join(result)
 
-    def _serialize_parsed_modifiers(self, test_configuration_converter, specifiers):
+    def _serialize_parsed_specifiers(self, test_configuration_converter, specifiers):
         result = []
-        result.extend(sorted(self.parsed_modifiers))
+        result.extend(sorted(self.parsed_specifiers))
         result.extend(test_configuration_converter.specifier_sorter().sort_specifiers(specifiers))
         return ' '.join(result)
 
     @staticmethod
     def _filter_redundant_expectations(expectations):
         if set(expectations) == set(['Pass', 'Skip']):
             return ['Skip']
         if set(expectations) == set(['Pass', 'Slow']):
             return ['Slow']
         return expectations
 
     @staticmethod
-    def _format_line(bugs, modifiers, name, expectations, comment, include_modifiers=True, include_expectations=True, include_comment=True):
-        new_modifiers = []
+    def _format_line(bugs, specifiers, name, expectations, comment, include_specifiers=True, include_expectations=True, include_comment=True):
+        new_specifiers = []
         new_expectations = []
-        for modifier in modifiers:
-            # FIXME: Make this all work with the mixed-cased modifiers (e.g. WontFix, Slow, etc).
-            modifier = modifier.upper()
-            if modifier in ('SLOW', 'SKIP', 'REBASELINE', NEEDS_REBASELINE_KEYWORD, NEEDS_MANUAL_REBASELINE_KEYWORD, 'WONTFIX'):
-                new_expectations.append(TestExpectationParser._inverted_expectation_tokens.get(modifier))
-            else:
-                new_modifiers.append(TestExpectationParser._inverted_configuration_tokens.get(modifier, modifier))
+        for specifier in specifiers:
+            # FIXME: Make this all work with the mixed-cased specifiers (e.g. WontFix, Slow, etc).
+            specifier = specifier.upper()
+            new_specifiers.append(TestExpectationParser._inverted_configuration_tokens.get(specifier, specifier))
 
         for expectation in expectations:
             expectation = expectation.upper()
             new_expectations.append(TestExpectationParser._inverted_expectation_tokens.get(expectation, expectation))
 
         result = ''
-        if include_modifiers and (bugs or new_modifiers):
+        if include_specifiers and (bugs or new_specifiers):
             if bugs:
                 result += ' '.join(bugs) + ' '
-            if new_modifiers:
-                result += '[ %s ] ' % ' '.join(new_modifiers)
+            if new_specifiers:
+                result += '[ %s ] ' % ' '.join(new_specifiers)
         result += name
         if include_expectations and new_expectations:
             new_expectations = TestExpectationLine._filter_redundant_expectations(new_expectations)
             result += ' [ %s ]' % ' '.join(sorted(set(new_expectations)))
         if include_comment and comment is not None:
             result += " #%s" % comment
         return result
 
 
 # FIXME: Refactor API to be a proper CRUD.
 class TestExpectationsModel(object):
     """Represents relational store of all expectations and provides CRUD semantics to manage it."""
 
     def __init__(self, shorten_filename=None):
         # Maps a test to its list of expectations.
         self._test_to_expectations = {}
 
-        # Maps a test to list of its modifiers (string values)
-        self._test_to_modifiers = {}
+        # Maps a test to list of its specifiers (string values)
+        self._test_to_specifiers = {}
 
         # Maps a test to a TestExpectationLine instance.
         self._test_to_expectation_line = {}
 
-        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)
         self._result_type_to_tests = self._dict_of_sets(TestExpectations.RESULT_TYPES)
 
         self._shorten_filename = shorten_filename or (lambda x: x)
 
     def _dict_of_sets(self, strings_to_constants):
         """Takes a dict of strings->constants and returns a dict mapping
         each constant to an empty set."""
         d = {}
         for c in strings_to_constants.values():
             d[c] = set()
         return d
 
-    def get_test_set(self, modifier, expectation=None, include_skips=True):
-        if expectation is None:
-            tests = self._modifier_to_tests[modifier]
-        else:
-            tests = (self._expectation_to_tests[expectation] &
-                self._modifier_to_tests[modifier])
-
+    def get_test_set(self, expectation, include_skips=True):
+        tests = self._expectation_to_tests[expectation]
         if not include_skips:
-            tests = tests - self.get_test_set(SKIP, expectation)
-
+            tests = tests - self.get_test_set(SKIP)
         return tests
 
     def get_test_set_for_keyword(self, keyword):
-        # FIXME: get_test_set() is an awkward public interface because it requires
-        # callers to know the difference between modifiers and expectations. We
-        # should replace that with this where possible.
         expectation_enum = TestExpectations.EXPECTATIONS.get(keyword.lower(), None)
         if expectation_enum is not None:
             return self._expectation_to_tests[expectation_enum]
-        modifier_enum = TestExpectations.MODIFIERS.get(keyword.lower(), None)
-        if modifier_enum is not None:
-            return self._modifier_to_tests[modifier_enum]
 
-        # We must not have an index on this modifier.
         matching_tests = set()
-        for test, modifiers in self._test_to_modifiers.iteritems():
-            if keyword.lower() in modifiers:
+        for test, specifiers in self._test_to_specifiers.iteritems():
+            if keyword.lower() in specifiers:
                 matching_tests.add(test)
         return matching_tests
 
     def get_tests_with_result_type(self, result_type):
         return self._result_type_to_tests[result_type]
 
     def get_tests_with_timeline(self, timeline):
         return self._timeline_to_tests[timeline]
 
-    def get_modifiers(self, test):
-        """This returns modifiers for the given test (the modifiers plus the BUGXXXX identifier). This is used by the LTTF dashboard."""
-        return self._test_to_modifiers[test]
-
-    def has_modifier(self, test, modifier):
-        return test in self._modifier_to_tests[modifier]
-
-    def has_keyword(self, test, keyword):
-        return (keyword.upper() in self.get_expectations_string(test) or
-                keyword.lower() in self.get_modifiers(test))
-
     def has_test(self, test):
         return test in self._test_to_expectation_line
 
     def get_expectation_line(self, test):
         return self._test_to_expectation_line.get(test)
 
     def get_expectations(self, test):
         return self._test_to_expectations[test]
 
     def get_expectations_string(self, test):
         """Returns the expectatons for the given test as an uppercase string.
         If there are no expectations for the test, then "PASS" is returned."""
         if self.get_expectation_line(test).is_skipped_outside_expectations_file:
             return 'NOTRUN'
 
-        if self.has_modifier(test, WONTFIX):
-            return TestExpectationParser.WONTFIX_MODIFIER.upper()
-
-        if self.has_modifier(test, SKIP):
-            return TestExpectationParser.SKIP_MODIFIER.upper()
-
         expectations = self.get_expectations(test)
         retval = []
 
+        # FIXME: WontFix should cause the test to get skipped without artificially adding SKIP to the expectations list.
+        if WONTFIX in expectations and SKIP in expectations:
+            expectations.remove(SKIP)
+
         for expectation in expectations:
             retval.append(self.expectation_to_string(expectation))
 
         return " ".join(retval)
 
     def expectation_to_string(self, expectation):
         """Return the uppercased string equivalent of a given expectation."""
         for item in TestExpectations.EXPECTATIONS.items():
             if item[1] == expectation:
                 return item[0].upper()
-        for item in TestExpectations.MODIFIERS.items():
-            if item[1] == expectation:
-                return item[0].upper()
         raise ValueError(expectation)
 
     def remove_expectation_line(self, test):
         if not self.has_test(test):
             return
         self._clear_expectations_for_test(test)
         del self._test_to_expectation_line[test]
 
     def add_expectation_line(self, expectation_line,
                              override_existing_matches=False,
                              merge_existing_matches=False,
                              model_all_expectations=False):
-        """Returns a list of warnings encountered while matching modifiers."""
+        """Returns a list of warnings encountered while matching specifiers."""
 
         if expectation_line.is_invalid():
             return
 
         for test in expectation_line.matching_tests:
             if (not (override_existing_matches or merge_existing_matches)
                     and self._already_seen_better_match(test, expectation_line)):
                 continue
 
             if merge_existing_matches or model_all_expectations:
                 expectation_line = TestExpectationLine.merge_expectation_lines(self.get_expectation_line(test), expectation_line, model_all_expectations)
 
             self._clear_expectations_for_test(test)
             self._test_to_expectation_line[test] = expectation_line
             self._add_test(test, expectation_line)
 
     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."""
         self._test_to_expectations[test] = expectation_line.parsed_expectations
         for expectation in expectation_line.parsed_expectations:
             self._expectation_to_tests[expectation].add(test)
 
-        self._test_to_modifiers[test] = expectation_line.modifiers
-        for modifier in expectation_line.parsed_modifiers:
-            mod_value = TestExpectations.MODIFIERS[modifier]
-            self._modifier_to_tests[mod_value].add(test)
+        self._test_to_specifiers[test] = expectation_line.specifiers
 
-        if TestExpectationParser.WONTFIX_MODIFIER in expectation_line.parsed_modifiers:
+        if WONTFIX in expectation_line.parsed_expectations:
             self._timeline_to_tests[WONTFIX].add(test)
         else:
             self._timeline_to_tests[NOW].add(test)
 
-        if TestExpectationParser.SKIP_MODIFIER in expectation_line.parsed_modifiers:
+        if SKIP in expectation_line.parsed_expectations:
             self._result_type_to_tests[SKIP].add(test)
         elif expectation_line.parsed_expectations == set([PASS]):
             self._result_type_to_tests[PASS].add(test)
         elif expectation_line.is_flaky():
             self._result_type_to_tests[FLAKY].add(test)
         else:
             # FIXME: What is this?
             self._result_type_to_tests[FAIL].add(test)
 
     def _clear_expectations_for_test(self, test):
         """Remove prexisting expectations for this test.
         This happens if we are seeing a more precise path
         than a previous listing.
         """
         if self.has_test(test):
             self._test_to_expectations.pop(test, '')
             self._remove_from_sets(test, self._expectation_to_tests)
-            self._remove_from_sets(test, self._modifier_to_tests)
             self._remove_from_sets(test, self._timeline_to_tests)
             self._remove_from_sets(test, self._result_type_to_tests)
 
     def _remove_from_sets(self, test, dict_of_sets_of_tests):
         """Removes the given test from the sets in the dictionary.
 
         Args:
           test: test to look for
           dict: dict of sets of files"""
         for set_of_tests in dict_of_sets_of_tests.itervalues():
@@ skipping 19 matching lines @@
 
         if len(prev_expectation_line.path) > len(expectation_line.path):
             # The previous path matched more of the test.
             return True
 
         if len(prev_expectation_line.path) < len(expectation_line.path):
             # This path matches more of the test.
             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.
+        # base path, so we need to check the two sets of specifiers.
 
         # FIXME: This code was originally designed to allow lines that matched
-        # more modifiers to override lines that matched fewer modifiers.
+        # more specifiers to override lines that matched fewer specifiers.
         # However, we currently view these as errors.
         #
-        # To use the "more modifiers wins" policy, change the errors for overrides
+        # To use the "more specifiers wins" policy, change the errors for overrides
         # to be warnings and return False".
 
         if prev_expectation_line.matching_configurations == expectation_line.matching_configurations:
             expectation_line.warnings.append('Duplicate or ambiguous entry lines %s:%s and %s:%s.' % (
                 self._shorten_filename(prev_expectation_line.filename), prev_expectation_line.line_numbers,
                 self._shorten_filename(expectation_line.filename), expectation_line.line_numbers))
             return True
 
         if prev_expectation_line.matching_configurations >= expectation_line.matching_configurations:
             expectation_line.warnings.append('More specific entry for %s on line %s:%s overrides line %s:%s.' % (expectation_line.name,
@@ skipping 22 matching lines @@
     """Test expectations consist of lines with specifications of what
     to expect from layout test cases. The test cases can be directories
     in which case the expectations apply to all test cases in that
     directory and any subdirectory. The format is along the lines of:
 
       LayoutTests/fast/js/fixme.js [ Failure ]
       LayoutTests/fast/js/flaky.js [ Failure Pass ]
       LayoutTests/fast/js/crash.js [ Crash Failure Pass Timeout ]
       ...
 
-    To add modifiers:
+    To add specifiers:
       LayoutTests/fast/js/no-good.js
       [ Debug ] LayoutTests/fast/js/no-good.js [ Pass Timeout ]
       [ Debug ] LayoutTests/fast/js/no-good.js [ Pass Skip Timeout ]
       [ Linux Debug ] LayoutTests/fast/js/no-good.js [ Pass Skip Timeout ]
       [ Linux Win ] LayoutTests/fast/js/no-good.js [ Pass Skip Timeout ]
 
     Skip: Doesn't run the test.
     Slow: The test takes a long time to run, but does not timeout indefinitely.
     WontFix: For tests that we never intend to pass on a given platform (treated like Skip).
 
     Notes:
       -A test cannot be both SLOW and TIMEOUT
       -A test can be included twice, but not via the same path.
       -If a test is included twice, then the more precise path wins.
       -CRASH tests cannot be WONTFIX
     """
 
     # FIXME: Update to new syntax once the old format is no longer supported.
     EXPECTATIONS = {'pass': PASS,
                     'audio': AUDIO,
                     'fail': FAIL,
                     'image': IMAGE,
                     'image+text': IMAGE_PLUS_TEXT,
                     'text': TEXT,
                     'timeout': TIMEOUT,
                     'crash': CRASH,
                     'missing': MISSING,
-                    'skip': SKIP,
-                    'needsrebaseline': NEEDS_REBASELINE,
-                    'needsmanualrebaseline': NEEDS_MANUAL_REBASELINE}
+                    TestExpectationParser.SKIP_MODIFIER: SKIP,
+                    TestExpectationParser.NEEDS_REBASELINE_MODIFIER: NEEDS_REBASELINE,
+                    TestExpectationParser.NEEDS_MANUAL_REBASELINE_MODIFIER: NEEDS_MANUAL_REBASELINE,
+                    TestExpectationParser.WONTFIX_MODIFIER: WONTFIX,
+                    TestExpectationParser.SLOW_MODIFIER: SLOW,
+                    TestExpectationParser.REBASELINE_MODIFIER: REBASELINE,
+    }
 
     EXPECTATIONS_TO_STRING = dict((k, v) for (v, k) in EXPECTATIONS.iteritems())
 
     # (aggregated by category, pass/fail/skip, type)
     EXPECTATION_DESCRIPTIONS = {SKIP: 'skipped',
                                 PASS: 'passes',
                                 FAIL: 'failures',
                                 IMAGE: 'image-only failures',
                                 TEXT: 'text-only failures',
                                 IMAGE_PLUS_TEXT: 'image and text failures',
                                 AUDIO: 'audio failures',
                                 CRASH: 'crashes',
                                 TIMEOUT: 'timeouts',
                                 MISSING: 'missing results'}
 
     EXPECTATION_ORDER = (PASS, CRASH, TIMEOUT, MISSING, FAIL, IMAGE, SKIP)
 
+    NON_TEST_OUTCOME_EXPECTATIONS = (REBASELINE, SKIP, SLOW, WONTFIX)
+
     BUILD_TYPES = ('debug', 'release')
 
-    MODIFIERS = {TestExpectationParser.SKIP_MODIFIER: SKIP,
-                 TestExpectationParser.WONTFIX_MODIFIER: WONTFIX,
-                 TestExpectationParser.SLOW_MODIFIER: SLOW,
-                 TestExpectationParser.REBASELINE_MODIFIER: REBASELINE,
-                 TestExpectationParser.NEEDS_REBASELINE_MODIFIER: NEEDS_REBASELINE,
-                 TestExpectationParser.NEEDS_MANUAL_REBASELINE_MODIFIER: NEEDS_MANUAL_REBASELINE,
-                 'none': NONE}
-
-    MODIFIERS_TO_STRING = dict((k, v) for (v, k) in MODIFIERS.iteritems())
-
     TIMELINES = {TestExpectationParser.WONTFIX_MODIFIER: WONTFIX,
                  'now': NOW}
 
     RESULT_TYPES = {'skip': SKIP,
                     'pass': PASS,
                     'fail': FAIL,
                     'flaky': FLAKY}
 
     @classmethod
     def expectation_from_string(cls, string):
         assert(' ' not in string)  # This only handles one expectation at a time.
         return cls.EXPECTATIONS.get(string.lower())
 
     @staticmethod
     def result_was_expected(result, expected_results, test_needs_rebaselining):
         """Returns whether we got a result we were expecting.
         Args:
             result: actual result of a test execution
             expected_results: set of results listed in test_expectations
             test_needs_rebaselining: whether test was marked as REBASELINE"""
+        if not (set(expected_results) - (set(TestExpectations.NON_TEST_OUTCOME_EXPECTATIONS))):
+            expected_results = set([PASS])
+
         if result in expected_results:
             return True
         if result in (PASS, TEXT, IMAGE, IMAGE_PLUS_TEXT, AUDIO, MISSING) and (NEEDS_REBASELINE in expected_results or NEEDS_MANUAL_REBASELINE in expected_results):
             return True
         if result in (TEXT, IMAGE_PLUS_TEXT, AUDIO) and (FAIL in expected_results):
             return True
         if result == MISSING and test_needs_rebaselining:
             return True
         if result == SKIP:
             return True
@@ skipping 80 matching lines @@
         self._has_warnings = False
         self._report_warnings()
         self._process_tests_without_expectations()
 
     # TODO(ojan): Allow for removing skipped tests when getting the list of
     # tests to run, but not when getting metrics.
     def model(self):
         return self._model
 
     def get_needs_rebaseline_failures(self):
-        return self._model.get_test_set_for_keyword(TestExpectationParser.NEEDS_REBASELINE_MODIFIER)
+        return self._model.get_test_set(NEEDS_REBASELINE)
 
     def get_rebaselining_failures(self):
         return self._model.get_test_set(REBASELINE)
 
     # FIXME: Change the callsites to use TestExpectationsModel and remove.
     def get_expectations(self, test):
         return self._model.get_expectations(test)
 
     # FIXME: Change the callsites to use TestExpectationsModel and remove.
-    def has_modifier(self, test, modifier):
-        return self._model.has_modifier(test, modifier)
-
-    # FIXME: Change the callsites to use TestExpectationsModel and remove.
     def get_tests_with_result_type(self, result_type):
         return self._model.get_tests_with_result_type(result_type)
 
     # FIXME: Change the callsites to use TestExpectationsModel and remove.
-    def get_test_set(self, modifier, expectation=None, include_skips=True):
-        return self._model.get_test_set(modifier, expectation, include_skips)
-
-    # FIXME: Change the callsites to use TestExpectationsModel and remove.
-    def get_modifiers(self, test):
-        return self._model.get_modifiers(test)
+    def get_test_set(self, expectation, include_skips=True):
+        return self._model.get_test_set(expectation, include_skips)
 
     # FIXME: Change the callsites to use TestExpectationsModel and remove.
     def get_tests_with_timeline(self, timeline):
         return self._model.get_tests_with_timeline(timeline)
 
     def get_expectations_string(self, test):
         return self._model.get_expectations_string(test)
 
     def expectation_to_string(self, expectation):
         return self._model.expectation_to_string(expectation)
 
     def matches_an_expected_result(self, test, result, pixel_tests_are_enabled):
         expected_results = self._model.get_expectations(test)
         if not pixel_tests_are_enabled:
             expected_results = self.remove_pixel_failures(expected_results)
         return self.result_was_expected(result, expected_results, self.is_rebaselining(test))
 
     def is_rebaselining(self, test):
-        return self._model.has_modifier(test, REBASELINE)
+        return REBASELINE in self._model.get_expectations(test)
 
     def _shorten_filename(self, filename):
         if filename.startswith(self._port.path_from_webkit_base()):
             return self._port.host.filesystem.relpath(filename, self._port.path_from_webkit_base())
         return filename
 
     def _report_warnings(self):
         warnings = []
         for expectation in self._expectations:
             for warning in expectation.warnings:
@@ skipping 38 matching lines @@
             index = self._expectations.index(expectation)
             self._expectations.remove(expectation)
 
             if index == len(self._expectations) or self._expectations[index].is_whitespace_or_comment():
                 while index and self._expectations[index - 1].is_whitespace_or_comment():
                     index = index - 1
                     self._expectations.pop(index)
 
         return self.list_to_string(self._expectations, self._parser._test_configuration_converter, modified_expectations)
 
-    def remove_rebaselined_tests(self, except_these_tests, filename):

[Dirk Pranke, 2013/07/26 23:07:28]

I was a bit surprised that this was gone, but I guess you removed it in one of
the previous cleanups of the rebaselining commands (I see the only callers now
are in fact the unittests).

-        """Returns a copy of the expectations in the file with the tests removed."""
-        def without_rebaseline_modifier(expectation):
-            return (expectation.filename == filename and
-                    not (not expectation.is_invalid() and
-                         expectation.name in except_these_tests and
-                         'rebaseline' in expectation.parsed_modifiers))
-
-        return self.list_to_string(filter(without_rebaseline_modifier, self._expectations), reconstitute_only_these=[])
-
     def _add_expectations(self, expectation_list):
         for expectation_line in expectation_list:
             if not expectation_line.expectations:
                 continue
 
             if self._model_all_expectations or self._test_config in expectation_line.matching_configurations:
                 self._model.add_expectation_line(expectation_line, model_all_expectations=self._model_all_expectations)
 
     def add_extra_skipped_tests(self, tests_to_skip):
         if not tests_to_skip:
@@ skipping 33 matching lines @@
             # If reconstitute_only_these is an empty list, we want to return original_string.
             # So we need to compare reconstitute_only_these to None, not just check if it's falsey.
             if reconstitute_only_these is None or expectation_line in reconstitute_only_these:
                 return expectation_line.to_string(test_configuration_converter)
             return expectation_line.original_string
 
         def nones_out(expectation_line):
             return expectation_line is not None
 
         return "\n".join(filter(nones_out, map(serialize, expectation_lines)))