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.

Sign up for GitHub

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 change-detection to Playwright #58767

Merged
merged 5 commits into from
Feb 13, 2024
Merged

Migrate change-detection to Playwright #58767

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
06cfa59
Migrate change-detection to Playwright
kevin940726 Feb 7, 2024
4162963
Do not retry page.reload
kevin940726 Feb 7, 2024
a05a995
Only resolve 'hasDialog' to true for the 'beforeunload' type
Mamaduka Feb 9, 2024
7ad27c8
Update the initial block creation method to avoid flakiness
Mamaduka Feb 9, 2024
a23241e
Fix route interception
Mamaduka Feb 9, 2024
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
31 changes: 18 additions & 13 deletions test/e2e/specs/editor/various/change-detection.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ const test = base.extend( {
},
} );

const POST_URLS = [
'/wp/v2/posts',
`rest_route=${ encodeURIComponent( '/wp/v2/posts' ) }`,
];

test.describe( 'Change detection', () => {
test.beforeEach( async ( { admin } ) => {
await admin.createNewPost();
Expand Down Expand Up @@ -276,7 +281,6 @@ test.describe( 'Change detection', () => {
} );

test( 'Should prompt if changes made while save is in-flight', async ( {
page,
editor,
pageUtils,
changeDetectionUtils,
Expand All @@ -295,12 +299,7 @@ test.describe( 'Change detection', () => {

await editor.canvas
.getByRole( 'textbox', { name: 'Add title' } )
.type( '!' );
await expect(
page
.getByRole( 'region', { name: 'Editor top bar' } )
.getByRole( 'button', { name: 'Save draft' } )
).toBeEnabled();
Copy link
Member

Choose a reason for hiding this comment

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

The button isn't restored when making changes in flight. I'm not sure if this is a regression. The behavior can be reproduced manually by throttling the network via DevTools.

.fill( 'Hello World!' );

await changeDetectionUtils.releaseSaveIntercept();

Expand Down Expand Up @@ -539,18 +538,24 @@ class ChangeDetectionUtils {
this.#continueInterceptedSave = res;
} );

await this.#page.route( '**/wp/v2/posts', async ( route ) => {
hadInterceptedSave = true;
await deferred;
await route.continue();
} );
await this.#page.route(
( url ) =>
POST_URLS.some( ( postUrl ) => url.href.includes( postUrl ) ),
Comment on lines +542 to +543
Copy link
Member

@Mamaduka Mamaduka Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That pattern matcher wasn't working with wp-env; maybe it could work with pretty permalinks, but we can't ensure that. I found this pattern to be the safe solution for now.

I usually check in UI Mode if requests were successfully intercepted.

Screenshot

CleanShot 2024-02-09 at 11 29 23

async ( route ) => {
hadInterceptedSave = true;
await deferred;
await route.continue();
}
);

return () => hadInterceptedSave;
};

releaseSaveIntercept = async () => {
this.#continueInterceptedSave?.();
await this.#page.unroute( '**/wp/v2/posts' );
await this.#page.unroute( ( url ) =>
POST_URLS.some( ( postUrl ) => url.href.includes( postUrl ) )
);
this.#continueInterceptedSave = null;
};
}
Loading