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

Update rejects_if_not_active.https.html #17920

Merged
merged 2 commits into from
Jul 19, 2019

Conversation

danyao
Copy link
Contributor

@danyao danyao commented Jul 18, 2019

Update this test to match the new proposed spec behavior in w3c/payment-request#872.

In Blink, after the iframe navigates, the original show promise will never settle. This causes the test to timeout. What we really want to check is that the original payment sheet is dismissed and a new payment sheet can be shown. This is what the test is updated to check.

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.

Worried about intermittent failures because of the timeout, but let’s see how we go.

@danyao
Copy link
Contributor Author

danyao commented Jul 19, 2019

Worried about intermittent failures because of the timeout, but let’s see how we go.

My understanding of event loop is not strong... My calculation was that the rejection should happen immediately on request2.show() if a payment sheet is already showing so any amount of timeout should be sufficient.

@danyao danyao merged commit ba57a8d into web-platform-tests:master Jul 19, 2019
@danyao danyao deleted the danyao-reject-on-inactive branch July 19, 2019 20:01
@marcoscaceres
Copy link
Contributor

The problem is that it makes assumptions about the test environment, which can be really slow sometimes. This is because, for example, we are testing against unoptimized builds of Firefox on random hardware, so there is little guarantee that anything can happen within 1 second (leading to intermittent failures).

Anyway, it may be ok because as you point out it should reject immediately 🤞if it fails randomly, we can always adjust it or come up with something different.

natechapin pushed a commit to natechapin/wpt that referenced this pull request Aug 23, 2019
Use a second payment sheet to detect that the first payment sheet is dismissed as a result of navigation instead of relying on the rejection of the first promise, which may never settle in some implementations (i.e. Blink).
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.

4 participants