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: add shards support #49761

Closed
wants to merge 3,660 commits into from

Conversation

rluvaton
Copy link
Member

PR-URL: #48639
Reviewed-By: Moshe Atlow [email protected]
Reviewed-By: Benjamin Gruenbaum [email protected]

mhdawson and others added 30 commits August 15, 2023 15:32
Fix warning about dereferencing null env

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#48954
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires
some careful justification but the default encoding can be eliminated
from sig.js entirely.

In Sign.prototype.update, we can safely remove the conditional
assignment of getDefaultEncoding() to encoding. This is because
SignUpdate() in crypto_sig.cc internally calls node::crypto::Decode,
which returns UTF8 for falsy encoding values. In other words, with the
conditional assignment, StringBytes::Write() ultimately receives the
encoding BUFFER, and without the conditional assignment, it receives the
encoding UTF8. However, StringBytes::Write() treats both encodings
identically, so there is no need to deviate from the internal default
encoding UTF8.

In Sign.prototype.sign, we can also safely remove the conditional
assignment of getDefaultEncoding() to encoding. Whether encoding is
falsy or 'buffer' makes no difference.

In Verify.prototype.verify, we can also safely remove the conditional
assignment of getDefaultEncoding() to sigEncoding. This is because the
function passes the sigEncoding to getArrayBufferOrView(), which passes
it to Buffer.from(). If sigEncoding is 'buffer', getArrayBufferOrView()
instead passes 'utf8' to Buffer.from(). Because the default encoding of
Buffer.from() is 'utf8', passing a falsy encoding to
getArrayBufferOrView() instead of 'buffer' results in the same behavior.

Refs: nodejs#47182
PR-URL: nodejs#49145
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
This version avoids the additional access to the embedder slot
when we already have a reference to the realm.

PR-URL: nodejs#49007
Refs: nodejs#48836
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
This reduce the number of embedder slot accesses and also removes
the assumption in a few binding methods that the current realm is
the principal realm of the current environment (which is not true
for shadow realms).

PR-URL: nodejs#49007
Refs: nodejs#48836
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: nodejs#49020
Reviewed-By: Luigi Pinca <[email protected]>
..and replace the similar code added for logging.

PR-URL: nodejs#49020
Reviewed-By: Luigi Pinca <[email protected]>
This (not particularly elegant) native addon tests the effect of
UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more
than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits
the number of concurrent async tasks to anything less than
UV_THREADPOOL_SIZE.

PR-URL: nodejs#49165
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20. It requires
some careful justification but the default encoding can be eliminated
from hash.js entirely. The reasoning is almost identical with that in
nodejs#49145 so I won't repeat it here.

Refs: nodejs#47182
PR-URL: nodejs#49167
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
getDefaultEncoding() always returns 'buffer' in Node.js 20.

In diffiehellman.js, this value is always used as input to either
toBuf(), encode(), or getArrayBufferOrView(). All of these functions
treat any falsy encoding just like 'buffer', so we can safely remove the
calls to getDefaultEncoding().

Refs: nodejs#47182
PR-URL: nodejs#49169
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Previously we assume that the objects are GC'ed after one
global.gc() returns, which is not necessarily always the case. Use
gcUntil() to run GC multiple times if they are not GC'ed in the
first time around.

PR-URL: nodejs#49053
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
The C++ implementation can now be done entirely in JS using WeakRef.
Re-implement it in JS instead to simplify the code.

PR-URL: nodejs#49053
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49053
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49199
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49019
Fixes: nodejs#48995
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Previously the ContextifyContext holds a MicrotaskQueueWrap which in
turns holds a MicrotaskQueue in a shared pointer. The indirection is
actually unnecessary, we can directly hold the MicrotaskQueue via
a unique pointer in ContextifyContext, the lifetime would still
remain the same but the graph would be simpler, and this removes
the additional JS -> C++ to create the wrapper object.

PR-URL: nodejs#48982
Reviewed-By: Stephen Belanger <[email protected]>
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: nodejs#46304
PR-URL: nodejs#46391
Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#48890
Refs: https://github.com/orgs/nodejs/discussions/44975
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#49215
Fixes: nodejs#49049
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
V8 now no longer supports serializing code cache in an isolate
with unfinalized read-only space. So guard the code cache regeneration
with the `is_building_snapshot()` flag. When the isolate is created
for snapshot generation, the code cache is going to be serialized
separately anyway, so there is no need to do it in the builtin loader.

PR-URL: nodejs#49108
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Fixes: nodejs#48112
Ref: nodejs#48208
PR-URL: nodejs#49184
Refs: nodejs#48208
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#49186
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Support for a single comma separates list for allow-fs-* flags is
removed. Instead now multiple flags can be passed to allow multiple
paths.

Fixes: nodejs/security-wg#1039
PR-URL: nodejs#49047
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Refs: nodejs#32930
PR-URL: nodejs#49180
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49175
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49112
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#49188
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#49189
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
V8 now requires the code cache to be compiled with a finalized
read-only space, so we need to serialize the snapshot to get
a finalized read-only space first, then deserialize it to compile
the code cache.

PR-URL: nodejs#49099
Refs: nodejs#47636
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13789
Reviewed-By: Yagiz Nizipli <[email protected]>
Previously this can be flaky because the there could be a delay
of the deallocation after gc() is invoked. Use gcUntil() to run
the GC multiple times to make the test more robust.

PR-URL: nodejs#49168
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Previously the test makes several assumptions about the absolute
values of the nodeTiming fields, which can make the test flaky
on slow machines. This patch rewrites the test to check the
relative values instead. It also updates the test to make it
work with workers instead of directly skipping in workers.

PR-URL: nodejs#49197
Refs: nodejs/reliability#638
Reviewed-By: Debadree Chatterjee <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
debadree25 and others added 24 commits September 19, 2023 10:54
PR-URL: nodejs#49548
Refs: nodejs#49537
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#49716
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#49690
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR-URL: nodejs#49685
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49698
Fixes: nodejs#49695
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Guy Bedford <[email protected]>
PR-URL: nodejs#49700
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
PR-URL: nodejs#49701
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
This reverts commit 48fcb20.

Refs: nodejs@18e00a577d74
PR-URL: nodejs#49708
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
PR-URL: nodejs#49714
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
In exmple of `util.inspect` with numericSeparator option,
calling `util.inspect` is missed. So actual result is different
from expected result.

PR-URL: nodejs#49717
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
When using .load the REPL would accumulate indentation with each line
including the indentation from all previous lines. Now it keeps the
indentation at the correct level.

Fixes: nodejs#47673
PR-URL: nodejs#49461
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Kohei Ueno <[email protected]>
Actual output of example in `mimeParams.set()` is mismatched.

PR-URL: nodejs#49718
Reviewed-By: Matthew Aitken <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49719
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
PR-URL: nodejs#49684
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#49705
Refs: nodejs/performance#106
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#49706
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: nodejs#49646
Fixes: nodejs#48937
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Co-authored-by: Antoine du Hamel <[email protected]>
Co-authored-by: Michaël Zasso <[email protected]>
PR-URL: nodejs#49725
Refs: nodejs#47342
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#49227
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Migrate the remaining error tests in the `test/message` folder
from Python to JS.

Fixes: nodejs#47707
PR-URL: nodejs#49721
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: nodejs#49755
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Fixes: nodejs#47993
PR-URL: nodejs#49713
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
PR-URL: nodejs#48639
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/tsc

@rluvaton rluvaton closed this Sep 22, 2023
@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Sep 22, 2023
@rluvaton rluvaton deleted the backport-48639-to-18 branch September 22, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. 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.