Skip to content

Commit

Permalink
test: fix a TODO and remove obsolete TODOs
Browse files Browse the repository at this point in the history
This removes outdated TODOs and adds a test for invalid input in
`fs.copyFile` and solves a TODO by doing so.

PR-URL: #20319
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR authored and MylesBorins committed May 4, 2018
1 parent 645a97a commit 2b8b40f
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 18 deletions.
3 changes: 0 additions & 3 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,6 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
// http://a@b@c/ => user:a@b host:c
// http://a@b?@c => user:a host:b path:/?@c

// v0.12 TODO(isaacs): This is not quite how Chrome does things.
// Review our test case against browsers more comprehensively.

var hostEnd = -1;
var atSign = -1;
var nonHost = -1;
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-cluster-http-pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ if (cluster.isMaster) {
http.createServer(common.mustCall((req, res) => {
assert.strictEqual(req.connection.remoteAddress, undefined);
assert.strictEqual(req.connection.localAddress, undefined);
// TODO common.PIPE?

res.writeHead(200);
res.end('OK');
Expand Down
23 changes: 11 additions & 12 deletions test/parallel/test-fs-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,22 +636,21 @@ if (!common.isAIX) {
);
}

// copyFile with invalid flags
// Check copyFile with invalid flags.
{
const validateError = (err) => {
assert.strictEqual(err.message,
'EINVAL: invalid argument, copyfile ' +
`'${existingFile}' -> '${nonexistentFile}'`);
assert.strictEqual(err.errno, uv.UV_EINVAL);
assert.strictEqual(err.code, 'EINVAL');
assert.strictEqual(err.syscall, 'copyfile');
return true;
const validateError = {
// TODO: Make sure the error message always also contains the src.
message: `EINVAL: invalid argument, copyfile -> '${nonexistentFile}'`,
errno: uv.UV_EINVAL,
code: 'EINVAL',
syscall: 'copyfile'
};

// TODO(joyeecheung): test fs.copyFile() when uv_fs_copyfile does not
// keep the loop open when the flags are invalid.
// See https://github.com/libuv/libuv/pull/1747
fs.copyFile(existingFile, nonexistentFile, -1,
common.expectsError(validateError));

validateError.message = 'EINVAL: invalid argument, copyfile ' +
`'${existingFile}' -> '${nonexistentFile}'`;
assert.throws(
() => fs.copyFileSync(existingFile, nonexistentFile, -1),
validateError
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-util-isDeepStrictEqual.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,6 @@ notUtilIsDeepStrict([1, , , 3], [1, , , 3, , , ]);
const err3 = new TypeError('foo1');
notUtilIsDeepStrict(err1, err2, assert.AssertionError);
notUtilIsDeepStrict(err1, err3, assert.AssertionError);
// TODO: evaluate if this should throw or not. The same applies for RegExp
// Date and any object that has the same keys but not the same prototype.
notUtilIsDeepStrict(err1, {}, assert.AssertionError);
}

Expand Down

0 comments on commit 2b8b40f

Please sign in to comment.