Migrate Splitting Merging test to Playwright #44916
Conversation
pooja-muchandikar
requested review from
ntwb,
nerrad and
ajitbohra
as code owners
Mamaduka
added
the
[Type] Automated Testing
|
Hi @Mamaduka, Could you please help me reviewing this PR? Do let me know if there are any suggestions. Thanks!! |
| expect( | ||
| await page.locator( '.block-editor-block-list__block' ) | ||
| ).toBeDefined(); |
There was a problem hiding this comment.
| expect( | |
| await page.locator( '.block-editor-block-list__block' ) | |
| ).toBeDefined(); | |
| await expect( | |
| page.locator( 'role=document[name=/Empty block/i]' ) | |
| ).toBeVisible(); | |
| await expect( | |
| page.locator( 'role=textbox[name="Add title"i]' ) | |
| ).toBeVisible(); |
Maybe we could write this as a role selector? What do you think?
| const content = await editor.getEditedPostContent(); | ||
| expect( content ).toBe( |
There was a problem hiding this comment.
Suggestion (non-blocking): When padding post content to inline snapshots, you can also use expect.poll to auto-wait for assertion. It can be used in most of the tests in this suite.
await expect.poll( editor.getEditedPostContent ).toBe( '' )
Note: The method does not work with toMatchSnapshot assertions.
There was a problem hiding this comment.
@pooja-muchandikar, sorry for miss understanding. There's no need to convert inline snapshot assertions to toMatchSnapshot.
For small text nodes, I would always recommend inline snapshots.
There was a problem hiding this comment.
Hi @Mamaduka,
I tried the above suggestion but somehow its failing in local, so that's the reason I have added toMatchSnapshot assertion.
Should I revert it?
There was a problem hiding this comment.
That's odd. Thanks for testing it locally. There's no need to revert. I will merge the PR once all checks are green.
Thanks again for the contribution!
There was a problem hiding this comment.
Thank you!
There was a problem hiding this comment.
Thanks for the PR, @pooja-muchandikar.
The migration looks good to me. I just left some minor suggestions.
|
Hi @Mamaduka, Rest other feedbacks are addressed.. |
| await page.keyboard.type( '-' ); | ||
|
|
||
| // Check the content. | ||
| expect( await editor.getEditedPostContent() ).toMatchSnapshot(); |
There was a problem hiding this comment.
This test used to have two expect checks and now it only has one. Why have these tests been modified?
|
@Mamaduka We need to make sure these tests remain the same when migrating them. |
|
@ellatrix, sorry I missed that 🙇 I will create PRs to add missing expectations. |
What?
Part of #38851.
Migrate splitting-merging.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/splitting-merging.spec.js