-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 flaky test by using waitForResponse to ensure the url details request completes #37901
Conversation
…uest completes and 404s
Size Change: 0 B Total Size: 1.13 MB ℹ️ View Unchanged
|
I'll give this a few re-runs, but fingers crossed it works 🤞 |
d0c893f
to
bcc03c8
Compare
af0b3a7
to
881f588
Compare
Spotted some other issues with the test. I'd misunderstood the way the test works, thinking That would explain why the test wasn't working before, it was waiting for the network request in the wrong place! 🤦 Hopefully now it should be working as expected 🤞 |
Passed 5 times, so hopefully that means it's solved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to run with this. Wasn't aware of wait for response. Seems good!
Thanks for trying this alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this and observe tests on the trunk.
@talldan, do you think a similar check can be used to fix this test as well - https://github.com/WordPress/gutenberg/runs/4785599397?check_suite_focus=true. I've seen it fail on the trunk a dozen times in the last couple of weeks. |
Description
Alternative to #37755, Fixes #37479
My previous attempt to make this test more robust used the
waitForNetworkIdle
function to ensure the url-details request completed so that the test can handle the 404 it returns. But that didn't seem to work consistently and the test was reported as flaky.Looking at the docs, I discovered puppeteer has a
waitForResponse
function that should improve the situation.