Skip to content
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

Migrate Block Switcher Test case to Playwright #50845

Closed

Conversation

pavanpatil1
Copy link
Contributor

What?

Part of #38851.
Migrate block-switcher.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-switcher.spec.js

@pavanpatil1 pavanpatil1 changed the title Migreate Block Switcher Test case to Playwright Migrate Block Switcher Test case to Playwright May 22, 2023
@juanfra juanfra added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label May 24, 2023
@kevin940726
Copy link
Member

Hi @pavanpatil1! This is still on my backlog but I'm currently busy with something else. I'll get back to this as soon as possible! Thanks for the PR!

Copy link
Member

@kevin940726 kevin940726 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Sorry for the late review! This is close though! 💯

await pageUtils.pressKeys( 'alt+F10' );

// Verify the block switcher exists.
expect( page.locator( 'role=button[name="List"i]' ) ).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A locator is always truthy. Let's try toBeVisible() instead? We can also scope it under the block toolbar.

Suggested change
expect( page.locator( 'role=button[name="List"i]' ) ).toBeTruthy();
const blockToolbar = page.getByRole( 'toolbar', { name: 'Block tools' } );
await expect( blockToolbar.getByRole( 'button', { name: 'List' } ) ) ).toBeVisible();


// Remove the quote block from the list of registered blocks.
await page.evaluate( () => {
wp.blocks.unregisterBlockType( 'core/quote' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do window.wp here so we don't need the wp const above.

await pageUtils.pressKeys( 'alt+F10' );

// Verify the block switcher exists.
expect( page.locator( 'role=button[name="List"i]' ) ).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

pageUtils,
} ) => {
const wp = '';
async function getAvailableBlockTransforms() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the function from the utils below?

await pageUtils.pressKeys( 'alt+F10' );

// Remove the paragraph and quote block from the list of registered blocks.
await page.evaluate( () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is executed before the key press in the original test. I think we should follow the original behavior :).

// Verify the block switcher exists.
expect(
page.locator(
'.block-editor-block-toolbar .block-editor-block-switcher'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Comment on lines +182 to +190
await this.page.click( 'role=button[name="List"i]' );
return this.page.evaluate( ( buttonSelector ) => {
return Array.from(
document.querySelectorAll( buttonSelector )
).map( ( button ) => {
return button.textContent;
} );
}, '.block-editor-block-switcher__popover .block-editor-block-switcher__transforms__menugroup button' );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can rewrite this into accessible selectors:

await this.getBlockSwitcher( blockName ).click();
return await this.page.getByRole( 'menu', { name: blockName } )
	.getByRole( 'menuitem' )
	.allTextContents();

I left out the implementation of getBlockSwitcher here though. Feel free to ask questions if it's not clear!

@Mamaduka
Copy link
Member

Migrated via #57381.

@Mamaduka Mamaduka closed this Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants