Method format refactored to make it more maintenable, replacing the
switch by a function factory, that returns the appropiated function
given the character (d, i , f, j, s).
Also, performance when formatting an string that contains several
consecutive % symbols is improved. The test:
`const numSamples = 10000000;
const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%';
var s;
console.time('Percents');
for (let i = 0; i < numSamples; i++) {
s = util.format(strPercents, 'test', 12);
}
console.timeEnd('Percents');`
Original time: 28399.708ms
After refactor: 23763.788ms
Improved: 16%
PR-URL: https://github.com/nodejs/node/pull/12407
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
When configuring node --without-ssl or --without-inspector these test
will fail. The underlying issue will be:
Inspector support is not available with this Node.js build
/work/nodejs/node/out/Release/node: bad option: --inspect=0
This commit adds checks to see if inspector support is enabled and if
not skips these tests.
PR-URL: https://github.com/nodejs/node/pull/13324
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@chromium.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
0.0.0.0 is more common than other special ipv4 addresses, so
it is possible that we may not get ENOTFOUND for such addresses.
Instead, this commit uses a less common address that is reserved
for documentation (RFC) use only.
PR-URL: https://github.com/nodejs/node/pull/13261
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Pure refactor, makes no functional changes but the renaming helped me
see more clearly what the relationship was between methods and
variables.
* Renamed methods to reduce number of slightly different names for the
same thing ("thread" vs "io thread", etc.).
* Added comments where it was useful to me.
PR-URL: https://github.com/nodejs/node/pull/13321
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test is flaky under load. These changes greatly improve reliability.
* Use a recurring interval to determine when the test should end rather
than a timer.
* Increase server timeout to 500ms to allow for events being delayed by
system load
Changing to an interval has the added benefit of reducing the test run
time from over 2 seconds to under 1 second.
Fixes: https://github.com/nodejs/node/issues/13307
PR-URL: https://github.com/nodejs/node/pull/13312
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
It takes time to build each of the addons used to test n-api.
Consolidate a few of the smaller ones to save build time.
PR-URL: https://github.com/nodejs/node/pull/13317
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Previously, napi_wrap() would only work with objects created from a
constructor returned by napi_define_class(). While the N-API team
was aware of this limitation, it was not clearly documented and is
likely to cause confusion anyway. It's much simpler if addons are
allowed to use any JS object. Also, the specific behavior of the
limitation is difficult to reimplement on other VMs that work
differently from V8.
V8 requires object internal fields to be declared on the object
prototype (which napi_define_class() used to do). Since it's too
late to modify the object prototype by the time napi_wrap() is
called, napi_wrap() now inserts a new object (with the internal
field) into the supplied object's prototype chain. Then it can be
retrieved from there later by napi_unwrap().
This change also includes improvements to the documentation for
napi_create_external(), partly to explain how it is different from
napi_wrap().
PR-URL: https://github.com/nodejs/node/pull/13250
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>
Functions that call `ECDH::BufferToPoint` were not clearing the
error stack on failure, so an invalid key could leave leftover
error state and cause subsequent (unrelated) signing operations
to fail.
PR-URL: https://github.com/nodejs/node/pull/13275
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Re-enable test-http-abort-stream-end and put it into parallel
category. Use system random port when calling server.listen()
and fix eslint errors.
After calling request.abort(), in order to avoid the buffered
data to trigger the 'data' event, explicitly remove 'data' event
listeners.
PR-URL: https://github.com/nodejs/node/pull/13260
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
On macOS, a watcher created with fs.watch() does not necessarily
start receiving events immediately. So it can miss a change by
fs.writefile() if it comes very soon after the watcher is created. Fix
test flakiness caused by this by using `setInterval()` to repeat the
write action.
PR-URL: https://github.com/nodejs/node/pull/13252
Fixes: https://github.com/nodejs/node/issues/13248
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: Michael Dawson <michael_dawson@ca.ibm.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>
One of the N-API weak-reference test cases already had to be made
asynchronous to handle different behavior in a newer V8 version:
https://github.com/nodejs/node/pull/12864
When porting N-API to Node-ChakraCore, we found more of the test
cases needed similar treatment:
https://github.com/nodejs/node-chakracore/issues/246 So to make
thes tests more robust (and avoid having differences in the test code
for Node-ChakraCore), I am refactoring the tests in this file to
insert a `setImmedate()` callback before every call to `gc()` and
assertions about the effects of the GC.
PR-URL: https://github.com/nodejs/node/pull/13121
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add testing for all types of typed arrays.
Add testing for napi_is_arraybuffer.
PR-URL: https://github.com/nodejs/node/pull/13244
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Allow binding to a randomly assigned port number with `--inspect=0`
or `--inspect-brk=0`.
PR-URL: https://github.com/nodejs/node/pull/5025
Refs: https://github.com/nodejs/node/issues/4419
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
When V8 is built from its master branch, it adds a " (candidate)"
suffix to the version string. Add support for that in the tests so it
does not fail with Node canary.
PR-URL: https://github.com/nodejs/node/pull/13282
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Currently this test will fail with the following error message when
configured --without-ssl:
Error: Node.js is not compiled with openssl crypto support
This commit checks for crypto and skips this tests if such support
is not available.
PR-URL: https://github.com/nodejs/node/pull/13253
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Check that callbacks are not executed when errors are expected to be
thrown.
PR-URL: https://github.com/nodejs/node/pull/13226
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Synchronize the argument list for `dns.resolve()` with what's in the
documentation.
Improve the error for a bad `rrtype` to be a `TypeError` rather than an
`Error`.
PR-URL: https://github.com/nodejs/node/pull/13090
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
In test-child-process-spawnsync-validation-errors, check that functions
used inappropriately as options are not invoked.
PR-URL: https://github.com/nodejs/node/pull/13205
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add napi_get_version function so that addons can
query the level of N-API supported.
PR-URL: https://github.com/nodejs/node/pull/13207
Fixes: https://github.com/nodejs/abi-stable-node/issues/231
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
This commit adds test coverage for the scenario where a socket's
handle has been closed prior to writing.
PR-URL: https://github.com/nodejs/node/pull/13171
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
`AsyncEvent` is not a good name given its semantics.
PR-URL: https://github.com/nodejs/node/pull/13192
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In Writable, 'finish' was not emitted when using writev() and
cork() in the event of an Error during the write. This commit
makes it consistent with the write() path, which emits 'finish'.
Fixes: https://github.com/nodejs/node/issues/11121
PR-URL: https://github.com/nodejs/node/pull/13195
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Use `common.mustNotCall()` to confirm that noop callbacks are not run
when functions throw errors.
PR-URL: https://github.com/nodejs/node/pull/13183
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
If node is configured --without-inspector/--without-ssl this test will
fail with the following error:
Path: inspector/test-bindings
inspector.js:8
throw new Error('Inspector is not available');
^
Error: Inspector is not available
at inspector.js:8:9
at NativeModule.compile (bootstrap_node.js:549:7)
This commit skips this test if the inspector is disabled.
PR-URL: https://github.com/nodejs/node/pull/13186
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: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Add test to cover napi_has_named_property
PR-URL: https://github.com/nodejs/node/pull/13178
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Use common.mustNotCall() to confirm that listener handles never run (as
no events are emitted).
PR-URL: https://github.com/nodejs/node/pull/13165
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>