In most cases, named functions match the variable or property to which
they are being assigned. That also seems to be the practice in a series
of PRs currently being evaluated that name currently-anonymous
functions.
This change applies that rule to instances in the code base that don't
comply with that practice.
This will be enforceable with a lint rule once we upgrade to ESLint
3.8.0.
PR-URL: https://github.com/nodejs/node/pull/9113
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
The `test/common` module has the capability to identify if any variable
is leaked to the global scope and fail the test. So that has to be
imported at the beginning.
PR-URL: https://github.com/nodejs/node/pull/7786
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
20285ad177 changed the format
of error messages throughout lib. However, the tests were not
updated to reflect these changes. This commit makes those
changes.
PR-URL: https://github.com/nodejs/node/pull/3727
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently there are many instances where assert.fail is directly passed
to a callback for error handling. Unfortunately this will swallow the
error as it is the third argument of assert.fail that sets the message
not the first.
This commit adds a new function to test/common.js that simply wraps
assert.fail and calls it with the provided message.
Tip of the hat to @trott for pointing me in the direction of this.
PR-URL: https://github.com/nodejs/node/pull/3453
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Port of joyent/node commits:
* e17c5a72b2
* 70dafa7b62
Pull over test-child-process-spawn-typeerror.js from v0.12, replacing
the existing test in master. The new test includes a broader set of
tests on the various arg choices and throws.
Reviewed-By: trevnorris - Trevor Norris <trevnorris@nodejs.org>
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: thefourtheye - Sakthipriyan Vairamani
PR-URL: https://github.com/nodejs/node/pull/2667
Fixes: https://github.com/nodejs/node/issues/2515
In the tests, we use "process.platform === 'win32'" in some places.
This patch replaces them with the "common.isWindows" for consistency.
PR-URL: https://github.com/nodejs/io.js/pull/2269
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.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 test wasn't checking directly that an assertion was thrown. Instead,
it was checking that spawn did not sucessfully spawn a non-existent
command.
However, the command chosen, dir, exists in GNU coreutils, so it exists
on Linux (though not on BSD derived OS X). The test as written passed on
Linux, even with the TypeError it is supposed to be checking for deleted
from spawn(). It would also pass on Windows if a ls.exe existed.
The approach is unnecessarily obscure, assert.throw() is for asserting
code throws, using it is more clear and works regardless of what
commands do or do not exist.
PR-URL: https://github.com/joyent/node/pull/8454
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Cherry-picked-from: 2ff29cc7e3
Conflicts:
test/parallel/test-child-process-spawn-typeerror.js
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.
You cannot spawn 'dir' on Windows because it's not an executable. Also,
some people might have 'ls' on their path on Windows, so I changed
invalidCmd to something that's highly unlikely to exist.
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Add stricter argument type checking to normalizeSpawnArguments().
Removes a number of extraneous checks in spawn().
Fix regression in handling of the optional args argument.
Add more thorough testing of spawn() arguments.
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Ensure TypeError is thrown, fix a bug where `env` option was
assuming the option was actually an object.
This case is especially bad because it then sets `env == null`
instead of using `process.env`.
Fix#7456
Signed-off-by: Fedor Indutny <fedor@indutny.com>
Rich Trott
Sakthipriyan Vairamani
Chuck Langford
cjihrig
Myles Borins
James M Snell
Roman Reiss
Sam Roberts
isaacs
Fedor Indutny
Alexis Campailla
Greg Sabia Tucker
Ben Noordhuis
Ryan Dahl
Oleg Efimov
Felix Geisendörfer