-
Notifications
You must be signed in to change notification settings - Fork 319
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
Bump puppeteer from 19.11.1 to 20.1.1 #3589
Bump puppeteer from 19.11.1 to 20.1.1 #3589
Conversation
Looks like the |
@romaricpascal Lots has changed! Let's see if my push works. Good to see Percy is matching the same version now too: ├─┬ govuk-frontend-helpers@ -> ./shared/helpers
│ ├─┬ @axe-core/puppeteer@4.6.1
│ │ └── puppeteer@20.1.1 deduped
│ └── puppeteer@20.1.1 deduped
├─┬ govuk-frontend-tasks@ -> ./shared/tasks
│ ├─┬ @percy/puppeteer@2.0.2
│ │ └── puppeteer@20.1.1 deduped
│ └── puppeteer@20.1.1 deduped
├─┬ govuk-frontend@ -> ./packages/govuk-frontend
│ └── puppeteer@20.1.1 deduped
├─┬ jest-puppeteer@8.0.6
│ └── puppeteer@20.1.1 deduped
└── puppeteer@20.1.1
Confirmed. The following PR replaced |
2e396cc
to
61428d4
Compare
61428d4
to
663cb3c
Compare
const browser = Browser.CHROME | ||
const platform = detectBrowserPlatform() | ||
|
||
// Download management | ||
const cacheDir = join(paths.root, '.cache', 'puppeteer') | ||
const cache = new Cache(cacheDir) | ||
|
||
// Downloaded versions | ||
// @ts-expect-error 'defaultBrowserRevision' is marked @internal | ||
const latest = puppeteer.defaultBrowserRevision | ||
const versions = fetcher.localRevisions() | ||
const buildId = await resolveBuildId(browser, platform, 'stable') | ||
const installed = cache.getInstalledBrowsers() | ||
|
||
// Download latest browser (unless cached) | ||
if (!versions.includes(latest)) { | ||
await fetcher.download(latest) | ||
if (!installed.some((install) => install.buildId === buildId)) { | ||
await cache.clear() | ||
|
||
// Remove outdated browser versions | ||
for (const version of versions) { | ||
await fetcher.remove(version) | ||
} | ||
// Install into cache directory | ||
await install({ browser, buildId, cacheDir }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, that's becoming much more involved to download the browsers 😬 Thanks for digging how to use the new API 😊
question Random thought (which you've likely explored already anyway): any chance we could leave that complexity back in the hands of Puppeteer by calling their CLI for installing Chrome?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully one day
I've treated this PR as "like for like" so we don't break anything, but whilst it appears from npm ls puppeteer
that Percy is using the same puppeteer
version it's actually downloading its own version still:
[percy] Downloading Chromium 929511...
[percy] Successfully downloaded Chromium 929511
We only really started downloading browsers ourselves because:
- Puppeteer stopped clearing cached downloads for previous versions
- Puppeteer cache on GitHub Actions started to grow and grow
- Percy downloads an older release of Chromium
Tomorrow I'll probably investigate await cache.clear()
so both Puppeteer (Chrome for Testing) and Percy (Chromium) can coexist without deleting each other's caches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romaricpascal Did some more testing and we're good to go
Percy is using the new Puppeteer v20 browser already (Chrome for Testing) via @percy/puppeteer for screenshots, and I've followed the steps in Skipping the asset discovery browser download to stop it downloading its own
Ready when you are
663cb3c
to
868efd9
Compare
86b4502
to
3771806
Compare
3771806
to
7605033
Compare
@@ -16,6 +16,8 @@ jobs: | |||
env: | |||
PERCY_PARALLEL_NONCE: ${{ github.run_id }}-${{ github.run_attempt }} | |||
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }} | |||
PERCY_POSTINSTALL_BROWSER: false | |||
PUPPETEER_SKIP_DOWNLOAD: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevents npm ci
from downloading ~140MB of browser when running manually:
https://github.com/alphagov/govuk-frontend/actions/workflows/screenshots.yml
@@ -16,6 +16,8 @@ jobs: | |||
env: | |||
PERCY_PARALLEL_NONCE: ${{ github.run_id }}-${{ github.run_attempt }} | |||
PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }} | |||
PERCY_POSTINSTALL_BROWSER: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevents Percy downloading Chromium:
[percy] Downloading Chromium 929511...
[percy] Successfully downloaded Chromium 929511
It can be skipped because we already download via Puppeteer:
govuk-frontend/shared/tasks/browser.mjs
Line 15 in 7605033
await download() |
Bumps [puppeteer](https://github.com/puppeteer/puppeteer) from 19.11.1 to 20.1.1. - [Release notes](https://github.com/puppeteer/puppeteer/releases) - [Changelog](https://github.com/puppeteer/puppeteer/blob/main/release-please-config.json) - [Commits](puppeteer/puppeteer@puppeteer-v19.11.1...puppeteer-v20.1.1) --- updated-dependencies: - dependency-name: puppeteer dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Puppeteer v20 now downloads Chrome (not Chromium) by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to go for me, only got a little nitpick on the naming of the percy config to make things a little more aligned with the rest of our configs (pre-approving as it's just a tiny renaming) 😊 Thanks for digging into both tools to get this through 🙌🏻
.percy.js
Outdated
* | ||
* @type {import('@percy/config').PercyConfigObject} | ||
*/ | ||
module.exports = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick Looks like Percy accepts percy.config.js
as a config file, it would align with the format of most our config files so I'd be keen to prefer that (especially as they didn't go for the .<PROJECT_NAME>rc.js
convention and just .<PROJECT_NAME>.js
) 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Done
Percy already uses the default Puppeteer browser for taking screenshots, but it was downloading another browser for asset discovery See: https://docs.percy.io/docs/skipping-asset-discovery-browser-download ``` [percy] Downloading Chromium 929511... [percy] Successfully downloaded Chromium 929511 ```
7605033
to
d915dfc
Compare
Bumps puppeteer from 19.11.1 to 20.1.1.
Release notes
Sourced from puppeteer's releases.
... (truncated)
Commits
3a6569e
chore: release main (#10137)9a1aff8
chore: implement locators with a click (#10009)9758cae
fix: rename PUPPETEER_DOWNLOAD_HOST to PUPPETEER_DOWNLOAD_BASE_URL (#10130)9e21d30
chore: update typescript (#10135)4615607
chore: add test console after navigation (#10110)3fb8135
docs: should keep track of README changes (#10133)6dbd53b
docs: remove a link to browserfetcher (#10129)7358e9e
refactor: use http-proxy-agent (#10132)8d08b2f
chore: update CfT Dashboard URLs (#10122)ead413b
chore: release main (#10117)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)