Replace @unroll hack with standard Jinja loops in canvas WPT generator #50080
+47
−72
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-6172744
branch
from
df8e6ba to
387664d
Compare
wpt-pr-bot
approved these changes
Jan 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
forloops.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}