Migrate Navigable toolbar test to Playwright #50900
Conversation
pooja-muchandikar
requested review from
kevin940726,
ntwb,
nerrad and
ajitbohra
as code owners
juanfra
added
the
[Type] Automated Testing
|
Hi @kevin940726, I hope you are going great!.. Could you please help me in reviewing this PR? Thanks!! |
|
Hi @pooja-muchandikar! Just want to note that this is still on my backlog, and I'm just a little busy with something else recently. I'll get back to it eventually! 🙇 |
| test.describe( 'Unified Toolbar', () => { | ||
| test( 'navigates into the toolbar by keyboard (Alt+F10)', async ( { | ||
| page, | ||
| pageUtils, | ||
| } ) => { |
There was a problem hiding this comment.
This whole test has coverage in Playwright already, so we can remove this Unified Toolbar section.
| pageUtils, | ||
| navigableToolbar, | ||
| } ) => { | ||
| const wp = ''; |
There was a problem hiding this comment.
Was this related to a wp is undefined kind of linting error? If so, I think it's that the places that use wp need to be window.wp and you can remove this const wp line.
| } ); | ||
| } ); | ||
| } ); | ||
|
|
||
| class navigableToolbar { |
There was a problem hiding this comment.
Classes in the other e2e tests capitalize the class names, so that's best to use that format here too for consistency: NavigableToolbar.
There was a problem hiding this comment.
I think we can drop the whole utility class here and replace isInBlockToolbar with this assertion, as it's more descriptive.
await expect(
page
.getByRole( 'toolbar', { name: 'Block Tools' } )
.getByRole( 'button', { name: 'Paragraph' } )
).toBeFocused();
| expect( scrollTopBefore ).toBe( scrollTopAfter ); | ||
| } ); | ||
|
|
||
| test( 'navigates into the toolbar by keyboard (Alt+F10)', async ( { |
There was a problem hiding this comment.
@jeryj mentioned that this is already covered in another test. We can remove it and leave only should not scroll page.
P.S. There's also a merge conflict that needs to be resolved.
There was a problem hiding this comment.
Yes I am working on that, but unfortunately these conflicts are not visible in my local so I am unable to fix it.. 😕
There was a problem hiding this comment.
I see conflicts when I try to rebase this branch on top of the trunk, but I can't push on this branch.
Creating a new PR with all the feedback applied might be easier.
There was a problem hiding this comment.
Sure I will do that...
There was a problem hiding this comment.
When the rebase is sorted (or a new PR), feel free to ping me for a review!
|
Closing this PR as the conflicts are not visible in local because of which unable to fix it.. Created new PR with the feedbacks addressed here - #51514 |
What?
Part of #38851.
Migrate navigable-toolbar.test.js to its Playwright version.
Why?
Part of #38851.
How?
See MIGRATION.md for migration steps.
Testing Instructions
Run
npm run test:e2e:playwright test/e2e/specs/editor/various/navigable-toolbar.spec.js