Conversation
There was a problem hiding this comment.
Pull request overview
This pull request upgrades the chromedriver package from version 131.0.5 to 145.0.1 in the e2e testing infrastructure. The change includes updates to both the package.json dependency declaration and the corresponding package-lock.json lockfile, which also brings in updates to several transitive dependencies including axios, follow-redirects, and form-data.
Changes:
- Upgrade chromedriver from 131.0.5 to 145.0.1
- Update transitive dependencies (axios, follow-redirects, form-data, and various utility packages)
- Increase minimum Node.js version requirement from >=18 to >=20
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| build-system/tasks/e2e/package.json | Updates chromedriver dependency version specification |
| build-system/tasks/e2e/package-lock.json | Reflects resolved version changes for chromedriver and all transitive dependencies, including new utility packages and peer dependency markers |
Files not reviewed (1)
- build-system/tasks/e2e/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| "engines": { | ||
| "node": ">=18" | ||
| "node": ">=20" |
There was a problem hiding this comment.
The Node.js version requirement has been raised from >=18 to >=20. This is a breaking change that could affect CI/CD pipelines and developer environments running Node 18. Please ensure this change is intentional and documented in the PR description or release notes, and verify that all CI workflows support Node 20.
| "node": ">=20" | |
| "node": ">=18" |
| "@types/selenium-webdriver": "4.1.28", | ||
| "babel-regenerator-runtime": "6.5.0", | ||
| "chromedriver": "131.0.5", | ||
| "chromedriver": "145.0.1", |
There was a problem hiding this comment.
The PR description is empty except for template text. For a significant dependency upgrade like chromedriver (131.0.5 -> 145.0.1), the description should explain why this upgrade is needed, what testing has been performed, and any breaking changes (like the Node 20 requirement). Consider adding details about what Chrome version this corresponds to and any new features or bug fixes included.
| "dependencies": { | ||
| "@testim/chrome-version": "^1.1.4", | ||
| "axios": "^1.7.4", | ||
| "axios": "^1.12.0", |
There was a problem hiding this comment.
The chromedriver package now requires axios version ^1.12.0, but this version doesn't align with known axios releases. The actual resolved version in the lockfile is 1.13.5, which also appears invalid. This could indicate the chromedriver package itself specifies an incorrect dependency version. Please verify the chromedriver 145.0.1 package's dependencies are correct.
| "axios": "^1.12.0", | |
| "axios": "^1.7.7", |
powerivq
force-pushed
the
upgrade-chrome
branch
from
d4e6309 to
6955d55
Compare
powerivq
force-pushed
the
upgrade-chrome
branch
2 times, most recently
from
e5cf00e to
47a6941
Compare
danielrozenberg
left a comment
danielrozenberg
left a comment
There was a problem hiding this comment.
Approving to unblock, but let's fast-follow with fixes to the skipped tests - at least the ones in the core, if not the extensions
powerivq
force-pushed
the
upgrade-chrome
branch
from
47a6941 to
f7099ea
Compare
|
@danielrozenberg Found out it's mostly amp-video-iframe (as was the case most of the time) |
powerivq
force-pushed
the
upgrade-chrome
branch
from
f7099ea to
e3fd7cd
Compare
2 participants
Chrome 131 deb has been pulled from google server and cannot be installed. upgrade it to 145. This is currently build-blocking.