Migrate Block Mover Test For Playwright #42039
Conversation
pooja-muchandikar
requested review from
ntwb,
nerrad and
ajitbohra
as code owners
|
Hi @kevin940726, I hope you are doing well! Could you please help me in reviewing this PR? Thanks!! |
| await page.click( '.block-editor-default-block-appender' ); | ||
| await page.keyboard.type( 'First Paragraph' ); | ||
| await page.keyboard.press( 'Enter' ); | ||
| await page.keyboard.type( 'Second Paragraph' ); |
There was a problem hiding this comment.
I think it would be good to simplify this part with something like:
await editor.insertBlock( { name: 'core/paragraph', attributes: { content: 'First Paragraph' } } );
await editor.insertBlock( { name: 'core/paragraph', attributes: { content: 'Second Paragraph' } } );There was a problem hiding this comment.
Worked on the feedbacks
| const count = await page.$$eval( | ||
| '.block-editor-block-mover', | ||
| ( el ) => el.length | ||
| ); | ||
| expect( count ).toBe( 1 ); |
There was a problem hiding this comment.
It'd be good to take advantage of Playwright's locator system here, and use the role selector:
const moveDownButton = page.locator( 'role=toolbar[name="Block tools"i] >> role=button[name="Move down"i]' );
const moveUpButton = page.locator( 'role=toolbar[name="Block tools"i] >> role=button[name="Move up"i]' );
await expect( moveDownButton ).toBeVisible();
await expect( moveUpButton ).toBeVisible();There was a problem hiding this comment.
Worked on the feedbacks
| await page.click( '.block-editor-default-block-appender' ); | ||
| await page.keyboard.type( 'First Paragraph' ); |
There was a problem hiding this comment.
Similar to the previous comment, this can change to:
await editor.insertBlock( { name: 'core/paragraph', attributes: { content: 'First Paragraph' } );There was a problem hiding this comment.
Worked on the feedbacks
| const count = await page.$$eval( | ||
| '.block-editor-block-mover', | ||
| ( el ) => el.length | ||
| ); | ||
| expect( count ).toBe( 0 ); |
There was a problem hiding this comment.
And this part could also be refactored to be use locators:
const moveDownButton = page.locator( 'role=toolbar[name="Block tools"i] >> role=button[name="Move down"i]' );
const moveUpButton = page.locator( 'role=toolbar[name="Block tools"i] >> role=button[name="Move up"i]' );
await expect( moveDownButton ).toBeHidden();
await expect( moveUpButton ).toBeHidden();There was a problem hiding this comment.
Worked on the feedbacks.
|
Hi @talldan, I have addressed all the feedbacks, could you please review again? Also I noticed that CI is failing but not related to the test case I worked on. I am little confused here 🤔
|
|
Thanks for making those changes!
Some of our old tests are not very reliable, which is one of the reasons for migrating them to Playwright. I've restarted them to see if they pass. |
|
Hi @talldan, good morning.. All the CI checks have passed except the End to End Tests / Admin -3 🤔
|
|
Did a another retry and managed to get it passing 👍 |
talldan
added
[Type] Task
|
Thank you :) |
What?
Part of #38851.
Migrate block-mover.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/block-mover.spec.js