Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the canvas test generation code #33613

Closed

Conversation

gsnedders
Copy link
Member

@gsnedders gsnedders commented Apr 12, 2022

This unifies the two largely-identical implementations that generate the canvas (<canvas> and OffscreenCanvas) tests, and gets rid of the duplication of templates.

This does, unfortunately, mean a small number of tests (~140 of the several thousand generated) have whitespace changes.

Hopefully this makes it easier for any future work on these scripts!

@jgraham
Copy link
Contributor

jgraham commented Apr 13, 2022

Would be useful to refactor this into one commit for the script changes and one commit for the generated output files.

@gsnedders gsnedders force-pushed the simplify-canvas-build branch from 3086985 to aa5c305 Compare April 13, 2022 16:08
@gsnedders
Copy link
Member Author

Would be useful to refactor this into one commit for the script changes and one commit for the generated output files.

Done!

This is running `pyupgrade --py36-plus` & `black` until they reached a fixed
point.
This unifies the two largely-identical implementations that generate the canvas
(<canvas> and OffscreenCanvas) tests, and gets rid of the duplication of
templates.

This does, unfortunately, mean a small number of tests (~140 of the several
thousand generated) have whitespace changes.

Hopefully this makes it easier for any future work on these scripts.
This is entirely changes to blank lines (see `git-diff --ignore-blank-lines`),
except for 2d.path.roundrect.radius.noargument.html which appears to have never
been generated since 31eb342 when the test name was modified.
@gsnedders
Copy link
Member Author

Apparently html/canvas/tools got substantially rewritten through 2023–4, and this never got reviewed, and as with all the changes, you probably are just as well off starting from scratch to actually land something comparable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants