Skip to content

Conversation

@opr
Copy link
Contributor

@opr opr commented Jun 8, 2023

What?

Adds an openPostInEditor function to the Admin class. This function will search for (by title), then open the specified page in the editor.

Why?

When testing, people may set up their test env with pre-published pages containing blocks they wish to test. The tests may need to open these pages to update block-specific settings to test different code paths.

It is not guaranteed that the IDs of the specific pages will be the same across runs, so it makes sense to be able to open the page by name, like a user would.

How?

It opens the admin page for the specified post type and searches for the specified post name. When the search is complete, it finds the link to edit the post and then clicks it. When the network is idle the function returns.

Testing Instructions

Set up wp-env.

Add a new test file, test/e2e/specs/editor/various/open-in-editor.spec.js and add the following code:

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

test.describe( 'openInEditor', () => {
	test( 'should open the page', async ( { admin, editor } ) => {
		await admin.createNewPost( {
			postType: 'post',
			title: 'My test post',
			content: '',
			excerpt: '',
		} );
		await editor.publishPost();
		await admin.openPostInEditor( 'My test post' );
		expect( await admin.page.title() ).toContain( 'My test post' );

		await admin.createNewPost( {
			postType: 'page',
			title: 'My test page',
			content: '',
			excerpt: '',
		} );
		await editor.publishPost();
		await admin.openPostInEditor( 'My test page', 'page' );
		expect( await admin.page.title() ).toContain( 'My test page' );
	} );
} );

Run npm run test:e2e:playwright -- open-in-editor and ensure the test passes.

Note: If you run this multiple times it will fail because the pages will be duplicated, so be sure to clear them between runs!

Open a test file, for example:

Testing Instructions for Keyboard

This PR does not affect the UI.

@opr opr added the [Tool] E2E Test Utils /packages/e2e-test-utils label Jun 8, 2023
@opr opr requested a review from kevin940726 as a code owner June 8, 2023 12:02
@opr opr self-assigned this Jun 8, 2023
@kevin940726
Copy link
Member

Thanks for the PR! I have some questions about the motivation:

people may set up their test env with pre-published pages containing blocks they wish to test.

I didn't imagine the utils would be used in this way. That's why we clear all posts/pages before starting the tests to guarantee a consistent testing environment. Under such an assumption, I would imagine the API to be implemented and used as the following instead:

const postContent = await fs.readFile( './my/test/post.html' );
const { id: postId } = await requestUtils.createPost( {
  title: 'My test post',
  content: postContent,
  status: 'publish',
} );
await admin.visitPostById( postId );

This ensures the post we're interacting with is always an isolated post that can't be intercepted by other parallel tests. It also enables full parallel testing in the future for more speed. It's also a lot faster because we don't have to visit the post list page to search for something by its title. The test is also portable since we just need the html file somewhere and don't need the post to exist in the database.

Most of the API are already supported. IIRC, we only need to implement visitPostById, which can also be inlined by replacing it with await admin.visitAdminPage( 'post.php', `post=${postId}&action=edit` ).

WDYT of this approach?

@opr
Copy link
Contributor Author

opr commented Jul 11, 2023

hi @kevin940726 thanks for the review and your patience while I responded!

I would imagine the API to be implemented and used as the following instead:

const postContent = await fs.readFile( './my/test/post.html' );
const { id: postId } = await requestUtils.createPost( {
  title: 'My test post',
  content: postContent,
  status: 'publish',
} );
await admin.visitPostById( postId );

ah this is a great idea, I see what you mean, I agree with all the reasons you mention about parallel testing too, thanks for the insight.

Most of the API are already supported. IIRC, we only need to implement visitPostById, which can also be inlined by replacing it with await admin.visitAdminPage( 'post.php', post=${postId}&action=edit ).

Would it make sense to create visitPostById in that case if it can easily be inlined? I wouldn't mind closing this PR as I don't feel visitPostById adds much value.

Please let me know if closing is your preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Tool] E2E Test Utils /packages/e2e-test-utils

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants