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

Fix E2E tests in WordPress 5.8 #8204

Closed
wants to merge 4 commits into from
Closed
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
28 changes: 28 additions & 0 deletions packages/e2e-test-utils/src/focusSelectedBlock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2021 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

async function focusSelectedBlock() {
// Ideally there shouuld be a UI way to do this. (Focus the selected block)
spacedmonkey marked this conversation as resolved.
Show resolved Hide resolved
await page.evaluate(() => {
wp.data
.dispatch('core/block-editor')
.selectBlock(
wp.data.select('core/block-editor').getSelectedBlockClientId(),
0
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the 0 here? AFAIK selectBlock only takes one argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

);
});
}
export default focusSelectedBlock;
1 change: 1 addition & 0 deletions packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export { default as createNewPost } from './createNewPost';
export { default as minWPVersionRequired } from './minWPVersionRequired';
export { default as visitBlockWidgetScreen } from './visitBlockWidgetScreen';
export { default as insertWidget } from './insertWidget';
export { default as focusSelectedBlock } from './focusSelectedBlock';
export {
getEditedPostContent,
setPostContent,
Expand Down
24 changes: 11 additions & 13 deletions packages/e2e-test-utils/src/insertBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,33 @@
/**
* WordPress dependencies
*/
import { searchForBlock } from '@wordpress/e2e-test-utils';
import { canvas, searchForBlock } from '@wordpress/e2e-test-utils';
/**
* Internal dependencies
*/
import focusSelectedBlock from './focusSelectedBlock';

/**
* Retrieves the document container by css class and checks
* to make sure the document's active element is within it.
*
* Differs from waitForInserterCloseAndContentFocus() in `@wordpress/e2e-test-utils`
* by using a simpler selector and optional chaining to avoid crashes.
* Retrieves the document container by css class and checks to make sure the document's active element is within it
*/
async function waitForInserterCloseAndContentFocus() {
await page.waitForFunction(() =>
document.body
.querySelector('.block-editor-block-list__layout')
?.contains(document.activeElement)
await canvas().waitForFunction(
() =>
document.activeElement.closest('.block-editor-block-list__layout') !==
null
);
}

/**
* Opens the inserter, searches for the given term, then selects the first
* result that appears. It then waits briefly for the block list to update.
*
* Avoids using waitForInserterCloseAndContentFocus() from `@wordpress/e2e-test-utils`
* because the selector it relies on does not exist in older versions of Gutenberg.
*
* @param {string} searchTerm The text to search the inserter for.
*/
async function insertBlock(searchTerm) {
swissspidy marked this conversation as resolved.
Show resolved Hide resolved
await searchForBlock(searchTerm);
await expect(page).toClick('button span', { text: searchTerm });
await focusSelectedBlock();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of focusing the block, could we close the inserter manually somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, in that case, since our versions of insertBlock and waitForInserterCloseAndContentFocus are now identical to the ones from Gutenberg, we can just use insertBlock from @wordpress/e2e-test-utils directly instead of copying everything.

Copy link
Contributor Author

@spacedmonkey spacedmonkey Jul 7, 2021

Choose a reason for hiding this comment

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

Sadly we can't as the version of @wordpress/e2e-test-utils is a month out of date. Also

It works properly with the minimum version of Gutenberg 9.2.0 or the minimum version of WordPress 5.6.0.

We support, 5.5, so have maintain our own version of these functions.

https://github.com/WordPress/gutenberg/tree/trunk/packages/e2e-test-utils

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sadly we can't as the version of @wordpress/e2e-test-utils is a month out of date.

Is it? At first glance I don't see a difference from the package published on npm that we use and the Gutenberg trunk branch.

I don't mind maintaining our own version of certain functions if they differ in logic, but here they seem identical to me. But happy to be corrected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This packages says, last published 1 month ago.

Screenshot 2021-07-07 at 12 22 52

Inserter was updated 12 minutes ago.

Screenshot 2021-07-07 at 12 22 45

What am I missing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems fine to me? Yes there's a recently modified file in there, but I don't see how that would affect us. That change is neither in core nor in the package version that we use. We also don't use any of the currently unreleased new features.


Putting this another way: if we just remove our own version of insertBlock and use the version from @wordpress/e2e-test-utils, do the e2e tests still pass? If yes, I'd say we can safely remove our copy.

// We should wait until the inserter closes and the focus moves to the content.
await waitForInserterCloseAndContentFocus();
}
Expand Down
5 changes: 3 additions & 2 deletions packages/e2e-tests/src/specs/wordpress/blockWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
visitBlockWidgetScreen,
insertWidget,
minWPVersionRequired,
focusSelectedBlock,
} from '@web-stories-wp/e2e-test-utils';

describe('Web Stories Widget Block', () => {
Expand All @@ -38,17 +39,17 @@ describe('Web Stories Widget Block', () => {

it('should insert a new web stories block', async () => {
await visitBlockWidgetScreen();

await expect(page).toClick('button[aria-label="Add block"]');
await page.type('.block-editor-inserter__search-input', 'Web Stories');
await expect(page).toClick('button span', { text: 'Web Stories' });
await focusSelectedBlock();

await page.waitForSelector('[data-testid="ws-block-configuration-panel"]');
await expect(page).toMatchElement(
'[data-testid="ws-block-configuration-panel"]'
);

await expect(page).toClick('[data-testid="ws-block-configuration-panel"]');

await expect(page).toClick('div.components-card__body', {
text: 'Story URL',
});
Expand Down
4 changes: 1 addition & 3 deletions packages/e2e-tests/src/specs/wordpress/webStoriesBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ const EMBED_BLOCK_CONTENT = `
<!-- /wp:web-stories/embed -->
`;

// TODO Fix https://github.com/google/web-stories-wp/issues/8160
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('Web Stories Block', () => {
describe('Web Stories Block', () => {
let stopRequestInterception;
let removeErrorMessage;
let removeError404Message;
Expand Down