Added duration to setTimeout and removed extraneous callback args,
as per Rich Trott's instructions
PR-URL: https://github.com/nodejs/node/pull/10291
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
file: test/parallel/test-domain-uncaught-exception.js
1. There are three setTimeout() in the file and they do not specify a
duration (the second argument), so I change them to setImmediate()
instead.
2. There are four callbacks that take an argument called `err` but that
argument is never used, so I removed them.
PR-URL: https://github.com/nodejs/node/pull/10193
Reviewed-By: Sam Roberts <sam@strongloop.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Fix the fast path for `buffer.fill()` with a single-character string.
The fast path only works for strings that are equivalent to a
single-byte buffer, but that condition was not checked properly
for the `utf8` or `utf16le` encodings and is always true for the
`latin1` encoding.
This change fixes these problems.
Fixes: https://github.com/nodejs/node/issues/9836
PR-URL: https://github.com/nodejs/node/pull/9837
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
There was a byte-order mismatch for `buffer#fill` on big-endian
platforms. Weirdly, the tests seemed to expect that wrong behaviour.
PR-URL: https://github.com/nodejs/node/pull/9837
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Instead of ignoring missing `.out` files for message/pseudo-tty tests,
raise an error to indicate that something is not quite right.
Ref: https://github.com/nodejs/node/pull/10037
PR-URL: https://github.com/nodejs/node/pull/10150
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Add a hostPattern variable for readable purposes
PR-URL: https://github.com/nodejs/node/pull/9653
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
After processing all the callbacks in the destroy_ids vector make sure
to clear() it otherwise the DestroyIdsCb() won't run again.
PR-URL: https://github.com/nodejs/node/pull/10400
Fixes: b49b496 "async_wrap: call destroy() callback in uv_idle_t"
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fix leaking the BIO in the error path. Introduced in commit 34febfbf4
("crypto: fix handling of root_cert_store").
PR-URL: https://github.com/nodejs/node/pull/9604
Refs: https://github.com/nodejs/node/pull/9409
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
SecureContext::AddRootCerts only parses the root certificates once and
keeps the result in root_cert_store, a global X509_STORE. This change
addresses the following issues:
1. SecureContext::AddCACert would add certificates to whatever
X509_STORE was being used, even if that happened to be root_cert_store.
Thus adding a CA certificate to a SecureContext would also cause it to
be included in unrelated SecureContexts.
2. AddCRL would crash if neither AddRootCerts nor AddCACert had been
called first.
3. Calling AddCACert without calling AddRootCerts first, and with an
input that didn't contain any certificates, would leak an X509_STORE.
4. AddCRL would add the CRL to whatever X509_STORE was being used. Thus,
like AddCACert, unrelated SecureContext objects could be affected.
The following, non-obvious behaviour remains: calling AddRootCerts
doesn't /add/ them, rather it sets the CA certs to be the root set and
overrides any previous CA certificates.
Points 1–3 are probably unimportant because the SecureContext is
typically configured by `createSecureContext` in `lib/_tls_common.js`.
This function either calls AddCACert or AddRootCerts and only calls
AddCRL after setting up CA certificates. Point four could still apply in
the unlikely case that someone configures a CRL without explicitly
configuring the CAs.
PR-URL: https://github.com/nodejs/node/pull/9409
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Setting reference count at the time of setting cert_store instead of
trying to manage it by modifying internal states in destructor.
PR-URL: https://github.com/nodejs/node/pull/9409
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Use zero-copy external string resources for storing the built-in JS
source code. Saves a few hundred kilobyte of memory and consistently
speeds up `benchmark/misc/startup.js` by 2.5%.
Everything old is new again! Commit 74954ce ("Add string class that
uses ExternalAsciiStringResource.") from 2011 did the same thing but
I removed that in 2013 in commit 34b0a36 ("src: don't use NewExternal()
with unaligned strings") because of a limitation in the V8 API.
V8 no longer requires that strings are aligned if they are one-byte
strings so it should be safe to re-enable external strings again.
PR-URL: https://github.com/nodejs/node/pull/5458
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
This commit addresses an inconsistency with eight tests. These
tests use the assert module, but named the variable ASSERT. This
goes against the project's typical coding style, and negatively
impacts global find and replace updates.
PR-URL: https://github.com/nodejs/node/pull/10544
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* add RegExp as second argument to assert.throws()
* replace process.on('exit', ...) boolean checks with common.mustCall()
* assert.equal() -> assert.strictEqual()
* add 1 ms duration as second argument to setTimeout()
* var -> const
PR-URL: https://github.com/nodejs/node/pull/10532
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* provide a RegExp for second argument to `assert.throws()`
* remove unused function arguments
* provide duration of 1 ms for `setTimeout()` calls that do not have a
duration
PR-URL: https://github.com/nodejs/node/pull/10530
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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 common.mustCall to control the functions execution automatically
* use let and const instead of var
* use assert.strictEqual instead of assert.equal
* use arrow functions
* remove console.error and unnecessary variables
PR-URL: https://github.com/nodejs/node/pull/10397
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Remove console.log statement. Replace error check with
assert.ifError().
PR-URL: https://github.com/nodejs/node/pull/10331
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* remove the manual control for functions execution
* use common.mustCall to control the functions execution automatically
* use let and const instead of var
* use assert.strictEqual instead assert.equal
PR-URL: https://github.com/nodejs/node/pull/10200
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@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 unnecessary variables
PR-URL: https://github.com/nodejs/node/pull/10440
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Swap var for let/const throughout the common.js module.
Change a snake case variable to camel case starting on line 168.
PR-URL: https://github.com/nodejs/node/pull/10177
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
When decodeStrings is false and given data is string, _write() function
receives the string data not `Buffer`.
PR-URL: https://github.com/nodejs/node/pull/10411
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Object.setPrototypeOf() -> Object.create()
PR-URL: https://github.com/nodejs/node/pull/10451
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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>
* use const instead of var
* use common.mustCall to control the functions execution automatically
* use assert.strictEqual instead of assert.equal
* use assert.notStrictEqual instead of assert.notEqual
* use arrow functions
PR-URL: https://github.com/nodejs/node/pull/10367
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var
* use assert.strictEqual instead of assert.equal
PR-URL: https://github.com/nodejs/node/pull/10429
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Extend `fs.realpathSync` to cache the results for paths that are not
symlinks in addition to caching symlink mappings.
PR-URL: https://github.com/nodejs/node/pull/10253
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This test runs based on a expectation that the stderr will get the
string 'Debugger listening on port'. But the actual message printed
to stderr has changed to 'Debugger listening on host:port'. So the
the actuals tests did not even start and eventually timeout.
Apart from that, changed `var`s to `let`s or `const`s.
Refs: https://github.com/nodejs/node/issues/10361
PR-URL: https://github.com/nodejs/node/pull/10370
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com
PR-URL: https://github.com/nodejs/node/pull/10390
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add configuration object createBenchmark object for buffer size &
iteration in buffer-base64-encode & buffer-base64-decode.js.
PR-URL: https://github.com/nodejs/node/pull/10175
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Follow up from 8eb19c4. Lower case `return` was not updated
PR-URL: https://github.com/nodejs/node/pull/10391
Ref: https://github.com/nodejs/node/pull/9554
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
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>
`./tools/macosx-firewall.sh` fails if run before build step. Since the
suggestion comes before the build steps in the document, this change
clarifies that the script should be run after building.
PR-URL: https://github.com/nodejs/node/pull/10311
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use common.mustCall to control the functions execution automatically
* use const instead of var
* use assert.strictEqual instead assert.equal
* use assert.ifError instead of throw error
PR-URL: https://github.com/nodejs/node/pull/10297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* use const instead of var for required modules
* use assert.strictEqual instead of assert.equal
PR-URL: https://github.com/nodejs/node/pull/10318
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In this test, I changed the var declarations to be either a let or a
const. For some of the callbacks, I added a mustCall check to ensure
that the functions have run. I also changed assert.equal() to
assert.strictEqual().
PR-URL: https://github.com/nodejs/node/pull/9962
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- var -> const where possible
- assert.equal -> assert.strictEqual
- passed the setTimeout function a second parameter for readability
- used assert.strictEqual for assert(!c) as it is expected to be 0 and
not some other value
PR-URL: https://github.com/nodejs/node/pull/10321
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.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>
10.5 error Please use assert.strictEqual() instead of assert.equal()
PR-URL: https://github.com/nodejs/node/pull/10047
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/9988
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
weyj4
Andy Chen
Siddhartha Sahai
Anna Henningsen
Gregory
Eugene Ostroukhov
Axel Monroy
Lance Ball
Trevor Norris
Ben Noordhuis
Adam Langley
Adam Majer
Tanuja-Sawant
William Kapke
cjihrig
Rich Trott
Adrian Estrada
Rob Adelmann
Paul Graham
Fumiya KARASAWA
Vse Mozhet Byt
Jeremy Yallop
Sakthipriyan Vairamani (thefourtheye)
Troy Connor
Myles Borins
Chase Starr
Daniel Sims
Emanuel Buholzer
Segu Riluvan
Ashita Nagesh