Scripts: Update Puppeteer to v13

[simonhammes]

Description

@wordpress/scripts

• Updated Puppeteer to v13

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• 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 (please manually search all *.native.js files for terms that need renaming or removal).

[gziolo]

Thabk you for starting this PR 💯

I found it very surprising that there is already v13 available:

https://github.com/puppeteer/puppeteer/releases

It looks like they made a typo in the API, and it needed to be fixed.

This branch has some failing e2e tests, but I hope that it’s something that a simple rebase with trunk would fix.

[simonhammes]

Most of the failing E2E-Tests are likely related to this upstream bug in puppeteer(-core): puppeteer/puppeteer#7836.

[gziolo]

Most of the failing E2E-Tests are likely related to this upstream bug in puppeteer(-core): puppeteer/puppeteer#7836.

Thank you for reporting back to the Puppeteer project 💯 We need to wait until the issue gets resolved before we can land changes. On our end everything is close to ready 👍🏻

[gziolo]

It looks like the newer version of Puppeteer fixed the bug we observed. However, I see some other test failures and it's hard to tell if that's something that happens in the trunk or we need to tweak those tests.

[gziolo]

@kevin940726, can you help debug the failing e2e tests for the widgets? It looks like there is something working differently in Puppeteer 13.x.

[kevin940726]

Hi! Thank you for making this PR! I haven't had the time to see what's going on in the widgets tests though but will take a look.

Might be worth noting that there's currently an ongoing discussion about migrating from puppeteer to playwright. Hence maybe this upgrade isn't needed unless it fixes some existing bugs.

[gziolo]

Might be worth noting that there's currently an ongoing discussion about migrating from puppeteer to playwright. Hence maybe this upgrade isn't needed unless it fixes some existing bugs.

Thank you for the update on the plans regarding migration to Playwright.

The more recent versions of puppeteer update bundled chromium from v93 to v98. Given that WordPress supports "Last 2 Chrome versions", it would be great to run tests using the same environment that real users have.

Would it be acceptable to skip those failing tests for widgets until you have time to look at them?

[kevin940726]

Would it be acceptable to skip those failing tests for widgets until you have time to look at them?

I don't think that's a good idea. puppeteer-testing-library relies on some internal APIs in chromium to query the elements, so it could break if chromium upgrades to a major version. However, it's very unlikely that a major version of chromium would break the web and make any of our existing code fail. (Note that we'll likely deprecate puppeteer-testing-library and migrate to other solutions while migrating to playwright.)

If we skip the tests now, then we're risking bugs to appear in the meantime. On the contract, if we don't upgrade chromium now, we are still going to upgrade to the latest version when migrating to playwright anyway.

Also worth mentioning that "last 2 chromium versions" is not quite the same as "last 2 chrome versions". I don't see much value in skipping tests to upgrade to the most updated versions.

[kevin940726]

Just did some digging and found the issue in #38195. Fortunately isn't that complex 😅 .

[gziolo]

Just did some digging and found the issue in #38195. Fortunately isn't that complex 😅 .

Awesome, the patch has landed 👍

[gziolo]

We have one failing e2e test to fix and we can land this PR: https://github.com/WordPress/gutenberg/runs/4941792118?check_suite_focus=true

I’m wondering if that test is stable in trunk.

[gziolo]

I added 82dc5c8 to try whether I can fix the failing e2e tests. I added a line that waits until the image finishes uploading.

I'm testing locally with npm run test-e2e -- --puppeteer-interactive packages/e2e-tests/specs/editor/blocks/cover.test.js to check how it works in the browser.

Edit: It didn't help. It needs further investigation.

[gziolo]

@Antonio-Laguna opened another PR #38539 that updates Puppeteer to the latest version to fix some sort of vulnerability. Let’s land this one with the latest version of Puppeteer and temporarily skip the failing test if we don’t find quickly a solution.

[kevin940726]

Let’s land this one with the latest version of Puppeteer and temporarily skip the failing test if we don’t find quickly a solution.

I don't think it's a good idea. The vulnerability seems to only affect nofe-fetch when used in user-facing code, but it's just a dependency in puppeteer and we're only using it for testing. To me, the cost of skipping working tests is much higher than not upgrading puppeteer now. Better take our time to fix the failing tests before upgrading puppeteer.

[gziolo]

@simonhammes, there is now https://www.npmjs.com/package/puppeteer/v/13.1.3 available.

[gziolo]

I finally made the test pass. It was hard to address because it was passing locally with the default speed, but I managed to reproduce it when the speeding up all actions with --puppeteer-slowmo=1 flag passed to the CLI call.