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 deprecated use of Puppeteer's page.waitFor #3236

Closed
aaemnnosttv opened this issue Apr 28, 2021 · 4 comments
Closed

Update deprecated use of Puppeteer's page.waitFor #3236

aaemnnosttv opened this issue Apr 28, 2021 · 4 comments
Labels
P1 Medium priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Infrastructure Engineering infrastructure & tooling

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Apr 28, 2021

Feature Description

v5 of Puppeteer has deprecated its waitFor function. We should update our use accordingly to avoid problems later when it is removed, but also to avoid extra noise in tests from this warning.

This function is still not removed yet, even in the latest v9 of Puppeteer so it isn't an emergency.

See also WordPress/gutenberg#28093


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

  • No new tests required.

Visual Regression Changes

  • N/A.

QA Brief

  • E2E tests as well as VRT/BackstopJS should not log deprecation messages about page.waitFor calls. You can check the PR or a develop GitHub Action run of E2E tests to verify this.

Changelog entry

  • N/A
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Infrastructure Engineering infrastructure & tooling labels Apr 28, 2021
@aaemnnosttv aaemnnosttv added the Good First Issue Good first issue for new engineers label Apr 28, 2021
@tofumatt tofumatt self-assigned this Apr 29, 2021
@tofumatt tofumatt added the QA: Eng Requires specialized QA by an engineer label Apr 29, 2021
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Apr 29, 2021
@aaemnnosttv aaemnnosttv self-assigned this May 13, 2021
@aaemnnosttv
Copy link
Collaborator Author

Upgrade Puppeteer in package.json so it includes these new methods

I don't think we need to add puppeteer as a top-level dependency, in fact, we just removed it as one in a recent issue. Did you find that this was necessary? The new method is available since v5.3 so it looks like our version already meets that.

While the above covers most of the issue, it looks like a few E2E tests need fixing, see errors here

If we do need to upgrade puppeteer first, it might be a good idea to create a new issue for upgrading @wordpress/scripts first which would also be good in preparation for #3340 as well.

@aaemnnosttv aaemnnosttv assigned tofumatt and unassigned aaemnnosttv May 14, 2021
@danielgent
Copy link
Contributor

@aaemnnosttv There already is an issue for upgrading wp-scripts and I'm already doing the IB for it (Sadly github actions being down means I can't continue, but I've got upgrades working up to v14) #3033

@fhollis fhollis added this to the Sprint 59 milestone Sep 22, 2021
@tofumatt tofumatt removed their assignment Sep 29, 2021
@tofumatt tofumatt removed the Good First Issue Good first issue for new engineers label Sep 29, 2021
@aaemnnosttv aaemnnosttv self-assigned this Sep 30, 2021
@aaemnnosttv
Copy link
Collaborator Author

  • Replace page.waitFor [...] with page.waitForDelay.

@tofumatt waitForDelay isn't a valid method, this should be waitForTimeout instead.

This looks good to go otherwise so I'll rename this one detail in the IB.

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Sep 30, 2021
@eugene-manuilov eugene-manuilov self-assigned this Oct 1, 2021
@fhollis fhollis modified the milestones: Sprint 59, Sprint 60 Oct 11, 2021
@fhollis fhollis added the Rollover Issues which role over to the next sprint label Oct 11, 2021
@fhollis fhollis removed this from the Sprint 60 milestone Oct 25, 2021
@fhollis fhollis added this to the Sprint 61 milestone Oct 25, 2021
@fhollis fhollis added Rollover Issues which role over to the next sprint and removed Rollover Issues which role over to the next sprint labels Oct 25, 2021
@FlicHollis FlicHollis added Rollover Issues which role over to the next sprint and removed Rollover Issues which role over to the next sprint labels Nov 8, 2021
@eclarke1 eclarke1 removed this from the Sprint 61 milestone Nov 8, 2021
@eugene-manuilov eugene-manuilov removed their assignment Nov 15, 2021
@tofumatt tofumatt assigned tofumatt and eugene-manuilov and unassigned tofumatt Nov 17, 2021
@tofumatt tofumatt removed their assignment Nov 18, 2021
@techanvil techanvil self-assigned this Nov 26, 2021
@techanvil
Copy link
Collaborator

QA ✅

Confirmed on recent runs of the VRT and E2E tests that no warnings about deprecated waitFor calls are being logged.

@techanvil techanvil removed their assignment Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

9 participants