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

Update Puppeteer #5862

Open
2 tasks
nfmohit opened this issue Sep 16, 2022 · 10 comments
Open
2 tasks

Update Puppeteer #5862

nfmohit opened this issue Sep 16, 2022 · 10 comments
Assignees
Labels
Next Up Issues to prioritize for definition P2 Low priority QA: Eng Requires specialized QA by an engineer Team M Issues for Squad 2 Type: Infrastructure Engineering infrastructure & tooling

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Sep 16, 2022

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

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 Fix flaky E2E tests #8229), ensure that no new E2E failures or instability introduced in this issue.

QA Brief

  • As this is an infrastructure ticket, the CR and MR engineers should perform QA:Eng.

Changelog entry

@aaemnnosttv aaemnnosttv added P2 Low priority Type: Infrastructure Engineering infrastructure & tooling labels Dec 20, 2022
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Dec 23, 2023
@eugene-manuilov
Copy link
Collaborator

@tofumatt @aaemnnosttv could you please check AC and notes in IB and give me your thumb up if it looks good to you?

@aaemnnosttv
Copy link
Collaborator

@eugene-manuilov SGTM, let's give it a try 👍

@aaemnnosttv aaemnnosttv removed their assignment Sep 6, 2024
@benbowler benbowler assigned benbowler and unassigned tofumatt Nov 19, 2024
@benbowler benbowler mentioned this issue Nov 20, 2024
19 tasks
@benbowler benbowler added the QA: Eng Requires specialized QA by an engineer label Jan 14, 2025
@benbowler
Copy link
Collaborator

benbowler commented Jan 14, 2025

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).

@eugene-manuilov
Copy link
Collaborator

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.

@benbowler
Copy link
Collaborator

benbowler commented Jan 14, 2025

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 #6357 by creating a string of follow up issues "Upgrade puppeteer to the next major version".

@eclarke1 eclarke1 added the Team M Issues for Squad 2 label Jan 16, 2025
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Jan 31, 2025
@eugene-manuilov
Copy link
Collaborator

@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.

@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Feb 6, 2025
@benbowler benbowler mentioned this issue Feb 6, 2025
19 tasks
@benbowler
Copy link
Collaborator

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

@benbowler benbowler removed their assignment Feb 12, 2025
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Feb 28, 2025
@benbowler benbowler self-assigned this Mar 3, 2025
@ivonac4
Copy link
Collaborator

ivonac4 commented Mar 19, 2025

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

@benbowler
Copy link
Collaborator

@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.

@ivonac4
Copy link
Collaborator

ivonac4 commented Mar 19, 2025

Okay, cool. Will push for that one to be done in the next sprint, hopefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Next Up Issues to prioritize for definition P2 Low priority QA: Eng Requires specialized QA by an engineer Team M Issues for Squad 2 Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

7 participants