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

'unhandledrejection' in payment-request/updateWith-method-pmi-handling-manual.https.html #15861

Closed
danyao opened this issue Mar 15, 2019 · 7 comments
Assignees

Comments

@danyao
Copy link
Contributor

danyao commented Mar 15, 2019

The individual test cases in payment-request/updateWith-method-pmi-handling-manual.https.html seem to pass all test assertions, but still end up with a test harness error due to an unhandledrejection event firing.

I can't figure out where the unhandled rejection comes from. All of these tests call PaymentRequestUpdateEvent.updateWith(invalidDetails), where invalidDetails contains an invalid payment method identifier. This causes the update to be aborted with a RangeError and rejects request.[[showPromise]], both are expected according to the spec. The test verifies this rejection with await promise_rejects(t, new RangeError(), showPromise);. What's strange is that the test report shows a pass for the test but with a harness error:

Harness status: Error

Failed to construct 'PaymentDetailsUpdate': Invalid payment method identifier format
undefined:undefined:undefined

It's also weird that the last test case on the page doesn't exhibit this behavior. The only difference between it and the others is that it calls done() right after the test method (here). I wonder if this is a race condition between done() and the unhandledrejection event handler triggering.

Because of this problem, the test page has to be reloaded after each test case. I'd like to fix it, but need help figuring out where the unhandled rejection comes from.

@marcoscaceres any ideas?

@danyao danyao self-assigned this Mar 15, 2019
@marcoscaceres
Copy link
Contributor

I wonder if this is a race condition between done() and the unhandledrejection event handler triggering.

Yes, that's definitely a bug... but not the bug.

Looking into it.

@marcoscaceres
Copy link
Contributor

This is really odd.... it's also as if Chrome is throwing an exception on updateWith(), but it's not actually throwing one. Testing in Safari, the test seems to work ok (they don't actually support the RangeError, but aborting works as expected).

Firefox is sadly broken still so can't even test the first part.

Sending a patch... but it doesn't seem to help. But at least the test runs in Safari.

@marcoscaceres
Copy link
Contributor

Ok, sent #15891 ... at least it fixes the done() one.

@marcoscaceres
Copy link
Contributor

If you figure it out, I’d be super interested to hear what is wrong. It’s likely the test, but I’m also at a loss.

@danyao danyao pinned this issue Mar 18, 2019
@gsnedders gsnedders unpinned this issue Mar 18, 2019
@gsnedders
Copy link
Member

@danyao pinning seems to be some global thing, which isn't that useful for a single test bug :)

@danyao
Copy link
Contributor Author

danyao commented Mar 19, 2019

Oops sorry I didn't realize this. Thanks @gsnedders!

@marcoscaceres
Copy link
Contributor

Closing, as this test no longer exists.

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

No branches or pull requests

3 participants