Update Puppeteer to version 23.9.0 #19115
Conversation
calixteman
force-pushed
the
puppeteer_23.9.0
branch
2 times, most recently
from
294da5f to
b187b38
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/b0c4665128bc512/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/d81433e824e9bb0/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/b0c4665128bc512/output.txt Total script time: 31.86 mins
Image differences available at: http://54.241.84.105:8877/b0c4665128bc512/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d81433e824e9bb0/output.txt Total script time: 49.80 mins
Image differences available at: http://54.193.163.58:8877/d81433e824e9bb0/reftest-analyzer.html#web=eq.log |
There was a problem hiding this comment.
There are many reported reftest failures in both Firefox and Chrome. I spot checked a few failures, and there are barely observable differences. Perhaps we should have a way to temporarily increase the tolerance to pixel differences for these scenarios to make it easier to quickly tell whether the differences warrant further attention. As long as there are no egregious errors, it should be ok to ignore them.
Between the original version and the proposed version Chrome jumped 3 major versions and Firefox 2 (soon 3 too since Firefox's major version bumped this week).
Old puppeteer version's referenced browsers: https://github.com/puppeteer/puppeteer/blob/puppeteer-v23.3.1/packages/puppeteer-core/src/revisions.ts#L11
chrome: '128.0.6613.137',
'chrome-headless-shell': '128.0.6613.137',
firefox: 'stable_130.0',
New puppeteer version's referenced browsers: https://github.com/puppeteer/puppeteer/blob/puppeteer-v23.9.0/packages/puppeteer-core/src/revisions.ts#L11
chrome: '131.0.6778.85',
'chrome-headless-shell': '131.0.6778.85',
firefox: 'stable_132.0.2',
| @@ -45,7 +45,7 @@ | |||
| "postcss-discard-comments": "^7.0.3", | |||
| "postcss-nesting": "^13.0.1", | |||
| "prettier": "^3.3.3", | |||
| "puppeteer": "23.3.1", | |||
| "puppeteer": "^23.9.0", | |||
There was a problem hiding this comment.
| "puppeteer": "^23.9.0", | |
| "puppeteer": "23.9.0", |
I know that @Snuffleupagus asked whether the range can be more specific at #19115 (comment) with the following question:
Can we "relax" the version requirements now, since we only locked it down previsouly because of the integration-test failures?
The answer is No, we should not relax the version. Puppeteer hardcodes browser versions and bumps their versions once in a while. Every browser update may potentially result in rendering differences, and consequently result in reftest failures.
If we were to use imprecise versions here, then a PDF.js PR may pass tests today and fail tomorrow, without any changes to the code in the PR. That is a debugging nightmare.
Therefore, we should continue to lock to specific Puppeteer versions and bump the version once in a while, and update all reftests at once.
There was a problem hiding this comment.
If we were to use imprecise versions here, then a PDF.js PR may pass tests today and fail tomorrow, without any changes to the code in the PR. That is a debugging nightmare.
I don't believe that's an issue though, since we have a package-lock.json file that should still guarantee that the versions cannot just change randomly!?
There was a problem hiding this comment.
I think so too. package.json only defines a range, and package-lock.json pins it to a fixed version for a reproducible environment; the latter should ensure that we can't be in the mentioned situation where tests pass one day and fail on another. We previously only pinned it in package.json because we found out that newer versions would actually break, so in that case there was a specific use case (see #18773 for the details), but in general we want to encourage updating to newer versions and it's normally also safe to do so. Note that bumping versions is already done explicitly using PRs, so if the reference tests fail due to rendering difference we'll always resolve that before actually merging the PR.
In short, I also don't see why we shouldn't be able to use the ^ range specifier here because I can't see a case where the tests would suddenly break without us noticing in PRs that go through review, or is there something I'm missing here?
There was a problem hiding this comment.
The use of package-lock.json would indeed avoid the scenario I mentioned before (PRs breaking), if npm ci is used to install dependencies, which is indeed the case, e.g
as seen at https://github.com/mozilla/botio-files-pdfjs/blob/5f24d5c1fee89fdf91cf5a8e73d5f1058439ad75/on_cmd_browsertest.js#L6
But unless someone bumps the versions in package-lock.json, CI will NOT use recent browser versions. This seems not a concern based on your comments, particularly:
Note that bumping versions is already done explicitly using PRs,
Assuming that you are referring to bumping versions in package-lock.json. If you are not regularly bumping puppeteer versions in package-lock.json, then tests will be run with older browser versions, without test coverage in the latest browser versions.
There was a problem hiding this comment.
Yes, this analysis is correct. We are regularly (usually bi-weekly) bumping versions in package-lock.json. Puppeteer was the only exception for this process because of #18773, but this PR should fix that issue so that it'll be taken along in the regular version bump rounds.
calixteman
force-pushed
the
puppeteer_23.9.0
branch
from
b187b38 to
dc453ea
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://54.241.84.105:8877/8125cc81405171b/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://54.193.163.58:8877/77f4646b76f8543/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/8125cc81405171b/output.txt Total script time: 32.36 mins
Image differences available at: http://54.241.84.105:8877/8125cc81405171b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/77f4646b76f8543/output.txt Total script time: 49.39 mins
|
|
It would be interesting to test this again, rebased to Also, can we please "unlock" the Puppeteer version number as discussed in #19115 (comment) and #19115 (comment)? |
calixteman
force-pushed
the
puppeteer_23.9.0
branch
from
dc453ea to
40ae3d3
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/ac6ffa1a919798a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/2b68ebe87832a12/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/ac6ffa1a919798a/output.txt Total script time: 29.44 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/2b68ebe87832a12/output.txt Total script time: 52.39 mins
|
|
The failures in the runs above are also ones I saw locally most of the time, and while some of them are intermittent some of them are also permafailing with the new Puppeteer versions. I'll try to have a look at this during the weekend to see if I can reproduce them locally and to attempt to find a fix because I do think we need to get them fixed before merging this to not have noise in the test results. |
calixteman
force-pushed
the
puppeteer_23.9.0
branch
from
40ae3d3 to
4c2d06c
Compare
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/37bdb5ea1ec2179/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/fb2a16c4224fca3/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/37bdb5ea1ec2179/output.txt Total script time: 11.16 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/fb2a16c4224fca3/output.txt Total script time: 24.18 mins
|
|
Superseded by PR #19208. |
No description provided.