PR-URL: https://github.com/nodejs/node/pull/20846
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Check actual expired time rather than relying on a number of calls to
setTimeout() in test-http2-session-timeout more robust.
PR-URL: https://github.com/nodejs/node/pull/20692
Fixes: https://github.com/nodejs/node/issues/20628
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fix a bug where the socket wasn't being correctly destroyed and
adjust existing tests, as well as add additional tests.
PR-URL: https://github.com/nodejs/node/pull/19852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
Use a shared function, for..in instead of Object.keys, do less work in
`setHeader` and instead defer some of it until later, and other minor
changes to improve clarity, as well as a slight boost in performance.
PR-URL: https://github.com/nodejs/node/pull/20250
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This change enables concurrent inspector sessions, through WebSocket
interface as well as JS interface, in any combination.
PR-URL: https://github.com/nodejs/node/pull/20137
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Also modifies the error messages so they include more information
and are more consistent.
- The message of ERR_SCRIPT_EXECUTION_INTERRUPTED now mentions
SIGINT and the trailing period is dropped for consistency.
- Added ERR_SCRIPT_EXECUTION_TIMEOUT and include the timeout
in the message.
PR-URL: https://github.com/nodejs/node/pull/20147
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The old error name and message were trying to be consistent with
ERR_BUFFER_TOO_LARGE but they were not really accurate.
The kStringMaxLength was measured in number of characters,
not number of bytes. The name ERR_STRING_TOO_LARGE also
seems a bit awkward. This patch tries to correct them before
they get released to users.
PR-URL: https://github.com/nodejs/node/pull/19864
Refs: https://github.com/nodejs/node/pull/19739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Close the connection immediately after receiving the `end` event to
avoid that the `timeout` event is fired before the connection closes
gracefully.
Move the test back to parallel.
PR-URL: https://github.com/nodejs/node/pull/19856
Fixes: https://github.com/nodejs/node/issues/19804
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Remove the `performance.getEntries()` and `performance.clear*()`
variants and eliminate the accumulation of the global timeline
entries. The design of this particular bit of the API is a memory
leak and performance footgun. The `PerformanceObserver` API is
a better approach to consuming the data in a more transient way.
PR-URL: https://github.com/nodejs/node/pull/19563
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Node instance may not know the real host and port user sees when
debug frontend connects through the SSH tunnel. This change fixes
'/json/list' response by using the value client provided in the host
header.
PR-URL: https://github.com/nodejs/node/pull/19664
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
This makes a effort to make sure all of these errors will actually
also show the received input.
On top of that it refactors a few tests for better maintainability.
It will also change the returned type to always be a simple typeof
instead of special handling null.
PR-URL: https://github.com/nodejs/node/pull/19445
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Assign a code to a user-facing error.
Turn other internal-only errors to checks.
PR-URL: https://github.com/nodejs/node/pull/19387
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
After this commit, all errors thrown from JS code in lib have an error
code.
PR-URL: https://github.com/nodejs/node/pull/19373
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test uses both `assert()` and `assert.ok()`. Use just `assert.ok()`.
Remove a comment that does not appear to match the code and does not
seem to explain much beyond the bare code itself.
PR-URL: https://github.com/nodejs/node/pull/19404
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The test is unreliable on some Windows platforms in its current form.
Make it more robust by using `setInterval()` to repeat the flooding
until an error is triggered.
PR-URL: https://github.com/nodejs/node/pull/19395
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
- Check if the watcher is active in JS land before
invoking the binding, act as a noop if the state of
the watcher does not match the expectation. This
avoids firing 'stop' when the watcher is already
stopped.
- Update comments, validate more arguments and
the type of the handle.
- Handle the errors from uv_fs_poll_start
- Create an `IsActive` helper method on StatWatcher
PR-URL: https://github.com/nodejs/node/pull/19345
Refs: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The test is unreliable on some Windows platforms in its current form.
Make it more robust by using `setInterval()` to repeat the flooding
until an error is triggered.
Fixes: https://github.com/nodejs/node/issues/18251
PR-URL: https://github.com/nodejs/node/pull/19349
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Provide a way to create pipes between native `StreamBase` instances
that acts more directly than a `.pipe()` call would.
PR-URL: https://github.com/nodejs/node/pull/18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
test-performance can fail due to resource constraints. Move it from
parallel to sequential so it does not compete with other tests for
resources.
Fixes: https://github.com/nodejs/node/issues/19197
PR-URL: https://github.com/nodejs/node/pull/19228
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
- Add an accessor property `initialized `to FSEventWrap to
check the state of the handle from the JS land
- Introduce ERR_FS_WATCHER_ALREADY_STARTED so calling start()
on a watcher that is already started will throw instead of
doing nothing silently.
- Introduce ERR_FS_WATCHER_NOT_STARTED so calling close()
on a watcher that is already closed will throw instead of
doing nothing silently.
- Validate the filename passed to fs.watch()
- Assert that the handle in the watcher are instances of
FSEvent instead of relying on the illegal invocation error
from the VM.
- Add more assertions in FSEventWrap methods now that we check
`initialized` and the filename in JS land before invoking
the binding.
- Use uvException instead of errornoException to create
the errors with the error numbers from libuv to make them
consistent with other errors in fs.
TODO:
- Improve fs.watchFile() the same way this patch improves fs.watch()
- It seems possible to fire both rename and change event from libuv
together now that we can check if the handle is closed via
`initialized` in JS land.
PR-URL: https://github.com/nodejs/node/pull/19089
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Display the constant name instead of a stream error code
in the error message, because the numerical codes give absolutely
no clue about what happened when an error is emitted.
PR-URL: https://github.com/nodejs/node/pull/18966
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
So it does not use `common.PORT` anymore.
PR-URL: https://github.com/nodejs/node/pull/18727
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Use common.platformTimeout() to give longer durations to Raspberry Pi
devices to make test more reliable.
PR-URL: https://github.com/nodejs/node/pull/18126
Fixes: https://github.com/nodejs/node/issues/16772
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This removes the `noAssert` argument and also adds some more tests.
PR-URL: https://github.com/nodejs/node/pull/18395
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This enables the eslint dot-notation rule for all code instead of
only in /lib.
PR-URL: https://github.com/nodejs/node/pull/18749
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Add punctuation and comments about code that should not throw.
Also remove a obsolete test and refactor some tests.
PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
There is actually no reason to use `assert.doesNotThrow()` in the
tests. If a test throws, just let the error bubble up right away
instead of first catching it and then rethrowing it.
PR-URL: https://github.com/nodejs/node/pull/18669
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/18676
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently when building --without-ssl a 'ERR_NO_CRYPTO' error is
reported.
This is not currently being picked up by the crypto-check lint rule as
it does not actually require any crypto modules directly, but instead
this is done by common/benchmark.
PR-URL: https://github.com/nodejs/node/pull/18724
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Due to extensive reliance on timings and the fs module, this test
is currently inherently flaky. Refactor it to simply use setImmediate
and only one busy loop.
PR-URL: https://github.com/nodejs/node/pull/18567
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The readline module wants a truthy time while using Timer.now() doesn't
necessarily guarantee that early on in the process' life. It also
doesn't actually resolve the timing issues experienced in an earlier
issue. Instead, this PR fixes the related tests and moves them back
to parallel.
Refs: https://github.com/nodejs/node/issues/14674
PR-URL: https://github.com/nodejs/node/pull/18563
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Initial set of fs.promises APIs with documentation and one
benchmark.
PR-URL: https://github.com/nodejs/node/pull/18297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
When an interval takes as long or longer to run as its timeout setting
and the roundtrip from rearm() to its deferal takes exactly 1ms, that
interval can then block the event loop. This is an edge case of another
recently fixed bug (which in itself was an edge case).
PR-URL: https://github.com/nodejs/node/pull/18486
Refs: https://github.com/nodejs/node/pull/15072
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Useful for executing in a shell because it accepts arguments as
an array instead of a string as exec does.
Depending on the circumstances,
that can prove to be useful if the arguments are already prepared.
PR-URL: https://github.com/nodejs/node/pull/18384
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This makes `assert.ifError` stricter by only accepting `null` and
`undefined` from now on. Before any truthy value was accepted.
PR-URL: https://github.com/nodejs/node/pull/18247
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reimplement uv.errname() as internal/util.getSystemErrorName() to
avoid the memory leaks caused by unknown error codes
and avoid calling into C++ for the error names. Also
expose it as a public API for external use.
PR-URL: https://github.com/nodejs/node/pull/18186
Refs: http://docs.libuv.org/en/v1.x/errors.html#c.uv_err_name
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
In further preparation for the Promises enabled fs API,
further encapsulate FSReqWrap details.
The intent here is to isolate, as much as possible, the
functionality of the various fs functions from the details
of how the results are notified. The promises implementation
will use a `FSReqPromise` alternative to `FSReqWrap` that
will use the same API so that both models can be used without
changing any of the actual implementation details for the
various methods.
PR-URL: https://github.com/nodejs/node/pull/18112
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/18151
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Make the deprecated `tls.createSecurePair()` method use other public
APIs only (`TLSSocket` in particular).
Since `tls.createSecurePair()` has been runtime-deprecated only
since Node 8, it probably isn’t quite time to remove it yet,
but this patch removes almost all of the code complexity that
is retained by it.
The API, as it is documented, is retained. However, it is very likely
that some users have come to rely on parts of undocumented API
of the `SecurePair` class, especially since some of the existing
tests checked for those. Therefore, this should definitely be
considered a breaking change.
PR-URL: https://github.com/nodejs/node/pull/17882
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/18018
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
* verify protections against ping and settings flooding
* Strictly handle and verify handling of unsolicited ping and
settings frame acks.
PR-URL: https://github.com/nodejs/node/pull/17969
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
The maxSessionMemory is a cap for the amount of memory an
Http2Session is permitted to consume. If exceeded, new
`Http2Stream` sessions will be rejected with an
`ENHANCE_YOUR_CALM` error and existing `Http2Stream`
instances that are still receiving headers will be
terminated with an `ENHANCE_YOUR_CALM` error.
PR-URL: https://github.com/nodejs/node/pull/17967
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Split them into their own benhmark file and use different byteLength
values.
PR-URL: https://github.com/nodejs/node/pull/11146
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/17924
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
`common.fail()` no longer exists as its functionality is now in
`assert.fail()`. Replace only two instances in the code base with
`assert.fail()`.
PR-URL: https://github.com/nodejs/node/pull/17845
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
The `auxData` field is not exposed to JavaScript, as DevTools uses it
for its `isDefault` parameter, which is implemented faithfully,
contributing to the nice indentation in the context selection panel.
Without the indentation, when `Target` domain gets implemented (along
with a single Inspector for cluster) in #16627, subprocesses and VM
contexts will be mixed up, causing confusion.
PR-URL: https://github.com/nodejs/node/pull/17720
Refs: https://github.com/nodejs/node/pull/14231#issuecomment-315924067
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
common.getTTYfd() is used in one test only. Move it's definition to that
test and out of the common module.
PR-URL: https://github.com/nodejs/node/pull/17781
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/17406
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This is a significant cleanup and refactoring of the
cleanup/close/destroy logic for Http2Stream and Http2Session.
There are significant changes here in the timing and ordering
of cleanup logic, JS apis. and various related necessary edits.
Currently, writeQueueSize is never used in C++ and barely used
within JS. Instead of constantly updating the value on the JS
object, create a getter that will retrieve the most up-to-date
value from C++.
For the vast majority of cases though, create a new prop on
Socket.prototype[kLastWriteQueueSize] using a Symbol. Use this
to track the current write size, entirely in JS land.
PR-URL: https://github.com/nodejs/node/pull/17650
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Check that `serverConnectionHandle.writeQueueSize === 0`
after a large write finished.
PR-URL: https://github.com/nodejs/node/pull/17564
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-child-process-pass-fd needs to launch many processes
simultaneously. On Fedora 24, this can result in EAGAIN "Resource
temporarily unavailable" errors. When this occurs, simply try to launch
a worker again.
PR-URL: https://github.com/nodejs/node/pull/17598
Fixes: https://github.com/nodejs/node/issues/17589
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Fix a nullptr dereference on abrupt termination when async call stack
recording is enabled.
Bug discovered while trying to write a regression test for the bug fix
in commit df79b7d821 ("src: fix missing handlescope bug in inspector".)
PR-URL: https://github.com/nodejs/node/pull/17577
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Add a comment explaining the test (especailly why it forks 80 processes.
Use destructuring and an arrow function callback.
PR-URL: https://github.com/nodejs/node/pull/17596
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
`common.PORT` should not be used in parallel tests because another test
may experience a collision with `common.PORT` when using port 0 to get
an open port. This has been observed to result in test failures in CI.
PR-URL: https://github.com/nodejs/node/pull/17410
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Adds `TCPSERVERWRAP` and `PIPESERVERWRAP` as provider types. This
makes it possible to distinguish servers from connections.
PR-URL: https://github.com/nodejs/node/pull/17157
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
`Promise` instances are already tracked by V8 itself.
This fixes `sequential/test-inspector-async-stack-traces-promise-then`
in debug mode (it previously crashed because our tracking
and the V8 tracking were not properly nested).
PR-URL: https://github.com/nodejs/node/pull/17118
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/707058
Fixes: https://github.com/nodejs/node/issues/17017
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This update does several significant things:
1. It eliminates the base Nghttp2* classes and folds those
in to node::http2::Http2Session and node::http2::Http2Stream
2. It makes node::http2::Http2Stream a StreamBase instance and
sends that out to JS-land to act as the [kHandle] for the
JavaScript Http2Stream class.
3. It shifts some of the callbacks from C++ off of the JavaScript
Http2Session class to the Http2Stream class.
4. It refactors the data provider structure for FD and Stream
based sending to help encapsulate those functions easier
5. It streamlines some of the functions at the C++ layer to
eliminate now unnecessary redirections
6. It cleans up node_http2.cc for better readability and
maintainability
7. It refactors some of the debug output
8. Because Http2Stream instances are now StreamBases, they are
now also trackable using async-hooks
9. The Stream::OnRead algorithm has been simplified with a
couple bugs fixed.
10. I've eliminated node_http2_core.h and node_http2_core-inl.h
11. Detect invalid handshake a report protocol error to session
12. Refactor out of memory error, improve other errors
13. Add Http2Session.prototype.ping
PR-URL: https://github.com/nodejs/node/pull/17105
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Report (for example) "node[1337]" as the human-readable name rather
than the more generic and less helpful "Node.js Main Context."
While not perfect yet, it should be an improvement to people that
debug multiple processes from DevTools, VS Code, etc.
PR-URL: https://github.com/nodejs/node/pull/17087
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
If the process was killed, then the exit code will be null, in which
case knowing the signal is really helpful.
PR-URL: https://github.com/nodejs/node/pull/16685
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Specify `buffer=fast` so that we only run that and not that along with
`buffer=slow` in two benchmarks.
PR-URL: https://github.com/nodejs/node/pull/17111
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
test-https-server-keep-alive-timeout relies on server timeouts and
whatnot that will be inherently unreliable on a busy host. The test
fails when run with a high `-j` value and higher `--repeat` value passed
to `tools/test.py`. Move the test to `sequential`.
PR-URL: https://github.com/nodejs/node/pull/16775
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: James M Snell <jasnell@gmail.com>
If the package.json does not contain the string '"main"', skip parsing
it to JSON.
Note that this changes the behavior of the module loader in the presence
of package.json files that don't contain legal JSON. Such files used to
throw an exception but now they are simply ignored unless they contain a
"main" property.
To me, that seems like a good trade-off: I observe a 25% reduction in
start-up time on a medium-sized application[0].
[0] https://github.com/strongloop/sls-sample-app
PR-URL: https://github.com/nodejs/node/pull/15767
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently the inspector module is always loaded and if it does not
return anything the inspector console setup is skipped.
This commit uses the process.config.variables.v8_enable_inspector
variable to only load the inspector module if it is enabled.
PR-URL: https://github.com/nodejs/node/pull/15008
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
The initials of expected in TypeError[ERR_INVALID_ARG_TYPE]
are inconsistent. This change is to unify them.
PR-URL: https://github.com/nodejs/node/pull/16401
Fixes: https://github.com/nodejs/node/issues/16383
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
For variables such as LD_LIBRARY_PATH and DYLD_LIBRARY_PATH that are
needed for dynamically linked binaries
PR-URL: https://github.com/nodejs/node/pull/16405
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Two instances of a similar test exist, with both testing the timeout on
the stream and neither on the session. Adjust one of them to test the
session timeout instead.
PR-URL: https://github.com/nodejs/node/pull/16754
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.
PR-URL: https://github.com/nodejs/node/pull/16777
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pause child on startup using inspect-brk=0 until the parent debugger
is ready.
PR-URL: https://github.com/nodejs/node/pull/15774
Fixes: https://github.com/nodejs/node/issues/14897
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* correctly reset write timers: currently reset timers on
both session & stream when write starts and when it ends.
* prevent large writes from timing out: when writing a large
chunk of data in http2, once the data is handed off to C++,
the JS session & stream lose all track of the write and will
timeout if the write doesn't complete within the timeout window
Fix this issue by tracking whether a write request is ongoing and
also tracking how many chunks have been sent since the most recent
write started. (Since each write call resets the timer.)
PR-URL: https://github.com/nodejs/node/pull/16525
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
When CPU is busy, the above sequential case fails occasionally,
expand the timeout value to fix it.
PR-URL: https://github.com/nodejs/node/pull/16314
Fixes: https://github.com/nodejs/node/issues/16310
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
With this change, we do async stack tracking only when explicitly
requested by the inspector client. This avoids unnecessary overhead
for clients that might not be interested in async stack traces.
PR-URL: https://github.com/nodejs/node/pull/16308
Fixes: https://github.com/nodejs/node/issues/16180
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
In PR [1], a bunch of properties were removed from the error thrown by
execSync and execFileSync. It turns out that some of those were still
supposed to be there, as the documentation states that the error
contains the entire result from the spawnSync call.
[1] https://github.com/nodejs/node/pull/13601
PR-URL: https://github.com/nodejs/node/pull/16060
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add updateWriteQueueSize which updates and returns queue size
(net & tls). Make _onTimeout check whether an active write
is ongoing and if so, call _unrefTimer rather than emitting
a timeout event.
Add http & https test that checks whether long-lasting (but
active) writes timeout or can finish writing as expected.
PR-URL: https://github.com/nodejs/node/pull/15791
Fixes: https://github.com/nodejs/node/issues/15082
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
`test-make-doc` fails in CI on Raspberry Pi devices where `test-ci-js`
runs the test but does not build the docs. We do not want to build the
docs in these cases. Move the test to the `doctool` suite and add that
suite to `IGNORED_SUITES` in `test.py`. Specify `doctool` as a test
suite to run with `make test` or `make test-ci` but not with the
`make test-ci-js` job run on cross-compiled fanned CI tests like we do
with Raspberry Pi devices in CI.
PR-URL: https://github.com/nodejs/node/pull/16301
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The test requires a file size limit that is greater than the string
size limit. Some aix machines might not meet this criteria so in
which case we should skip the test.
PR-URL: https://github.com/nodejs/node/pull/16273
Fixes: https://github.com/nodejs/node/issues/16319
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: MichaëZasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Allow zero operations for short buffer benchmark tests.
PR-URL: https://github.com/nodejs/node/pull/16296
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
When there are at least 2 timers set by setInterval whose callback
execution are longer than interval, the eventloop will be blocked.
This commit fix the above bug.
PR-URL: https://github.com/nodejs/node/pull/15072
Fixes: https://github.com/nodejs/node/issues/15068
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions will happen.
This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.
PR-URL: https://github.com/nodejs/node/pull/16281
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
clean up code by using fixtures helper module
instead of fixturesDir directly.
PR-URL: https://github.com/nodejs/node/pull/15847
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Import fixturesDir path from common/fixturesDir
module rather than from common.
PR-URL: https://github.com/nodejs/node/pull/15887
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Change all assert.strictEqual() to have actual value 1st
and expected value 2nd.
PR-URL: https://github.com/nodejs/node/pull/16008
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/15817
Reviewed-By: Ryan Graham <r.m.graham@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Supply `type=asc` option in test-benchmark-http to reduce runs in all
benchmark files to one combination of options.
PR-URL: https://github.com/nodejs/node/pull/16137
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Added comments to the tests to better describe what the test is doing.
PR-URL: https://github.com/nodejs/node/pull/16003
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-http-server-consumed-timeout will fail if the host is sufficiently
loaded that a 25ms interval takes more than 200ms to be invoked. Skip
the test in that situation.
PR-URL: https://github.com/nodejs/node/pull/15688
Fixes: https://github.com/nodejs/node/issues/14312
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
- Group all relevant methods/states into a C++ class.
- Uses internal fields instead of the less efficient v8::External for
storing the pointer to the C++ object.
- Use AsyncWrap to allow instrumenting callback states.
PR-URL: https://github.com/nodejs/node/pull/15643
Refs: https://github.com/nodejs/node/issues/13503
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This changes the disparity of bufferedRequestCount and the actual buffer
on file _stream_writable.js
PR-URL: https://github.com/nodejs/node/pull/15661
Fixes: https://github.com/nodejs/node/issues/6758
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-https-writable-true-after-close fails intermittently when run with
a lot of competing processes. Move it to sequential for stability.
PR-URL: https://github.com/nodejs/node/pull/15705
Fixes: https://github.com/nodejs/node/issues/15700
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Make the `http2` module always available.
The `--expose-http2` cli flag is made a non-op
PR-URL: https://github.com/nodejs/node/pull/15535
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Increase server timeout, reduce frequency of calls and
unbind timeout after runs are done in order to avoid
race conditions. Temporarily moved to sequential.
Fixes: https://github.com/nodejs/node/issues/15326
PR-URL: https://github.com/nodejs/node/pull/15338
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.
PR-URL: https://github.com/nodejs/node/pull/15373
Fixes: https://github.com/nodejs/node/issues/15372
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use cleaner `process.stdin.write('.exit')` to exit the process rather
than `proc.kill()`.
* Move test to sequential. It uses the default port 9229. It will fail
if another inspector test (or test using port 0) is already using that
port. So it needs to be run sequentially rather than in parallel with
other tests. (We haven't seen many failures with it yet because there
aren't a lot of other inspector tests in parallel at this time.)
PR-URL: https://github.com/nodejs/node/pull/15141
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously, this test would contain a DNS query that timed out
after 60 seconds, thus occupying one of the parallel test slots
for that period.
Fix that by creating a new channel for that request, and cancelling
it immediately.
PR-URL: https://github.com/nodejs/node/pull/15319
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Mostly shared/duplicated logic between all benchmark test files, so
creating a new common module to store it.
PR-URL: https://github.com/nodejs/node/pull/15004
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reasons:
- `test-async-wrap-getasyncid` binds a handle, so move to
sequential because port cannot be already in use.
- `test-dgram-implicit-bind-failure` requires a hardcoded
port number to properly send socket packet.
- `test-http-agent-uninitialized-with-handle` requires a
hardcoded port number to properly send http request.
- `test-http-agent-uninitialized` requires a hardcoded port
number to properly send http request.
- `test-net-localport` requires a hardcoded port number
for assertions.
In addition this replaces two common.PORTs with a dynamic port.
PR-URL: https://github.com/nodejs/node/pull/15151
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Move test reliant on timer triggering in a timely fahion from parallel
to sequential. The test can fail under high load when the timer is
triggered too late and the `\r` and `\n` are treated as separate lines.
PR-URL: https://github.com/nodejs/node/pull/15066
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
test-timers-blocking-callback may fail erroneously on
resource-constrained machines due to the timing nature of the test.
There is likely no way around the timing issue. This change tries to
decrease the probability of the test failing erroneously by having it
retry a small number of times on failure.
Tested on 0.10.38 (which has a bug that this test was written for) and
(modifying the test slightly to remove ES6 stuff) the test still seems
to fail 100% of the time there, which is what we want/expect.
PR-URL: https://github.com/nodejs/node/pull/14831
Fixes: https://github.com/nodejs/node/issues/14792
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Allows env vars to be passed through to child processes. This is needed
for things like NODE_TEST_DIR or LD_LIBRARY_PATH if testing the shared
library.
PR-URL: https://github.com/nodejs/node/pull/14822
Refs: https://github.com/nodejs/node/pull/13390
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Two test cases in `test-readline-interface` are sensitive to resource
constraints (probably due to `\r` and `\n` not arriving within the
appropriate delay to be treated as a single line ending). Move those
tests to `sequential`.
PR-URL: https://github.com/nodejs/node/pull/14681
Fixes: https://github.com/https://github.com/nodejs/node/issues/14674
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Adds a new `../common/fixtures' module to begin normalizing
`test/fixtures` use. Our test code is a bit inconsistent with
regards to use of the fixtures directory. Some code uses
`path.join()`, some code uses string concats, some other
code uses template strings, etc. In mnay cases, significant
duplication of code is seen when accessing fixture files, etc.
This updates many (but by no means all) of the tests in the
test suite to use the new consistent API. There are still
many more to update, which would make an excelent Code-n-Learn
exercise.
PR-URL: https://github.com/nodejs/node/pull/14332
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
* use common.mustCall() instead of exit handler
* use execSync instead of exec so test is reliable under load
* move from sequential to parallel
PR-URL: https://github.com/nodejs/node/pull/14541
Fixes: https://github.com/nodejs/node/issues/11826
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
* add block scoping
* rename block-scoped identifiers (e.g., filenameTwo -> filename)
* use common.mustCall() instead of exit handler
* use common.mustNotCall() as appropriate
* order modules per test writing guide
PR-URL: https://github.com/nodejs/node/pull/14534
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
ESLint 4.x has stricter linting than previous versions. We are currently
using the legacy indentation rules in the test directory. This commit
changes the indentation of files to comply with the stricter 4.x linting
and enable stricter linting in the test directory.
PR-URL: https://github.com/nodejs/node/pull/14431
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
`module.children` is supposed to be the list of modules included by this
module but lib/module.js failed to update the list when the included
module was retrieved from `Module._cache`.
Fixes: https://github.com/nodejs/node/issues/7131
PR-URL: https://github.com/nodejs/node/pull/14132
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Move test-http-server-keep-alive-timeout-slow-server and
test-http-server-keep-alive-timeout-slow-client-headers from parallel to
sequential to resolve test flakiness on freebsd10-64.
Fixes: https://github.com/nodejs/node/issues/14033
Refs: https://github.com/nodejs/node/pull/9317
PR-URL: https://github.com/nodejs/node/pull/14377
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
This makes the naming more consistent with existing properties like
isFreeBSD where the capitalization of the property name is consistent
with the conventional styling of the operating system.
PR-URL: https://github.com/nodejs/node/pull/14263
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Specify more configuration options to reduce run time by about a third.
PR-URL: https://github.com/nodejs/node/pull/14180
Fixes: https://github.com/nodejs/node/issues/14177
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.
PR-URL: https://github.com/nodejs/node/pull/14183
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Because of a race condition, connection listener may not be invoked if
test is run under load. Remove `common.mustCall()` wrapper from the
listener. Move the test to `parallel` because it now works under load.
Make similar change to http test to keep them in synch even though it is
much harder to trigger the race in http.
PR-URL: https://github.com/nodejs/node/pull/14134
Fixes: https://github.com/nodejs/node/issues/14133
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
The sequential/test-regress-GH-4027 test is flaky with an increased
system load, failing when the watched file is unlinked before the
first state of the watched file is retrieved.
After increasing the delay before unlinking and calling setTimeout
after watchFile, the flakiness stopped reproducing.
PR-URL: https://github.com/nodejs/node/pull/14010
Fixes: https://github.com/nodejs/node/issues/13800
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
* Make changes to `test-https-set-timeout-server` to resolve
inconsistencies with its http counterpart:
- Apply the changes analogous to those in GH-13802 to the https test.
- Add a missing `common.mustCall()` wrapper.
- Make small stylistic changes (e.g., remove unnecessary line breaks
in comments) to make it visually consistent with the http test.
* Use arrow functions.
PR-URL: https://github.com/nodejs/node/pull/13935
Fixes: https://github.com/nodejs/node/issues/13588
Refs: https://github.com/nodejs/node/pull/13802
Refs: https://github.com/nodejs/node/pull/13625
Refs: https://github.com/nodejs/node/pull/13822
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make test-http(s)-set-timeout-server tests more similar and resolve the
following issues:
* `test-https-set-timeout-server.js` was missing some `assert`
statements, including with `http` module
* Both files were missing some calls to `common.mustCall()`
* Both files were calling `createServer()` in different ways
PR-URL: https://github.com/nodejs/node/pull/13822
Refs: https://github.com/nodejs/node/issues/13588
Refs: https://github.com/nodejs/node/pull/13625
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previous realization produces some false positive and false negative
results due to:
* conflicts between unescaped user input and RegExp special characters;
* conflicts between parsing with `\b` RegExp symbol and non
alphanumeric characters in section names.
The doc does not mention any such restrictions.
PR-URL: https://github.com/nodejs/node/pull/13841
Fixes: https://github.com/nodejs/node/issues/13728
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Remove needless RegExp flag
In fixed case, `/g` flag is needless in the boolean context.
* Remove needless RegExp capturing
Use non-capturing grouping or remove capturing completely when:
* capturing is useless per se, e.g. in test() check;
* captured groups are not used afterward at all;
* some of the later captured groups are not used afterward.
* Use test, not match/exec in boolean context
match() and exec() return a complicated object,
unneeded in a boolean context.
* Do not needlessly repeat RegExp creation
This commit takes RegExp creation out of cycles and other repetitions.
As long as the RegExp does not use /g flag and match indices,
we are safe here.
In tests, this fix hardly gives a significant performance gain,
but it increases clarity and maintainability,
reassuring some RegExps to be identical.
RegExp in functions are not taken out of their functions:
while these functions are called many times
and their RegExps are recreated with each call,
the performance gain in test cases
does not seem to be worth decreasing function self-dependency.
PR-URL: https://github.com/nodejs/node/pull/13770
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Make test-http(s)-set-timeout-server tests more similar and resolve the
following issues:
* `test-http-set-timeout-server.js` had a `secReceived` check in
`serverResponseTimeoutWithPipeline()` that was added to prevent
flakiness, but this did not exist in the https counterpart.
* `test-https-set-timeout-server.js` utilized `common.mustCall()`,
`common.mustNotCall()`, etc., while the http counterpart utilized the
old method of checking flags on exit of the process.
Refs: https://github.com/nodejs/node/issues/13588
PR-URL: https://github.com/nodejs/node/pull/13625
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Use common.mustNotCall() in test/sequential/test-fs-watch.js in
situations where the call to watch() is expected to throw.
PR-URL: https://github.com/nodejs/node/pull/13595
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Allows NODE_TEST_DIR to be set (necessary to avoid path length issues
with common.PIPE).
PR-URL: https://github.com/nodejs/node/pull/13390
Refs: https://github.com/nodejs/node/issues/12708#issuecomment-297847882
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* Use common.mustNotCall() and common.mustCall() as appropriate
* Use block scoping
* Move assertions out of `exit` handler and into callbacks
* Order assert.strictEqual() args per docs
* Remove console.log() calls
* Move test from `parallel` to `sequential` so `common.PORT` can be used
without conflicting with OS-provided ports in other `parallel` tests
PR-URL: https://github.com/nodejs/node/pull/13273
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.
PR-URL: https://github.com/nodejs/node/pull/13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fixed sequential test-net-connect-local-error by swapping port
and localPort in net.connect options.
PR-URL: https://github.com/nodejs/node/pull/13064
Fixes: https://github.com/nodejs/node/issues/13055
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Add `--set key=""` to cut down the time it takes to run
test-benchmark-http by about a third.
Alphabetize options in `--set` list.
PR-URL: https://github.com/nodejs/node/pull/13109
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
The usage of common.PORT could cause undesired port collisions when run
in parallel. The following test was moved to sequential.
test-net-reconnect-error.js
PR-URL: https://github.com/nodejs/node/pull/13033
Refs: https://github.com/nodejs/node/issues/12376
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Use `common.mustCall()` and `common.mustNotCall()` to check that
callbacks are invoked the expected number of times in
test-net-listen-shared-ports.
PR-URL: https://github.com/nodejs/node/pull/13010
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Remove common.PORT from, test-net-connect-immediate-destroy,
test-net-options-lookup, test-net-connect-local-error,
test-net-connect-handle-econnrefused, test-net-socket-destroy-twice,
test-net-better-error-messages-port-hostname, test-net-localerror,
to reduce possibility that a dynamic port used in another test will
collide with common.PORT.
Moved test-net-listen-shared-ports, test-net-better-error-messages-port
from tests/parallel to test/sequential
Refs: https://github.com/nodejs/node/issues/12376
PR-URL: https://github.com/nodejs/node/pull/12473
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This is first in a hoped-for series of moves away from a monolithic
common.js that is loaded for every test and towards a more modular
approach. (In the end, common.js will hopefully contain checks for
variables leaking into the global space and perhaps some of the more
ubiquitous functions like common.mustCall().)
Move the WPT testing code to its own module.
PR-URL: https://github.com/nodejs/node/pull/12736
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Use block scope for local variables only used in a small code block.
Fixed inverse arguments in some usages of Assert.strictEqual.
PR-URL: https://github.com/nodejs/node/pull/12728
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
When configured --without-ssl the inspect-brk option will not be
available and the process will exit with a exit value of 9 "Invalid
Argument/Bad option".
This commit adds a skipIfInspectorDisabled check since --without-ssl
implies that no inspector support is build as well.
PR-URL: https://github.com/nodejs/node/pull/12757
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-https-set-timeout-server fails under load. Move it to sequential so
it is not competing with other tests.
PR-URL: https://github.com/nodejs/node/pull/12704
Fixes: https://github.com/nodejs/node/issues/10130
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
test-benchmark-child-process failures reveal that
child-process-exec-stdout benchmark sometimes leaves around a stray
yes.exe process. Add code to terminate the process.
PR-URL: https://github.com/nodejs/node/pull/12658
Ref: https://github.com/nodejs/node/issues/12560
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12622
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reversed "actual" and "expected" arguments for assert.strictEqual().
Replaced constructor with regular expression for assert.throws().
PR-URL: https://github.com/nodejs/node/pull/12595
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
https://github.com/nodejs/node/issues/5085 has been completed so
presumably test-fs-watch is not flaky on AIX anymore. Remove flaky
designation from sequential.status.
PR-URL: https://github.com/nodejs/node/pull/12564
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12470
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
sequential/test-benchmark-child-process is still failing sometimes flaky
on Windows in CI. Mark it as flaky in sequential.status until it gets
sorted.
PR-URL: https://github.com/nodejs/node/pull/12561
Ref: https://github.com/nodejs/node/issues/12560
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
test-benchmark-child-process sometimes times out on Windows in CI.
Minimize the number of configurations that run so it will (hopefully)
not time out anymore. Set dur=0.
PR-URL: https://github.com/nodejs/node/pull/12518
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
In the 2017-04-05 meeting, the CTC agreed to remove support for the
legacy debugger in 8.0.0. This is the first step in this direction.
Refs: https://github.com/nodejs/CTC/issues/94
PR-URL: https://github.com/nodejs/node/pull/12197
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This test is allocating much more memory than necessary to actually
reproduce the original problem. Lowering the amount of memory allocated
increases performance at least in some cases and makes this test less
likely to time out on SmartOS.
PR-URL: https://github.com/nodejs/node/pull/11177
Ref: https://github.com/nodejs/node/issues/10166
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Export a new common.noop no-operation function for general use.
Allow using common.mustCall() without a fn argument to simplify
test cases.
Replace various non-op functions throughout tests with common.noop
PR-URL: https://github.com/nodejs/node/pull/12027
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Currently, benchmark code is not exercised at all in CI. This adds a
minimal test for net benchmarks. If this is deemed acceptable, similar
minimal tests for other benchmarks can be written. Additionally, as
issues and edge cases are uncovered, checks for them can be added.
PR-URL: https://github.com/nodejs/node/pull/11979
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
test-domain-abort-on-uncaught is flaky under load. Move it to sequential
so it is not competing with other tests for resources.
PR-URL: https://github.com/nodejs/node/pull/11817
Fixes: https://github.com/nodejs/node/issues/11814
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently when node is build --without-ssl and the test are run,
there are a number of failing test due to tests expecting crypto
support to be available. This commit fixes fixes the failure and
instead skips the tests that expect crypto to be available.
PR-URL: https://github.com/nodejs/node/pull/11631
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The intention of test case is to make sure that `timeout` property is honored
and the code in context terminates and throws correct exception. However in
test case, the code inside context would complete before `timeout` for windows
and would sometimes fail. Updated the code so it guarantee to not complete
execution until timeout is triggered.
Fixes: https://github.com/nodejs/node/issues/11261
PR-URL: https://github.com/nodejs/node/pull/11530
Reviewed-By: James M Snell <jasnell.gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
This commit adds a mustNotCall() helper for testing. This provides
an alternative to using common.fail() as a callback, or creating
a callback function for the sole purpose of calling common.fail().
PR-URL: https://github.com/nodejs/node/pull/11152
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Even after being moved to `sequential` in
1ce05ad540, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.
The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.
On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.
This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.
PR-URL: https://github.com/nodejs/node/pull/10903
Fixes: https://github.com/nodejs/node/issues/10073
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
test-fs-readfile-tostring-fail is resource-intensive and is flaky in
CI. Move to sequential tests so it is not competing for resources with
other tests.
PR-URL: https://github.com/nodejs/node/pull/10744
Fixes: https://github.com/nodejs/node/issues/10742
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Use assert.strictEqual instead of assert.equal in tests, manually
convert types where necessary.
PR-URL: https://github.com/nodejs/node/pull/10698
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Manually fix issues that eslint --fix couldn't do automatically.
PR-URL: https://github.com/nodejs/node/pull/10685
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Formatting changes for upcoming linter update.
PR-URL: https://github.com/nodejs/node/pull/10561
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
* use const and let instead of var
* use assert.strictEqual instead of assert.equal
* use arrow functions
* removed unwanted console log
PR-URL: https://github.com/nodejs/node/pull/10531
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
add an --inspect-brk option which breaks on
first line of user script. same behavior as old
--debug-brk flag.
PR-URL: https://github.com/nodejs/node/pull/8979
Reviewed-By: Eugene Ostroukhov <eostroukhov@chromium.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
test-http-client-timeout-with-data has failed here and there in CI on
FreeBSD and OS X. The test has a socket timeout set to 50ms and a timer
set for 100ms. However, they are not necessarily set in the same tick of
the event loop and their ordering is therefore not guaranteed.
Instead of using a timer, this change listens for an event on the
listener to know when the socket timeout has occurred and then runs the
code originally in the timer.
Additional refactoring: Replaced `process.on('exit', ...)` checks with
`common.mustCall()` and replaced usage of `assert.equal()` with
`assert.strictEqual()`.
PR-URL: https://github.com/nodejs/node/pull/10431
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const and let instead of var
* use common.mustCall to control functions executions
* use assert.strictEqual instead of assert.equal
* use assert.ifError to handle errors
* use arrow functions
* remove console.log and process.stdout.write
PR-URL: https://github.com/nodejs/node/pull/10452
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
1. Lot of repeating code has been refactored to a function
2. Errors in async calls are properly asserted
3. Fail the test if the callbacks are not fired
PR-URL: https://github.com/nodejs/node/pull/10384
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
* used let and const instead of var
* used assert.strictEqual instead assert.equal
PR-URL: https://github.com/nodejs/node/pull/10357
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-timers-same-timeout-wrong-list-deleted was flaky under load because
there is no guarantee that a timer will fire within a given period of
time. It had an exit handler that checked that the process was finishing
in less than twice as much as a timer was set for. Under load, the
timer could take over 200ms to fire even if it was set for 100ms, so
this was causing the test to be flaky on CI from time to time.
However, that timing check is unnecessary to identify the regression
that the test was written for. When run with a version of Node.js that
does not contain the fix that accompanied the test in its initial
commit, an assertion indicating that there were still timers in the
active timer list fired. So, this commit removes the exit handler timing
check and relies on the existing robust active timers list length check.
This allows us to move the test back to parallel because it does not
seem to fail under load anymore.
The test was refactored slightly, removing duplicated code to a
function, using `assert.strictEqual()` instead of `assert.equal()`,
changing a 10ms timer to 1ms, and improving the messages provided by
assertions.
Fixes: https://github.com/nodejs/node/issues/8459
PR-URL: https://github.com/nodejs/node/pull/10362
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-buffer-creation-regression is flaky on some SmartOS hosts in CI,
timing out. Move to sequential so it does not compete with other tests
for resources. Reduce three test cases to just the one needed to
identify the regression.
PR-URL: https://github.com/nodejs/node/pull/10161
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Requiring a file from a directory that contains an invalid package.json
file should throw an error.
PR-URL: https://github.com/nodejs/node/pull/10044
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
