Navigation Menu

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

move WebCryptoAPI to any.js #29276

Merged
merged 9 commits into from Jun 11, 2021
Merged

Conversation

lucacasonato
Copy link
Member

@lucacasonato lucacasonato commented Jun 8, 2021

This PR is an alternative to #27714 that doesn't break the worker tests.

I split the PR into 5 commits, each changing one of the subfolders of the
WebCryptoAPI folder. Other commits are minor fixes.

I did not change any indentation to make reviewing easier.

@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

Comment on lines +270 to +274
promise_test(function() {
return Promise.all(all_promises)
.then(function() {done();})
.catch(function() {done();})
}, "setup");
Copy link
Member Author

Choose a reason for hiding this comment

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

.any.worker.js tests automatically call done(). This is used to prevent the test suite from preemptively exiting before these promises have resolved.

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Didn't fully check everything, but since it looks like there's no changes to the overall number of test passes/failures, it's probably fine. One comment below.

@lucacasonato
Copy link
Member Author

Please hold on merging. Some of these tests look to be flaky now. https://github.com/web-platform-tests/wpt/pull/29276/checks?check_run_id=2801619886. I am concerned it is related to the top level promise_test change. Wan't to investigate this real quick.

@lucacasonato
Copy link
Member Author

Ok investigated, and seems these tests were previously already flaky: https://crbug.com/1045472.

@Ms2ger Ms2ger merged commit 1423625 into web-platform-tests:master Jun 11, 2021
@lucacasonato lucacasonato deleted the crypto_any_js branch June 11, 2021 13:17
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.

None yet

3 participants