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

bypass bogus AOM crashtests that assume abandoned syntax will be added. #41576

Merged

Conversation

cookiecrook
Copy link
Contributor

@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!

@@ -1,6 +1,11 @@
<html class="test-wait">
Copy link
Contributor Author

@cookiecrook cookiecrook Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers... The tests pass by removing this "test-wait" classname.

The diffs below wrap a conditional if/else around blocks that only work in Chrome, because it relies on an abandoned API proposal (getComputedAccessibleNode). The larger diff blocks are the same, but indented so every line is different.

This allows WPT to keep the crashtest running for Chrome without penalizing other engines with 4 bogus failures.

Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In broad strokes: I wonder if we should move all the AOM tests to a top-level aom directory, rather than having a miscellaneous accessibility directory—everywhere else we group things by spec, after all. This would also arguably make it easier to ignore tests for the abandoned spec, treating it the same as any other spec (say, WebUSB) which doesn't have cross-vendor support.

accessibility/crashtests/aom-in-destroyed-iframe.html Outdated Show resolved Hide resolved
accessibility/crashtests/aom-in-destroyed-iframe.html Outdated Show resolved Hide resolved
remove references to "chromium only"
@cookiecrook
Copy link
Contributor Author

In broad strokes: I wonder if we should move all the AOM tests to a top-level aom directory, rather than having a miscellaneous accessibility directory—everywhere else we group things by spec, after all. This would also arguably make it easier to ignore tests for the abandoned spec, treating it the same as any other spec (say, WebUSB) which doesn't have cross-vendor support.

That directory is several years old, and I have no issue with moving it... However, it's only crashtests... I added a ReadMe back in the Spring that effectively says... "Don't put new tests here unless they fit nowhere else."

cc @mfreed7 and @aleventhal since they have committed to the dir recently.

@cookiecrook
Copy link
Contributor Author

@gsnedders all the new accessibility tests are in spec-relevant directories:

@cookiecrook cookiecrook changed the title bypass bogus AOM crashtests that assume abaondoned API will be added. bypass bogus AOM crashtests that assume abandoned API will be added. Aug 22, 2023
@cookiecrook cookiecrook changed the title bypass bogus AOM crashtests that assume abandoned API will be added. bypass bogus AOM crashtests that assume abandoned syntax will be added. Aug 22, 2023
@mfreed7
Copy link
Contributor

mfreed7 commented Aug 22, 2023

cc @mfreed7 and @aleventhal since they have committed to the dir recently.

I'll definitely defer to @aleventhal - I've only committed here once (happened to be a week ago) but that was just to correct an existing test.

@aleventhal
Copy link
Contributor

LGTM on general approach -- I didn't review line by line.

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine to me!

@cookiecrook cookiecrook merged commit b685181 into web-platform-tests:master Aug 23, 2023
19 checks passed
@cookiecrook cookiecrook deleted the fix-bogus-aom-tests branch November 14, 2023 08:33
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.

bypass abandoned syntax in four AOM crashtests
6 participants