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

Fix timeout vs. task-queue race conditions in promise rejection tests #25698

Merged
merged 1 commit into from Feb 10, 2020

Conversation

@pshaughn
Copy link
Member

pshaughn commented Feb 6, 2020

These two tests were assuming that timeouts and promise rejection events would have a temporal relationship not actually guaranteed by spec, leading to test timeouts if the DOM manipulation task queue was running slower than anticipated.

fix #22207, fix #22295

Of course, the upstream WPT results need looking at to be sure this hasn't done something weird to another browser.

@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 6, 2020

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21618.

@pshaughn pshaughn force-pushed the pshaughn:rejectiontests branch from a152391 to 7684afb Feb 6, 2020
@servo-wpt-sync
Copy link
Collaborator

servo-wpt-sync commented Feb 6, 2020

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21618.

@jdm
Copy link
Member

jdm commented Feb 10, 2020

Great catch!
@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Feb 10, 2020

📌 Commit 7684afb has been approved by jdm

@highfive highfive assigned jdm and unassigned asajeffrey Feb 10, 2020
@bors-servo
Copy link
Contributor

bors-servo commented Feb 10, 2020

Testing commit 7684afb with merge 143b06f...

bors-servo added a commit that referenced this pull request Feb 10, 2020
Fix timeout vs. task-queue race conditions in promise rejection tests

<!-- Please describe your changes on the following line: -->
These two tests were assuming that timeouts and promise rejection events would have a temporal relationship not actually guaranteed by spec, leading to test timeouts if the DOM manipulation task queue was running slower than anticipated.

fix #22207, fix #22295

Of course, the upstream WPT results need looking at to be sure this hasn't done something weird to another browser.
@bors-servo
Copy link
Contributor

bors-servo commented Feb 10, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Feb 10, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Feb 10, 2020

Testing commit 7684afb with merge 5bcd0d8...

bors-servo added a commit that referenced this pull request Feb 10, 2020
Fix timeout vs. task-queue race conditions in promise rejection tests

<!-- Please describe your changes on the following line: -->
These two tests were assuming that timeouts and promise rejection events would have a temporal relationship not actually guaranteed by spec, leading to test timeouts if the DOM manipulation task queue was running slower than anticipated.

fix #22207, fix #22295

Of course, the upstream WPT results need looking at to be sure this hasn't done something weird to another browser.
@bors-servo
Copy link
Contributor

bors-servo commented Feb 10, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 5bcd0d8 to master...

@bors-servo bors-servo merged commit 7684afb into servo:master Feb 10, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.