TestExpectations cannot represent variants with spaces

https://bugs.webkit.org/show_bug.cgi?id=269484
rdar://123026314

Reviewed by Jonathan Bedard.

This converts variants to always be percent-encoded (specifically: the variant
should be encoded identically to the output from the URL parser).

Along with this, this fixes various places where we only handled query variants
(and not fragment variants).

* Tools/Scripts/webkitpy/common/find_files.py:
(_normalized_find.sorted_paths_generator): Handle fragment variants
* Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy.py:
(LayoutTestFinder._expand_variants): De-duplicate code, handle fragment passed variants, percent encode everything
(LayoutTestFinder._percent_encoded_variant): Actually do percent encoding of variants
* Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy_unittest.py:

Canonical link: https://commits.webkit.org/274749@main

Tools/Scripts/webkitpy/common/find_files.py

@@ -44,6 +44,7 @@
 The callback has to take three arguments: filesystem, dirname and filename."""
 
 import itertools
+import re
 
 
 def find(filesystem, base_dir, paths=None, skipped_directories=None, file_filter=None, directory_sort_key=None):
@@ -76,16 +77,28 @@ def _normalized_find(filesystem, paths, skipped_directories, file_filter, direct
     sort_fn = (lambda lst: sorted(lst, key=directory_sort_key)) if directory_sort_key else (lambda lst: lst[:])
 
     def sorted_paths_generator(path, function):
-        base_path, separator, variant = path.partition('?')
-        if not separator:
-            return sort_fn(function(base_path))
+        m = re.search(
+            "^(?P<path>[^?#]*)(?P<variant>(?P<query>\\?[^#]*)?(?P<fragment>#.*)?)$",
+            path,
+        )
+        if not m.group("variant"):
+            return sort_fn(function(m.group("path")))
+
         # This isn't perfect, you won't be able glob the variant parts of the test name,
-        # but this is ultimately a design flaw stemming from a layout test not being a file
-        result = ['{}{}{}'.format(part, separator, variant) for part in function(base_path)]
+        # but this is ultimately a design flaw stemming from a layout test not being a
+        # file.
+        result = [
+            "{}{}".format(part, m.group("variant"))
+            for part in function(m.group("path"))
+        ]
         if result:
             return sort_fn(result)
+
+        # If we haven't matched anything, we might have a question-mark in path which we
+        # actually want to match as a glob, rather than treat it as a variant.
         return sort_fn(function(path))
 
+
     paths_to_walk = itertools.chain(*(sorted_paths_generator(path, filesystem.glob) for path in paths))
     all_files = itertools.chain(*(sorted_paths_generator(
         path, lambda p: filesystem.files_under(p, skipped_directories, file_filter),

Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy.py

@@ -30,6 +30,8 @@
 import json
 import logging
 import re
+import sys
+import urllib
 
 from webkitpy.common import find_files
 from webkitpy.layout_tests.models.test import Test
@@ -174,31 +176,25 @@ def _expand_variants(self, files):
             if "web-platform-tests" not in f:
                 expanded.append(f)
                 continue
-            f, variant_separator, passed_variant = f.partition('?')
+            m = re.search("^(?P<path>[^?#]*)(?P<variant>(?P<query>\\?[^#]*)?(?P<fragment>#.*)?)$", f)
+            f = m.group("path")
+            passed_variant = self._percent_encoded_variant(m.group("variant"))
             opened_file = fs.open_text_file_for_reading(f)
             try:
                 first_line = opened_file.readline()
                 if not first_line:
                     expanded.append(f)
                     continue
 
+                variants = []
                 if first_line.strip() == TEMPLATED_TEST_HEADER:
-                    variants = []
                     for line in iter(opened_file.readline, ''):
                         results = re.match(r'<!--\s*META:\s*variant=(\S*)\s*-->', line)
                         if not results:
                             continue
                         variant = results.group(1)
-                        if self._is_valid_variant(variant):
-                            variants.append(variant)
-                    if variants:
-                        for variant in variants:
-                            if not passed_variant or variant.startswith(variant_separator + passed_variant):
-                                expanded.append(f + variant)
-                    else:
-                        expanded.append(f)
+                        variants.append(variant)
                 else:
-                    variants = []
                     for line in iter(opened_file.readline, ''):
                         try:
                             line = line.lstrip()
@@ -220,21 +216,48 @@ def _expand_variants(self, files):
                             while line[end_index] not in end_chars:
                                 end_index += 1
                             variant = line[start_index:end_index]
-                            if self._is_valid_variant(variant):
-                                variants.append(line[start_index:end_index])
+                            variants.append(variant)
                         except IndexError:
                             continue
-                    if len(variants):
-                        for variant in variants:
-                            if not passed_variant or variant.startswith(variant_separator + passed_variant):
-                                expanded.append(f + variant)
-                    else:
-                        expanded.append(f)
+
+                variants = [v for v in variants if self._is_valid_variant(v)]
+                if len(variants):
+                    for variant in variants:
+                        variant = self._percent_encoded_variant(variant)
+                        if not passed_variant or variant.startswith(passed_variant):
+                            expanded.append(f + variant)
+                else:
+                    expanded.append(f)
             except UnicodeDecodeError:
                 expanded.append(f)
                 continue
         return expanded
 
+    def _percent_encoded_variant(self, variant):
+        m = re.search("^(?P<path>[^?#]*)(?P<variant>(?P<query>\\?[^#]*)?(?P<fragment>#.*)?)$", variant)
+        path, _, query, fragment = m.groups()
+        assert m.group("path") == ""
+
+        # This is all code points not in the "query percent-encode set" [URL], minus
+        # characters urllib.parse.quote never quotes.
+        safe_query = "!$%&'()*+,/:;=?@[\\]^`{|}~"
+
+        # This is all code points not in the "fragment percent-encode set" [URL], minus
+        # characters urllib.parse.quote never quotes.
+        safe_fragment = "!#$%&'()*+,/:;=?@[\\]^{|}~"
+
+        query = "" if query is None else query
+        fragment = "" if fragment is None else fragment
+
+        if sys.version_info > (3,):
+            query = urllib.parse.quote(query, safe=safe_query, encoding="utf-8")
+            fragment = urllib.parse.quote(fragment, safe=safe_fragment, encoding="utf-8")
+        else:
+            query = urllib.quote(query.encode("utf-8"), safe=safe_query).decode("ascii")
+            fragment = urllib.quote(fragment.encode("utf-8"), safe=safe_fragment).decode("ascii")
+
+        return "{}{}".format(query, fragment)
+
     def _is_valid_variant(self, variant):
         return variant == "" or (len(variant) > 1 and variant[0] in ("?", "#")) and variant != "?#"
 

Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy_unittest.py

@@ -399,6 +399,9 @@ def test_find_template_variants_meta(self):
 <meta name="variant" content="?#">
 <meta name="variant" content="?1#a">
 <meta name="variant" content="nonsense">
+<meta name=variant content="?only open()ed, not aborted">
+<meta name=variant content="?aborted immediately after send()">
+<meta name=variant content="?call abort() after TIME_NORMAL_LOAD">
         """)
         tests_found = [t.test_path for t in finder.find_tests_by_path(find_paths)]
         self.assertEqual(
@@ -409,10 +412,48 @@ def test_find_template_variants_meta(self):
                 "web-platform-tests/variant_test.html#a-m",
                 "web-platform-tests/variant_test.html#n-z",
                 "web-platform-tests/variant_test.html?1#a",
+                "web-platform-tests/variant_test.html?only%20open()ed,%20not%20aborted",
+                "web-platform-tests/variant_test.html?aborted%20immediately%20after%20send()",
+                "web-platform-tests/variant_test.html?call%20abort()%20after%20TIME_NORMAL_LOAD",
             ],
             tests_found,
         )
 
+    def test_find_template_variants_meta_passed_variants(self):
+        finder = self.finder
+
+        path = finder._port.layout_tests_dir() + "/web-platform-tests/variant_test.html"
+
+        find_paths = [
+            path + "?a b",
+            path + "?c%20d",
+            path + "#m n",
+            path + "#o%20p",
+            path + "?e%20f#q%20r",
+        ]
+
+        finder._filesystem.maybe_make_directory(finder._filesystem.dirname(path))
+        finder._filesystem.write_text_file(
+            path,
+            """<!doctype html>
+<meta name=variant content="?a b">
+<meta name=variant content="?c%20d">
+<meta name=variant content="#m n">
+<meta name=variant content="#o%20p">
+<meta name=variant content="?e f#q r">
+        """,
+        )
+        tests_found = [t.test_path for t in finder.find_tests_by_path(find_paths)]
+        self.assertEqual(
+            ['web-platform-tests/variant_test.html?a%20b',
+             'web-platform-tests/variant_test.html?c%20d',
+             'web-platform-tests/variant_test.html#m%20n',
+             'web-platform-tests/variant_test.html#o%20p',
+             'web-platform-tests/variant_test.html?e%20f#q%20r'],
+            tests_found,
+        )
+
+
     def test_find_template_variants_comment(self):
         find_paths = ["web-platform-tests"]
         finder = self.finder