* Do not require if test is skipped.
* Do not re-require without need.
* Sort requiring by module names.
PR-URL: https://github.com/nodejs/node/pull/14008
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@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>
This change removes `common.noop` from the Node.js internal testing
common module.
Over the last few weeks, I've grown to dislike the `common.noop`
abstraction.
First, new (and experienced) contributors are unaware of it and so it
results in a large number of low-value nits on PRs. It also increases
the number of things newcomers and infrequent contributors have to be
aware of to be effective on the project.
Second, it is confusing. Is it a singleton/property or a getter? Which
should be expected? This can lead to subtle and hard-to-find bugs. (To
my knowledge, none have landed on master. But I also think it's only a
matter of time.)
Third, the abstraction is low-value in my opinion. What does it really
get us? A case could me made that it is without value at all.
Lastly, and this is minor, but the abstraction is wordier than not using
the abstraction. `common.noop` doesn't save anything over `() => {}`.
So, I propose removing it.
PR-URL: https://github.com/nodejs/node/pull/12822
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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>
PR-URL: https://github.com/nodejs/node/pull/14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/14011
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* use common.mustNotCall() to confirm callback is not invoked
* whitespace change per test-writing guide
PR-URL: https://github.com/nodejs/node/pull/13996
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* Use common.mustCall() to confirm callback is invoked.
* Change spacing of require statements to conform to test-writing guide.
PR-URL: https://github.com/nodejs/node/pull/13993
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Since V8 5.9 V8 installs a default signal handler for some signals
when creating a default platform instance that prints a stack trace.
However, Node already does the same thing, so it would seem like the
two different stack traces would be printed; also, the V8 handler
would lead to a `SIGSEGV` under some circumstances, rather than
letting the abort continue normally.
Resolve this by disabling V8’s signal handler by default.
PR-URL: https://github.com/nodejs/node/pull/13985
Fixes: https://github.com/nodejs/node/issues/13865
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13852
Refs: https://github.com/nodejs/node/issues/12586
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
* Reorder require() statements per test-writing guide
* Use common.mustNotCall() to check that callback is not invoked
* Use common.mustCall() to check that callback is invoked
PR-URL: https://github.com/nodejs/node/pull/13977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
* re-order require() and crypto check per test writing guide
* use common.mustNotCall() to confirm callback is not invoked
PR-URL: https://github.com/nodejs/node/pull/13975
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
The at-exit addons test uses asserts like the other addons tests,
but at-exit is the only one that undefines NDEBUG to make sure
that asserts are enabled. This commit removes the undef for
consistency.
PR-URL: https://github.com/nodejs/node/pull/13998
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13974
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13733
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This commit verifies that Node's internal errors are recognized
by V8's IsNativeError(), which is exposed in Node as
process.binding('util').isNativeError().
PR-URL: https://github.com/nodejs/node/pull/13965
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* Add exit listener to child process to check return code. Previously,
child process faiilure would not cause the test to fail.
* Use common.mustNotCall() to guarantee callback is not invoked.
* Insert blank line per test writing guide.
PR-URL: https://github.com/nodejs/node/pull/13904
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Using objectMode with stream_wrap has not worked properly
before and would end in an error.
Therefore prohibit the usage of objectMode alltogether.
This also improves the handling performance due to the
cheaper chunk check and by using explicit statements as they
produce better code from the compiler.
PR-URL: https://github.com/nodejs/node/pull/13863
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13940
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
- Categorize all nonspacing marks (Mn) and enclosing marks (Me) as
0-width
- Categorize all spacing marks (Mc) as non-0-width.
- Treat soft hyphens (a format character Cf) as non-0-width.
- Do not treat all unassigned code points as 0-width; instead, let ICU
select the default for that character per UAX #11.
- Avoid getting the General_Category of a character multiple times as it
is an intensive operation.
Refs: http://unicode.org/reports/tr11/
PR-URL: https://github.com/nodejs/node/pull/13918
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use common.mustNotCall() to check that callback is not invoked.
* Add space per test writing guide.
PR-URL: https://github.com/nodejs/node/pull/13905
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use common.mustNotCall() to confirm SIGINT listeners are not being
invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.
PR-URL: https://github.com/nodejs/node/pull/13902
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* order require() statements per test writing guide
* add keydir variable to make readFileSync() calls more readable
* make `next` argument to test() optional
* use common.mustCall() to guarantee second test runs
PR-URL: https://github.com/nodejs/node/pull/13896
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
PR-URL: https://github.com/nodejs/node/pull/13857
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
1) Add missing lazy assert call
2) Remove obsolete error type
3) Name undocumented error type more appropriate
4) Consolidate error type style (rely on util.format
instead of using a function)
5) Uppercase the first letter from error messages
6) Improve some internal error parameters
PR-URL: https://github.com/nodejs/node/pull/13857
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
These methods are Javascript-accessible so they get an implicit
HandleScope. The extra scope is unneeded and can be dropped.
PR-URL: https://github.com/nodejs/node/pull/13859
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
The motivation for this commit is that we are building Node with
--shared-openssl and in our case the system OpenSSL version
supports FIPS.
The tests in test-crypto-fips that toggle fips mode on/off using the
config file option might succeed and return 1 instead of an error
being thrown from OpenSSL (which is what happens for a default build
but the error is not processed/displayed in any way at the moment):
openssl config failed: error:060B10A7:digital envelope
routines:ALG_MODULE_INIT:fips mode not supported
Note that this only concerns the test that use the configuration file
option which is different from when calling the fips setter as
the handling of the configuration file is handled by OpenSSL, so it
is not possible for us to try to call the fips setter as that would
throw an error ("Error: Cannot set FIPS mode in a non-FIPS build.").
The suggestion is to skips these tests when --shared-openssl is used.
PR-URL: https://github.com/nodejs/node/pull/13786
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use common.mustCall()/common.mustNotCall() as appropriate
* reorder require() statements per test writing guide
* add comment
PR-URL: https://github.com/nodejs/node/pull/13893
Reviewed-By: Refael Ackermann <refack@gmail.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: James M Snell <jasnell@gmail.com>
* Use `common.mustCall()` to guarantee callback invocations
* Order modules according to test writing guide
PR-URL: https://github.com/nodejs/node/pull/13886
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.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>
We don't use the global Buffer throughout the lib/ to avoid circular
dependency issues in core, but we actually don't need to require it on
test files. So remove them on:
+ test/parallel/test-stream-uint8array.js
+ test/parallel/test-stream2-finish-pipe.js
+ test/parallel/test-tls-session-cache.js
+ test/parallel/test-vm-cached-data.js
Refs: https://github.com/nodejs/node/issues/13836
PR-URL: https://github.com/nodejs/node/pull/13844
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Vse Mozhet Byt
Jaime Bernardo
Refael Ackermann
Rich Trott
Alexey Orlenko
Tobias Nießen
Ruben Bridgewater
Anna Henningsen
Nataly Shrits
Daniel Bevenius
starkwang
cjihrig
João Reis
Timothy Gu
Ezequiel Garcia
Julien Klepatch
Eugene Ostroukhov
XadillaX
lena
Zongmin Lei
OriLev