[rebaseline-cl] Skip optimization for invalid WPT type-suffix pairs

WPTs may only have baselines in a few cases. Both `rebaseline-cl` and
`run_(web|wpt)_tests.py --reset-results` should not create invalid
baseline types, so there's no need to check for `*-expected.*` that will
almost certainly not exist.

As one might suppose, this filtering has a negligible impact when
optimizing only a few files (as `web_tests/PRESUBMIT.py` usually does),
but speeds up:
  blink_tool.py optimize-baselines --all --check --verbose

from 6:30 to 3:45.

Bug: 1364190
Change-Id: I915ab8cae96be6653cb085ba0be533364410e93c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4939519
Reviewed-by: Weizhong Xia <weizhong@google.com>
Commit-Queue: Jonathan Lee <jonathanjlee@google.com>
Cr-Commit-Position: refs/heads/main@{#1209747}

third_party/blink/tools/blinkpy/common/net/web_test_results.py

@@ -28,19 +28,26 @@
 
 import collections
 import json
-from typing import Dict, List, NamedTuple, Optional
+from typing import Dict, List, Literal, NamedTuple, Optional
 
 from blinkpy.common.memoized import memoized
 from blinkpy.web_tests.layout_package import json_results_generator
 from blinkpy.web_tests.models.test_failures import FailureImage
 from blinkpy.web_tests.models.typ_types import ResultType
+from blinkpy.web_tests.port.base import Port
 
 
 class Artifact(NamedTuple):
     url: str
     digest: Optional[str] = None
 
 
+# For CLI compatibility, we would like a list of baseline extensions without
+# the leading dot.
+# TODO: Investigate changing the CLI.
+BaselineSuffix = Literal[tuple(ext[1:] for ext in Port.BASELINE_EXTENSIONS)]
+
+
 class WebTestResult:
     def __init__(self, test_name, result_dict,
                  artifacts: Dict[str, List[Artifact]]):
@@ -52,7 +59,7 @@ def __repr__(self):
         return "WebTestResult(test_name=%s, result_dict=%s)" % \
             (repr(self._test_name), repr(self._result_dict))
 
-    def baselines_by_suffix(self) -> Dict[str, List[Artifact]]:
+    def baselines_by_suffix(self) -> Dict[BaselineSuffix, List[Artifact]]:
         baselines = {}
         # Add extensions for mismatches.
         for artifact_name, suffix in [

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

@@ -30,8 +30,10 @@
 
 import logging
 import optparse
+from typing import get_args
 
 from blinkpy.common.checkout.baseline_optimizer import BaselineOptimizer
+from blinkpy.common.net.web_test_results import BaselineSuffix
 from blinkpy.tool.commands.rebaseline import AbstractRebaseliningCommand
 
 _log = logging.getLogger(__name__)
@@ -52,6 +54,7 @@ def __init__(self):
                 default=False,
                 help='Show missing baselines as well.'),
         ] + self.platform_options)
+        self._baseline_suffix_list = get_args(BaselineSuffix)
         self._optimizer_class = BaselineOptimizer  # overridable for testing
         self._baseline_optimizer = None
         self._port = None

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

@@ -3,15 +3,20 @@
 # found in the LICENSE file.
 
 import functools
+import itertools
 import logging
 import optparse
+from typing import Collection, List, Optional, Set, Tuple
 
 from blinkpy.common.checkout.baseline_optimizer import BaselineOptimizer
+from blinkpy.common.net.web_test_results import BaselineSuffix
 from blinkpy.tool.commands.command import resolve_test_patterns
 from blinkpy.tool.commands.rebaseline import AbstractParallelRebaselineCommand
 
 _log = logging.getLogger(__name__)
 
+OptimizationTask = Tuple[str, str, BaselineSuffix]
+
 
 class OptimizeBaselines(AbstractParallelRebaselineCommand):
     name = 'optimize-baselines'
@@ -58,19 +63,17 @@ def execute(self, options, args, tool):
             _log.error("No port names match '%s'", options.platform)
             return 1
 
-        port = tool.port_factory.get(options=options)
-        test_set = self._get_test_set(port, options, args)
+        self._host_port = tool.port_factory.get(options=options)
+        test_set = self._get_test_set(options, args)
         if not test_set:
             _log.error('No tests to optimize. Ensure all listed tests exist.')
             return 1
 
         worker_factory = functools.partial(Worker,
                                            port_names=port_names,
                                            options=options)
-        baseline_suffix_list = options.suffixes.split(',')
+        tasks = self._make_tasks(test_set, options.suffixes.split(','))
         with self._message_pool(worker_factory) as pool:
-            tasks = [(self.name, test_name, suffix) for test_name in test_set
-                     for suffix in baseline_suffix_list]
             pool.run(tasks)
         if options.check:
             if self._successful:
@@ -81,15 +84,40 @@ def execute(self, options, args, tool):
                              'to fix these issues.')
                 return 2
 
-    def _get_test_set(self, port, options, args):
+    def _make_tasks(
+            self, test_set: Set[str],
+            suffixes: Collection[BaselineSuffix]) -> List[OptimizationTask]:
+        tasks = []
+        for test_name, suffix in itertools.product(sorted(test_set), suffixes):
+            wpt_type = self._get_wpt_type(test_name)
+            if (not wpt_type or
+                    # Only legacy reftests can dump text output, not WPT
+                    # reftests.
+                    wpt_type in {'testharness', 'wdspec'} and suffix == 'txt'
+                    # Some manual tests are run as pixel tests (crbug.com/1114920),
+                    # so `png` is allowed in that case.
+                    or wpt_type == 'manual' and suffix == 'png'):
+                tasks.append((self.name, test_name, suffix))
+        return tasks
+
+    def _get_wpt_type(self, test_name: str) -> Optional[str]:
+        for wpt_dir, url_base in self._host_port.WPT_DIRS.items():
+            if test_name.startswith(wpt_dir):
+                manifest = self._host_port.wpt_manifest(wpt_dir)
+                return manifest.get_test_type(
+                    manifest.file_path_for_test_url(
+                        test_name[len(f'{wpt_dir}/'):]))
+        return None  # Not a WPT.
+
+    def _get_test_set(self, options, args):
         if options.all_tests:
-            test_set = set(port.tests())
+            test_set = set(self._host_port.tests())
         else:
-            test_set = resolve_test_patterns(port, args)
+            test_set = resolve_test_patterns(self._host_port, args)
         virtual_tests_to_exclude = {
             test
             for test in test_set
-            if port.lookup_virtual_test_base(test) in test_set
+            if self._host_port.lookup_virtual_test_base(test) in test_set
         }
         test_set -= virtual_tests_to_exclude
         return test_set
@@ -115,7 +143,8 @@ def start(self):
             self._port_names,
             check=self._options.check)
 
-    def handle(self, name: str, source: str, test_name: str, suffix: str):
+    def handle(self, name: str, source: str, test_name: str,
+               suffix: BaselineSuffix):
         successful = self._optimizer.optimize(test_name, suffix)
         if self._options.check and not self._options.verbose and successful:
             # Without `--verbose`, do not show optimization logs when a test

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

@@ -2,9 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import json
 import optparse
 import textwrap
 
+from blinkpy.common.path_finder import PathFinder
 from blinkpy.common.system.log_testing import LoggingTestCase
 from blinkpy.tool.commands.optimize_baselines import OptimizeBaselines
 from blinkpy.tool.commands.rebaseline_unittest import BaseTestCase
@@ -104,6 +106,52 @@ def test_execute_with_test_name_file(self):
         self.assertTrue(self._exists(test_port, 'optimized-expected.txt'))
         self.assertTrue(self._exists(test_port, 'skipped-expected.txt'))
 
+    def test_execute_filter_suffixes_for_valid_wpt_types(self):
+        """Skip optimization for invalid test type-suffix combinations."""
+        finder = PathFinder(self.tool.filesystem)
+        self.tool.filesystem.write_text_file(
+            finder.path_from_wpt_tests('MANIFEST.json'),
+            json.dumps({
+                'version': 8,
+                'url_base': '/',
+                'items': {
+                    'manual': {
+                        'test-manual.html': ['abcdef', [None, {}]],
+                    },
+                    'testharness': {
+                        'testharness.html': ['abcdef', [None, {}]],
+                    },
+                    'reftest': {
+                        'reftest.html': ['abcdef', [None, [], {}]],
+                    },
+                    'print-reftest': {
+                        'print-reftest.html': ['abcdef', [None, [], {}]],
+                    },
+                    'crashtest': {
+                        'test-crash.html': ['abcdef', [None, {}]],
+                    },
+                },
+            }))
+        self.tool.filesystem.write_text_file(
+            finder.path_from_web_tests('wpt_internal', 'MANIFEST.json'),
+            json.dumps({}))
+
+        exit_code = self.command.execute(
+            optparse.Values({
+                'suffixes': 'txt,wav,png',
+                'all_tests': False,
+                'platform': 'test-mac-mac10.10',
+                'check': True,
+                'test_name_file': None,
+                'manifest_update': False,
+                'verbose': False,
+            }), ['external/wpt'], self.tool)
+        self.assertLog([
+            'INFO: Checking external/wpt/test-manual.html (png)\n',
+            'INFO: Checking external/wpt/testharness.html (txt)\n',
+            'INFO: All baselines are optimal.\n',
+        ])
+
     def test_check_optimal(self):
         test_port = self.tool.port_factory.get('test')
         self._write_test_file(test_port, 'another/test.html',

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

@@ -42,6 +42,7 @@
     Optional,
     Set,
     Tuple,
+    get_args,
 )
 from urllib.parse import urlparse
 
@@ -51,7 +52,11 @@
 from blinkpy.common.path_finder import WEB_TESTS_LAST_COMPONENT
 from blinkpy.common.memoized import memoized
 from blinkpy.common.net.results_fetcher import Build
-from blinkpy.common.net.web_test_results import Artifact, WebTestResult
+from blinkpy.common.net.web_test_results import (
+    Artifact,
+    BaselineSuffix,
+    WebTestResult,
+)
 from blinkpy.tool.commands.command import (
     Command,
     check_dir_option,
@@ -60,15 +65,10 @@
 from blinkpy.web_tests.models import test_failures
 from blinkpy.web_tests.models.test_expectations import SystemConfigurationEditor, TestExpectations
 from blinkpy.web_tests.models.typ_types import RESULT_TAGS, ResultType
-from blinkpy.web_tests.port import base, factory
+from blinkpy.web_tests.port import factory
 
 _log = logging.getLogger(__name__)
 
-# For CLI compatibility, we would like a list of baseline extensions without
-# the leading dot.
-# TODO(robertma): Investigate changing the CLI.
-BASELINE_SUFFIX_LIST = tuple(ext[1:] for ext in base.Port.BASELINE_EXTENSIONS)
-
 
 class AbstractRebaseliningCommand(Command):
     """Base class for rebaseline-related commands."""
@@ -101,7 +101,7 @@ class AbstractRebaseliningCommand(Command):
         help='Local results directory to use.')
     suffixes_option = optparse.make_option(
         '--suffixes',
-        default=','.join(BASELINE_SUFFIX_LIST),
+        default=','.join(get_args(BaselineSuffix)),
         action='store',
         help='Comma-separated-list of file types to rebaseline.')
     builder_option = optparse.make_option(
@@ -122,8 +122,7 @@ class AbstractRebaseliningCommand(Command):
               'one test per line.'))
 
     def __init__(self, options=None):
-        super(AbstractRebaseliningCommand, self).__init__(options=options)
-        self._baseline_suffix_list = BASELINE_SUFFIX_LIST
+        super().__init__(options=options)
         self.expectation_line_changes = ChangeSet()
         self._tool = None
         self._results_dir = None
@@ -598,7 +597,8 @@ def _format_line(self, task: RebaselineTask) -> str:
     def unstaged_baselines(self):
         """Returns absolute paths for unstaged (including untracked) baselines."""
         baseline_re = re.compile(r'.*[\\/]' + WEB_TESTS_LAST_COMPONENT +
-                                 r'[\\/].*-expected\.(txt|png|wav)$')
+                                 r'[\\/].*-expected\.(' +
+                                 '|'.join(get_args(BaselineSuffix)) + ')$')
         unstaged_changes = self._tool.git().unstaged_changes()
         return sorted(self._tool.git().absolute_path(path)
                       for path in unstaged_changes