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

E2E Utils: add setPreferences and editPost utils #55099

Merged
merged 20 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fe70678
Create editor.setPreferences util
WunderBart Oct 5, 2023
aecd765
Migrate createNewPost to TS; use editor.setPreferences
WunderBart Oct 5, 2023
ecef484
Create visitPostEditor util
WunderBart Oct 5, 2023
86c8f79
Allow for setting preferences context
WunderBart Oct 5, 2023
a3c356e
Pass context correctly
WunderBart Oct 5, 2023
8ac5f11
Make visitSiteEditor consistent with other utils
WunderBart Oct 5, 2023
148c15b
Update tests to use editor.setPreferences()
WunderBart Oct 5, 2023
c3f20f5
Merge branch 'trunk' into add/e2e-utils-editor-preferences
WunderBart Oct 6, 2023
ddfd869
Make action param fixed
WunderBart Oct 11, 2023
256f5b9
Update specs
WunderBart Oct 11, 2023
34f93b3
Pass props as object to evaluate call
WunderBart Oct 11, 2023
6e5feb7
Merge remote-tracking branch 'origin' into add/e2e-utils-editor-prefe…
WunderBart Oct 16, 2023
59720fe
Merge createNewPost into visitPostEditor
WunderBart Oct 17, 2023
d2aba51
Update visitSiteEditor to use URLSearchParams
WunderBart Oct 17, 2023
9559d8e
Restore canvas check to avoid unrelated changes
WunderBart Oct 17, 2023
7658a0a
Fix accidental page->editor rename
WunderBart Oct 18, 2023
36b26c2
Increase perf tests action timeout to 2 minutes.
WunderBart Oct 18, 2023
99c2b2d
Remove canvas waiter from visitSiteEditor as it's actually breaking s…
WunderBart Oct 19, 2023
8b7ffd2
Merge remote-tracking branch 'origin' into add/e2e-utils-editor-prefe…
WunderBart Oct 23, 2023
fa96ad2
Drop visitPostEditor in favor of createNewPost + editPost
WunderBart Nov 3, 2023
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
47 changes: 0 additions & 47 deletions packages/e2e-test-utils-playwright/src/admin/create-new-post.js

This file was deleted.

38 changes: 38 additions & 0 deletions packages/e2e-test-utils-playwright/src/admin/create-new-post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Internal dependencies
*/
import type { Admin } from './';

interface NewPostOptions {
postType?: string;
title?: string;
content?: string;
excerpt?: string;
showWelcomeGuide?: boolean;
}

/**
* Creates new post.
*
* @param this
* @param options Options to create new post.
*/
export async function createNewPost(
this: Admin,
options: NewPostOptions = {}
) {
const query = new URLSearchParams();
const { postType, title, content, excerpt } = options;

if ( postType ) query.set( 'post_type', postType );
if ( title ) query.set( 'post_title', title );
if ( content ) query.set( 'content', content );
if ( excerpt ) query.set( 'excerpt', excerpt );

await this.visitAdminPage( 'post-new.php', query.toString() );

await this.editor.setPreferences( 'core/edit-post', {
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious to know if we need to do this every time, seems wasteful to me. Could we just do this in global-setup with a network request? I know that the preference API doesn't seem stable in e2e tests though 😅,

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea. The only time the welcome guide needs to be enabled is when we're actually testing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there an existing util for setting prefs via a network request? 🤔

welcomeGuide: options.showWelcomeGuide ?? false,
fullscreenMode: false,
} );
}
24 changes: 24 additions & 0 deletions packages/e2e-test-utils-playwright/src/admin/edit-post.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Internal dependencies
*/
import type { Admin } from '.';

/**
* Open the post with given ID in the editor.
*
* @param this
* @param postId Post ID to visit.
*/
export async function editPost( this: Admin, postId: string | number ) {
const query = new URLSearchParams();

query.set( 'post', String( postId ) );
query.set( 'action', 'edit' );

await this.visitAdminPage( 'post.php', query.toString() );

await this.editor.setPreferences( 'core/edit-post', {
welcomeGuide: false,
fullscreenMode: false,
} );
}
13 changes: 10 additions & 3 deletions packages/e2e-test-utils-playwright/src/admin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,36 @@ import type { Browser, Page, BrowserContext } from '@playwright/test';
import { createNewPost } from './create-new-post';
import { getPageError } from './get-page-error';
import { visitAdminPage } from './visit-admin-page';
import { editPost } from './edit-post';
import { visitSiteEditor } from './visit-site-editor';
import type { PageUtils } from '../page-utils';
import type { Editor } from '../editor';

type AdminConstructorProps = {
page: Page;
pageUtils: PageUtils;
editor: Editor;
};

export class Admin {
browser: Browser;
page: Page;
pageUtils: PageUtils;
context: BrowserContext;
browser: Browser;
pageUtils: PageUtils;
editor: Editor;

constructor( { page, pageUtils }: AdminConstructorProps ) {
constructor( { page, pageUtils, editor }: AdminConstructorProps ) {
this.page = page;
this.context = page.context();
this.browser = this.context.browser()!;
this.pageUtils = pageUtils;
this.editor = editor;
}

/** @borrows createNewPost as this.createNewPost */
createNewPost: typeof createNewPost = createNewPost.bind( this );
/** @borrows editPost as this.editPost */
editPost: typeof editPost = editPost.bind( this );
/** @borrows getPageError as this.getPageError */
getPageError: typeof getPageError = getPageError.bind( this );
/** @borrows visitAdminPage as this.visitAdminPage */
Expand Down
83 changes: 31 additions & 52 deletions packages/e2e-test-utils-playwright/src/admin/visit-site-editor.ts
Original file line number Diff line number Diff line change
@@ -1,72 +1,51 @@
/**
* WordPress dependencies
*/
import { addQueryArgs } from '@wordpress/url';

/**
* Internal dependencies
*/
import type { Admin } from './';

export interface SiteEditorQueryParams {
postId: string | number;
postType: string;
interface SiteEditorOptions {
postId?: string | number;
postType?: string;
path?: string;
canvas?: string;
showWelcomeGuide?: boolean;
}

const CANVAS_SELECTOR = 'iframe[title="Editor canvas"i] >> visible=true';

/**
* Visits the Site Editor main page
*
* By default, it also skips the welcome guide. The option can be disabled if need be.
* Visits the Site Editor main page.
*
* @param this
* @param query Query params to be serialized as query portion of URL.
* @param skipWelcomeGuide Whether to skip the welcome guide as part of the navigation.
* @param options Options to visit the site editor.
*/
export async function visitSiteEditor(
this: Admin,
query: SiteEditorQueryParams,
skipWelcomeGuide = true
options: SiteEditorOptions = {}
) {
const path = addQueryArgs( '', {
...query,
} ).slice( 1 );

await this.visitAdminPage( 'site-editor.php', path );

if ( skipWelcomeGuide ) {
await this.page.evaluate( () => {
window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-site', 'welcomeGuide', false );

window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-site', 'welcomeGuideStyles', false );

window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-site', 'welcomeGuidePage', false );

window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-site', 'welcomeGuideTemplate', false );
const { postId, postType, path, canvas } = options;
const query = new URLSearchParams();

if ( postId ) query.set( 'postId', String( postId ) );
if ( postType ) query.set( 'postType', postType );
if ( path ) query.set( 'path', path );
if ( canvas ) query.set( 'canvas', canvas );

await this.visitAdminPage( 'site-editor.php', query.toString() );

if ( ! options.showWelcomeGuide ) {
await this.editor.setPreferences( 'core/edit-site', {
welcomeGuide: false,
welcomeGuideStyles: false,
welcomeGuidePage: false,
welcomeGuideTemplate: false,
} );
}

// Check if the current page has an editor canvas first.
if ( ( await this.page.locator( CANVAS_SELECTOR ).count() ) > 0 ) {
// The site editor initially loads with an empty body,
// we need to wait for the editor canvas to be rendered.
await this.page
.frameLocator( CANVAS_SELECTOR )
.locator( 'body > *' )
.first()
.waitFor();
}

Comment on lines -58 to -67
Copy link
Member

Choose a reason for hiding this comment

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

This code removal doesn't look related to the changes proposed in this PR. As discussed in #54911, there were usually good reasons for introducing similar guardrails.

Maybe we should split this into a separate PR. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, nice catch! I actually forgot to restore this part. I wasn't planning on removing it in this PR, only running a single round without it to see how it goes. Restoring it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's good, though, that now we know only perf tests fail without it - these tests require specific handling, so it might be good to move this safeguard there if it's not necessary here for the regular E2Es. Anyway, it's just a note for myself, I guess - let's leave that for another PR.

Copy link
Member Author

@WunderBart WunderBart Oct 17, 2023

Choose a reason for hiding this comment

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

Restored in 9559d8e See comment below

Copy link
Member Author

@WunderBart WunderBart Oct 24, 2023

Choose a reason for hiding this comment

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

I had to remove it as the slight timing change caused by using setPreferences in visitSiteEditor revealed that the part above is actually faulty. Before, it checked the canvas iframe(s) before they were attached, so that check has always been skipped and moved instantly to wait until the loading spinner is hidden. Now, because setPreferences adds a little delay by waiting until the window.wp.data is available, all the editor iframes get attached in the meantime, which ends up violating the locator strict mode:

at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/site-editor/patterns.spec.js:165:3
Error:   1) [chromium] › site-editor/patterns.spec.js:137:2 › Patterns › search and filter patterns ───────

    Retry #1 ───────────────────────────────────────────────────────────────────────────────────────
    Error: locator.waitFor: Error: strict mode violation: locator('iframe[title="Editor canvas"i]').locator('visible=true') resolved to 3 elements:
        1) <iframe tabindex="-1" aria-hidden="true" title="Editor c…></iframe> aka locator('#edit-site-patterns-synced-footer')
        2) <iframe tabindex="-1" aria-hidden="true" title="Editor c…></iframe> aka locator('#edit-site-patterns-unsynced-header')
        3) <iframe tabindex="-1" aria-hidden="true" title="Editor c…></iframe> aka locator('#edit-site-patterns-unsynced-footer')

ℹ️ The other iframes are previews of the patterns created in that test.

// TODO: Ideally the content underneath the canvas loader should be marked inert until it's ready.
/**
* @todo This is a workaround for the fact that the editor canvas is seen as
* ready and visible before the loading spinner is hidden. Ideally, the
* content underneath the loading overlay should be marked inert until the
* loading is done.
*/
await this.page
.locator( '.edit-site-canvas-loader' )
// Bigger timeout is needed for larger entities, for example the large
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e-test-utils-playwright/src/editor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { publishPost } from './publish-post';
import { saveDraft } from './save-draft';
import { selectBlocks } from './select-blocks';
import { setContent } from './set-content';
import { setPreferences } from './set-preferences';
import { showBlockToolbar } from './show-block-toolbar';
import { saveSiteEditorEntities } from './site-editor';
import { setIsFixedToolbar } from './set-is-fixed-toolbar';
Expand Down Expand Up @@ -76,6 +77,8 @@ export class Editor {
selectBlocks: typeof selectBlocks = selectBlocks.bind( this );
/** @borrows setContent as this.setContent */
setContent: typeof setContent = setContent.bind( this );
/** @borrows setPreferences as this.setPreferences */
setPreferences: typeof setPreferences = setPreferences.bind( this );
/** @borrows showBlockToolbar as this.showBlockToolbar */
showBlockToolbar: typeof showBlockToolbar = showBlockToolbar.bind( this );
/** @borrows setIsFixedToolbar as this.setIsFixedToolbar */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Internal dependencies
*/
import type { Editor } from './index';

type PreferencesContext =
| 'core/edit-post'
| 'core/edit-site'
| 'core/customize-widgets';

/**
* Set the preferences of the editor.
*
* @param this
* @param context Context to set preferences for.
* @param preferences Preferences to set.
*/
export async function setPreferences(
this: Editor,
context: PreferencesContext,
preferences: Record< string, any >
) {
await this.page.waitForFunction( () => window?.wp?.data );

await this.page.evaluate(
async ( props ) => {
for ( const [ key, value ] of Object.entries(
props.preferences
) ) {
await window.wp.data
.dispatch( 'core/preferences' )
.set( props.context, key, value );
}
},
{ context, preferences }
);
}
4 changes: 2 additions & 2 deletions packages/e2e-test-utils-playwright/src/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ const test = base.extend<
lighthousePort: number;
}
>( {
admin: async ( { page, pageUtils }, use ) => {
await use( new Admin( { page, pageUtils } ) );
admin: async ( { page, pageUtils, editor }, use ) => {
await use( new Admin( { page, pageUtils, editor } ) );
},
editor: async ( { page }, use ) => {
await use( new Editor( { page } ) );
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/specs/editor/blocks/query.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ test.describe( 'Query block', () => {
} );

test.beforeEach( async ( { admin } ) => {
await admin.createNewPost( { postType: 'page', title: 'Query Page' } );
await admin.createNewPost( {
postType: 'page',
title: 'Query Page',
} );
} );

test.afterEach( async ( { requestUtils } ) => {
Expand Down
17 changes: 4 additions & 13 deletions test/e2e/specs/editor/local/demo.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,12 @@ test.describe( 'New editor state', () => {
test( 'content should load, making the post dirty', async ( {
page,
admin,
editor,
} ) => {
await admin.visitAdminPage( 'post-new.php', 'gutenberg-demo' );
await page.waitForFunction( () => {
if ( ! window?.wp?.data?.dispatch ) {
return false;
}
window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-post', 'welcomeGuide', false );

window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-post', 'fullscreenMode', false );

return true;
await editor.setPreferences( 'core/edit-site', {
welcomeGuide: false,
fullscreenMode: false,
} );

const isDirty = await page.evaluate( () => {
Expand Down
6 changes: 2 additions & 4 deletions test/e2e/specs/editor/various/block-renaming.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ test.describe( 'Block Renaming', () => {
pageUtils,
} ) => {
// Turn on block list view by default.
await page.evaluate( () => {
window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-site', 'showListViewByDefault', true );
await editor.setPreferences( 'core/edit-site', {
showListViewByDefault: true,
} );

const listView = page.getByRole( 'treegrid', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,8 @@ class PostEditorTemplateMode {

async disableTemplateWelcomeGuide() {
// Turn off the welcome guide.
await this.page.evaluate( () => {
window.wp.data
.dispatch( 'core/preferences' )
.set( 'core/edit-post', 'welcomeGuideTemplate', false );
await this.editor.setPreferences( 'core/edit-post', {
welcomeGuideTemplate: false,
} );
}

Expand Down
5 changes: 4 additions & 1 deletion test/e2e/specs/editor/various/preview.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,10 @@ test.describe( 'Preview with private custom post type', () => {
admin,
page,
} ) => {
await admin.createNewPost( { postType: 'not_public', title: 'aaaaa' } );
await admin.createNewPost( {
postType: 'not_public',
title: 'aaaaa',
} );

// Open the view menu.
await page.click( 'role=button[name="Preview"i]' );
Expand Down
Loading
Loading