Replace @unroll hack with standard Jinja loops in canvas WPT generator

The @unroll directive was added as a way of generating a statement with the cross product of a number of possible argument values. It was added before the test generator was migrated to Jinja. With Jinja, we can now do the equivalent unrolling by simply using standard `for` loops. The @... directives are just disguised regex string replacements and their interaction with Jinja is delicate, often broken and hard to maintain. The string replacement often fails for multi-line/statement content or when combined with Jinja templating. We would be better off just using standard Jinja templating. The @unroll directive was only used in a single test and the interaction with Jinja template expansion made the test definition very hard to understand. The test is easier to follow if we just use Jinja logic. Bug: 40207206 Change-Id: I6e31becd7c224d00c3d1d5a32bf8f47ea6ba411c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6172744 Reviewed-by: Yi Xu <[email protected]> Commit-Queue: Jean-Philippe Gravel <[email protected]> Cr-Commit-Position: refs/heads/main@{#1406444}
web-platform-tests · Jan 15, 2025 · 9aa25c6 · 9aa25c6
1 parent 06be9fb
commit 9aa25c6
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 72 deletions.
diff --git a/html/canvas/tools/gentestutilsunion.py b/html/canvas/tools/gentestutilsunion.py
@@ -78,46 +78,6 @@ def _escape_js(string: str) -> str:
     return string
 
 
-def _unroll(text: str) -> str:
-    """Unrolls text with all possible permutations of the parameter lists.
-
-    Example:
-    >>> print _unroll('f = {<a | b>: <1 | 2 | 3>};')
-    // a
-    f = {a: 1};
-    f = {a: 2};
-    f = {a: 3};
-    // b
-    f = {b: 1};
-    f = {b: 2};
-    f = {b: 3};
-    """
-    patterns = []  # type: List[Tuple[str, List[str]]]
-    while True:
-        match = re.search(r'<([^>]+)>', text)
-        if not match:
-            break
-        key = f'@unroll_pattern_{len(patterns)}'
-        values = text[match.start(1):match.end(1)]
-        text = text[:match.start(0)] + key + text[match.end(0):]
-        patterns.append((key, [value.strip() for value in values.split('|')]))
-
-    def unroll_patterns(text: str,
-                        patterns: List[Tuple[str, List[str]]],
-                        label: Optional[str] = None) -> List[str]:
-        if not patterns:
-            return [text]
-        patterns = patterns.copy()
-        key, values = patterns.pop(0)
-        return (['// ' + label] if label else []) + list(
-            itertools.chain.from_iterable(
-                unroll_patterns(text.replace(key, value), patterns, value)
-                for value in values))
-
-    result = '\n'.join(unroll_patterns(text, patterns))
-    return result
-
-
 def _expand_nonfinite(method: str, argstr: str, tail: str) -> str:
     """
     >>> print _expand_nonfinite('f', '<0 a>, <0 b>', ';')
@@ -198,9 +158,6 @@ def _expand_test_code(code: str) -> str:
 
     code = _remove_extra_newlines(code)
 
-    # Unroll expressions with a cross-product-style parameter expansion.
-    code = re.sub(r'@unroll ([^;]*;)', lambda m: _unroll(m.group(1)), code)
-
     code = re.sub(r'@nonfinite ([^(]+)\(([^)]+)\)(.*)', lambda m:
                   _expand_nonfinite(m.group(1), m.group(2), m.group(3)),
                   code)  # Must come before '@assert throws'.

diff --git a/html/canvas/tools/yaml-new/filters.yaml b/html/canvas/tools/yaml-new/filters.yaml
@@ -834,37 +834,55 @@
   desc: Test exceptions on CanvasFilter() dropShadow object
   code: |
     // Should not throw an error.
-    @unroll {{ filter_declaration | replace("param", "{\-
-        name: 'dropShadow', \-
-        <dx | dy | floodOpacity>: \-
-        <10 | -1 | 0.5 | null | true | false | [] | [20] | '30'>}") }};
-    @unroll {{ filter_declaration | replace("param", "{\-
-        name: 'dropShadow', \-
-        <stdDeviation>: \-
-        <10 | -1 | 0.5 | null | true | false | [] | [20] | '30' | \-
-         [10, -1] | [0.5, null] | [true, false] | [[], [20]] | \-
-         ['30', ['40']]>}") }};
-    @unroll {{ filter_declaration | replace("param", "{\-
-        name: 'dropShadow', \-
-        <floodColor>: \-
-        <'red' | 'canvas' | 'rgba(4, -3, 0.5, 1)' | '#aabbccdd' |
-          '#abcd'>}") }};
+    {% for param_name in ['dx', 'dy', 'floodOpacity'] -%}
+      // {{ param_name }}
+      {% for value in [10, -1, 0.5, 'null', 'true', 'false', [], [20],
+                       "'30'"] -%}
+        {{ filter_declaration | replace("param",
+            "{name: 'dropShadow', %s: %s}" | format(param_name, value)) }};
+      {% endfor %}
+    {% endfor -%}
+
+    // stdDeviation
+    {% for value in [10, -1, 0.5, 'null', 'true', 'false', [], [20],
+                     "'30'", [10, -1], '[0.5, null]', '[true, false]',
+                     [[], [20]], "['30', ['40']]"] -%}
+      {{ filter_declaration | replace("param",
+          "{name: 'dropShadow', stdDeviation: %s}" | format(value)) }};
+    {% endfor -%}
+
+    // floodColor
+    {% for value in ['red', 'canvas', 'rgba(4, -3, 0.5, 1)', '#aabbccdd',
+                     '#abcd'] -%}
+      {{ filter_declaration | replace("param",
+          "{name: 'dropShadow', floodColor: '%s'}" | format(value)) }};
+    {% endfor %}
 
     // Should throw a TypeError.
-    @unroll @assert throws TypeError {{ filter_declaration | replace("param", \-
-        "{name: 'dropShadow', \-
-        <dx | dy | floodOpacity>: \-
-        <NaN | Infinity | -Infinity | undefined | 'test' | {} | [1, 2]>}") }};
-    @unroll @assert throws TypeError {{ filter_declaration | replace("param", \-
-        "{name: 'dropShadow', \-
-        <stdDeviation>: \-
-        <NaN | Infinity | -Infinity | undefined | 'test' | {} | [1, 2, 3] | \-
-         [1, NaN] | [1, Infinity] | [1, -Infinity] | [1, undefined] | \-
-         [1, 'test'] | [1, {}] | [1, [2, 3]]>}") }};
-    @unroll @assert throws TypeError {{ filter_declaration | replace("param", \-
-        "{name: 'dropShadow', \-
-        <floodColor>: \-
-        <'test' | 'rgba(NaN, 3, 2, 1)' | 10 | undefined | null | NaN>}") }};
+    {% for param_name in ['dx', 'dy', 'floodOpacity'] -%}
+      // {{ param_name }}
+      {% for value in ['NaN', 'Infinity', '-Infinity', 'undefined', "'test'",
+                       {}, [1, 2]] -%}
+        @assert throws TypeError {{ filter_declaration | replace("param",
+          "{name: 'dropShadow', %s: %s}" | format(param_name, value)) }};
+      {% endfor %}
+    {% endfor -%}
+
+    // stdDeviation
+    {% for value in ['NaN', 'Infinity', '-Infinity', 'undefined', "'test'", {},
+                     [1, 2, 3], '[1, NaN]', '[1, Infinity]', '[1, -Infinity]',
+                     '[1, undefined]', "[1, 'test']", '[1, {}]',
+                     '[1, [2, 3]]'] -%}
+      @assert throws TypeError {{ filter_declaration | replace("param",
+        "{name: 'dropShadow', stdDeviation: %s}" | format(value)) }};
+    {% endfor -%}
+
+    // floodColor
+    {% for value in ["'test'", "'rgba(NaN, 3, 2, 1)'", 10, 'undefined', 'null',
+                     'NaN'] -%}
+      @assert throws TypeError {{ filter_declaration | replace("param",
+        "{name: 'dropShadow', floodColor: %s}" | format(value)) }};
+    {% endfor -%}
   append_variants_to_name: false
   variants:
   - layers: