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

Fix svg graphics element tests #227

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johannesodland
Copy link
Contributor

@johannesodland johannesodland commented Feb 4, 2024

Depends on #244

Adding another PR for discussion:

The svg-graphics-element tests are currently flaky, making the tests fail randomly when submitting PRs.
This is due to three separate issues:

  1. We don't support svg elements subjects, as offsetTop/Left/Parent are not available for SVGElement
  2. Injecting 1s in the animation shorthand failed after removing auto duration
  3. WPT subtests might run before animations are polyfilled

The first two issues are easy to solve, but the third is more complicated.

The polyfill waits for animationstart before polyfilling animations. At this point the wpt test might have started already, causing the test to fail or time out.

It is possible to proxy the promise_test() function, so that it waits for the polyfill before running the test. However, the wpt test runner only allows one injected script. To make the test pass then we either have to:

  • Setup the proxy in the production build
  • Have a separate test build with the proxy included

It seems like several others polyfill projects have run into similar problems (web-platform-tests/wpt#38227). I wish it was possible to inject multiple scripts into the wpt test runner, but that might not be feasible.

@johannesodland johannesodland force-pushed the fix-svg-graphics-element-tests branch from c72bd12 to 1bfe1d9 Compare February 4, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant