[web tests] Separate expectations using more than one mutually exclusive specifier

This CL also adds the ability to remove configurations from expectations that do
not have bugs. This CL also affects the behavior of the rebaseline and try-flag tools
so that it will create expectations which do not have more than one mutually exclusive specifier.
It also adds a new presubmit check which prevents users from creating expectations that contain
more than one mutually exclusive specifier. We are doing this because the new test expectations
format does not allow expectations to have more than one mutually exclusive specifier.

Bug: 986447
Change-Id: Ic97c4f1bba1e57b71f4624ef88ad69b5c8122a9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1970297
Commit-Queue: Rakib Hasan <rmhasan@google.com>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726015}

third_party/blink/tools/blinkpy/tool/commands/rebaseline_unittest.py

@@ -406,7 +406,7 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
              'bug(z) [ Linux ] userscripts/first-test.html [ Failure ]\n'))
 
     def test_rebaseline_updates_expectations_file_all_platforms(self):
-        self._write(self.test_expectations_path, 'Bug(x) userscripts/first-test.html [ Failure ]\n')
+        self._write(self.test_expectations_path, 'userscripts/first-test.html [ Failure ]\n')
         self._setup_mock_build_data()
         test_baseline_set = TestBaselineSet(self.tool)
         test_baseline_set.add('userscripts/first-test.html', Build('MOCK Mac10.11'))
@@ -415,13 +415,15 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
 
         new_expectations = self._read(self.test_expectations_path)
         self.assertMultiLineEqual(
-            new_expectations, 'Bug(x) [ Linux Mac10.10 Win ] userscripts/first-test.html [ Failure ]\n')
+            new_expectations, ('[ Win ] userscripts/first-test.html [ Failure ]\n'
+                               '[ Linux ] userscripts/first-test.html [ Failure ]\n'
+                               '[ Mac10.10 ] userscripts/first-test.html [ Failure ]\n'))
 
     def test_rebaseline_handles_platform_skips(self):
         # This test is just like test_rebaseline_updates_expectations_file_all_platforms(),
         # except that if a particular port happens to SKIP a test in an overrides file,
         # we count that as passing, and do not think that we still need to rebaseline it.
-        self._write(self.test_expectations_path, 'Bug(x) userscripts/first-test.html [ Failure ]\n')
+        self._write(self.test_expectations_path, 'userscripts/first-test.html [ Failure ]\n')
         self._write('NeverFixTests', 'Bug(y) [ Android ] userscripts [ WontFix ]\n')
         self._setup_mock_build_data()
         test_baseline_set = TestBaselineSet(self.tool)
@@ -431,7 +433,9 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
 
         new_expectations = self._read(self.test_expectations_path)
         self.assertMultiLineEqual(
-            new_expectations, 'Bug(x) [ Linux Mac10.10 Win ] userscripts/first-test.html [ Failure ]\n')
+            new_expectations, ('[ Win ] userscripts/first-test.html [ Failure ]\n'
+                               '[ Linux ] userscripts/first-test.html [ Failure ]\n'
+                               '[ Mac10.10 ] userscripts/first-test.html [ Failure ]\n'))
 
     def test_rebaseline_handles_skips_in_file(self):
         # This test is like test_rebaseline_handles_platform_skips, except that the
@@ -451,7 +455,8 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
         new_expectations = self._read(self.test_expectations_path)
         self.assertMultiLineEqual(
             new_expectations,
-            ('Bug(x) [ Linux Mac10.10 ] userscripts/first-test.html [ Failure ]\n'
+            ('Bug(x) [ Linux ] userscripts/first-test.html [ Failure ]\n'
+             'Bug(x) [ Mac10.10 ] userscripts/first-test.html [ Failure ]\n'
              'Bug(y) [ Win ] userscripts/first-test.html [ Skip ]\n'))
 
     def test_rebaseline_handles_smoke_tests(self):
@@ -469,7 +474,9 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
 
         new_expectations = self._read(self.test_expectations_path)
         self.assertMultiLineEqual(
-            new_expectations, 'Bug(x) [ Linux Mac10.10 Win ] userscripts/first-test.html [ Failure ]\n')
+            new_expectations, ('Bug(x) [ Win ] userscripts/first-test.html [ Failure ]\n'
+                               'Bug(x) [ Linux ] userscripts/first-test.html [ Failure ]\n'
+                               'Bug(x) [ Mac10.10 ] userscripts/first-test.html [ Failure ]\n'))
 
     # In the following test cases, the tests produce no outputs (e.g. clean
     # passing reftests, skipped tests, etc.). Hence, there are no baselines to
@@ -562,7 +569,9 @@ class TestRebaselineUpdatesExpectationsFiles(BaseTestCase):
 
         new_expectations = self._read(self.test_expectations_path)
         self.assertMultiLineEqual(
-            new_expectations, 'Bug(foo) [ Linux Mac10.10 Win ] userscripts/all-pass.html [ Failure ]\n')
+            new_expectations, ('Bug(foo) [ Win ] userscripts/all-pass.html [ Failure ]\n'
+                               'Bug(foo) [ Linux ] userscripts/all-pass.html [ Failure ]\n'
+                               'Bug(foo) [ Mac10.10 ] userscripts/all-pass.html [ Failure ]\n'))
         self.assertEqual(self.tool.executive.calls, [])
 
     def test_rebaseline_test_passes_unexpectedly_everywhere(self):

third_party/blink/tools/blinkpy/web_tests/lint_test_expectations.py

@@ -36,6 +36,7 @@ from blinkpy.common.host import Host
 from blinkpy.common.system.log_utils import configure_logging
 from blinkpy.web_tests.models import test_expectations
 from blinkpy.web_tests.port.factory import platform_options
+from blinkpy.web_tests.models.test_expectations import TestExpectationLine
 
 _log = logging.getLogger(__name__)
 
@@ -57,11 +58,19 @@ def lint(host, options):
         ports_to_lint[0].web_tests_dir(), 'WebGPUExpectations')
     paths = [wpt_overrides_exps_path, web_gpu_exps_path]
     expectations_dict = {}
+    all_system_specifiers = set()
+    all_build_specifiers = set(ports_to_lint[0].ALL_BUILD_TYPES)
     for path in paths:
         if host.filesystem.exists(path):
             expectations_dict[path] = host.filesystem.read_text_file(path)
+
     for port in ports_to_lint:
         expectations_dict.update(port.all_expectations_dict())
+        config_macro_dict = port.configuration_specifier_macros()
+        if config_macro_dict:
+            all_system_specifiers.update({s.lower() for s in config_macro_dict.keys()})
+            all_system_specifiers.update(
+                {s.lower() for s in reduce(lambda x, y: x + y, config_macro_dict.values())})
         for path in port.extra_expectations_files():
             if host.filesystem.exists(path):
                 expectations_dict[path] = host.filesystem.read_text_file(path)
@@ -77,7 +86,8 @@ def lint(host, options):
                 _log.error(warning)
                 failures.append('%s: %s' % (path, warning))
                 _log.error('')
-        for lineno, line in enumerate(content.split('\n'), 1):
+        exp_lines = content.split('\n')
+        for lineno, line in enumerate(exp_lines, 1):
             if line.strip().startswith('Bug('):
                 error = (("%s:%d Expectation '%s' has the Bug(...) token, "
                           "The token has been removed in the new expectations format") %
@@ -86,6 +96,23 @@ def lint(host, options):
                 failures.append(error)
                 _log.error('')
 
+        for lineno, line in enumerate(exp_lines, 1):
+            if line.strip().startswith('#') or not line.strip():
+                continue
+            exp_line = TestExpectationLine.tokenize_line(
+                host.filesystem.basename(path), line, lineno, ports_to_lint[0])
+            specifiers = set(s.lower() for s in exp_line.specifiers)
+            system_intersection = specifiers & all_system_specifiers
+            build_intersection = specifiers & all_build_specifiers
+            for intersection in [system_intersection, build_intersection]:
+                if len(intersection) < 2:
+                    continue
+                error = (("%s:%d Expectation '%s' has multiple specifiers that are mutually exclusive.\n"
+                          "The mutually exclusive specifiers are %s") %
+                         (host.filesystem.basename(path), lineno, line, ', '.join(intersection)))
+                _log.error(error)
+                _log.error('')
+                failures.append(error)
     return failures
 
 

third_party/blink/tools/blinkpy/web_tests/lint_test_expectations_unittest.py

@@ -45,6 +45,8 @@ class FakePort(object):
         self.name = name
         self.path = path
 
+    ALL_BUILD_TYPES = ('debug', 'release')
+
     def test_configuration(self):
         return None
 
@@ -62,7 +64,7 @@ class FakePort(object):
         return []
 
     def configuration_specifier_macros(self):
-        return []
+        return {}
 
     def get_option(self, _, val):
         return val

third_party/blink/tools/blinkpy/web_tests/models/test_configuration.py

@@ -116,7 +116,7 @@ class TestConfigurationConverter(object):
 
     def __init__(self, all_test_configurations, configuration_macros=None):
         self._all_test_configurations = all_test_configurations
-        self._configuration_macros = configuration_macros or {}
+        self.configuration_macros = configuration_macros or {}
         self._specifier_to_configuration_set = {}
         self._specifier_sorter = SpecifierSorter()
         self._collapsing_sets_by_size = {}
@@ -148,7 +148,7 @@ class TestConfigurationConverter(object):
         return self._specifier_sorter
 
     def _expand_macros(self, specifier):
-        expanded_specifiers = self._configuration_macros.get(specifier)
+        expanded_specifiers = self.configuration_macros.get(specifier)
         return expanded_specifiers or [specifier]
 
     def to_config_set(self, specifier_set, error_list=None):
@@ -266,9 +266,9 @@ class TestConfigurationConverter(object):
 
         # 4) Substitute specifier subsets that match macros within each set:
         #   (win7, win10, release) -> (win, release)
-        self.collapse_macros(self._configuration_macros, specifiers_list)
+        self.collapse_macros(self.configuration_macros, specifiers_list)
 
-        macro_keys = set(self._configuration_macros.keys())
+        macro_keys = set(self.configuration_macros.keys())
 
         # 5) Collapsing macros may have created combinations the can now be abbreviated.
         #   (win7, release), (linux, x86, release), (linux, x86_64, release)

third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py

@@ -31,6 +31,7 @@
 from collections import defaultdict
 
 import logging
+import itertools
 import re
 
 from blinkpy.common.path_finder import PathFinder
@@ -565,15 +566,33 @@ class TestExpectationLine(object):
         if self.name is None:
             return '' if self.comment is None else '#%s' % self.comment
 
-        if test_configuration_converter and self.bugs:
+        if test_configuration_converter:
             specifiers_list = test_configuration_converter.to_specifiers_list(self.matching_configurations)
+            system_specifiers = set(
+                s.upper() for s in test_configuration_converter.configuration_macros.keys())
+            if system_specifiers:
+                system_specifiers.update(
+                    [s.upper() for s in reduce(lambda x, y: x + y, test_configuration_converter.configuration_macros.values())])
+            build_specifiers = set(s.upper() for s in TestExpectations.BUILD_TYPES)
             result = []
             for specifiers in specifiers_list:
                 # FIXME: this is silly that we join the specifiers and then immediately split them.
-                specifiers = self._serialize_parsed_specifiers(test_configuration_converter, specifiers).split()
+                specifiers = set(
+                    s.upper() for s in 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, specifiers, self.name, expectations, self.comment))
-            return '\n'.join(result) if result else None
+                system_intersection = system_specifiers & specifiers
+                build_intersection = build_specifiers & specifiers
+                if not system_intersection and not build_intersection:
+                    result.append(self._format_line(self.bugs, [], self.name, expectations, self.comment))
+                elif not system_intersection or not build_intersection:
+                    result.extend(
+                        [self._format_line(self.bugs, [s], self.name, expectations, self.comment)
+                         for s in system_intersection or build_intersection])
+                else:
+                    result.extend(
+                        [self._format_line(self.bugs, [sys, build], self.name, expectations, self.comment)
+                         for sys, build in itertools.product(system_intersection, build_intersection)])
+            return '\n'.join(result) or None
 
         return self._format_line(self.bugs, self.specifiers, self.name, self.expectations, self.comment,
                                  include_specifiers, include_expectations, include_comment)

third_party/blink/tools/blinkpy/web_tests/models/test_expectations_unittest.py

@@ -520,7 +520,8 @@ Bug(y) [ Win Mac Debug ] failures/expected/foo.html [ Crash ]
 
         actual_expectations = expectations.remove_configurations([('failures/expected/foo.html', test_config)])
 
-        self.assertEqual("""Bug(x) [ Linux Win10 Release ] failures/expected/foo.html [ Failure ]
+        self.assertEqual("""Bug(x) [ Win10 Release ] failures/expected/foo.html [ Failure ]
+Bug(x) [ Linux Release ] failures/expected/foo.html [ Failure ]
 Bug(y) [ Win Mac Debug ] failures/expected/foo.html [ Crash ]
 """, actual_expectations)
 
@@ -713,7 +714,6 @@ Bug(y) [ Mac ] failures/expected/foo.html [ Crash ]
         actual_expectations = expectations.remove_configurations([('failures/expected/foo.html', test_config)])
         actual_expectations = expectations.remove_configurations(
             [('failures/expected/foo.html', host.port_factory.get('test-win-win10', None).test_configuration())])
-
         self.assertEqual("""Bug(x) [ Win Debug ] failures/expected/foo.html [ Failure Timeout ]
 Bug(y) [ Mac ] failures/expected/foo.html [ Crash ]
 """, actual_expectations)
@@ -775,24 +775,24 @@ class TestExpectationSerializationTests(unittest.TestCase):
     def test_unparsed_to_string(self):
         expectation = TestExpectationLine()
 
-        self.assertEqual(expectation.to_string(self._converter), '')
+        self.assertEqual(expectation.to_string(), '')
         expectation.comment = ' Qux.'
-        self.assertEqual(expectation.to_string(self._converter), '# Qux.')
+        self.assertEqual(expectation.to_string(), '# Qux.')
         expectation.name = 'bar'
-        self.assertEqual(expectation.to_string(self._converter), 'bar # Qux.')
+        self.assertEqual(expectation.to_string(), 'bar # Qux.')
         expectation.specifiers = ['foo']
         # FIXME: case should be preserved here but we can't until we drop the old syntax.
-        self.assertEqual(expectation.to_string(self._converter), '[ FOO ] bar # Qux.')
+        self.assertEqual(expectation.to_string(), '[ FOO ] bar # Qux.')
         expectation.expectations = ['bAz']
-        self.assertEqual(expectation.to_string(self._converter), '[ FOO ] bar [ BAZ ] # Qux.')
+        self.assertEqual(expectation.to_string(), '[ FOO ] bar [ BAZ ] # Qux.')
         expectation.expectations = ['bAz1', 'baZ2']
-        self.assertEqual(expectation.to_string(self._converter), '[ FOO ] bar [ BAZ1 BAZ2 ] # Qux.')
+        self.assertEqual(expectation.to_string(), '[ FOO ] bar [ BAZ1 BAZ2 ] # Qux.')
         expectation.specifiers = ['foo1', 'foO2']
-        self.assertEqual(expectation.to_string(self._converter), '[ FOO1 FOO2 ] bar [ BAZ1 BAZ2 ] # Qux.')
+        self.assertEqual(expectation.to_string(), '[ FOO1 FOO2 ] bar [ BAZ1 BAZ2 ] # Qux.')
         expectation.warnings.append('Oh the horror.')
-        self.assertEqual(expectation.to_string(self._converter), '')
+        self.assertEqual(expectation.to_string(), '')
         expectation.original_string = 'Yes it is!'
-        self.assertEqual(expectation.to_string(self._converter), 'Yes it is!')
+        self.assertEqual(expectation.to_string(), 'Yes it is!')
 
     def test_unparsed_list_to_string(self):
         expectation = TestExpectationLine()

third_party/blink/tools/blinkpy/web_tests/try_flag.py

@@ -42,7 +42,8 @@ class TryFlag(object):
         self._git_cl = git_cl
         self._expectations_model = TestExpectationsModel()
         self._test_configuration_converter = TestConfigurationConverter(
-            set(BUILDER_CONFIGS.values()))
+            set(BUILDER_CONFIGS.values()),
+            self._host.port_factory.get().configuration_specifier_macros())
         self._filesystem = self._host.filesystem
         self._path_finder = PathFinder(self._filesystem)
         self._git = self._host.git()

third_party/blink/tools/blinkpy/web_tests/try_flag_unittest.py

@@ -153,7 +153,8 @@ class TryFlagTest(unittest.TestCase):
             '### 2 unexpected failures:',
             '',
             'Bug(none) something/fail-everywhere.html [ Failure ]',
-            'Bug(none) [ Linux Win ] something/fail-win-and-linux.html [ Failure ]',
+            'Bug(none) [ Win ] something/fail-win-and-linux.html [ Failure ]',
+            'Bug(none) [ Linux ] something/fail-win-and-linux.html [ Failure ]',
             ''
         ]))