[EuiContextMenuPanel] Convert flakey Jest focus/keyboard tests to Cypress #5261
Conversation
There was a problem hiding this comment.
Hey y'all! After some evaluation I decided to split my Jest->Cypress flaky test work into a separate PR from my Jest+Cypress code coverage work. Hopefully this should be easier to follow along.
In case you're curious, I went ahead and did the coverage %s on this change in #5262 - it went up a little from converting to Cypress! 😄
| it('is set on the first focusable element by default if there are no items and hasFocus is true', () => { | ||
| mount( | ||
| <EuiContextMenuPanel> | ||
| <button data-test-subj="button">Hello world</button> |
There was a problem hiding this comment.
I added Hello world text here to make the button focus more obvious in the Cypress browser real-time preview
| cy.wait(100); // Intermittent flake workaround: without this, the first downarrow key does not always focus into the menu items as expected | ||
| }); | ||
|
|
||
| it('focuses the panel by default', () => { |
There was a problem hiding this comment.
FUN FACT: Did you know that if you blindly copy/paste Jest tests into Cypress and don't remove the async from it('focuses the panel by default', async () => {, you will get a bunch of indecipherable warnings and all your tests will pass on group run even when they shouldn't? 🤦♀️ ☠️
Posting this so others hopefully don't make the same mistake as me in the future lol
| .type('{downarrow}') | ||
| .type('{leftarrow}') | ||
| .then(() => { | ||
| expect(showPreviousPanelHandler).to.be.called; |
There was a problem hiding this comment.
NB: expect gets called immediately where as cy. commands are async promises, so we have to put the stub expect in a then chain for the test to pass as expected
| it('up arrow key wraps to last menu item', async () => { | ||
| component.simulate('keydown', { key: keys.ARROW_DOWN }); | ||
| component.simulate('keydown', { key: keys.ARROW_UP }); | ||
|
|
||
| await tick(20); | ||
| expect(findTestSubject(component, 'itemC').getDOMNode()).toBe( | ||
| document.activeElement | ||
| ); | ||
| }); |
There was a problem hiding this comment.
This test felt redundant to me with lines 364-371 so I removed it in favor of just one test 🤷♀️
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5261/ |
|
Wahoo! Thanks Greg! |
Summary
I brought up recently in Slack that I was seeing intermittent failures from
EuiContextMenuPanel's focus/keyboard behavior tests. I saw them again last week on a random PR and got annoyed enough to convert them to Cypress, which should (fingers crossed) have fewer flaking issues with focus behavior.QA
yarn test-cypress-devcontext_menu_panel.spec.tsxNB: I did initially encounter intermittent failures with the
down arrow key focuses the first menu itemandsubsequently, down arrow key focuses the next menu itemtests. Adding acy.wait()of 100 milliseconds seemed to fix those intermittent failures. I reran approx tests 10x locally with no intermittent failures after that, although only the CI gods will know after this if we still get flakes 😅Checklist
No changelog, internal/dev only change