E2E tests migration to Playwright #17791

adimoldovan · 2020-11-12T16:02:45Z

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

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
Added command to clean the output dir before tests run
Other small tweaks like locators changes to make tests pass or more resilient

Left to do

Change user agent
Make all tests pass. I'm assuming that current failures are not related to the migration, but still, a confirmation is needed.
There is a lot of room for code cleanup. E.g. some logging may be obsolete now as Playwright has really good debugging logging with DEBUG=pw:api option. Possibly some methods in page-helper are also obsolete.

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

Functional pre-connection tests

- Grouped test output under a single output path

- Replaced 'networkidle0' and 'networkidle2' to 'networkidle - Replaced page.waitFor to page.waitForTimeout

- Save screenshots on failure

jetpackbot · 2020-11-12T16:06:49Z

Scheduled Jetpack release: December 1, 2020.
Scheduled code freeze: November 23, 2020

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

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 3230cd4

kraftbj · 2020-11-12T19:16:03Z

tests/e2e/lib/global-teardown.js

@@ -61,5 +60,5 @@ module.exports = async function ( globalConfig ) {
 	if ( process.env.CI ) {
 		await processSlackLog();
 	}
-	await teardown( globalConfig );
+	// await teardown( globalConfig );


Minor nitpick: Is this left here for anything in particular or missed in the cleanup?

Good catch. I actually forgot about this one, tests not complaining about it...
This was a call to a jest-puppeteer function and I was not sure what to replace it with, or now, even if it should be replaced at all.
Maybe @brbrr can help.

it coming from jest-puppeteer dependency that we using: https://github.com/smooth-code/jest-puppeteer#create-your-own-globalsetup-and-globalteardown

I guess we playwright counterpart does not need that (I'm not sure why tho)

kraftbj · 2020-11-12T19:23:54Z

tests/e2e/README.md

@@ -53,10 +53,10 @@ You can run the e2e tests locally using this command:
 yarn test-e2e
 ```

-Puppeteer runs headless by default (i.e. browser is not visible). However, sometimes it's useful to observe the browser while running tests. To see the browser window and the running tests you can pass `PUPPETEER_HEADLESS=false` as follows:
+Puppeteer runs headless by default (i.e. browser is not visible). However, sometimes it's useful to observe the browser while running tests. To see the browser window and the running tests you can pass `HEADLESS=false` as follows:


There are various references to Puppeteer still in the codebase and documentation.

Indeed, I left a lot of those in documentation blocks mostly. I pushed a change which replaced all except one for a piece o code that still needs to be evaluated if it should stay or go.

- Fixed locator for block editor.

brbrr · 2020-11-13T15:48:27Z

tests/e2e/jest.config.js

 	],
+	testRunner: 'jasmine2',


I understand that this line was added basically to make tests work. But I wonder if this is an optimal solution.

From what I gathered, with playwright we have a few options for the test runner:

jasmine2 (which is used in the current configuration)

jest-circus

playwright-test

Could you talk about the pros and cons of these as well? I know that circus supports test re-run which I find really appealing. Also I wonder what benefits playwright-test could provide.

Yes, this was a fix for an error thrown by jest-allure library. I don't really understand how come it wasn't failing before, with the Puppeteer suite of dependencies.
This was the only working solution I could find, based on this: zaqqaz/jest-allure#8
It's not ideal, it was the way to move forward while keeping the Allure reports.
I will look into other options.

Oh, I got it. I thought the source of this error was our own usage of jasmine. I guess we would need to move away from jest-allure if we'd like to switch to circus runner.

I wonder if there are alternatives to jest-allure that would work with circus runner.

ebinnion · 2020-11-30T19:51:45Z

Leaving a note that we closed this PR in favor of #17814 since this PR was created from a forked repo.

adimoldovan added 7 commits November 11, 2020 22:51

Introduced Playwright

9d4b107

Functional pre-connection tests

Functional connection tests

91aabf3

Updated waitForNavigation option and locators

df7ba7f

- Added video capture

0d4b6a8

- Grouped test output under a single output path

Changed config format from js to json

c237a89

- Removed obsolete methods waitAndType and waitAndClick

b8d733e

- Replaced 'networkidle0' and 'networkidle2' to 'networkidle - Replaced page.waitFor to page.waitForTimeout

- Clean test output before running

1159ded

- Save screenshots on failure

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

- API changes: addCookie and waitForXpath

82181ab

kraftbj requested a review from brbrr November 12, 2020 16:15

jeherve added [Status] In Progress E2E Tests labels Nov 12, 2020

kraftbj reviewed Nov 12, 2020

View reviewed changes

- Replaced Puppeteer occurrences in comments and documentation.

a562672

- Fixed locator for block editor.

brbrr added the Trial Project label Nov 13, 2020

brbrr reviewed Nov 13, 2020

View reviewed changes

Fixes for free-blocks and pro-blocks tests.

3230cd4

adimoldovan closed this Nov 16, 2020

adimoldovan mentioned this pull request Nov 16, 2020

E2E tests migration to Playwright #17814

Closed

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 #17791

E2E tests migration to Playwright #17791

adimoldovan commented Nov 12, 2020

jetpackbot commented Nov 12, 2020 •

edited

Loading

kraftbj Nov 12, 2020

adimoldovan Nov 13, 2020

brbrr Nov 13, 2020

kraftbj Nov 12, 2020

adimoldovan Nov 13, 2020

brbrr Nov 13, 2020

adimoldovan Nov 13, 2020

brbrr Nov 16, 2020

ebinnion commented Nov 30, 2020

E2E tests migration to Playwright #17791

E2E tests migration to Playwright #17791

Conversation

adimoldovan commented Nov 12, 2020

Changes proposed in this Pull Request:

Added dependencies:

Removed dependencies:

Configuration changes

Code changes due to API differences

New features

Changes unrelated to migration

Left to do

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

Testing instructions:

Proposed changelog entry for your changes:

jetpackbot commented Nov 12, 2020 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebinnion commented Nov 30, 2020

jetpackbot commented Nov 12, 2020 •

edited

Loading