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 Classic block tests to Playwright #46689

Merged
merged 3 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

145 changes: 0 additions & 145 deletions packages/e2e-tests/specs/editor/blocks/classic.test.js

This file was deleted.

176 changes: 176 additions & 0 deletions test/e2e/specs/editor/blocks/classic.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/**
* External dependencies
*/
const path = require( 'path' );
const fs = require( 'fs/promises' );
const os = require( 'os' );
const { v4: uuid } = require( 'uuid' );

/**
* WordPress dependencies
*/
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );

test.use( {
mediaUtils: async ( { page }, use ) => {
await use( new MediaUtils( { page } ) );
},
} );

test.describe( 'Classic', () => {
test.beforeEach( async ( { admin } ) => {
await admin.createNewPost();
} );

test.afterAll( async ( { requestUtils } ) => {
await requestUtils.deleteAllMedia();
} );

test( 'should be inserted', async ( { editor, page, pageUtils } ) => {
await editor.insertBlock( { name: 'core/freeform' } );
// Ensure there is focus.
await page.click( '.mce-content-body' );
await page.keyboard.type( 'test' );
// Move focus away.
await pageUtils.pressKeyWithModifier( 'shift', 'Tab' );

await expect.poll( editor.getEditedPostContent ).toBe( 'test' );
} );

test( 'should insert media, convert to blocks, and undo in one step', async ( {
editor,
mediaUtils,
page,
pageUtils,
} ) => {
await editor.insertBlock( { name: 'core/freeform' } );
// Ensure there is focus.
await page.click( '.mce-content-body' );
await page.keyboard.type( 'test' );

await page.getByRole( 'button', { name: /Add Media/i } ).click();
Copy link
Member

@WunderBart WunderBart Jan 27, 2023

Choose a reason for hiding this comment

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

Note
Just a heads up that the name matching will be case-insensitive and search for a substring by default in Playwright >=1.28. For exact matching, the exact param will need to be used.

See https://playwright.dev/docs/api/class-page#page-get-by-role-option-name


const modalGalleryTab = page.getByRole( 'tab', {
name: 'Create gallery',
} );

await expect( modalGalleryTab ).toBeVisible();
await modalGalleryTab.click();

const filename = await mediaUtils.upload(
page.locator( '.media-modal .moxie-shim input[type=file]' )
);

// Wait for upload
await expect(
page.locator( `role=checkbox[name="${ filename }"i]` )
).toBeChecked();

const createGallery = page.getByRole( 'button', {
name: 'Create a new gallery',
} );
await expect( createGallery ).not.toBeDisabled();
await createGallery.click();
await page.click( 'role=button[name="Insert gallery"i]' );
Copy link
Member

Choose a reason for hiding this comment

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

Can we also use the getByRole method here?

Suggested change
await page.click( 'role=button[name="Insert gallery"i]' );
await page.getByRole( 'button' , { name: /Insert gallery/i } ).click();

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a matter of preference. I usually find page.click() simpler in similar cases.

We had a similar discussion here #45193 (comment).

Copy link
Member

@WunderBart WunderBart Jan 27, 2023

Choose a reason for hiding this comment

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

Thanks for the context! I can see how the page.click() can be more straightforward in some cases. However, a good argument for discontinuing it is that those text role selectors are not in the documentation anymore, so it can be confusing for folks that don't have a lot of experience with Playwright-specific selectors. The getBy* locators are the officially recommended ones now.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I just found out the other day that the role selector is not in the doc anymore. It'd be nice if we recommend using getByRole in our doc from now on too. I think we can also migrate all existing usages in one go during a playwright upgrade if that's feasible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's migrate all existing usages in one go during the next Playwright update.


await pageUtils.pressKeyWithModifier( 'shift', 'Tab' );
await expect
.poll( editor.getEditedPostContent )
.toMatch( /\[gallery ids=\"\d+\"\]/ );

await editor.clickBlockToolbarButton( 'Convert to blocks' );
const galleryBlock = page.locator(
'role=document[name="Block: Gallery"i]'
);
Comment on lines +82 to +84
Copy link
Member

Choose a reason for hiding this comment

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

Can we use getByRole here as well?

await expect( galleryBlock ).toBeVisible();

// Focus on the editor so that keyboard shortcuts work.
// See: https://github.com/WordPress/gutenberg/issues/46844
await galleryBlock.focus();

// Check that you can undo back to a Classic block gallery in one step.
await pageUtils.pressKeyWithModifier( 'primary', 'z' );
await expect(
page.locator( 'role=document[name="Block: Classic"i]' )
Copy link
Member

Choose a reason for hiding this comment

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

Also getByRole here, and a couple more below! 😅

).toBeVisible();
await expect
.poll( editor.getEditedPostContent )
.toMatch( /\[gallery ids=\"\d+\"\]/ );

await editor.clickBlockToolbarButton( 'Convert to blocks' );
await expect
.poll( editor.getEditedPostContent )
.toMatch( /<!-- wp:gallery/ );
} );

test( 'Should not fail after save/reload', async ( {
editor,
page,
pageUtils,
} ) => {
// Based on docs routing diables caching.
// See: https://playwright.dev/docs/api/class-page#page-route
await page.route( '**', ( route ) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

The playwright doesn't have page.setCacheEnabled; this is the best replacement I could find for now.

Copy link
Member

Choose a reason for hiding this comment

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

Pretty cool trick!

route.continue();
} );

await editor.insertBlock( { name: 'core/freeform' } );
// Ensure there is focus.
await page.click( '.mce-content-body' );
await page.keyboard.type( 'test' );
// Move focus away.
await pageUtils.pressKeyWithModifier( 'shift', 'Tab' );

await page.click( 'role=button[name="Save draft"i]' );

await expect(
page.locator( 'role=button[name="Saved"i]' )
).toBeDisabled();

await page.reload();
await page.unroute( '**' );

const errors = [];
page.on( 'pageerror', ( exception ) => {
errors.push( exception );
} );

const classicBlock = page.locator(
'role=document[name="Block: Classic"i]'
);

await expect( classicBlock ).toBeVisible();
await classicBlock.click();

expect( errors.length ).toBe( 0 );
await expect.poll( editor.getEditedPostContent ).toBe( 'test' );
} );
} );

class MediaUtils {
constructor( { page } ) {
this.page = page;

this.TEST_IMAGE_FILE_PATH = path.join(
__dirname,
'..',
'..',
'..',
'assets',
'10x10_e2e_test_image_z9T8jK.png'
);
}

async upload( inputElement ) {
Copy link
Member Author

@Mamaduka Mamaduka Dec 28, 2022

Choose a reason for hiding this comment

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

@kevin940726, now we have three places where we use a similar util method. Would it make sense to extract this into global media utility?

Copy link
Member

Choose a reason for hiding this comment

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

Sure! Makes sense to me 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I will follow-up with this.

const tmpDirectory = await fs.mkdtemp(
path.join( os.tmpdir(), 'gutenberg-test-image-' )
);
const filename = uuid();
const tmpFileName = path.join( tmpDirectory, filename + '.png' );
await fs.copyFile( this.TEST_IMAGE_FILE_PATH, tmpFileName );

await inputElement.setInputFiles( tmpFileName );

return filename;
}
}