test: fix a TODO and remove obsolete TODOs

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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · May 4, 2018 · 2b8b40f · 2b8b40f
1 parent 645a97a
commit 2b8b40f
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 18 deletions.
diff --git 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;

diff --git 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-fs-error-messages.js b/test/parallel/test-fs-error-messages.js
@@ -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

diff --git 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);
 }