Navigation Editor e2e tests: migrate from response mocking to rest api util #34869
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
talldan
added
[Type] Automated Testing
|
Size Change: +83 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
talldan
force-pushed
the
remove/navigation-editor-e2e-test-endpoint-mocking
branch
from
ee5986f
to
2ebccfa
Compare
|
The "Menu name editor" tests fail for me when testing locally—investigating the reason of the failure. |
Mamaduka
reviewed
Sep 17, 2021
Comment on lines
436
to
437
| const snackbar = await page.waitForSelector( | ||
| '.components-snackbar' |
There was a problem hiding this comment.
This is the selector that causes failure locally for me.
How reliable is this method for checking if the Menu is saved? I remember Widgets had flaky tests because of the Snackbars, but this might not be related.
There was a problem hiding this comment.
Yep, unusual, I'm not sure why since it passes with the --puppeteer-interactive flag.
Anyhow, I changed it to this
const snackbar = await page.waitForSelector(
'.components-snackbar',
{ visible: true }
);and it makes it work again.
I'm aware of some other bugs in puppeteer related to waitForSelector, and using visible: true forces puppeteer to use a different waiting strategy (MutationObserver is the default, visible changes the strategy to polling), so it must be something related to that.
There was a problem hiding this comment.
and using visible: true forces puppeteer to use a different waiting strategy
I wasn't aware of this. Thanks for the information.
|
Looks good on the surface, happy to give it a deeper look on Monday |
|
Hi, @talldan Unfortunately, I'm still getting the same error when running locally. But it might be the issue on my end, so let's wait for others to run tests and approve. I think we should also rebase updates tests on the latest trunk. The screen is in active development means that things can diverge much faster. We accidentally introduced failing e2e tests because of this last week #34874. |
talldan
force-pushed
the
remove/navigation-editor-e2e-test-endpoint-mocking
branch
from
08c7b4c
to
2d3932b
Compare
I just managed to re-create it too (it passed every time before, so was pretty sure I'd fixed it 😢 ). Here's the screenshot artifact for the failed test, clearly a snackbar visible and the class name is present in the HTML artifact: sidenote: The editor could probably show 'Untitled' in the header. So I'm thinking this could just be skipped for now. I don't have any good ideas for fixing it because it doesn't seem like it should be broken 🤔 . |
|
I remember that @kevin940726 worked on a similar fix for the Widgets Screen e2e tests. Maybe he has some suggestions. |
|
I couldn't reproduce that failure locally. If it only happens sometimes, that sounds related to timing. If you can reproduce it every time, it could maybe be related to a WordPress version or a specific message logged by Chrome. E.g. maybe we could let go of matching the exact error string somehow? Other than that, this PR looks good and I'm okay with skipping the test for now as long as it doesn't stay disabled. |
adamziel
approved these changes
Sep 21, 2021
talldan
deleted the
remove/navigation-editor-e2e-test-endpoint-mocking
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #34765 (near enough)
This was a lot more difficult than I anticipated. The tough thing with menu items is that they usually link to other resources (pages, posts, categories), and those need to be created before the menu items so that we have valid ids and urls for the menu items. Otherwise the menu items are considered invalid.
Menu items also require two batch requests, once to create the items initially, and then once we have the ids a second request to set the correct
parents using those ids.The last thing is that now we're using the REST API, the ids and urls of items are unstable, they change between test runs, so can no longer be used in snapshots. I wrote a function to go through block attributes and ensure the value of those attributes is not stored in the snapshot (the type of the value is used instead).
What I haven't done (in the interested of not making this PR huge):