Previously our tests did not check this codepath as seen at
coverage.nodejs.org
PR-URL: https://github.com/nodejs/node/pull/9555
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Previously our tests did not check these codepaths as seen at
coverage.nodejs.org
PR-URL: https://github.com/nodejs/node/pull/9556
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
* minor layout changes for clarity
* assert.equal() and assert.ok() swapped out for assert.strictEqual()
* var -> const for modules included via require()
PR-URL: https://github.com/nodejs/node/pull/9544
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This is a part of Code And Learn at NodeFest 2016 Challenge
Fixes: https://github.com/nodejs/code-and-learn/issues/58
PR-URL: https://github.com/nodejs/node/pull/9578
Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit improves the test cases in
test-stream2-objects.js by using assert.strictEqual
instead of assert.equal.
This is a part of Code And Learn at NodeFest 2016
Fixes: https://github.com/nodejs/code-and-learn/issues/58
PR-URL: https://github.com/nodejs/node/pull/9565
Reviewed-By: Shigeki Ohtsu <ohtsu@ohtsu.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
There are two instances of `setTimeout()` called without a duration in
`inspector-helper.js`. Change to `setImmediate()` for clarity that it
isn't a mistake.
PR-URL: https://github.com/nodejs/node/pull/9499
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Move copy/pasted callback into its own function.
PR-URL: https://github.com/nodejs/node/pull/9498
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
`test-regress-GH-897` is dependent on a timer firing within a period of
time. Especially on some of the FreeBSD hosts on CI, we have seen tests
like that fail when run in parallel. (This may have nothing to do with
FreeBSD and may just mean that the hosts are resource-constrained.) Move
this test to sequential as we have done with several other
timer-dependent tests recently.
The test has also been refactored and documented via comments.
PR-URL: https://github.com/nodejs/node/pull/9487
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
The test `debugger/test-debugger-repl-break-in-module` (and probably
others) was failing because the handshake message for debugging is no
longer `listening on port <port>` but is instead `listening on
<address>:<port>`.
This change makes the check less strict so as to hopefully future-proof
it at least a little bit against subsequent changes.
This test failure is not caught in CI because currently debugger tests
are not run in CI.
PR-URL: https://github.com/nodejs/node/pull/9486
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
The `/` character does not need to be escaped when occurring inside a
character class in a regular expression. Remove such instances of
escaping in the code base.
PR-URL: https://github.com/nodejs/node/pull/9485
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Calling JS during GC is a no-no. So intead create a queue of all ids
that need to have their destroy() callback called and call them later.
Removed checking destroy() in test-async-wrap-uid because destroy() can
be called after the 'exit' callback.
Missing a reliable test to reproduce the issue that caused the
FATAL_ERROR.
PR-URL: https://github.com/nodejs/node/pull/10096
Fixes: https://github.com/nodejs/node/issues/8216
Fixes: https://github.com/nodejs/node/issues/9465
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
test-debug-signal-cluster contains a watchdog timer that results in
false positives in CI. Remove the watchdog timer and let the test runner
determine that the test has timed out.
PR-URL: https://github.com/nodejs/node/pull/9476
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The changes introdcued here replace the deprecated
v8 method SetNamedPropertyHandler() to SetHandler()
in node.cc.
Prior to refactoring, the method defined callbacks
when accessing object properties defined by Strings
and not Symbols.
test/parallel/test-v8-interceptStrings-not-Symbols.js
demonstrates that this behaviour remained unchanged
after refactoring.
PR-URL: https://github.com/nodejs/node/pull/9062
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
There are multiple reports of Windows7 not being able to resolve
localhost on some setups (web search also confirms that). This change
will advertise "127.0.0.1" as inspector host name.
Fixes: https://github.com/nodejs/node/issues/9382
Fixes: https://github.com/nodejs/node/issues/9188
PR-URL: https://github.com/nodejs/node/pull/9451
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-stream2-readable-empty-buffer-no-eof fails on resource-constrained
machines due to its use of timers. Removing timers makes it more
reliable and doesn’t affect the validity of the test, as it only uses
relative timing relations.
Failures were noticed on freebsd10-64 in CI. I am able to replicate the
failure with `tools/test.py --repeat=100 -j 100`. When run alone, it
passes reliably.
Refs: https://github.com/nodejs/node/pull/9359
PR-URL: hkttps://github.com/nodejs/node/pull/9360
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Avoid the `exit` command to be sent more than once. It prevents from
undesired errors emitted on `proc.stdin`.
Remove the watchdog timer so the test does not fail in case it takes
longer to complete.
PR-URL: https://github.com/nodejs/node/pull/9490
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James Snell <jasnell@gmail.com>
Remove the timer just in case the test takes longer to complete.
PR-URL: https://github.com/nodejs/node/pull/9460
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Timer-dependent tests fail frequently on certain platforms in CI when
run in parallel with other tests, likely due to competition for
resources. Move test-repl-timeout-throw to sequential to avoid this
problem. Also did some minor refactoring (var->const and more use of
assert.strictEqual of looser assertions).
PR-URL: https://github.com/nodejs/node/pull/9431
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
PR-URL: https://github.com/nodejs/node/pull/7376
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
The timer is not necessary. The test will timeout via the test harness
if the test fails. This should resolve spurious CI failures.
PR-URL: https://github.com/nodejs/node/pull/9361
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* var -> const
* remove function names where V8 inference is as good or better
* add function names where there is no V8 inference
* assert.equal -> strictEqual
* move assertion from exit handler to response end handler
PR-URL: https://github.com/nodejs/node/pull/9344
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use assert.strictEqual() instead of assert.equal(),
=== instead of ==, and const instead of var.
PR-URL: https://github.com/nodejs/node/pull/9308
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove unneeded timers from some tests and move others from parallel
testing to sequential testing.
This is to resolve test failures on freebsd10-64 on CI. The failures
are all due to timers firing later than expected. Timers firing later
than they are set for can happen on resource-constrained hosts and is
not a bug.
In general, it may be wise to put tests that depend on timing into
sequential testing rather than parallel testing, as the timing can
be affected by other simultaneously-running test processes.
Fixes: https://github.com/nodejs/node/issues/8041
Fixes: https://github.com/nodejs/node/issues/9227
PR-URL: https://github.com/nodejs/node/pull/9317
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Johan Bergstrom <bugs@bergstroem.nu>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
The test was sometimes timing out due to a race condition. In OS X,
events for `fs.watch()` might only start showing up after a delay. This
is a limitation of the operating system. To work around that, there was
a timer in the test that delayed the writing of the file by 100ms.
However, sometimes that was not enough, and so the event never fired,
and the test timed out.
Change the timer to an interval so that it fires repeatedly until it is
picked up. This change only affects OS X.
Fixes: https://github.com/nodejs/node/issues/8511
PR-URL: https://github.com/nodejs/node/pull/9303
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='
PR-URL: https://github.com/nodejs/node/pull/9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The test has two test cases, but only the first was being run due to a
small bug. This change fixes the bug.
PR-URL: https://github.com/nodejs/node/pull/9305
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Instead of writing to the REPL’s input stream for the alignment
spaces in `.editor` mode, let `readline` handle the spaces
properly (echoing them using `_ttyWrite` and adding them to the
current line buffer).
Fixes: https://github.com/nodejs/node/issues/9189
PR-URL: https://github.com/nodejs/node/pull/9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
In `.editor` mode, `repl.write()` would have crashed when the
`key` argument was not present, because the overwritten
`_ttyWrite` of REPLs doesn’t check for the absence of a second
argument like `readline.write()` does.
Since the docs indicate that the argument is optional, add
a check paralleling the one in `readline.write()`.
PR-URL: https://github.com/nodejs/node/pull/9207
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replaces function expressions with ES6 arrow functions as well as
improve the comparison operator to check if operands are of same types.
PR-URL: https://github.com/nodejs/node/pull/9239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Replace calls to assert.equal() and assert.notEqual() with
assert.strictEqual() and assert.strictNotEqual() respectively.
PR-URL: https://github.com/nodejs/node/pull/9263
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Removed the errorTimer from test-http-set-timeout.js, as this timer is
not necessary to test the setTimeout functionality.
Also edited the console.log message on line 8 to log the correct
timeout duration. Changed var to const, and added common.mustCall() to
on timeout and on error callbacks.
Fixes: https://github.com/nodejs/node/issues/9256
PR-URL: https://github.com/nodejs/node/pull/9264
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Writing data to TLSWrap instance during handshake will result in it
being queued in `write_item_queue_`. This queue won't get cleared up
until the end of the handshake.
Technically, it gets cleared on `~TLSWrap` invocation, however this
won't ever happen because every `WriteWrap` holds a reference to the
`TLSWrap` through JS object, meaning that they are doomed to be alive
for eternity.
To breach this dreadful contract a knight shall embark from the
`close` function to kill the dragon of memory leak with his magic
spear of `destroySSL`.
`destroySSL` cleans up `write_item_queue_` and frees `SSL` structure,
both are good for memory usage.
PR-URL: https://github.com/nodejs/node/pull/9586
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
- Changed `assert.ok()` to `assert.strictEqual()`.
- Changed `var` to `const` where possible.
PR-URL: https://github.com/nodejs/node/pull/9231
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
This commit adds coverage for the timeout option used by
child_process exec() and execFile().
PR-URL: https://github.com/nodejs/node/pull/9208
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This fixes one of the tests that has been failing on CI on freebsd for
a bit by removing an unnecessary timer.
PR-URL: https://github.com/nodejs/node/pull/9199
Fixes: https://github.com/nodejs/node/issues/7929
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
59714cb7b3 introduced the
`util.inspect.custom` symbol, but it was exported as
`customInspectSymbol` by `internal/util.js` and referenced as
`inspectSymbol` by `buffer.js`.
PR-URL: https://github.com/nodejs/node/pull/9289
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is a partial revert of
14d1a8a631, which coerced the offset
of Buffer#slice() using the | operator. This causes some edge
cases to be handled incorrectly. This commit restores the old
behavior, but converts offsets to integers using Math.trunc().
This commit does not revert any tests, and adds an additional
regression test.
Refs: https://github.com/nodejs/node/issues/9096
Refs: https://github.com/nodejs/node/pull/9101
PR-URL: https://github.com/nodejs/node/pull/9341
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
As shown in https://github.com/nodejs/node/issues/9096, the offset and
end value of the `slice` call are coerced to numbers and then passed to
`FastBuffer`, which internally truncates the mantissa part if the number
is actually a floating point number. This actually affects the new
length of the slice calculation. For example,
> const original = Buffer.from('abcd');
undefined
> original.slice(original.length / 3).toString()
'bc'
This happens because, starting value of the slice is 4 / 3, which is
1.33 (approximately). Now, the length of the slice is calculated as
the difference between the actual length of the buffer and the starting
offset. So, it becomes 2.67 (4 - 1.33). Now, a new `FastBuffer` is
constructed, with the following values as parameters,
1. actual buffer object,
2. starting value, which is 1.33 and
3. the length 2.67.
The underlying C++ code truncates the numbers and they become 1 and 2.
That is why the result is just `bc`.
This patch makes sure that all the offsets are coerced to integers
before any calculations are done.
Fixes: https://github.com/nodejs/node/issues/9096
PR-URL: https://github.com/nodejs/node/pull/9101
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
This commit adds coverage for errors returned by execFileSync()
when the child process exits with a non-zero code.
PR-URL: https://github.com/nodejs/node/pull/9211
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Commit 93a44d5 ("src: fix deferred events not working with -e") defers
evaluation of the script to the next tick.
A side effect of that change is that 'beforeExit' listeners run before
the actual script. 'beforeExit' is emitted when the event loop is
empty but process.nextTick() does not ref the event loop.
Fix that by using setImmediate(). Because it is implemented in terms
of a uv_check_t handle, it interacts with the event loop properly.
Fixes: https://github.com/nodejs/node/issues/8534
PR-URL: https://github.com/nodejs/node/pull/8821
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove parallel/test-v8-inspector-json-protocol, it duplicates the test
found in inspector/test-inspector.
PR-URL: https://github.com/nodejs/node/pull/9184
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fix a nullptr dereference when an invalid path is requested.
Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.
PR-URL: https://github.com/nodejs/node/pull/9184
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add a test for _writableState.needDrain.
PR-URL: https://github.com/nodejs/node/pull/8799
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Related: https://github.com/nodejs/node/issues/8686
By convention, inspector protocol targets do not advertise connection
URLs when the frontend is already connected as multiple inspector
protocol connections are not supported.
PR-URL: https://github.com/nodejs/node/pull/8919
Reviewed-By: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Jeremiah Senkpiel
Rich Trott
MURAKAMI Masahiko
Yoshiya Hinosawa
Trevor Norris
Michael Macherey
AnnaMag
Eugene Ostroukhov
Anna Henningsen
Santiago Gimeno
Gibson Fahnestock
Kyle E. Mitchell
Isobel Redelmeier
Gerges Beshay
Deverick
Ben Noordhuis
BethGriggs
Fedor Indutny
Jeena Lee
cjihrig
Evan Lucas
Charmander
Sakthipriyan Vairamani (thefourtheye)
Italo A. Casas