E2E tests migration to Playwright #17814

adimoldovan · 2020-11-16T10:27:01Z

Closed in favor of 18762

This is a POC for a migration of E2E tests from Puppeteer to Playwright.

This PR is replacing 17791, which was opened from a forked repo.

Changes proposed in this Pull Request:

Added dependencies:

playwright
jest-playwright-preset
playwright-video
@ffmpeg-installer/ffmpeg

Removed dependencies:

puppeteer
jest-puppeteer

Configuration changes

Removed jest-puppeteer.config.js and added jest-playwright.config.js instead
Added JEST_PLAYWRIGHT_CONFIG env variable in jest.config.js instead of CLI as it never changes
added testRunner: 'jasmine2' in jest.config.js to fix "referenceError: jasmine is not defined"
I had an error reading the config files and I could only fix it by changing the format from .js to .json. So default.js and local-tests.js are now default.json and local-tests.json

Code changes due to API differences

replaced waitAndClick and waitAndType with Playwright's page.click and page.type. Removed waitAndClick and waitAndType as they are now obsolete
replaced page.waitFor with page.waitForTimeout
replaced page.setCookie() with page.browserContext().addCookies()
replaced page.$x with page.$$(xpath='locator')
replaced page.waitForXPath() with page.waitForSelector()
replaced visibility options visible: true and hidden: true with state: 'visible' and state: 'hidden'
replaced page.setViewport() with page.setViewportSize()
replaced networkidle0 and networkidle0 with networkidle
updated env variables names, like PUPPETEER_HEADLES to HEADLESS

New features

Added video capture support, under a toggle controlled by CAPTURE_VIDEO env variable. Run with CAPTURE_VIDEO=true yarn test-e2e to capture video.

Changes unrelated to migration

Grouped test output (logs, videos, screenshots) under a single directory tests/e2e/output. I was not able to move allure-results though
Test output folder is stored in Github Actions as build artefacts
Added command to clean the output dir before tests run
Other small tweaks like locators changes to make tests pass or more resilient

Does this pull request change what data or activity we track or use?

No

Testing instructions:

All E2E tests need to pass

Proposed changelog entry for your changes:

no changelog entry needed

jetpackbot · 2020-11-16T10:33:39Z

	Warnings
⚠️	`pre-commit` hook was skipped for one or more commits

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17814

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against d0068b1

tests/e2e/lib/setup-env.js

brbrr

Looks solid!

I left a few comments, and questions for changes that I can not understand.

tests/e2e/lib/flows/jetpack-connect.js

tests/e2e/lib/blocks/mailchimp.js

tests/e2e/jest-playwright.config.js

tests/e2e/lib/blocks/simple-payments.js

tests/e2e/lib/pages/wp-admin/plugins.js

tests/e2e/lib/pages/wpcom/login.js

brbrr · 2020-11-23T17:28:02Z

tests/e2e/lib/setup-env.js

+	let filePath;
+
+	try {
+		filePath = await takeScreenshot( currentBlock, name );


Why do we need to take screenshots on failures locally?

Why not? The reports with screenshots are very helpful in debugging issues.

I guess it depends on how one would debug issues locally.

I prefer running E2E tests in headful mode with E2E_DEBUG=true, which will stop the execution on failure, timeout, or exception, which is quite handy, so I can poke around and see what's happened.

tests/e2e/lib/setup-env.js

tests/e2e/specs/plugin-updater.test.js

brbrr

We are getting there! Would you mind taking a look at the E2E GH actions? They are failing with this TypeError: TypeError [ERR_INVALID_OPT_VALUE_ENCODING]: The value "logs/e2e-slack.log" is invalid for option "encoding"

tests/e2e/jest-playwright.config.js

modules/module-headings.php

tests/e2e/lib/pages/wp-admin/plugins.js

brbrr · 2020-11-24T12:53:48Z

tests/e2e/lib/setup-env.js

+	if ( captureVideo ) {
+		video = await saveVideo(
+			page,
+			path.resolve( config.get( 'testOutputDir' ), `video/video_${ new Date().getTime() }.mp4` )


Is there a way to somehow link these videos to actual tests? I guess at this moment we don't know which specific test would run, but maybe in afterAll hook we can rename the file?

I moved the video capture logic into jasmine reporter listeners, giving access to test names. All videos now meaningful names. I know that jasmine will be dropped at some point, but a new runner should provide the same functionality and we can move the video logic in there.

brbrr · 2020-11-24T12:58:18Z

tests/e2e/lib/setup-env.js

+	const userAgent = await page.evaluate( () => navigator.userAgent );
+	logger.info( `User agent: ${ userAgent }` );
+
+	if ( captureVideo ) {


To be honest, I don't think this belongs to setupBrowser. Also, since setupBrowser is being called in beforeAll and afterEach we end up with 28 video files.

I guess it does not directly related to the goal of this PR, but I'll still share my thoughts.

It would be nice to have a video per test, and save it only if test is failing (I don't think we successful runs as much)

I totally agree. But also, calling setupBrowser in an afterEach doesn't sound right to me either.
I guess I already mentioned, the entire setup could do some refactoring, maybe combined with a test runner migration. Currently my feeling is that everything is all over the place. Probably a custom environment would make most sense.
And true, not in the scope of this PR. But I will find a better place for video handling.

Solved now, see #17814 (comment)

By also moving the logic of setting user agent from setupBrowser the method remained empty, so I removed it.

brbrr · 2020-11-24T16:09:13Z

I would like to note that in this PR, tests took about 2 minutes more to complete.
For example:
7m 19s in master: https://github.com/Automattic/jetpack/runs/1448461525
9m 30s in this branch: https://github.com/Automattic/jetpack/pull/17814/checks?check_run_id=1448587774

adimoldovan · 2020-11-25T13:11:45Z

I would like to note that in this PR, tests took about 2 minutes more to complete.
For example:
7m 19s in master: https://github.com/Automattic/jetpack/runs/1448461525
9m 30s in this branch: https://github.com/Automattic/jetpack/pull/17814/checks?check_run_id=1448587774

Other runs took less, but still an average of 1 minute longer:
7m 59s: https://github.com/Automattic/jetpack/runs/1449989038?check_suite_focus=true
8m 10s: https://github.com/Automattic/jetpack/runs/1449487928?check_suite_focus=true
8m 16s: https://github.com/Automattic/jetpack/runs/1448447559?check_suite_focus=true

I wondered if video capture has any significant effect, but it seems it doesn't.

I'll take a quick look, but I think a more in depth analysis is required here and maybe optimisations in tests/setup/teardown code would be required.

brbrr · 2020-11-25T14:34:14Z

tests/e2e/lib/page-helper.js

 /**
 * Internal dependencies
 */
 import logger from './logger';
 import { execSyncShellCommand } from './utils-helper';

-/**


One of the reasons for these custom methods was to log every test action, to see what's going on. You mentioned that there is DEBUG=pw:api env variable that we can use to achieve similar behavior. I'd like to hear your take on this, and maybe how would you approach it.

Indeed those methods were logging actions, if/where they were used. Playwright's auto-wait introduces the risk of not using those methods in a consistent way.
I also believe the logged information was only useful in case you're debugging, otherwise it's polluting the log. In case of failure, the failing action is logged anyway, with element locator if relevant. With Playwright, if you want to debug, running the tests with DEBUG=pw:api will give the same result, but with more information and in a consistent way.

I've added the option under the E2E_DEBUG, so there's a single debug toggle. So now, E2E_DEBUG=true will also trigger pw:api logging.

Also, I'm not very familiar with Winston, but I'm assuming there as way to filter pw:api log entries. Then have a different debug file with only those entries. Or have a file with everything and a clean one without those entries. There are more ideas.

Co-authored-by: Brandon Kraft <[email protected]> Co-authored-by: Brad Jorsch <[email protected]> Co-authored-by: Yaroslav Kukharuk <[email protected]>

Co-authored-by: Brandon Kraft <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Brandon Kraft <[email protected]>

Bumps [https-proxy-agent](https://github.com/TooTallNate/node-https-proxy-agent) from 2.2.2 to 2.2.4. - [Release notes](https://github.com/TooTallNate/node-https-proxy-agent/releases) - [Commits](TooTallNate/proxy-agents@2.2.2...2.2.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Lazy Images: do not include js linting config in production * Update packages/lazy-images/.gitattributes Co-authored-by: Brad Jorsch <[email protected]> * Fix wrong auto-merge * Fix spacing Co-authored-by: Brad Jorsch <[email protected]>

* store: add media-source store (init) * extensions: add store * store: improve registering media source * store: add readme for media source * audio-player: move out global var from int. dep. * store/media-source: Add unit tests * store/media-source: test: Add jest-environment jsdom comment * Update extensions/store/media-source/storeDefinition.js Co-authored-by: Matthew Reishus <[email protected]> * Revert "store/media-source: test: Add jest-environment jsdom comment" This reverts commit b0dabf8. * media-source: add eslint.js file * media-source: simplify file structure. jsdoc. * store: move eslint rules to extensions folder * media-source-state: update to new structure * media-source: minor doc changes Co-authored-by: Matthew Reishus <[email protected]>

From February 2021, in order to provide feedback on pull requests, Code Scanning workflows must be configured with both `push` and `pull_request` triggers. This is because Code Scanning compares the results from a pull request against the results for the base branch to tell you only what has changed between the two. Early in the beta period we supported displaying results on pull requests for workflows with only `push` triggers, but have discontinued support as this proved to be less robust. See https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#scanning-pull-requests for more information on how best to configure your Code Scanning workflows.

- Removed empty setupBrowser

…nto test/e2e-playwright-migration # Conflicts: # .github/workflows/e2e-tests.yml # .gitignore # package.json # projects/plugins/jetpack/tests/e2e/config/default.json # projects/plugins/jetpack/tests/e2e/jest-playwright.config.js # tests/e2e/.eslintrc.js # tests/e2e/.gitignore # tests/e2e/README.md # tests/e2e/config/encrypted.enc # tests/e2e/jest.config.js # tests/e2e/lib/blocks/eventbrite.js # tests/e2e/lib/blocks/mailchimp.js # tests/e2e/lib/blocks/pinterest.js # tests/e2e/lib/blocks/simple-payments.js # tests/e2e/lib/blocks/word-ads.js # tests/e2e/lib/flows/jetpack-connect.js # tests/e2e/lib/global-teardown.js # tests/e2e/lib/logger.js # tests/e2e/lib/page-helper.js # tests/e2e/lib/pages/jetpack-connect/site-topic.js # tests/e2e/lib/pages/jetpack-connect/site-type.js # tests/e2e/lib/pages/jetpack-connect/user-type.js # tests/e2e/lib/pages/page.js # tests/e2e/lib/pages/postFrontend.js # tests/e2e/lib/pages/wp-admin/block-editor.js # tests/e2e/lib/pages/wp-admin/dashboard.js # tests/e2e/lib/pages/wp-admin/in-place-authorize.js # tests/e2e/lib/pages/wp-admin/in-place-plans.js # tests/e2e/lib/pages/wp-admin/jetpack.js # tests/e2e/lib/pages/wp-admin/login.js # tests/e2e/lib/pages/wp-admin/plugins.js # tests/e2e/lib/pages/wp-admin/sidebar.js # tests/e2e/lib/pages/wpcom/authorize.js # tests/e2e/lib/pages/wpcom/checkout.js # tests/e2e/lib/pages/wpcom/connections.js # tests/e2e/lib/pages/wpcom/login.js # tests/e2e/lib/pages/wpcom/my-plan.js # tests/e2e/lib/pages/wpcom/pick-a-plan.js # tests/e2e/lib/pages/wpcom/plans.js # tests/e2e/lib/pages/wpcom/thank-you.js # tests/e2e/lib/plan-helper.js # tests/e2e/lib/reporters/screenshot.js # tests/e2e/lib/setup-env.js # tests/e2e/lib/tunnel-manager.js # tests/e2e/lib/utils-helper.js # tests/e2e/specs/free-blocks.test.js # tests/e2e/specs/plugin-updater.test.js # tests/e2e/specs/pro-blocks.test.js # yarn.lock

adimoldovan added Trial Project [Status] In Progress E2E Tests labels Nov 16, 2020

adimoldovan requested review from kraftbj and brbrr November 16, 2020 10:27

matticbot added the Touches WP.com Files label Nov 16, 2020

kraftbj assigned adimoldovan Nov 17, 2020

brbrr reviewed Nov 23, 2020

View reviewed changes

tests/e2e/lib/setup-env.js Outdated Show resolved Hide resolved

brbrr reviewed Nov 23, 2020

View reviewed changes

brbrr reviewed Nov 24, 2020

View reviewed changes

tests/e2e/jest-playwright.config.js Outdated Show resolved Hide resolved

modules/module-headings.php Outdated Show resolved Hide resolved

tests/e2e/lib/pages/wp-admin/plugins.js Outdated Show resolved Hide resolved

brbrr reviewed Nov 24, 2020

View reviewed changes

brbrr reviewed Nov 25, 2020

View reviewed changes

ebinnion mentioned this pull request Nov 30, 2020

E2E tests migration to Playwright #17791

Closed

kraftbj and others added 13 commits January 14, 2021 09:10

Monorepo: Reorganize all projects (#18257)

e13487d

Co-authored-by: Brandon Kraft <[email protected]> Co-authored-by: Brad Jorsch <[email protected]> Co-authored-by: Yaroslav Kukharuk <[email protected]>

E2E Tests: Disable Allure reports generation (#18342)

66461a1

Co-authored-by: Brandon Kraft <[email protected]>

Reorg: Remove temp branch from executing action (#18345)

fd0a916

ESlint: Fix i18n relates issue (#18353)

01a60a8

Actions: Improve build-artifacts (#18352)

004d8cf

Bump ini from 1.3.5 to 1.3.8 (#18351)

6b0657c

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Brandon Kraft <[email protected]>

Packages: Do not include .github in dist (#18361)

e56f5f4

Changelog: add initial changelog for 9.3.1 (#18331)

669e9ad

Rename root-level project (#18367)

6f07c39

adimoldovan added 17 commits February 5, 2021 13:02

Added manual run option for e2e workflow

f2c0f8a

Use custom user agent

3826cc1

Removed workflow_dispatch for e2e workflow

d4fdf8b

[not verified] Updates following PR suggestions

b4a0fbd

Changed networkidle with domcontentloaded

b8066df

Updated user agent

e3ac8bb

Updated user agent

a8a0d81

Fixed log path

f7a4365

Fixed log path

5443f0a

Dynamic user agent

c82d1f2

Renamed video files

fcbb01a

- Moved user agent logic into before all

64535b6

- Removed empty setupBrowser

Fix open handles caused by video capture

bedf602

Clean-up remains of old waitForSelector method

5d961ef

Add DEBUG = 'pw:api' under E2E_DEBUG variable

308a652

Workflow and gitignore changes to match master

de803bb

adimoldovan requested review from gibrown and a team as code owners February 5, 2021 11:22

matticbot added the [Status] Needs Tracks Review Added/removed/modified a tracks event. label Feb 5, 2021

github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Feb 5, 2021

brbrr removed request for a team and gibrown February 5, 2021 13:45

adimoldovan added 2 commits February 5, 2021 16:21

Fixed broken dependencies and eslint issues

978db97

Unknown saveVideo and eslint errors

16eadaa

adimoldovan mentioned this pull request Feb 9, 2021

E2E tests replace Puppeteer with Playwright #18762

Merged

adimoldovan closed this Feb 9, 2021

adimoldovan deleted the test/e2e-playwright-migration branch June 21, 2021 12:39

kraftbj removed the [Status] In Progress label Oct 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2E tests migration to Playwright #17814

E2E tests migration to Playwright #17814

adimoldovan commented Nov 16, 2020 •

edited

Loading

jetpackbot commented Nov 16, 2020 •

edited

Loading

brbrr left a comment

brbrr Nov 23, 2020

adimoldovan Nov 23, 2020

brbrr Nov 24, 2020

brbrr left a comment

brbrr Nov 24, 2020

adimoldovan Nov 24, 2020

brbrr Nov 24, 2020

adimoldovan Nov 24, 2020

adimoldovan Nov 24, 2020

brbrr commented Nov 24, 2020

adimoldovan commented Nov 25, 2020

brbrr Nov 25, 2020

adimoldovan Nov 25, 2020

E2E tests migration to Playwright #17814

E2E tests migration to Playwright #17814

Conversation

adimoldovan commented Nov 16, 2020 • edited Loading

Closed in favor of 18762

Changes proposed in this Pull Request:

Added dependencies:

Removed dependencies:

Configuration changes

Code changes due to API differences

New features

Changes unrelated to migration

Does this pull request change what data or activity we track or use?

Testing instructions:

Proposed changelog entry for your changes:

jetpackbot commented Nov 16, 2020 • edited Loading

brbrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brbrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brbrr commented Nov 24, 2020

adimoldovan commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adimoldovan commented Nov 16, 2020 •

edited

Loading

jetpackbot commented Nov 16, 2020 •

edited

Loading