From 46e2f50eed8d593fb12a0501a9ef3dd185a79488 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 05:44:14 +0200 Subject: [PATCH 1/5] test: remove obsolete TODOs --- test/parallel/test-cluster-http-pipe.js | 1 - test/parallel/test-util-isDeepStrictEqual.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/test/parallel/test-cluster-http-pipe.js b/test/parallel/test-cluster-http-pipe.js index 9e58fb297b28fe..9e039541cd26f1 100644 --- a/test/parallel/test-cluster-http-pipe.js +++ b/test/parallel/test-cluster-http-pipe.js @@ -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'); diff --git a/test/parallel/test-util-isDeepStrictEqual.js b/test/parallel/test-util-isDeepStrictEqual.js index 356a9a71324971..938781a43084a5 100644 --- a/test/parallel/test-util-isDeepStrictEqual.js +++ b/test/parallel/test-util-isDeepStrictEqual.js @@ -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); } From 1f204dd546377b05bafd4a7374f7ab4c2daa8c06 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 05:44:34 +0200 Subject: [PATCH 2/5] test: add fs.copyFile test This adds a test for invalid input and solves a TODO by doing so. --- test/parallel/test-fs-error-messages.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 61e51585a028c2..3968fd316aa4eb 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -636,21 +636,18 @@ 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 = { + message: 'EINVAL: invalid argument, copyfile ' + + `'${existingFile}' -> '${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)); assert.throws( () => fs.copyFileSync(existingFile, nonexistentFile, -1), From b6b64d96e19ab61f5b7aec95090a0630516c0c36 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 15:18:18 +0200 Subject: [PATCH 3/5] fixup: remove another TODO --- lib/url.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/url.js b/lib/url.js index ac9879a650fce6..e4326e80b5d948 100644 --- a/lib/url.js +++ b/lib/url.js @@ -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; From d09cb7007ca9660d472c97e5394a758c242ceecf Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 17:51:26 +0200 Subject: [PATCH 4/5] fixup: address test failure --- test/parallel/test-fs-error-messages.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 3968fd316aa4eb..fbc2c0d571ca4c 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -639,8 +639,7 @@ if (!common.isAIX) { // Check copyFile with invalid flags. { const validateError = { - message: 'EINVAL: invalid argument, copyfile ' + - `'${existingFile}' -> '${nonexistentFile}'`, + message: `EINVAL: invalid argument, copyfile -> '${nonexistentFile}'`, errno: uv.UV_EINVAL, code: 'EINVAL', syscall: 'copyfile' @@ -649,6 +648,8 @@ if (!common.isAIX) { fs.copyFile(existingFile, nonexistentFile, -1, common.expectsError(validateError)); + validateError.message = 'EINVAL: invalid argument, copyfile ' + + `'${existingFile}' -> '${nonexistentFile}'`; assert.throws( () => fs.copyFileSync(existingFile, nonexistentFile, -1), validateError From 83f63f1016c4c10b0df7dbb3c0026ef7e0901ff6 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 26 Apr 2018 17:52:18 +0200 Subject: [PATCH 5/5] fixup: add todo --- test/parallel/test-fs-error-messages.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index fbc2c0d571ca4c..28141f33f62965 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -639,6 +639,7 @@ if (!common.isAIX) { // Check copyFile with invalid flags. { 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',