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

Conversation

spacedmonkey
Copy link
Contributor

@spacedmonkey spacedmonkey commented Jul 6, 2021

Context

Summary

TLDR, in WordPress 5.8+, the block needs to be focused, before this is out was automatically focused and the inserter closed.

Much of these changes are copied from upstream.

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

QA

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

UAT

  • UAT should use the same steps as above.

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This PR contains automated tests (unit, integration, and/or e2e) to verify the code works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #8160

@spacedmonkey spacedmonkey added P2 Should do soon Type: Infrastructure Changes impacting testing infrastructure or build tooling Pod: WP & Infra Group: Blocks Issues related to the provided Gutenberg Blocks Package: E2E Test Utils /packages/e2e-test-utils Package: E2E Tests /packages/e2e-tests labels Jul 6, 2021
@spacedmonkey spacedmonkey self-assigned this Jul 6, 2021
@google-cla google-cla bot added the cla: yes label Jul 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2021

Size Change: 0 B

Total Size: 2.2 MB

ℹ️ View Unchanged
Filename Size
assets/css/carousel-view-rtl.css 701 B
assets/css/carousel-view.css 701 B
assets/css/edit-story-rtl.css 277 B
assets/css/edit-story.css 277 B
assets/css/stories-dashboard-rtl.css 276 B
assets/css/stories-dashboard.css 276 B
assets/css/vendors-edit-story-rtl.css 706 B
assets/css/vendors-edit-story.css 706 B
assets/css/web-stories-block-rtl.css 3.26 kB
assets/css/web-stories-block.css 3.3 kB
assets/css/web-stories-embed-rtl.css 288 B
assets/css/web-stories-embed.css 288 B
assets/css/web-stories-list-styles-rtl.css 2.3 kB
assets/css/web-stories-list-styles.css 2.31 kB
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B
assets/css/web-stories-theme-style-twentyten.css 143 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 263 B
assets/css/web-stories-theme-style-twentytwentyone.css 264 B
assets/css/web-stories-widget-rtl.css 484 B
assets/css/web-stories-widget.css 484 B
assets/js/carousel-view.js 3.72 kB
assets/js/chunk-fonts-********************.js 45.9 kB
assets/js/chunk-web-stories-template-0-********************.js 472 B
assets/js/chunk-web-stories-template-10-********************.js 7.94 kB
assets/js/chunk-web-stories-template-12-********************.js 466 B
assets/js/chunk-web-stories-template-16-********************.js 7.86 kB
assets/js/chunk-web-stories-template-18-********************.js 477 B
assets/js/chunk-web-stories-template-22-********************.js 8.11 kB
assets/js/chunk-web-stories-template-24-********************.js 424 B
assets/js/chunk-web-stories-template-28-********************.js 9.73 kB
assets/js/chunk-web-stories-template-30-********************.js 459 B
assets/js/chunk-web-stories-template-34-********************.js 8.54 kB
assets/js/chunk-web-stories-template-36-********************.js 451 B
assets/js/chunk-web-stories-template-4-********************.js 7.96 kB
assets/js/chunk-web-stories-template-40-********************.js 6.99 kB
assets/js/chunk-web-stories-template-42-********************.js 444 B
assets/js/chunk-web-stories-template-46-********************.js 9.2 kB
assets/js/chunk-web-stories-template-48-********************.js 461 B
assets/js/chunk-web-stories-template-52-********************.js 6.06 kB
assets/js/chunk-web-stories-template-54-********************.js 481 B
assets/js/chunk-web-stories-template-58-********************.js 7.98 kB
assets/js/chunk-web-stories-template-6-********************.js 498 B
assets/js/chunk-web-stories-template-60-********************.js 488 B
assets/js/chunk-web-stories-template-64-********************.js 7.84 kB
assets/js/chunk-web-stories-template-66-********************.js 508 B
assets/js/chunk-web-stories-template-70-********************.js 8.33 kB
assets/js/chunk-web-stories-template-72-********************.js 445 B
assets/js/chunk-web-stories-template-76-********************.js 8.65 kB
assets/js/chunk-web-stories-textset-0-********************.js 5.29 kB
assets/js/chunk-web-stories-textset-1-********************.js 6.81 kB
assets/js/chunk-web-stories-textset-2-********************.js 7.92 kB
assets/js/chunk-web-stories-textset-3-********************.js 15.4 kB
assets/js/chunk-web-stories-textset-4-********************.js 4.43 kB
assets/js/chunk-web-stories-textset-5-********************.js 5.71 kB
assets/js/chunk-web-stories-textset-6-********************.js 5.5 kB
assets/js/chunk-web-stories-textset-7-********************.js 10.4 kB
assets/js/edit-story.js 551 kB
assets/js/lightbox.js 986 B
assets/js/stories-dashboard.js 450 kB
assets/js/tinymce-button.js 3.48 kB
assets/js/vendors-chunk-ffmpeg-********************.js 5.6 kB
assets/js/vendors-edit-story-********************.js 64.1 kB
assets/js/vendors-edit-story-stories-dashboard-********************.js 231 kB
assets/js/vendors-web-animations-js-********************.js 14.6 kB
assets/js/web-stories-activation-notice.js 65.1 kB
assets/js/web-stories-block.js 565 kB
assets/js/web-stories-embed.js 493 B
assets/js/web-stories-widget.js 984 B

compressed-size-action

.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.

`//button//span[contains(text(), '${searchTerm}')]`
);
await insertButton.click();
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.

@spacedmonkey
Copy link
Contributor Author

Closing in favour #8223

@swissspidy swissspidy deleted the fix/widget-error branch July 7, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Blocks Issues related to the provided Gutenberg Blocks P2 Should do soon Package: E2E Test Utils /packages/e2e-test-utils Package: E2E Tests /packages/e2e-tests Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants