commit dc72f923ed0173e81dda73770ccca93c90c9c22b
author Jean-Philippe Gravel <jpgravel@chromium.org> Thu Apr 04 08:21:35 2024
committer Blink WPT Bot <blink-w3c-test-autoroller@chromium.org> Thu Apr 04 10:26:43 2024
tree 327524706501a3f86c180fa3b0210885797cb4ab
parent 283fe08e8649884a66633f269c03b751b65b0c26

Move test uniqueness check outside of _generate_test

This logic really belongs to the main loop, where the used_tests
variable, keeping track of the tests created so far, lives.

Bug: 330885075
Change-Id: I19e525132865abab2e84a5bc39ae0e2a42a3f7bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5374600
Commit-Queue: Jean-Philippe Gravel <jpgravel@chromium.org>
Reviewed-by: Andres Ricardo Perez <andresrperez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1282135}

html/canvas/tools/gentestutilsunion.py

diff --git a/html/canvas/tools/gentestutilsunion.py b/html/canvas/tools/gentestutilsunion.py
index 31be4d0..3aad13b 100644
--- a/html/canvas/tools/gentestutilsunion.py
+++ b/html/canvas/tools/gentestutilsunion.py
@@ -471,25 +471,14 @@
 
 
 def _generate_test(test: _TestParams, jinja_env: jinja2.Environment,
-                   used_tests: DefaultDict[str, Set[_CanvasType]],
                    output_dirs: _OutputPaths) -> None:
     _validate_test(test)
 
-    name = test['name']
-
-    enabled_canvas_types = test['canvas_types']
-
     params = dict(test)
     params.update({
         'code': _expand_test_code(test['code']),
     })
 
-    already_tested = used_tests[name].intersection(enabled_canvas_types)
-    if already_tested:
-        raise InvalidTestDefinitionError(
-            f'Test {name} is defined twice for types {already_tested}')
-    used_tests[name].update(enabled_canvas_types)
-
     output_files = output_dirs.sub_path(params['file_name'])
 
     if 'reference' in test or 'html_reference' in test:
@@ -546,6 +535,15 @@
     return test_variants
 
 
+def _check_uniqueness(tested: DefaultDict[str, Set[_CanvasType]], name: str,
+                      canvas_types: FrozenSet[_CanvasType]) -> None:
+    already_tested = tested[name].intersection(canvas_types)
+    if already_tested:
+        raise InvalidTestDefinitionError(
+            f'Test {name} is defined twice for types {already_tested}')
+    tested[name].update(canvas_types)
+
+
 def generate_test_files(name_to_dir_file: str) -> None:
     """Generate Canvas tests from YAML file definition."""
     output_dirs = _OutputPaths(element='../element', offscreen='../offscreen')
@@ -608,7 +606,9 @@
             sub_dir = _get_test_sub_dir(variant['file_name'], name_to_sub_dir)
             output_sub_dirs = output_dirs.sub_path(sub_dir)
 
+            _check_uniqueness(used_tests, variant['name'],
+                              variant['canvas_types'])
             _generate_expected_image(variant, output_sub_dirs)
-            _generate_test(variant, jinja_env, used_tests, output_sub_dirs)
+            _generate_test(variant, jinja_env, output_sub_dirs)
 
     print()