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

Fail early if @position-try is not supported #50552

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

foolip
Copy link
Member

@foolip foolip commented Feb 7, 2025

No description provided.

@foolip
Copy link
Member Author

foolip commented Feb 7, 2025

I'm doing this because the test fails with an unclear harness error in Firefox and Safari:
https://wpt.fyi/results/css/css-anchor-position/at-position-try-invalidation-shadow-dom.html?run_id=5200154028408832&run_id=5067916582322176&run_id=5983905834598400&run_id=5118776041537536

It's better if none of the tests run and the error is clear.

@foolip foolip marked this pull request as ready for review February 7, 2025 20:11
@foolip foolip requested a review from lilles February 7, 2025 20:11
@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

@foolip
Copy link
Member Author

foolip commented Feb 7, 2025

@lilles what do you think of this refactoring? You can see the results here:

https://wpt.fyi/results/css/css-anchor-position/at-position-try-invalidation-shadow-dom.html?label=pr_head&max-count=1&pr=50552

It makes the results a lot less readable at a glance, since there's just one subtest with a unhelpful name.

The key thing I want to achieve is no harness error in Firefox and Safari, which you can see here:
https://wpt.fyi/results/css/css-anchor-position/at-position-try-invalidation-shadow-dom.html?run_id=5200154028408832&run_id=5067916582322176&run_id=5983905834598400&run_id=5118776041537536

I think it would also be good if there aren't any passing subtests if @position-try isn't supported at all, although that's just a side benefit.

There are other ways of doing it, perhaps most obviously by putting the insertRule call inside the 3rd subtest. That would leave two passing subtests, and to avoid that we'd have to sprinkle in some assert_implements in the first two subtests...

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

Successfully merging this pull request may close these issues.

2 participants