Many tests use require() to import modules that subsequently never gets
used. This removes those imports and, in a few cases, removes other
unused variables from tests.
PR-URL: https://github.com/nodejs/node/pull/4684
Reviewed-By: Myles Borins <mborins@us.ibm.com>
On single core Windows systems, process.send() would cause an
EPIPE because of the ordering of the IPC channel disconnect and
the process.send().
The test was originally only relevant for non-Windows platforms,
so this commit merely skips the test on Windows.
Fixes: https://github.com/nodejs/node/issues/4450
PR-URL: https://github.com/nodejs/node/pull/4457
Reviewed-By: Rich Trott <rtrott@gmail.com>
Three tests designated as flaky on Linux have not failed on the
continuous integration server in a long time. Removing flaky designation
for these tests.
Fixes: https://github.com/nodejs/node/issues/4446
PR-URL: https://github.com/nodejs/node/pull/4519
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Use common.platformTimeout() to fix flaky
test-stream2-readable-empty-buffer-no-eofi on Raspberry Pis.
Fixes: https://github.com/nodejs/node/issues/4493
PR-URL: https://github.com/nodejs/node/pull/4516
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell<jasnell@gmail.com>
test-child-process-fork-net2.js checks that things happen within
certain time constraints, thus doubling as a benchmark test in addition
to a functionality test.
This change removes the time check, as it was causing the test to fail
on SmartOS and Windows (and possibly elsewhere) when the tests were
run in parallel on CI. There is no guarantee that other tests won't
consume enough resources to slow this test down, so don't check the time
constraints (beyond the generous timeout that the test is given by
test.py in the first place, of course).
If we want to do benchmark/performance tests, we should keep them
separate from pure functionality tests. The time check may have been a
remnant of the distant past when Node.js was much slower. It predates
io.js
Ref: https://github.com/nodejs/node/pull/4476
PR-URL: https://github.com/nodejs/node/pull/4494
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Refactor test to remove unnecessary booleans and one unnecesary timer.
Instead, throw Error objects where appropriate and rely on
common.mustCall().
The timer seemed to be the source of an issue when parallelizing tests.
Ref: https://github.com/nodejs/node/pull/4476#issuecomment-168080875
PR-URL: https://github.com/nodejs/node/pull/4490
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add a comment to clarify how the tests work and their purpose.
Also removes unnecessary assignment of domain module to a variable.
PR-URL: https://github.com/nodejs/node/pull/4474
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
Improves the message when an assertion fires in the
test-net-pipe-connect-errors so that it indicates the incorrect value
received rather than merely reporting that the value is incorrect.
PR-URL: https://github.com/nodejs/node/pull/4461
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Many test modules load assert but do not use it. This change removes
those instances.
It also removes a handful of other unused variables when they were
nearby.
PR-URL: https://github.com/nodejs/node/pull/4438
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It is possible that the internal hnadleMessage() might try to send to
a channel that has been closed. The result can be an AssertionError.
Guard against this.
Fixes: https://github.com/nodejs/node/issues/4205
PR-URL: https://github.com/nodejs/node/pull/4418
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
`aethrow` is defined as a function returned from makeBlock() but the
function is never used and the call to makeBlock() does not run any
tests.
PR-URL: https://github.com/nodejs/node/pull/4405
Reviewed-By: James M Snell <jasnell@gmail.com>
https requests with different SNI values should not be sent over the
same connection, even if the `host` is the same. Server may want to
present different certificate or route the incoming TLS connection
differently, depending on the received servername extension.
Fix: https://github.com/nodejs/node/issues/3940
PR-URL: https://github.com/nodejs/node/pull/4389
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
assert.deepEqual: when actual and expected are typed arrays,
wrap them in a new Buffer each to increase performance
significantly.
PR-URL: https://github.com/nodejs/node/pull/4330
Fixes: https://github.com/nodejs/node/issues/4294
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1c85849973 "fixed"
test-domain-exit-dispose-again by changing its logic to test that
process.domain was cleared properly in case an error was thrown from a
timer's callback.
However, it became clear when reviewing a recent change that refactors
lib/timers.js that it was not quite the intention of the original test.
Thus, this change adds the original implementation of
test-domain-exit-dispose-again back, with comments that make its
implementation easier to understand.
It also preserve the changes made by
1c85849973, but it moves them to a new
test file named test-timers-reset-process-domain-on-throw.js.
PR: #4256
PR-URL: https://github.com/nodejs/node/pull/4256
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
socket.destroy() triggers a 'close' event from the socket which triggers
the onClose handler of HTTPAgent which calls self.removeSocket(). So by
calling self.removeSocket() prior to socket.destroy() we end up with two
calls to self.removeSocket().
If there are pending requests, removeSocket ends up creating a new socket.
So if there are pending requests, each time a request completes, we tear
down one socket and create two more. So the total number of sockets grows
exponentially and without regard for any maxSockets settings. This was
noticed in https://github.com/nodejs/node/issues/4050. Let's get rid of
the extra calls to removeSocket so we only call it once per completed
request.
PR-URL: https://github.com/nodejs/node/pull/4172
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Load the certificate chain from the PFX file the same as we do it for a
regular certificate chain.
Fix: #4127
PR-URL: https://github.com/nodejs/node/pull/4165
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
If the deprecated NODE_REPL_HISTORY_FILE is set to default
node history file path ($HOME/.node_repl_history) and the file
doesn't exist, then node creates the file and then crashes when
it tries to parse that file as JSON thinking that it's an older
JSON formatted history file. This fixes that bug.
This patch also prevents node repl from throwing if the old
history file is empty or if $HOME/.node_repl_history is empty.
Fixes: https://github.com/nodejs/node/issues/4102
PR-URL: https://github.com/nodejs/node/pull/4108
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
When a client calls read() with a nonzero argument
on a Socket, that Socket sets this._consuming to true.
It never sets this._consuming back to false.
ChildProcess.flushStdio() currently doesn't flush
any streams where _consuming is truthy. But, that means
that it never flushes any stream that has ever been read from.
This prevents a child process from ever closing if one of
its streams has been read from, causing issue #4049. This
commit allows consuming streams to be flushed, and the
child process to emit a close event.
Fixes: https://github.com/nodejs/node/issues/4049
PR-URL: https://github.com/nodejs/node/pull/4071
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
A number of tests in `test/parallel` were importing the `util` module
via `require()` but not using `util` for anything. This removes those
`require()` statements.
PR-URL: https://github.com/nodejs/node/pull/4562
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
common.js needs to be loaded in all tests so that there is checking
for variable leaks and possibly other things. However, it does not
need to be assigned to a variable if nothing in common.js is referred
to elsewhere in the test.
The main tradeoff for this bit of code churn is that it gets the code
base most of the way to being able to enable the no-unused-vars rule in
eslint.
(The non-tooling benefit is that it lessens cognitive load when reading
tests as it is an immediate indication that none of the functions or
properties in common.js will be used by the test.)
PR-URL: https://github.com/nodejs/node/pull/4563
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fix module loading of third-party modules in the REPL by inheriting
module.paths from the REPL's parent module.
Commit ee72ee7 ("module,repl: remove repl require() hack") introduced
a regression where require() of modules in node_modules directories
no longer worked in the REPL (and fortunately only in the REPL.)
It turns out we didn't have test coverage for that but we do now.
Fixes: https://github.com/nodejs/node/issues/4208
PR-URL: https://github.com/nodejs/node/pull/4215
Reviewed-By: Roman Reiss <me@silverwind.io>
A number of REPL tests define the same ArrayStream object. This
commit moves the repeated code into common.js.
PR-URL: https://github.com/nodejs/node/pull/4027
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Conflicts:
test/common.js
Move the method that was added in commit 8ca412b from earlier this month
from lib/util.js to lib/internal/util.js.
Avoids exposing a method that we may not wish to expose just yet, seeing
how it relies on implementation details.
PR-URL: https://github.com/nodejs/node/pull/4026
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove a hack that was introduced in commit bb6d468d from November 2010.
This is groundwork for a follow-up commit that makes it possible to use
internal modules in lib/repl.js.
PR-URL: https://github.com/nodejs/node/pull/4026
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Conflicts:
lib/module.js
This commit adds the decorateErrorStack() method. This function
uses the internal util's getHiddenValue() method to extract
arrow messages from error objects and attach them to the
error's stack trace.
PR-URL: https://github.com/nodejs/node/pull/4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit adds an internal util method that makes hidden
values in the C++ layer visible in JS.
PR-URL: https://github.com/nodejs/node/pull/3988
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Conflicts:
lib/internal/util.js
Without these changes, the pi1-raspbian-wheezy CI node was timing
out on these tests.
PR-URL: https://github.com/nodejs/node/pull/4387
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
On Windows there can exist some race condition where the
notification of the client's `socket.destroy()` isn't received
before the server writes to the socket. This race condition was
more evident/reproducible on a single core system.
This commit fixes the flakiness by waiting until the server's
connection event handler has been called to destroy the client
socket and perform the server socket write.
Fixes: https://github.com/nodejs/node/issues/4057
PR-URL: https://github.com/nodejs/node/pull/4342
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: João Reis <reis@janeasystems.com>
Rewrite the test so that stderr reordering of the child processes won't
confuse the test's expectations.
PR-URL: https://github.com/nodejs/node/pull/4310
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It does not currently have any explicit tests to verify the behavior.
PR-URL: https://github.com/nodejs/node/pull/4283
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Check the stderr output in the `close` event as it's not guaranteed to
be fully available when the `exit` event is fired.
PR: #4364
PR-URL: https://github.com/nodejs/node/pull/4364
Reviewed-By: Julien Gilli <jgilli@fastmail.fm>
When sending a very large buffer (400000 bytes) the test fails due to
the client socket from the `a` server erroring with `ECONNRESET`.
There's a race condition between the closing of this socket and the `ssl`
socket closing on the other side of the connection. To improve things,
destroy the socket as soon as possible: in the `end` event of the `dest`
socket.
PR-URL: https://github.com/nodejs/node/pull/4195
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Make sure all the data is read before checking its validity.
Remove `gotHello` variable and just check that the ssl `end` event
is received.
Remove unused variables.
PR-URL: https://github.com/nodejs/node/pull/4195
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
When loading directory instead of file, no error message
is displayed. It's good to display error message for
this scenario.
PR-URL: https://github.com/nodejs/node/pull/4170
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
* Document that Symbol can used as event names.
* Add test for using Symbol as event names
PR-URL: https://github.com/nodejs/node/pull/4151
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If not running on Windows it skips the long path tests in:
* test-fs-long-path.js
* test-require-long-path.js
Fixes: https://github.com/nodejs/node/issues/2255
PR-URL: https://github.com/nodejs/node/pull/4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
If JS throws an object whose toString() method throws, then Node
attempts to print an empty message, but actually prints garbage.
This commit checks for this case, and prints a message instead.
Fixes: https://github.com/nodejs/node/issues/4079
PR-URL: https://github.com/nodejs/node/pull/4112
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
test-net-socket-local-address had a race condition that resulted in
unreliability on FreeBSD and Windows. This changes fixes the issue.
Fixes: https://github.com/nodejs/node/issues/2475
PR-URL: https://github.com/nodejs/node/pull/4109
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
This commit fixes two issues in test-cluster-disconnect-handles:
1. If the master's TCP connection to the worker fails, the worker
process stays alive and causes many other tests that use the same
common port number to also fail (with EADDRINUSE).
2. One particular problem that can cause the master's TCP connection
to fail is attempting an IPv6 connection to the worker when no IPv6
network interfaces are available.
PR-URL: https://github.com/nodejs/node/pull/4084
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It can happen that the HTTP connection is closed before the server has received
all the requests, thus the server close condition is never reached. To solve
this, close the server when the socket is fully closed.
PR-URL: https://github.com/nodejs/node/pull/4041
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
test-http-client-timeout-with-data fails on Raspberry Pi in CI from time
to time.
Use a platform-based timeout to improve reliability.
PR-URL: https://github.com/nodejs/node/pull/4015
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Roman Reiss <me@silverwind.io>
The modification time of a file is assumed to happen at the
exact time when it was requested. As the utime API specification
delcares that the resolution of the result is 1 second,
relax the constrain to 1 second helps the test case to be
robust and consistent under different load conditions in the system
PR-URL: https://github.com/nodejs/node/pull/3981
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Rich Trott
Brian White
Fedor Indutny
Claudio Rodriguez
Julien Gilli
Dave
Mudit Ameta
Ben Noordhuis
cjihrig
Evan Lucas
Jeremiah Senkpiel
Santiago Gimeno
Prince J Wesley
Bryan English
Rafał Pocztarski
Gireesh Punathil