* 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>
* 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>
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>
* use `common.mustNotCall()` to confirm callback is not called
* reorder modules to conform with test-writing guide
* match full error message in `assert.throws()`
PR-URL: https://github.com/nodejs/node/pull/13721
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: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.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>
* 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>
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>
* /m is needless if ^ and $ are not used
* /g is needless in split()
* /g is needless in test() with one-time RegExp/String
PR-URL: https://github.com/nodejs/node/pull/13690
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use `common.mustCall()` to track callback invocations
* Remove console.log() statements unrelated to the test
* Add blank line to conform with test-writing guide
PR-URL: https://github.com/nodejs/node/pull/13802
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@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>
Confirm that callback is not being invoked in
test-http-eof-on-connect.js.
PR-URL: https://github.com/nodejs/node/pull/13587
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use `common.mustNotCall()` in place of `common.noop` where appropriate
* Increase specificity of regular expressions (that is, make them match
the whole error string rather than part of the error string) in
`assert.throws()` calls
PR-URL: https://github.com/nodejs/node/pull/13501
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
* Check that noop callback is or isn't invoked as appropriate using
common.mustCall() and common.mustNotCall()
* Fix typo in array literal
PR-URL: https://github.com/nodejs/node/pull/13480
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Use `common.mustNotCall()` to confirm that callback is not invoked when
`dns.lookup()` throws.
PR-URL: https://github.com/nodejs/node/pull/13456
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
* Change common.noop to common.mustNotCall() to verify callback is not
invoked.
* Add destructuring assignment for clarity. Yeah, clarity. That's why.
PR-URL: https://github.com/nodejs/node/pull/13443
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@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>
Use `common.mustCall()` to make sure noop function is called as
expected.
PR-URL: https://github.com/nodejs/node/pull/13259
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use `common.mustNotCall()` in test-stream2-objects.js to confirm that
noop function is never invoked.
PR-URL: https://github.com/nodejs/node/pull/13249
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-stream2-basic runs in a few seconds. It can be moved to parallel.
PR-URL: https://github.com/nodejs/node/pull/13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Use common.mustCall() in test-fs-makeStatsCallback to confirm that the
callback is invoked.
PR-URL: https://github.com/nodejs/node/pull/13132
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@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>
Timers should work even if the user has monkey-patched `.call()` and
`.apply()` to undesirable values.
PR-URL: https://github.com/nodejs/node/pull/12960
Ref: https://github.com/nodejs/node/issues/12956
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
bootstrap_node.js was directly parsing process.execArgv to see if
internals should be exposed, even though the argv was already parsed by
node. This is unusual and unnecessary, change it to set the option value
from the parser onto the config binding.
Backport-PR-URL: https://github.com/nodejs/node/pull/14483
PR-URL: https://github.com/nodejs/node/pull/12245
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12935
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@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>
* 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.
Backport-PR-URL: https://github.com/nodejs/node/pull/14370
PR-URL: https://github.com/nodejs/node/pull/13770
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Also, add tests to ensure they will always return this, and to confirm
they return this when these doc changes are back-ported to earlier
release lines.
PR-URL: https://github.com/nodejs/node/pull/13553
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In non-buffer tests, change usage of the Buffer constructor to one of
the recommended alternatives.
Backport-PR-URL: https://github.com/nodejs/node/pull/14339
PR-URL: https://github.com/nodejs/node/pull/13649
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: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Brian White <mscdex@mscdex.net>
PR-URL: https://github.com/nodejs/node/pull/13685
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Add common.mustCall().
* Add check for error existence, not only for error details.
* Use test(), not match() in a boolean context.
* Properly reverse meanings of assert messages.
PR-URL: https://github.com/nodejs/node/pull/13680
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
PR-URL: https://github.com/nodejs/node/pull/13626
Fixes: https://github.com/nodejs/node/issues/13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add error listener to ignore `ECONNRESET`. Makes test reliable while it
still segfaults (as expected) on Node.js 7.7.3. It might not be possible
to eliminate the probable race causing `ECONNRESET` without also
eliminating the required segfault-inducing part of the test. (Or maybe
it's totally possible. If you figure it out, hey cool, submit a pull
request.)
PR-URL: https://github.com/nodejs/node/pull/13529
Fixes: https://github.com/nodejs/node/issues/13184
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit regains test coverage for EventEmitter#once() with
four or more arguments. To avoid similar regressions in the
future, once() is called with enough arguments to cover all of
the separate code paths.
PR-URL: https://github.com/nodejs/node/pull/13524
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Alexey Orlenko
Rich Trott
Nataly Shrits
Daniel Bevenius
Julien Klepatch
XadillaX
lena
Zongmin Lei
OriLev
sallen450
Vse Mozhet Byt
Aditya Anand
Timothy Gu
Tobias Nießen
jeyanthinath
Sam Roberts
Refael Ackermann
James M Snell
cjihrig
João Reis