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

[Experiment] Use puppeteer-testing-library in e2e tests #28380

Merged
merged 11 commits into from Apr 23, 2021

Conversation

kevin940726
Copy link
Member

@kevin940726 kevin940726 commented Jan 21, 2021

Description

An alternative approach to #27260.

This PR integrates with puppeteer-testing-library. A queries/matchers library I created to help us write accessible and consistent e2e tests with puppeteer and jest.

You can find the full documentation of the library in its README. Or, here's a simplified version of it:

Basically there are only two queries available: find and findAll.

find(query), well, finds a single element which matches the query, or throw an error if there're more than one matching elements or there're no matching elements. It will by default wait for up to 3 seconds until it finds an element, or throw an error if timeout. The query will return an ElementHandle if found.

The query is just a collection of accessible properties of the DOM node, like role, name, value, etc. The most commonly used fields are, by the order of their priories, role, name, text, selector.

// <button>My button</button>
const button = await find({
  role: 'button',
  name: 'My button',
});

findAll finds all matching elements, simply a multiple version of find. It returns an array of ElementHandles.

// <button>Button 1</button>
// <button>Button 2</button>
const buttons = await findAll({
  role: 'button',
});

There're also some helpful matchers: toMatchQuery, toHaveFocus, toBeVisible, to name a few.

This PR refactors widgets test (adding-widgets.test.js) to use puppeteer-testing-library to demonstrate its usage and the before&after.

How has this been tested?

npm run test-e2e -- packages/e2e-tests/specs/widgets/adding-widgets.test.js

Types of changes

New feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@kevin940726 kevin940726 added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Jan 21, 2021
@github-actions
Copy link

github-actions bot commented Jan 21, 2021

Size Change: +579 B (0%)

Total Size: 1.47 MB

Filename Size Change
build/block-editor/index.js 130 kB +23 B (0%)
build/block-library/index.js 154 kB +29 B (0%)
build/blocks/index.js 48.7 kB +112 B (0%)
build/components/index.js 285 kB +58 B (0%)
build/core-data/index.js 17 kB -153 B (-1%)
build/customize-widgets/index.js 8.25 kB -1 B (0%)
build/data/index.js 9.17 kB +299 B (+3%)
build/edit-navigation/index.js 17.1 kB +63 B (0%)
build/edit-post/index.js 339 kB -4 B (0%)
build/edit-site/index.js 28.9 kB +152 B (+1%)
build/element/index.js 4.62 kB +1 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/api-fetch/index.js 3.41 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.62 kB 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-editor/style-rtl.css 12.6 kB 0 B
build/block-editor/style.css 12.6 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 503 B 0 B
build/block-library/blocks/button/style.css 503 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 368 B 0 B
build/block-library/blocks/buttons/style.css 368 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 436 B 0 B
build/block-library/blocks/columns/style.css 435 B 0 B
build/block-library/blocks/cover/editor-rtl.css 605 B 0 B
build/block-library/blocks/cover/editor.css 605 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.23 kB 0 B
build/block-library/blocks/cover/style.css 1.23 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 301 B 0 B
build/block-library/blocks/file/editor.css 300 B 0 B
build/block-library/blocks/file/frontend.js 765 B 0 B
build/block-library/blocks/file/style-rtl.css 255 B 0 B
build/block-library/blocks/file/style.css 255 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB 0 B
build/block-library/blocks/freeform/editor.css 2.44 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 704 B 0 B
build/block-library/blocks/gallery/editor.css 705 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.09 kB 0 B
build/block-library/blocks/gallery/style.css 1.09 kB 0 B
build/block-library/blocks/group/editor-rtl.css 160 B 0 B
build/block-library/blocks/group/editor.css 160 B 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 281 B 0 B
build/block-library/blocks/latest-comments/style.css 282 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/legacy-widget/editor-rtl.css 398 B 0 B
build/block-library/blocks/legacy-widget/editor.css 399 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 597 B 0 B
build/block-library/blocks/navigation-link/editor.css 597 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 1.07 kB 0 B
build/block-library/blocks/navigation-link/style.css 1.07 kB 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.24 kB 0 B
build/block-library/blocks/navigation/editor.css 1.24 kB 0 B
build/block-library/blocks/navigation/style-rtl.css 272 B 0 B
build/block-library/blocks/navigation/style.css 271 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 239 B 0 B
build/block-library/blocks/page-list/editor.css 240 B 0 B
build/block-library/blocks/page-list/style-rtl.css 167 B 0 B
build/block-library/blocks/page-list/style.css 167 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B 0 B
build/block-library/blocks/post-excerpt/style.css 69 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/post-title/style-rtl.css 60 B 0 B
build/block-library/blocks/post-title/style.css 60 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 103 B 0 B
build/block-library/blocks/preformatted/style.css 103 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 83 B 0 B
build/block-library/blocks/query-loop/editor.css 82 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/query/editor-rtl.css 810 B 0 B
build/block-library/blocks/query/editor.css 809 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 189 B 0 B
build/block-library/blocks/search/editor.css 189 B 0 B
build/block-library/blocks/search/style-rtl.css 359 B 0 B
build/block-library/blocks/search/style.css 362 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 251 B 0 B
build/block-library/blocks/separator/style.css 251 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 512 B 0 B
build/block-library/blocks/shortcode/editor.css 512 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 440 B 0 B
build/block-library/blocks/site-logo/editor.css 441 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 154 B 0 B
build/block-library/blocks/site-logo/style.css 154 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 796 B 0 B
build/block-library/blocks/social-links/editor.css 795 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB 0 B
build/block-library/blocks/social-links/style.css 1.33 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 308 B 0 B
build/block-library/blocks/spacer/editor.css 308 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 552 B 0 B
build/block-library/blocks/template-part/editor.css 551 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 568 B 0 B
build/block-library/blocks/video/editor.css 569 B 0 B
build/block-library/blocks/video/style-rtl.css 173 B 0 B
build/block-library/blocks/video/style.css 173 B 0 B
build/block-library/common-rtl.css 1.31 kB 0 B
build/block-library/common.css 1.31 kB 0 B
build/block-library/editor-rtl.css 9.83 kB 0 B
build/block-library/editor.css 9.82 kB 0 B
build/block-library/reset-rtl.css 502 B 0 B
build/block-library/reset.css 503 B 0 B
build/block-library/style-rtl.css 9.44 kB 0 B
build/block-library/style.css 9.44 kB 0 B
build/block-library/theme-rtl.css 692 B 0 B
build/block-library/theme.css 693 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/components/style-rtl.css 16.2 kB 0 B
build/components/style.css 16.2 kB 0 B
build/compose/index.js 11.6 kB 0 B
build/customize-widgets/style-rtl.css 630 B 0 B
build/customize-widgets/style.css 631 B 0 B
build/data-controls/index.js 837 B 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 787 B 0 B
build/dom-ready/index.js 577 B 0 B
build/dom/index.js 5.12 kB 0 B
build/edit-navigation/style-rtl.css 2.86 kB 0 B
build/edit-navigation/style.css 2.86 kB 0 B
build/edit-post/classic-rtl.css 454 B 0 B
build/edit-post/classic.css 454 B 0 B
build/edit-post/style-rtl.css 6.96 kB 0 B
build/edit-post/style.css 6.95 kB 0 B
build/edit-site/style-rtl.css 4.9 kB 0 B
build/edit-site/style.css 4.89 kB 0 B
build/edit-widgets/index.js 16.7 kB 0 B
build/edit-widgets/style-rtl.css 2.97 kB 0 B
build/edit-widgets/style.css 2.98 kB 0 B
build/editor/index.js 42.6 kB 0 B
build/editor/style-rtl.css 3.9 kB 0 B
build/editor/style.css 3.9 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.77 kB 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 4.04 kB 0 B
build/is-shallow-equal/index.js 699 B 0 B
build/keyboard-shortcuts/index.js 2.53 kB 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/index.js 3.19 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/media-utils/index.js 5.39 kB 0 B
build/notices/index.js 1.85 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.95 kB 0 B
build/primitives/index.js 1.42 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/react-i18n/index.js 1.45 kB 0 B
build/redux-routine/index.js 2.83 kB 0 B
build/reusable-blocks/index.js 3.8 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/rich-text/index.js 13.5 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 3.01 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@gziolo gziolo added [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Type] New API New API to be used by plugin developers or package users. labels Jan 21, 2021
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Very nice!

I like that it's easier to read as a code reviewer, that it tests the editor as it "appears" to a screen reader instead of relying on CSS selectors that might change, and that the tests might be a little less prone to intermittent failure because find implicitly uses waitForSelector.

I'm keen to try it out in widgets. If it works well for us, then we can use it in other parts of Gutenberg. If it doesn't, we can remove it.

Does it affect how long tests take to run? By how much?

@kevin940726
Copy link
Member Author

kevin940726 commented Jan 26, 2021

The original adding-widgets test using the CSS/XPath selector took about 14s to finish, while this PR using puppeteer-testing-library takes about 21s to finish. So it's 33% slower... To me, it's worth the extra time for the confidence and readability in our tests though.

The bottleneck seems to be the role and name queries, which usually takes about 400ms for each query in a DOM-heavy page. It can potentially be boosted if we adopt aria query eventually, however it's not in a very stable phase right now. The good thing about abstracting away the query library is that we can change the underlying query system any time without impacting the user facing code, since they're all just implementation details.

@gziolo
Copy link
Member

gziolo commented Jan 26, 2021

It’s great that you explore all those improvements, I appreciate all the efforts 👍🏻
Have you seen #23238? @youknowriad tried replacing puppeteer with playwright which waits for elements to show in the UI by design. It’s the biggest difference between those two projects. Have you considered also switching to Playwright to achieve some of the goals? I personally think it's a quite important shift in how those selectors work in contrast to how it's handled with DOM-based (querySelector or XPath) selectors from Puppeteer that operate on the current DOM tree.

I see that puppeteer-testing-library uses waitForTimeout internally:
https://github.com/kevin940726/puppeteer-testing-library/blob/bbd58a1a34b995688f02d603f2f8e7907644d0b4/src/queries.ts#L124-L133

Noting, that it's discouraged from usage in Gutenberg with an ESLint rule:

gutenberg/.eslintrc.js

Lines 116 to 120 in c888d27

{
selector:
'CallExpression[callee.object.name="page"][callee.property.name="waitForTimeout"]',
message: 'Prefer page.waitForSelector instead.',
},

Not sure if that is an issue, but it's hidden in this PR so I thought I will mention it.

As for the API proposed:

const button = await find({
  role: 'button',
  name: 'My button',
});

I agree that it looks clean and simple on the surface. The remaining question is whether, for folks writing tests, it'd be more preferable to use over web APIs they already might know querySelector, querySelectorAll or XPath that are integrated into the official Puppeteer API. There is over 100 e2e test specs today, so it's another challenge to convince people that this new API would work better for them. Rewriting all the existing tests is doable but it's rather impractical because of the scale and the chances that some errors would be introduced. With that said, if you have a good strategy how to document it, communicate this new API, and convince people to use it as a default way for querying then make it happen 😄

@noisysocks
Copy link
Member

I see that puppeteer-testing-library uses waitForTimeout internally:
https://github.com/kevin940726/puppeteer-testing-library/blob/bbd58a1a34b995688f02d603f2f8e7907644d0b4/src/queries.ts#L124-L133

I think that that usage of waitForTimeout is okay because really what it's doing is using waitForTimeout to implement a version of waitForSelector that uses a puppeteer-testing-library selector instead of a regular selector.

@noisysocks
Copy link
Member

Rewriting all the existing tests is doable but it's rather impractical because of the scale and the chances that some errors would be introduced. With that said, if you have a good strategy how to document it, communicate this new API, and convince people to use it as a default way for querying then make it happen 😄

I think there's no harm in trying out this new approach in a small area of the codebase (widgets). If it helps a lot with test stability (it might not!) then we can think about how to roll it out more broadly, whether that means re-writing tests (probably not for the reasons you mention) or just recommending this for new tests.

@kevin940726
Copy link
Member Author

Have you seen #23238?

I didn’t know about that PR! It looks like puppeteer-testing-library can still work with playwright nicely though, since they basically share the same API.

The remaining question is whether, for folks writing tests, it'd be more preferable to use over web APIs they already might know querySelector, querySelectorAll or XPath that are integrated into the official Puppeteer API.

I totally agree that convincing others to adopt this new API might be the most important part. I can try to write up some more docs or perhaps some tutorials to help developers know the benefits and the trade-offs. I'll also reach out to the javascript testing community outside Gutenberg and gain some feedbacks from them.

@gziolo
Copy link
Member

gziolo commented Jan 27, 2021

I think there's no harm in trying out this new approach in a small area of the codebase (widgets). If it helps a lot with test stability (it might not!) then we can think about how to roll it out more broadly

Yes, it sounds like a reasonable plan 👍

Base automatically changed from master to trunk March 1, 2021 15:45
@kevin940726 kevin940726 force-pushed the experiment/puppeteer-testing-library branch from c4301c9 to 16a029f Compare April 21, 2021 07:35
@kevin940726 kevin940726 force-pushed the experiment/puppeteer-testing-library branch from 16a029f to df28ee8 Compare April 21, 2021 07:54
@kevin940726 kevin940726 force-pushed the experiment/puppeteer-testing-library branch 2 times, most recently from f95e303 to 3ae06e4 Compare April 21, 2021 15:39
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

I'd be happy to try this if we can convert it from a draft and get the tests passing.

I do wonder if there's a way we can indicate to other developers that this is being trialled as an experiment ... something like a linting rule for import { something } from 'puppeteer-testing-library'; that has to be specifically allowed in the widgets test file?

@gziolo Are you ok with the changes to the config now?

@kevin940726 kevin940726 force-pushed the experiment/puppeteer-testing-library branch from 3ae06e4 to f5a1cba Compare April 22, 2021 04:26
@gziolo
Copy link
Member

gziolo commented Apr 22, 2021

@gziolo Are you ok with the changes to the config now?

Sure, it's fine.

@kevin940726 kevin940726 marked this pull request as ready for review April 22, 2021 15:01
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Lets try this out 👍

@talldan talldan merged commit 5c78bb8 into trunk Apr 23, 2021
@talldan talldan deleted the experiment/puppeteer-testing-library branch April 23, 2021 03:19
@github-actions github-actions bot added this to the Gutenberg 10.6 milestone Apr 23, 2021
@WunderBart
Copy link
Member

Howdy! 👋 I've noticed this experiment while revisiting the idea of migration to Playwright. Since the last time it was considered/tried, Playwright got quite a few updates that IMO have moved it to the front of the competition, making it a solid candidate for Gutenberg. I'm writing here because it seems to me like adopting this library would make the potential migration much more difficult due to API differences (unless that lib itself moves to Playwright). I think that moving to Playwright would actually lower the dev entry threshold and increase the overall stability of the tests (e.g. thanks to explicit waits with auto-wait). I'm happy to make a case for Playwright using some examples/comparisons, but right now I just wanted to raise the concern, and find out the current status and plans for this experiment.

For the most prominent features that IMO make Playwright the goto framework, check out the auto-waiting mechanism, a wide range of supported selectors, debugging tools, and browsers support.

@kevin940726
Copy link
Member Author

@WunderBart Hi, thanks for the ping!

This library is designed to work with playwright, as long as we keep using chromium to run our tests. We might need a few tweak here and there, but it shouldn't be a blocker. I'm happy to help when the migration begin. I still believe that the queries and helpful matchers this library brings have benefits, even with all the more advanced selectors playwright supports.

@swissspidy
Copy link
Member

as long as we keep using chromium to run our tests.

Which means this will prevent any attempts at running tests on other browsers like Firefox :/

@kevin940726
Copy link
Member Author

Which means this will prevent any attempts at running tests on other browsers like Firefox :/

Yep, as for the current implementation, it will only work in chromium-based browsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] New API New API to be used by plugin developers or package users. [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants