-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Preview tests: Wait for the post-status change #43874
Conversation
Size Change: +1.95 kB (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
Another difference I noticed in a successful test run trace, the "View Options" dropdown is closed. Probably closing on focus-outside doesn't work in every case. I'm considering adding logic to close it manually. |
b3cd1f6
to
3583885
Compare
@kevin940726, should we give this solution a try? |
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.
Thanks for looking into this! I think we can give this a try!
); | ||
|
||
// Close "View" dropdown if open. | ||
if ( await previewToggle.isVisible() ) { |
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 think the previewToggle
will always be visible here unless we add [include-hidden]
to the locator selector?
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.
Maybe we should assert expanded status. We only want to close this if it's expanded.
Edit: The "View" button doesn't have the aria-hidden
attribute set.
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 think waiting for the status change might be enough in this case. Let's remove this visible check and merge this PR to test it out?
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.
Sounds good. Updated in ea84e04.
Co-authored-by: Kai Hao <kevin830726@gmail.com>
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.
Awesome! Thanks!
What?
PR tries to fix #37976.
I'm having trouble reproducing failure locally, so this is a bit of a wild guess based on the traces I examined. A few examples I checked showed that post-status switching is still in flight when test types
Draft
.It might be safer to wait for the actual status change before running the last part of the tests.
An example trace could be found here - https://github.com/WordPress/gutenberg/actions/runs/2969205018.
Use
npx playwright show-trace <path-to-trace-zip>
for viewing them.Testing Instructions
Screenshots or screencast