-
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
Fix flaky widgets e2e tests by waiting for the snackbar correctly #33317
Conversation
Size Change: 0 B Total Size: 1.07 MB ℹ️ View Unchanged
|
24e84bd
to
be0d41c
Compare
'gutenberg-test-plugin-disables-the-css-animations' | ||
); | ||
// Reduced motion is needed to immediately show and dismiss the snackbars. | ||
await page.emulateMediaFeatures( [ |
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.
We probably want to enable it globally too.
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.
LGTM, though I feel the snapshot thing should be in a separate PR.
I don't mind if you merge it though 😄
packages/e2e-tests/CHANGELOG.md
Outdated
### New Features | ||
|
||
- Bail out tests if a prior snapshot failed. Fixed a bug which failing snapshots won't trigger screenshots [#33317](https://github.com/WordPress/gutenberg/pull/33317). | ||
|
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.
Seems unrelated, should it be in a separate PR?
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 was just being lazy 😅, I can do that in another PR.
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.
Moved it here: #33448
// Close the snackbar. | ||
const savedSnackbar = await find( savedSnackbarQuery ); | ||
await savedSnackbar.click(); | ||
await expect( savedSnackbarQuery ).not.toBeFound(); |
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.
Does it need to be dismissed for the test to work? If so, I think that's ok, but otherwise it should probably be in a separate test rather than something that happens any time widgets are saved.
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.
They should be dismissed, or else there will multiple snackbars on the page, and we won't know when the widgets are saved. Which was the reason it was flaky before 😅
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.
Ah, I see. If only the snackbar disappearing speed was proportional to the speed the e2e tests move. 😄
Could consider adding a comment like 'avoids multiple snackbars being present on the screen'.
6faf1a9
to
5e941fe
Compare
Description
Related to #33275, but probably won't fix that entirely.
I think one reason behind the flaky e2e tests is that we are waiting for the wrong elements when the screen is being saved. We are waiting for elements with text of
Widgets saved.
, expecting it to be the snackbar element. But it turns out that there will always be a a11y element that contains that text.Hence, the waiting resolves immediately, the widgets has not yet been saved. This is not always failing because we wait for another 500ms after saving, which is the root of the flakiness.
The solution is to correctly wait for the snackbar elements, and dismiss them once it's shown. We have to also enable reduced motion mode to disable the snackbar transition. (I think we should enable it for all of our e2e tests though, WDYT?)
How has this been tested?
E2E tests should pass.
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).