Multi-block selection and rich text test: wait for expected UI to appear #47173
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
jsnajdr
added
[Feature] Rich Text
|
Size Change: 0 B Total Size: 1.33 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 4c2ce38. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3928739658
|
|
The first screenshot is weird, I guess it's better to avoid the "no results" message if search is in progress. |
youknowriad
approved these changes
Jan 16, 2023
The "no results" message is not there, there's not even any search going on. The inserter popover looks initially like this:
It's all caused by this const currentShownBlockTypes = useAsyncList( filteredBlockTypes );
return (
<div>
{ filteredBlockTypes.length === 0 & <InserterNoResults /> }
{ currentShownBlockTypes.map( renderType ) }
</div>
);
That's why Tab + Enter keyboard events will click on the "Browse all" button. That's wrong, the test doesn't want to go there at all. |
|
Fun fact: This issue came up in my Playwright migration proposal PR, where running |
|
@WunderBart There are other rich-text/multi-selection tests that are failing intermittently, namely:
These tests select two blocks, with Shift+ArrowDown or Shift+ArrowUp, and them move them up or down. When the test fails, it's like if only the first block was selected and moved. In other words, the move is done before the selection is updated after Shift+Arrow. A possible solution is to wait for the selection to finish, i.e., for the You can implement this fix if you want. I debugged the test a bit yesterday, but didn't get to actually do and try out the fix. |
Sounds good. I'll take a look shortly! 👍 |
|
PR for improving the multiple selection stability: #47423 |
juanmaguitar
added
the
[Type] Code Quality
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.
Fixes some flaky e2e tests when testing multi-block selection and rich-text formatting. What all these tests have in common is that they do a lot of keyboard events in quick succession, like: type
/columnsinto block inserter, then tab, tab, arrow right, enter...But the expected UI elements we're navigating to and focusing don't always appear so fast. There is for example
useAsyncListin block inserter search results, which initially renders and empty list and starts populating it only afterrequestIdleCallback. Popovers also take some time to appear and focus. Etc.Some examples of actual failures. Here the test did Tab and Enter, expecting to select a paragraph block in block inserter popover:
But what really happened is that the paragraph block item was not there yet, and the Tab tabbed to the "Browse all" button, clicked on it, and then, instead of entering paragraph content, typed into sidebar inserter search field.
Another test was supposed to tab (four times) into the color popover and select a specific color there:
But when the tabbing was being done, the popover wasn't there yet, so instead the editor keyboard-navigated to the right sidebar and selected the "Styles" button there (notice the focus ring around it).
I'm fixing this by adding some
waitForcalls that wait for expected elements to appear.These flaky tests fail much more often when concurrent mode is enabled, so I was forced to fix them while working on #46467.