A few tests in test/gc include the http module twice. Remove duplicate
require().
PR-URL: https://github.com/nodejs/node/pull/4606
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
It's not guaranteed that the socket data is received in the same chunk
as the upgrade response. Listen for the `data` event to make sure all
the data is received.
PR-URL: https://github.com/nodejs/node/pull/4602
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.
Fixes: https://github.com/nodejs/node/issues/4559
PR-URL: https://github.com/nodejs/node/pull/4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
If one were to set NODE_REPL_HISTORY to a string that contains only a
space (" "), then the history file would be created with that name
which can cause problems are certain systems.
PR-URL: https://github.com/nodejs/node/pull/4539
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
It's not guaranteed that the socket data is received in the same chunk
as the upgrade response. Listen for the `data` event to make sure all
the data is received.
PR-URL: https://github.com/nodejs/node/pull/4520
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
It avoids the creation of unnecessary handles. This issue is causing
intermitent failures in `test-cluster-disconnect-race` on `FreeBSD`
and `OS X`.
The problem is that the `worker2.disconnect` is being called on the
master before the `queryServer` is handled, causing the worker to
be deleted, then the Server handle is created afterwards. Later on,
when `removeWorker` is called from the `exit` handler, there are no
workers left, but one handle, thus the `AssertionError`.
Add a new `test/sequential/test-cluster-disconnect-leak` based on
`test-cluster-disconnect-race` that creates lots of workers and fails
consistently without this patch.
PR-URL: https://github.com/nodejs/node/pull/4465
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.
Fixes: https://github.com/nodejs/node/issues/4456
PR-URL: https://github.com/nodejs/node/pull/4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Before this commit, it was possible on Windows for the server's
'connection' handler to be called *after* the client socket's
'connect' handler. This caused the 'message' event to be missed
and the test would never end (timing out in CI). This problem
was more easily reproducible on a low resource (slow CPU)
Windows (2012r2) installation.
This commit waits until both handlers have been called before
sending the handle to the master process.
Fixes: https://github.com/nodejs/node/issues/3957
PR-URL: https://github.com/nodejs/node/pull/4444
Reviewed-By: Rich Trott <rtrott@gmail.com>
Windows would die with ECONNRESET most times when running
this particular test. This commit makes handling these errors
more tolerable.
PR-URL: https://github.com/nodejs/node/pull/4442
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
There is no guarantee that the `suicide` property of a worker in the
master process is going to be set when the `disconnect` and `exit`
events are emitted.
To fix it, wait for the ACK of the suicide message from the master
before disconnecting the worker. Also, there's no need to send the
suicide message from the worker if the disconnection has been
initiated in the master.
Add `test-cluster-disconnect-suicide-race` that forks a lot of workers
to consistently reproduce the issue this patch tries to solve.
Modify `test-regress-GH-3238` so it checks both the `kill` and
`disconnect` cases. Also take into account that the `disconnect` event
may be received after the `exit` event.
PR-URL: https://github.com/nodejs/node/pull/4349
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
In some conditions it can happen that the client-side socket is
destroyed before the server-side socket has gracefully closed, thus
causing a 'ECONNRESET' error in this socket. To solve this, also close
gracefully in the client side.
PR-URL: https://github.com/nodejs/node/pull/3966
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Copy client CA certs and cert store when asynchronously selecting
`SecureContext` during `SNICallback`. We already copy private key,
certificate, and certificate chain, but the client CA certs were
missing.
Fix: #2772
PR-URL: https://github.com/nodejs/node/pull/3537
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.
PR-URL: https://github.com/nodejs/node/pull/4477
Reviewed-By: Rich Trott <rtrott@gmail.com>
test-fs-realpath.js was writing files to the fixture dir. This changes
it to use the temp directory instead. This also replaces some of the
string concatenation for paths with uses of path.join() and
path.relative().
PR-URL: https://github.com/nodejs/node/pull/4489
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
A few tests assumed that temp dirs always lived in the same
parent folder as fixtures. Make these use `common.tmpDir` instead.
PR-URL: https://github.com/nodejs/node/pull/3325
Reviewed-By: Joao Reis <reis@janeasystems.com>
In CI we previously passed `NODE_COMMON_PIPE` to the test runner to
avoid long filenames. Add an option to the test runner that allows the
user to change the temporary directory instead. This also allows us to
run test suites in parallel since `NODE_COMMON_PIPE` otherwise would
have been used from multiple tests at the same time.
PR-URL: https://github.com/nodejs/node/pull/3325
Reviewed-By: Joao Reis <reis@janeasystems.com>
Remove all remaining unused variables from tests in test/parallel.
PR-URL: https://github.com/nodejs/node/pull/4511
Reviewed-By: James M Snell<jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
The http tests seem especially prone to including unused variables.
This change removes them.
PR-URL: https://github.com/nodejs/node/pull/4422
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Debug mode slows execution speed. There is work afoot to enable Debug
mode runs on the continuous integration infrastructure for the project.
Some tests are timing out, such as test-net-GH-5504.js.
This change doubles the timeout returned from `common.platformTimeout()`
when running in Debug mode. It also removes an unused variable from the
aforementioned test-net-GH-5504.js.
PR-URL: https://github.com/nodejs/node/pull/4431
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Some of the TLS tests have variables that do not get used. This removes
those variables.
PR-URL: https://github.com/nodejs/node/pull/4424
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Remove a handful of variables that are declared but never used in the
tests for the net module.
PR-URL: https://github.com/nodejs/node/pull/4430
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In addition to removing unused vars, this also fixes an instance where
booleans were set presumably to check something but then never used.
This now confirms that the events that were setting the booleans are
fired.
PR-URL: https://github.com/nodejs/node/pull/4425
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Remove unused vars in tests
PR-URL: https://github.com/nodejs/node/pull/4536
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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>
Rich Trott
Santiago Gimeno
Evan Lucas
Brian White
Santiago Gimeno
Fedor Indutny
Michaël Zasso
Ben Noordhuis
James M Snell
cjihrig
Johan Bergström
Claudio Rodriguez
Julien Gilli
Dave