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

Future: Replace Puppeteer with Playwright #4949

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

birkskyum
Copy link
Member

@birkskyum birkskyum commented Oct 31, 2024

Ongoing work on improving the test suite.

Initially we had Selenium in Query / Browser suite, and custom mocks of webgl1 in Render suite.

February 2022 - Browser, Query: Selenium to Playwright

May 8th 2023 - Browser, Query: Playwright to Puppeteer

Google chrome made a splash. Until this point headless and headful chrome were two different browsers, and now they cleaned it up so the headless became a "real chrome", which is a lot better. Only puppeteer supported this initially, so we switched to be able to leverage it in (query + browser):

May 9th 2023 - Render: Custom Webgl1 Mocks to Puppeteer:

Now - Browser, Query, Render: Puppeteer to Playwright

This is now old history, and since, playwright has due to a nicer dx, and multi-browser support (firefox / webkit / chromium), managed to become the No. 1 browser runner, and it's possible to it thoughout.

As an example of the better DX, this is i.e. a diff from browser.test.ts:

Puppeteer:

await page.emulateMediaFeatures([
    {name: 'prefers-reduced-motion', value: 'reduce'},
]);

Playwright:

await page.emulateMedia({reducedMotion: 'reduce'});
Screenshot 2024-10-31 at 18 51 53

Rendet test HTML reporting works - here a full run on a mac.

Screenshot 2024-10-31 at 20 01 09

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!

@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.81%. Comparing base (d3f2bca) to head (38cc518).
Report is 4 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (d3f2bca) and HEAD (38cc518). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d3f2bca) HEAD (38cc518)
9 8
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4949       +/-   ##
===========================================
- Coverage   90.48%   71.81%   -18.68%     
===========================================
  Files         265      265               
  Lines       38113    38113               
  Branches     3166     2607      -559     
===========================================
- Hits        34488    27372     -7116     
- Misses       2650     9490     +6840     
- Partials      975     1251      +276     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HarelM
Copy link
Collaborator

HarelM commented Oct 31, 2024

Looks like a good direction, let's make sure the coverage is working as expected as I had to fight with it back then...

@birkskyum
Copy link
Member Author

@HarelM , how to I check the coverage? is it specific to the render test suite?

@HarelM
Copy link
Collaborator

HarelM commented Oct 31, 2024

If this is only a refactoring, the coverage should stay the same between main branch and this one.
You can open an empty PR and compare the report in codecov, probably selectively.

@birkskyum birkskyum changed the title Replace Puppeteer with Playwright Future: Replace Puppeteer with Playwright Oct 31, 2024
@birkskyum
Copy link
Member Author

It seems like the render tests take 2.5x longer to run with the current state of this PR (26 min. vs ~ 11 min), so that has to be reduced significantly before this PR can be merged.

@HarelM
Copy link
Collaborator

HarelM commented Oct 31, 2024

Yes, that's a concern...

@birkskyum
Copy link
Member Author

birkskyum commented Oct 31, 2024

The performance overhead appear to come from improved isolation / cross-browser architecture with some abstractions layers - it just is a bit steep price. I'll revisit this when the vitest browser mode is stable, as it might bring forward some options for performance improvements:

@birkskyum birkskyum mentioned this pull request Oct 31, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants