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

Remove deadlock from payment-is-showing test #17262

Merged
merged 4 commits into from Jun 11, 2019

Conversation

danyao
Copy link
Contributor

@danyao danyao commented Jun 11, 2019

payment-is-show.https.html tests that if multiple contexts try to call PaymentRequest.show(), all but the first one rejects.

Before this patch, each PaymentRequest.show() is wrapped inside its own test_driver.bless(). This creates a deadlock when running the tests directly in the browser: the first payment sheet prevents the user from interacting with the test page to click on the button generated by test_driver.bless(), which in turn blocks the test from proceeding, which will eventually abort the payment sheet. This deadlock breaks the web test runner.

This patch combines all PaymentRequset.show() calls of a single test case inside a single test_driver.bless() call. This way, each test only requires user to click on a single block.

The only exceptions are the two tests that trigger popups and call .show() from the main test page. The test requires a user gesture to bring the main page back into foreground. User has to click on two buttons to make these tests proceed.

Copy link
Contributor

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. As an aside, we should get agreement on them quotes and some way of enforcing that to avoid unnecessary changes.

@danyao
Copy link
Contributor Author

danyao commented Jun 11, 2019

Thanks @marcoscaceres for the review! I used https://prettier.io which you recommended in an earlier thread to format the code. It replaced the quotes. What do you prefer? I'll figure out how to configure prettier default to match.

@danyao danyao merged commit 1bdd1b1 into web-platform-tests:master Jun 11, 2019
@danyao danyao deleted the danyao-payment-is-showing branch June 11, 2019 14:09
@marcoscaceres
Copy link
Contributor

I’m a double-quotes semicolon kinda person.

@danyao
Copy link
Contributor Author

danyao commented Jun 11, 2019

SG. I'll follow that convention in the future, and see if there's a way to get a style enforcer added to the presubmit.

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