Many of the tests use variables to track when callback functions
are invoked or events are emitted. These variables are then
asserted on process exit. This commit replaces this pattern in
straightforward cases with common.mustCall(). This makes the
tests easier to reason about, leads to a net reduction in lines
of code, and uncovered a few bugs in tests. This commit also
replaces some callbacks that should never be called with
common.fail().
PR-URL: https://github.com/nodejs/node/pull/7753
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
A number of test files use IIFEs to separate distinct tests from
each other in the same file. The project has been moving toward
using block scopes and let/const in favor of IIFEs. This commit
moves IIFE tests to block scopes. Some additional cleanup such
as use of strictEqual() and common.mustCall() is also included.
PR-URL: https://github.com/nodejs/node/pull/7694
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.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>
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.
PR-URL: https://github.com/nodejs/node/pull/4408
Reviewed-By: James M Snell <jasnell@gmail.com>
Enable linting for the test directory. A number of changes was made so
all tests conform the current rules used by lib and src directories. The
only exception for tests is that unreachable (dead) code is allowed.
test-fs-non-number-arguments-throw had to be excluded from the changes
because of a weird issue on Windows CI.
PR-URL: https://github.com/nodejs/io.js/pull/1721
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
In this situation:
writable.on('error', handler);
readable.pipe(writable);
writable.removeListener('error', handler);
writable.emit('error', new Error('boom'));
there is actually no error handler, but it doesn't throw, because of the
fix for stream.once('error', handler), in 23d92ec.
Note that simply reverting that change is not valid either, because
otherwise this will emit twice, being handled the first time, and then
throwing the second:
writable.once('error', handler);
readable.pipe(writable);
writable.emit('error', new Error('boom'));
Fix this with a horrible hack to make the stream pipe onerror handler
added before any other userland handlers, so that our handler is not
affected by adding or removing any userland handlers.
Closes#6007.
Problem: Since stream.pipe() is registering it's own error handlers on
the source and destination stream, it needs to replicate the
EventEmitter 'error' emitting semantics of throwing an error if there
are no other listeners. However, there was a off-by-one error because
the check for remaining listeners was done after cleanup() which means
the pipe's own listener was no longer included.
This would cause 'error' events on either the dest or the source to
throw if there was one other error listener, and while swallowing
the 'error' event if there was no other listener.
Solution: I added a test demonstrating the two issues and fixed the
problem by correcting the off-by-one error.
Fixes#1095.
cjihrig
Rich Trott
Roman Reiss
isaacs
Fedor Indutny
Felix Geisendörfer
Ryan Dahl
Oleg Efimov
Tobie Langel