WPT importer: Use concise lists of version specifiers in expectation lines.

third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Updates layout test expectations and baselines when updating w3c tests.
 
 Specifically, this class fetches results from try bots for the current CL, then
 (1) downloads new baseline files for any tests that can be rebaselined, and
 (2) updates the generic TestExpectations file for any other failing tests.
 """
 
 import argparse
 import copy
 import logging
 
+from webkitpy.common.memoized import memoized
 from webkitpy.common.net.git_cl import GitCL
 from webkitpy.common.net.rietveld import Rietveld
 from webkitpy.common.webkit_finder import WebKitFinder
-from webkitpy.layout_tests.models.test_expectations import TestExpectationLine
+from webkitpy.layout_tests.models.test_expectations import TestExpectationLine, TestExpectations
 from webkitpy.w3c.test_parser import TestParser
 
 _log = logging.getLogger(__name__)
 
 MARKER_COMMENT = '# ====== New tests from w3c-test-autoroller added here ======'
 
 
 class WPTExpectationsUpdater(object):
 
     def __init__(self, host):
@@ skipping 36 matching lines @@
         test_expectation_lines = self.create_line_list(test_expectations)
         self.write_to_test_expectations(test_expectation_lines)
         return 0
 
     def get_issue_number(self):
         """Returns current CL number. Can be replaced in unit tests."""
         return GitCL(self.host).get_issue_number()
 
     def get_try_bots(self):
         """Returns try bot names. Can be replaced in unit tests."""
+        # TODO(qyearsley): This method is unnecessary; unit tests can set up
+        # a BuilderList with try builder names, instead of overriding this.
         return self.host.builders.all_try_builder_names()
 
     def get_failing_results_dict(self, build):
         """Returns a nested dict of failing test results.
 
         Retrieves a full list of layout test results from a builder result URL.
         Collects the builder name, platform and a list of tests that did not
         run as expected.
 
         Args:
@@ skipping 35 matching lines @@
             test_name = result.test_name()
             test_dict[test_name] = {
                 full_port_name: {
                     'expected': result.expected_results(),
                     'actual': result.actual_results(),
                     'bug': 'crbug.com/626703'
                 }
             }
         return test_dict
 
-    def _port_name_to_platform_specifier(self, port_name):
-        """Maps a port name to the platform specifier used in expectation lines.
-
-        For example:
-            linux-trusty -> Trusty
-            mac-mac10.11 -> Mac10.11.
-        """
-        builder_name = self.host.builders.builder_name_for_port_name(port_name)
-        specifiers = self.host.builders.specifiers_for_builder(builder_name)
-        return specifiers[0]
-
     def merge_dicts(self, target, source, path=None):
         """Recursively merges nested dictionaries.
 
         Args:
             target: First dictionary, which is updated based on source.
             source: Second dictionary, not modified.
 
         Returns:
             An updated target dictionary.
         """
@@ skipping 102 matching lines @@
                         'platform': {
                             'expected: 'PASS',
                             'actual': 'FAIL',
                             'bug': 'crbug.com/11111'
                         }
                     }
                 }
 
         Returns:
             A list of test expectations lines with the format:
             ['BUG_URL [PLATFORM(S)] TEST_MAME [EXPECTATION(S)]']

[jeffcarp, 2017/02/13 20:42:18]

I know this isn't part of the CL but should this be TEST_NAME?

[qyearsley, 2017/02/14 00:00:51]

Yep! Unless mame has some meaning I'm not aware of...

         """
         line_list = []
         for test_name, port_results in merged_results.iteritems():
-            for port_names in port_results:
+            for port_names in sorted(port_results):
                 if test_name.startswith('external'):
-                    bug_part = port_results[port_names]['bug']
-                    specifier_part = '[ %s ]' % ' '.join(self.to_list(port_names))
-                    expectations_part = '[ %s ]' % ' '.join(self.get_expectations(port_results[port_names]))
-                    line = ' '.join([bug_part, specifier_part, test_name, expectations_part])
-                    line_list.append(line)
+                    line_parts = [port_results[port_names]['bug']]
+                    specifier_part = self.specifier_part(self.to_list(port_names), test_name)
+                    if specifier_part:
+                        line_parts.append(specifier_part)
+                    line_parts.append(test_name)
+                    line_parts.append('[ %s ]' % ' '.join(self.get_expectations(port_results[port_names])))
+                    line_list.append(' '.join(line_parts))
         return line_list
 
+    def specifier_part(self, port_names, test_name):
+        """Returns the specifier part for a new test expectations line.
+
+        Args:
+            port_names: A list of full port names that the line should apply to.
+            test_name: The test name for the expectation line.
+
+        Returns:
+            The specifier part of the new expectation line, e.g. "[ Mac ]".
+            This will be an empty string if the line should apply to all platforms.
+        """
+        specifiers = []
+        for name in sorted(port_names):
+            specifiers.append(self.host.builders.version_specifier_for_port_name(name))
+        port = self.host.port_factory.get()
+        specifiers = self.simplify_specifiers(specifiers, port.configuration_specifier_macros())
+        specifiers.extend(self.skipped_specifiers(test_name))

[qyearsley, 2017/02/13 19:54:23]

The purpose of this is so that Android is included in the list of specifiers, so
that it can be simplified out for tests that aren't run on Android.

+        if not specifiers:
+            return ''
+        return '[ %s ]' % ' '.join(specifiers)
+
     @staticmethod
     def to_list(tuple_or_value):
+        """Converts a tuple to a list, and a string value to a one-item list."""
         if isinstance(tuple_or_value, tuple):
             return list(tuple_or_value)
         return [tuple_or_value]
 
+    def skipped_specifiers(self, test_name):
+        """Returns a list of platform specifiers for which the test is skipped."""
+        # TODO(qyearsley): Change Port.skips_test so that this can be simplified.
+        specifiers = []
+        for port in self.all_try_builder_ports():
+            generic_expectations = TestExpectations(port, tests=[test_name], include_overrides=False)
+            full_expectations = TestExpectations(port, tests=[test_name], include_overrides=True)
+            if port.skips_test(test_name, generic_expectations, full_expectations):
+                specifiers.append(self.host.builders.version_specifier_for_port_name(port.name()))
+        return specifiers
+
+    @memoized
+    def all_try_builder_ports(self):
+        """Returns a list of Port objects for all try builders."""
+        return [self.host.port_factory.get_from_builder_name(name) for name in self.get_try_bots()]
+
+    @staticmethod
+    def simplify_specifiers(specifiers, configuration_specifier_macros):  # pylint: disable=unused-argument
+        """Converts a list of specifiers to an equivalent and maybe shorter list.
+
+        The input strings are all case-insensitive, but the strings in the
+        return value will all be capitalized.
+
+        Args:
+            specifiers: A list of lower-case specifiers.
+            configuration_specifier_macros: A dict mapping "macros" for
+                groups of specifiers to lists of specific specifiers. In
+                practice, this is a dict mapping operating systems to
+                supported versions, e.g. {"win": ["win7", "win10"]}.
+
+        Returns:
+            A shortened list of specifiers. For example, ["win7", "win10"]
+            would be converted to ["Win"]. If the given list covers all
+            supported platforms, then an empty list is returned.
+        """
+        specifiers = {specifier.lower() for specifier in specifiers}
+        for macro, versions in configuration_specifier_macros.iteritems():
+            versions = set(versions)
+            if versions <= specifiers:
+                specifiers -= versions
+                specifiers.add(macro)
+        if specifiers == set(configuration_specifier_macros):
+            return []
+        return sorted(specifier.capitalize() for specifier in specifiers)

[qyearsley, 2017/02/13 19:54:23]

This is the main part of this change.

+
     def write_to_test_expectations(self, line_list):
         """Writes to TestExpectations.
 
         The place in the file where the new lines are inserted is after a
         marker comment line. If this marker comment line is not found, it will
         be added to the end of the file.
 
         Args:
             line_list: A list of lines to add to the TestExpectations file.
         """
@@ skipping 83 matching lines @@
 
         Args:
             test_path: A file path relative to the layout tests directory.
                 This might correspond to a deleted file or a non-test.
         """
         absolute_path = self.host.filesystem.join(self.finder.layout_tests_dir(), test_path)
         test_parser = TestParser(absolute_path, self.host)
         if not test_parser.test_doc:
             return False
         return test_parser.is_jstest()