[v18.x backport] test_runner various features#46360
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
PR-URL: nodejs#45865Fixes: nodejs#45535 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: nodejs#45868 PR-URL: nodejs#45882 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: James M Snell <[email protected]>
Tailing slash of url.href is ommited. PR-URL: nodejs#45928 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Signed-off-by: RafaelGSS <[email protected]> PR-URL: nodejs#45876 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#45887 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
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 <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
PR-URL: nodejs#45914 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#45803 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#45803 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Co-authored-by: Daniel Lemire <[email protected]> PR-URL: nodejs#45803 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Signed-off-by: Daeyeon Jeong <[email protected]> PR-URL: nodejs#45793 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]>
PR-URL: nodejs#45935 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
Update the GitHub workflow action used for closing stalled issues and PRs. PR-URL: nodejs#45937 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Mestery <[email protected]>
User can check output of example easily. PR-URL: nodejs#45915 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#45940 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Jacob Smith <[email protected]>
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 <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Co-authored-by: Luigi Pinca <[email protected]> PR-URL: nodejs#45778Fixes: nodejs#43355 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#45957 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Qingyu Deng <[email protected]>
Error message in document is different from actual result. PR-URL: nodejs#45920 Reviewed-By: Kohei Ueno <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
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 <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
This reverts commit 09f33c9. PR-URL: nodejs#45948 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#45947 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Cope with the delay(to the next function call) of v8::Isolate::TerminateExecution() PR-URL: nodejs#45620 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
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 <[email protected]> PR-URL: nodejs#45956 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#45972 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#45960 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Christian Clauss <[email protected]>
PR-URL: nodejs#45967 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
We already always specify a value, and failing to do so would likely be a bug. PR-URL: nodejs#46164 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
- add text discussed by the TSC Signed-off-by: Michael Dawson <[email protected]> PR-URL: nodejs#46121 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Joe Sepi <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
Refs nodejs#45189 PR-URL: nodejs#45990 Refs: nodejs#45189 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
tls.createServer() and new tls.Server() ignore secureContext option. PR-URL: nodejs#46224 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#45904Fixes: nodejs#45838 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Refs: nodejs#39316 PR-URL: nodejs#46205 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#46242 Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
closesnodejs#45987 PR-URL: nodejs#46042Fixes: nodejs#45987 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
Fixes: nodejs#45755 PR-URL: nodejs#45760 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Juan José Arboleda <[email protected]> PR-URL: nodejs#45241 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Member
GeoffreyBooth commented Jan 26, 2023
Aren’t the reporters behind |
PR-URL: nodejs#46257 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#46257 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This makes the test compatible with off-thread loaders. Co-Authored-By: Geoffrey Booth <[email protected]> PR-URL: nodejs#46220 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]>
`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 <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
`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 <[email protected]>
Fixes: nodejs#42866 PR-URL: nodejs#46226 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Arguments of some APIs are mismatched and 2 APIs are not as described. PR-URL: nodejs#45678 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#46212 Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tierney Cyren <[email protected]>
Collaborator
nodejs-github-bot commented Jan 26, 2023
Fixes: nodejs#44612 PR-URL: nodejs#45264 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#45712Fixes: nodejs#45648 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#46030 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This commit updates the test runner to make the built in test reporters internal modules. PR-URL: nodejs#46092 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Moshe Atlow <[email protected]>
MoLowforce-pushed the backport-v18-test-runner-reporters branch from 1ea2fa2 to e304cc5Compare
Member
ruyadorno commented Jan 27, 2023
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. |
MemberAuthor
MoLow commented Jan 28, 2023
then let's wait for the next release 🙏🏻 |
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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