Fix flaky Navigation block e2e test by mocking out URL details endpoint to avoid 404 #37501
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
getdave
added
[Type] Bug
|
Size Change: 0 B Total Size: 1.13 MB ℹ️ View Unchanged
|
7 tasks
ajlende
approved these changes
Dec 17, 2021
There was a problem hiding this comment.
Seems like a better workaround than page.waitForNetworkIdle(), so +1 for the immediate fix.
I haven't been very involved in the navigation editor, so I'm probably missing something. But is there an issue/pr for addressing this in the NavEditor component or fetchUrlData function so the workaround isn't needed here?
A good question but I believe this isn't a flaw of the component. The request will 404 however it's handled by This is because the request is made to a post which is a draft. The fetchUrlData function needs to be unaware as to whether the requested resource is an external or internal URL. I did a lot of experimenting with this when I created the "rich data" feature so I'm confident mocking the endpoint is the best route in this regard. Thanks for merging the fix 🙇 |
11 tasks
|
Hi, folks I don't think this PR fully resolved this issue. I have seen the same tests failures a lot lately. |
|
Yeh looks like only partially. I was trying all day to debug this as it would only fail on CI for me. |
|
@getdave, I also had no luck debugging it locally. |
12 tasks
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
Since we re-enabled the Navigation block e2e tests the CI tests have been flaky.
This PR fixes #37479 by mocking out the
url-detailsendpoint to avoid having to handle the 404 using less resilient methods.Note this commit was cherry picked from #37454 which also contains the fix.
How has this been tested?
Run tests check they pass on CI.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).