-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Block-editor-keyboard-shortcut test case #50930
Add Block-editor-keyboard-shortcut test case #50930
Conversation
Hey there! I wanted to express my gratitude for the PR you submitted. By the way, have you had a chance to check out the latest best practices guide? It's packed with some really helpful tips and tricks that can make a big difference in how we write reliable and readable e2e tests. One key suggestion is to make good use of the |
@kevin940726 Can you please recheck that for me? |
Hi @alvitazwar! I've been busy with something else recently, but this is still on my backlog! I'll get back to it as soon as possible 🙇 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR removes the specs/editor/various/autosave.test.js
file instead of the block-editor-keyboard-shortcuts.test.js
.
Additional notes:
- I'm not sure if
await expect.poll( editor.getEditedPostContent ).toBe( MatchObject );
assertions check anything related to keyboard navigation. - Let's combine two
test.describe
into one. The grouping usually makes it harder to know where the new tests go. I think it's better to split tests into multiple files if there's a need for a separate test group.
await page.keyboard.type( '3rd' ); | ||
await expect.poll( editor.getEditedPostContent ).toBe( MatchObject ); | ||
await page.keyboard.press( 'ArrowUp' ); | ||
page.locator( '[aria-label="Multiple selected blocks"]' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call the just the locator doesn't do anything. This should be converted into a proper assertion.
What?
Based on #38570, part of #38851. Migrate block-editor-keyboard-shortcuts.test.js
Why?
See #38570 for its background and rationale.
This is split into a new PR for easier review.
How?
See #38570 for the proposed migration steps.
Testing Instructions
npm run test:e2e:playwright /test/e2e/specs/editor/various/block-editor-keyboard-shortcuts.spec.js
Screenshots or screencast
Screen.Recording.2023-05-25.at.12.21.20.AM.mov