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

[v18.x backport] test_runner various features #46360

Closed

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Jan 26, 2023

this is a backport for a batch of PRS that depend on each other, so it was convenient to backport in bulk

#45264
#45712
#45923
#46030
#46092

anonrig and others added 30 commits January 24, 2023 16:39
PR-URL: nodejs#45865
Fixes: nodejs#45535
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
If we are in an artifically created Environment that has no args set,
and uv_exepath returns an error (for instance, if /proc is not mounted
on a Linux system), then we crash with a nullptr deref attempting to
use argv[0].

Fixes: nodejs#45901
PR-URL: nodejs#45902
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Refs: nodejs#45868
PR-URL: nodejs#45882
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Tailing slash of url.href is ommited.

PR-URL: nodejs#45928
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: nodejs#45876
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
PR-URL: nodejs#45887
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit exposes uv_available_parallelism() as an alternative
to cpus().length. uv_available_parallelism() is inspired by
Rust's available_parallelism().

PR-URL: nodejs#45895
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
With the introduction of os.availableParallelism(), users should
no longer rely on os.cpus().length to determine the amount of
available parallelism. This commit adds a note to the os.cpus()
docs.

PR-URL: nodejs#45895
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#45914
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: nodejs#45803
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: nodejs#45803
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Co-authored-by: Daniel Lemire <daniel@lemire.me>
PR-URL: nodejs#45803
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: nodejs#45793
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
daae938 broke addons which create their own `Isolate`
instances, because enabling the shared-readonly-heap feature
of V8 requires all snapshots used for different `Isolate`s to
be identical. Usage of addons that do this has probably
decreased quite a bit since Worker threads were introduced
in Node.js, but it’s still a valid use case, and in any case
the breakage was probably not intentional (although the referenced
commit did require test changes because of this issue).

This commit addresses this issue partially by caching the
V8 snapshot parameters and ignoring ones passed in from users
in `NewIsolate()` when this feature is enabled, and makes
the `NodeMainInstance` snapshot-based isolate creation
also re-use this code.

PR-URL: nodejs#45885
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#45935
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Update the GitHub workflow action used for closing stalled issues
and PRs.

PR-URL: nodejs#45937
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
User can check output of example easily.

PR-URL: nodejs#45915
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#45940
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
If port is used for `url.hostname`, `url.hostname`
is not working. So remove port from example in
`url.hostname`.

PR-URL: nodejs#45927
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#45778
Fixes: nodejs#43355
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#45957
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Qingyu Deng <i@ayase-lab.com>
Error message in document is different from actual result.

PR-URL: nodejs#45920
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Starting vcbuild.bat for cross-compiling from powershell was failing the
licensertf step because it couldn't find x64_node_exe after downloading.

PR-URL: nodejs#45890
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This reverts commit 09f33c9.

PR-URL: nodejs#45948
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#45947
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Cope with the delay(to the next function call) of
v8::Isolate::TerminateExecution()

PR-URL: nodejs#45620
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Jesse has informed me that he won't be able to contribute to this
initiative, unfortunately. If you would like to know why, feel free to
ask him or me privately. I would like to take over.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: nodejs#45956
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#45972
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#45960
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Christian Clauss <cclauss@me.com>
PR-URL: nodejs#45967
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
aduh95 and others added 10 commits January 26, 2023 13:38
We already always specify a value, and failing to do so would likely be
a bug.

PR-URL: nodejs#46164
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
- add text discussed by the TSC

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs#46121
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Joe Sepi <sepi@joesepi.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Refs nodejs#45189

PR-URL: nodejs#45990
Refs: nodejs#45189
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
tls.createServer() and new tls.Server() ignore secureContext option.

PR-URL: nodejs#46224
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#45904
Fixes: nodejs#45838
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Refs: nodejs#39316
PR-URL: nodejs#46205
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#46242
Reviewed-By: Ruy Adorno <ruyadorno@google.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
closes nodejs#45987

PR-URL: nodejs#46042
Fixes: nodejs#45987
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Fixes: nodejs#45755
PR-URL: nodejs#45760
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: James M Snell <jasnell@gmail.com>
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: nodejs#45241
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@GeoffreyBooth
Copy link
Member

Aren’t the reporters behind --experimental-reporter? If so then I think they can land on both lines, but I defer to @nodejs/releasers

nodejs-github-bot and others added 8 commits January 26, 2023 14:21
PR-URL: nodejs#46257
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#46257
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This makes the test compatible with off-thread loaders.

Co-Authored-By: Geoffrey Booth <webadmin@geoffreybooth.com>
PR-URL: nodejs#46220
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
`event.returnvalue` is described as legacy in spec.
Plus, add missed '#'(private member) of defaultPrevented
in implementation.

Refs: https://dom.spec.whatwg.org/#interface-event
Refs: https://developer.mozilla.org/en-US/docs/Web/API/Event/returnValue
PR-URL: nodejs#46175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
`options.categories` is string[]. So used `validateStringArray`

Refs: https://nodejs.org/dist/latest-v19.x/docs/api/tracing.html#trace_eventscreatetracingoptions
PR-URL: nodejs#46012
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fixes: nodejs#42866
PR-URL: nodejs#46226
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Arguments of some APIs are mismatched and 2 APIs are not as
described.

PR-URL: nodejs#45678
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#46212
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tierney Cyren <hello@bnb.im>
@nodejs-github-bot
Copy link
Collaborator

fossamagna and others added 5 commits January 26, 2023 23:46
Fixes: nodejs#44612
PR-URL: nodejs#45264
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#45712
Fixes: nodejs#45648
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Move the logic for handling --test-reporter out of the
general module loader and into the test_runner subsystem.

PR-URL: nodejs#45923
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#46030
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit updates the test runner to make the built in test
reporters internal modules.

PR-URL: nodejs#46092
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@MoLow MoLow force-pushed the backport-v18-test-runner-reporters branch from 1ea2fa2 to e304cc5 Compare January 26, 2023 21:47
@ruyadorno
Copy link
Member

Personally I haven't heard of an exception to the rule for experimental APIs and I would prefer if we could wait the regular 4-week-from-landing-on-current period of time.

@MoLow
Copy link
Member Author

MoLow commented Jan 28, 2023

then let's wait for the next release 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet