Update Puppeteer
Feature Description
While working with #5154, we noticed that the Puppeteer version being used is quite outdated. As a result, we're not able to use new methods like waitForNetworkIdle which were introduced in the newer versions. We should try to update Puppeteer (and preferably its related packages) to the latest version without breaking anything.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- The puppeteer dependency is upgrade to its latest version if possible. If impossible, then it should be updated to a version that we can without rewriting a lot of tests.
Implementation Brief
- [ ] Once #10013 is merged, update the packages
jest-puppeteerandpuppeteerto the latest version. - [ ] Address any issues causing E2E tests to break:
- TODO: IB author should check issues that this causes within the E2E test suite to assess work needed to fix in a draft PR.
Related fix
- [ ] Remove the console ignore for blockEditor.transformStyles in
tests/e2e/config/bootstrap.js(details in #11117)
Test Coverage
- E2E tests should be passing reliably in develop at the point of working on this issue (Some known issues are tracked through tickets such as https://github.com/google/site-kit-wp/issues/8229), ensure that no new E2E failures or instability introduced in this issue.
QA Brief
Changelog entry
@tofumatt @aaemnnosttv could you please check AC and notes in IB and give me your thumb up if it looks good to you?
@eugene-manuilov SGTM, let's give it a try 👍
After testing this it's definitely not simple to jump straight to the latest version of puppeteer. However splitting the E2E infrastructure unlocks the upgrade pathway.
I suggest we take the approach of #6357, by creating tickets to upgrade to the next version only, and once finishing each version upgrade, opening a new ticket to work on the next major version upgrade.
Merging the attached PR, will get us to these versions:
"jest-puppeteer": "^6.2.0",
"puppeteer": "^14.4.1"
It also adds the ability to set a new node version in tests/e2e/.nvmrc, which is honoured within the E2E GitHub also (even though currently we can use node 14 still).
I think this ticket will be addressed when we work on the #10013. Perhaps we should close this one or priorities that one to complete first and then decide whether to close this one.
Hey @eugene-manuilov, based on the comment on the IB, the test PR #10020 does successfully split the E2E package to it's own package.json and updates the E2E workflow to be able to switch node versions and install it's own packages and run tests in this location.
If we close this issue I suggest linking the PR #10020 in a comment or the description as this could shortcut some of the requirements in that ticket.
Also in the experimentation in this ticket I found that with a separate package.json we can upgrade the packages as follows without all E2E tests failing:
"jest-puppeteer": "^6.2.0",
"puppeteer": "^11.0.0"
So we could do that there then create work as we are doing in https://github.com/google/site-kit-wp/issues/6357 by creating a string of follow up issues "Upgrade puppeteer to the next major version".
@benbowler
Also in the experimentation in this ticket I found that with a separate package.json we can upgrade the packages as follows without all E2E tests failing:
"jest-puppeteer": "^6.2.0", "puppeteer": "^11.0.0"
Do you remember why you couldn’t upgrade it to the latest version? I think we need to aim upgrading it to the latest version in one go rather than fixing issues with intermediate versions.
Some thoughts to IB author:
Since we use puppeteer as a direct dependency for e2e tests only, we can probably decouple e2e related scripts and dependencies and put them within e2e tests folder as a separate package.json file. In this case, these dependencies won't conflict with the dependencies that we need for development and storybook stories and upgrade can go smoother. Of course, this will require updating e2e workflows to account for the new location of e2e node modules.
This is not needed in IB, it was for you and wasn't supposed to appear in the final IB. Please, remove it.
If we close this issue I suggest linking the PR 10020 in a comment or the description as this could shortcut some of the requirements in that ticket.
No need to close this one, let's keep it to fix the problem.
I'm un-assigning myself as the blockers are unlikely to be complete before my imminent parental leave.
So far while we still have shared dependencies such as @storybook/addon-storyshots in the root package.json we will have upgrade issues for puppeteer hence why I've added #10092 and others as blockers here. Hopefully with #10082 completed we can get past npm 8 and don't encounter this issue
Hey @benbowler , still planning to work on this one? If not, could you please unassign your self so someone from Team M can pick it up? Thank you
@ivonac4 yes. It's waiting on the node upgrade ticket as many of these upgrade tickets are. Hence why I've not updated it yet.
Okay, cool. Will push for that one to be done in the next sprint, hopefully.
@ankitrox you can take this over if you like.