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

E2E tests replace Puppeteer with Playwright #18762

Merged
merged 23 commits into from
Feb 11, 2021

Conversation

adimoldovan
Copy link
Member

@adimoldovan adimoldovan commented Feb 9, 2021

This PR replaces Puppeteer with Playwright as the browser automation library used in E2E tests.

The initial POC was part of PR 17814, closed now in favor of this one.

Changes proposed in this Pull Request:

Added dependencies:

playwright
jest-playwright-preset

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"
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
Changes unrelated to migration
  • Grouped test output (logs, videos, screenshots) under a single directory tests/e2e/output

Jetpack product discussion

n/a

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

n/a

Testing instructions:

All E2E tests need to pass

Proposed changelog entry for your changes:

no changelog entry needed

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello adimoldovan! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D56699-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link

jetpackbot commented Feb 9, 2021

Scheduled Jetpack release: February 16, 2021.
Scheduled code freeze: February 8, 2021

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 1f2ab8f

@adimoldovan adimoldovan added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Feb 9, 2021
@adimoldovan adimoldovan self-assigned this Feb 9, 2021
brbrr
brbrr previously approved these changes Feb 10, 2021
Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

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

There is a small linting issue in README file, otherwise, it looks good!

@adimoldovan adimoldovan requested a review from brbrr February 10, 2021 10:48
brbrr
brbrr previously approved these changes Feb 10, 2021
if ( ! CI ) {
executablePath = '/Applications/Google Chrome.app/Contents/MacOS/Google Chrome';
}
let { E2E_DEBUG, HEADLESS, SLOWMO } = process.env;

if ( E2E_DEBUG ) {
process.env.DEBUG = 'pw:browser|api|error';
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this doesn't really work. This code works in a jest's setup process, while tests are running in separate processes. And I believe the environment is not shared between these processes.

Try accessing DEBUG value in one of the tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually it works. Not sure how, they're probably shared or inherited.

Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

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

LGTM!

@adimoldovan adimoldovan merged commit f6a0245 into master Feb 11, 2021
@adimoldovan adimoldovan deleted the test/e2e-tests-migrate-to-playwright branch February 11, 2021 13:06
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 11, 2021
@github-actions github-actions bot added this to the 9.4.1 milestone Feb 11, 2021
@jeherve jeherve modified the milestones: 9.4.1, 9.5 Feb 11, 2021
@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Needs Changelog labels Feb 11, 2021
@brbrr
Copy link
Contributor

brbrr commented Feb 11, 2021

r220908-wpcom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Tests Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants