1942260 - Update the version of Node for tests to use at least Node 22

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, task)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The current version 18 of node that is used on test machines will reach its support end by May 2025. We should consider to upgrade node to at least version 20 LTS. Given that there is as well version 22 LTS available should we upgrade directly to the latest one?

We need it for running newer Puppeteer tests (they actually rely on 22LTS now) and for now we have a workaround in place to fallback to tsx instead of using the new --experimental-strip-types feature. Also when Node 18 reaches its end Puppeteer will stop supporting this version of Node.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Mike, if you could give some feedback that would be appreciated. Thanks!

Comment 2

[Mike Hommey [:glandium]]

I'm not sure what feedback you're looking for.

Comment 3

[Mark Banner (:standard8)]

Node 22 would be more useful unless there's objections/issues. We just hit a case in bug 1939658, which would need Node 22.1.0.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Mike Hommey [:glandium] from comment #2)

I'm not sure what feedback you're looking for.

I think that the question in comment 0 was clear enough (there is nothing else on this bug otherwise)?

But as Mark now stated as well, maybe we should skip version 20 (LTS) and directly upgrade to 22 (LTS)?

Comment 5

[Mike Hommey [:glandium]]

The only question I see in comment 0 is "Given that there is as well version 22 LTS available should we upgrade directly to the latest one?" and I don't have any insight wrt that.

Comment 6

[Kershaw Chang [:kershaw]]

FYI: We plan to test WebTransport over HTTP/2 with fallback components, which also requires Node.js version 22.
Thanks.

Comment 7

[Kershaw Chang [:kershaw]]

Hi Mike,

Any chance you could make this happen a bit sooner?
Or could you guide me on how to do this?

Thanks.

Comment 8

[Mike Hommey [:glandium]]

You're free to submit a patch editing taskcluster/kinds/toolchain/node.yml and taskcluster/kinds/fetch/toolchains.yml appropriately.

Comment 9

[Julian Descottes [:jdescottes]]

Updating the summary to reflect that we would rather directly update to 22.

Comment 10

[Kershaw Chang [:kershaw]]

Apparently, we need to build node 22 from source (like what we did for node 18).
Unfortunately, I got this build failure and I have no idea how to fix it.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I don't know much about C++ but searching a bit on the web opens the question for me if common_reference_t is a c++20 concept feature. GCC 11.4 doesn't seem to support C++20 by default. Would maybe using the -std=c++20 compiler flag be an option to get it working? But just a shot in the dark...

Comment 12

[Mike Hommey [:glandium]]

Attachment: Bug 1942260 - Build node 22 for Linux from source.

Comment 13

[Mike Hommey [:glandium]]

The attached patch (and patch queue below that) should get you started. You may need to adjust the max-run-time for the task.

Comment 14

[BugBot [:suhaib / :marco]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:glandium, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 15

[Mike Hommey [:glandium]]

The attached patch is not enough to address comment 0. It's a stepping stone for someone to test the new version of node.

Comment 16

[Kershaw Chang [:kershaw]]

(In reply to Mike Hommey [:glandium] from comment #15)

The attached patch is not enough to address comment 0. It's a stepping stone for someone to test the new version of node.

Sorry for the delay.

I think we should land that patch and resolve this bug. I'll file another bug for using node 22 in xpcshell-test.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I'm happy to see this patch landed as is, and we can file a separate bug as well for getting puppeteer tests upgraded to Node 22.

Mike, or do you want to see all tasks that use Node 18 upgraded at once unless they show failures?

Comment 18

[Mark Banner (:standard8)]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #17)

I'm happy to see this patch landed as is, and we can file a separate bug as well for getting puppeteer tests upgraded to Node 22.

Mike, or do you want to see all tasks that use Node 18 upgraded at once unless they show failures?

We should be able to upgrade everything at the same time, the amount of work should be about the same unless there's issues on a particular build set. We have patterns to follow (I might have done it already, but I haven't had time to look).

I don't see an issue with attaching a second patch to this bug to add the additional items, especially given what Mike has already said.

Comment 19

[Mark Banner (:standard8)]

Attachment: Bug 1942260 - Use Node 22 for tests on CI. r?#firefox-build-system-reviewers!

Comment 20

[Mark Banner (:standard8)]

I've added a patch that I think upgrades this correctly. There is a try push here, feel free to add more builds if I've missed the ones that apply to your area.

I'm kinda hoping to land this before the ESR branches, so that we have the same version of node on ESR140 which might help with uplifts.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2cbb362f6ee
Build node 22 for Linux from source. r=firefox-build-system-reviewers,sergesanspaille
https://hg.mozilla.org/integration/autoland/rev/ea4930ad6ffd
Use Node 22 for tests on CI. r=firefox-build-system-reviewers,perftest-reviewers,glandium,sparky

Comment 22

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c2cbb362f6ee
https://hg.mozilla.org/mozilla-central/rev/ea4930ad6ffd

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

FYI the version of Node that we bumped to is 22.11.0, which sadly has issues with node packages that support ESM modules like pixelmatch. In those cases installing the package will actually fail as it can be seen here in my try build for the Puppeteer 24.10.0 upgrade:

https://treeherder.mozilla.org/logviewer?job_id=511731353&repo=try&lineNumber=473

It should be fixed in Node 22.12.0 which enabled ESM support by default. I'll file a new bug to get Node upgraded.

Comment 24

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #21)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2cbb362f6ee
Build node 22 for Linux from source.
r=firefox-build-system-reviewers,sergesanspaille
https://hg.mozilla.org/integration/autoland/rev/ea4930ad6ffd
Use Node 22 for tests on CI.
r=firefox-build-system-reviewers,perftest-reviewers,glandium,sparky

Perfherder has detected a browsertime performance change from push ea4930ad6ffd9683e3177619553f90a213ba9e88.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
17%	bing-search FirstVisualChange	linux1804-64-shippable-qr	bytecode-cached fission warm webrender	110.24 -> 91.62	Before/After
14%	welcome loadtime	linux1804-64-shippable-qr	cold fission webrender	74.20 -> 63.60	Before/After
11%	welcome fcp	linux1804-64-shippable-qr	cold fission webrender	125.28 -> 111.27	Before/After
9%	paypal FirstVisualChange	linux1804-64-shippable-qr	fission warm webrender	142.66 -> 129.36	Before/After
7%	paypal ContentfulSpeedIndex	linux1804-64-shippable-qr	fission warm webrender	164.67 -> 153.14	Before/After
...	...	...	...	...	...
3%	paypal SpeedIndex	linux1804-64-shippable-qr	fission warm webrender	358.18 -> 346.77	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 45298

The following documentation link provides more information about this command.