Navigation Editor: Fix failing e2e tests #32043
Merged
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
Mamaduka
added
the
[Type] Automated Testing
|
Size Change: +26 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
Mamaduka
changed the title
|
Response mocking is fragile. You have to mock out all response and in a specific order. I have a vague memory of a simialr issue when I wrote the original Nav Block e2e tests. |
@getdave I think I discovered the reason for some of those fragilities. Every time the mock function is called it resets any previously set up mocks. So if for example I first made a call to mock pages and menuItems, but then make a second call that only mocks menuItems, the pages mock is discarded. With that understanding, I haven't had too many issues using them in these tests, they've been pretty stable. In the long run ideally these tests would use real data. The difficult aspect is that menus require a lot of external data (pages, tags etc.) and resetting the test environment to a clean slate every time could be time consuming and hard to maintain. |
|
Since we're discussing mocking responses, how can I mock bad/unsuccessful responses? I want to mock this scenario: gutenberg/packages/edit-navigation/src/store/actions.js Lines 136 to 138 in cc41feb While working on these fixes, I noticed that one "empty name save" test has diverged from actual behavior. |
|
@talldan It looks like now a different e2e test is failing for trunk https://github.com/WordPress/gutenberg/runs/2635970904. Maybe I should change how the "dirty" state is checked in tests - #32033 (comment). Should we skip the problematic test for now? I'm happy to work on fixes again, but it's difficult to reproduce issues locally exactly. cc @youknowriad. |
|
@Mamaduka That test seems to have caught a legitimate problem. I see this failed HTTP request when loading the navigation screen: |
|
Actually seeing a lot of duplicate/similar requests when loading the screen, some with |
|
@talldan I don't see those issues locally, but I don't use Going to try and reproduce the issue with |
|
I believe the response will return 404 if there're no menus, and this will be the case for the "Create a new menu" state. Do you see a 404 response when editing menus? I just want to narrow it down for debugging. |
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
I did minor refactoring and tracked down issues with the "name editing" e2e tests.
Tests were failing primarily in two scenarios:
I can't 100% point my finger on the cause of the last one, but I think it's an issue with response mocking. I could only reproduce this when running tests in headless mode or with only the
--puppeteer-devtoolsflag.Fixes #32033.
How has this been tested?
wp-envpackage is up to date.npm run test-e2e -- packages/e2e-tests/specs/experiments/navigation-editor.test.jsfew times.Types of changes
Bugfix
Checklist:
*.native.jsfiles for terms that need renaming or removal).