test, fs: introduce helper function to test fs.* methods as fsPromises.* methods

test/common/README.md

@@ -120,11 +120,33 @@ an expected warning does not have a code then `common.noWarnCode` can be used
 to indicate this.
 
 ### fileExists(pathname)
-* pathname [<string>]
+* `pathname` [<string>]
 * return [<boolean>]
 
 Checks if `pathname` exists
 
+### fsTest(method, args, options)
+* `method` [<string>]
+* `args` [<Array>]
+* `options` [<Object>]
+
+Run both `fs[method]()` and `fsPromises[method]()` passing `args`.
+`util.callbackify()` is used to convert the `fsPromises` version of the method
+into one that takes a callback. The last value in `args` must be a callback that
+is expected to run once for each of the two invocations.
+
+The `options` object may contain a `setup` property that is a function that is
+run before each test. This function might refresh the temporary directory if
+both tests need to use it, for example. The `options` object may also contain a
+`throws` property that is a boolean indicating if the `fs[method]()` function is
+expected to throw. The `fsPromises[method]()` function will be expected to
+reject. Lastly, the `options` object may contain a `differentFiles` array. If
+`differentFiles` is included, then the first element is inserted as the first
+argument (typically the path to a file) when testing the callback-based API and
+the second element is inserted as the first argument when testing the
+Promise-based API. This can be useful when testing APIs that modify the target
+file.
+
 ### getArrayBufferViews(buf)
 * `buf` [<Buffer>]
 * return [<ArrayBufferView[]>]

test/common/index.js

@@ -892,3 +892,37 @@ exports.isCPPSymbolsNotMapped = exports.isWindows ||
                                 exports.isAIX ||
                                 exports.isLinuxPPCBE ||
                                 exports.isFreeBSD;
+
+// Run test for both fs.method and fsPromises.method with args.
+// The last value in args must be a callback that is expected to run once for
+// each call to the method named in `method`.
+exports.fsTest = (method, args, options = {}) => {
+  const fsPromises = fs.promises;
+
+  if (options.differentFiles)
+    args.unshift(options.differentFiles[0]);
+
+  if (options.setup)
+    options.setup();
+  const checkResults = args.pop();
+
+  const wrappedCheck = (...callbackArgs) => {
+    checkResults(...callbackArgs);
+    if (options.setup)
+      options.setup();
+    const callbackified = util.callbackify(fsPromises[method]);
+    if (options.differentFiles)
+      args[0] = options.differentFiles[1];
+    callbackified(...args, exports.mustCall(checkResults));
+    return true;
+  };
+
+  if (options.throws) {
+    assert.throws(
+      () => { fs[method](...args, exports.mustNotCall()); },
+      wrappedCheck
+    );
+  } else {
+    fs[method](...args, exports.mustCall(wrappedCheck));
+  }
+};

test/parallel/test-fs-access.js

@@ -62,38 +62,37 @@ assert.strictEqual(typeof fs.R_OK, 'number');
 assert.strictEqual(typeof fs.W_OK, 'number');
 assert.strictEqual(typeof fs.X_OK, 'number');
 
-fs.access(__filename, common.mustCall(assert.ifError));
-fs.access(__filename, fs.R_OK, common.mustCall(assert.ifError));
-fs.access(readOnlyFile, fs.F_OK | fs.R_OK, common.mustCall(assert.ifError));
+common.fsTest('access', [__filename, assert.ifError]);
+common.fsTest('access', [__filename, fs.R_OK, assert.ifError]);
+common.fsTest('access', [readOnlyFile, fs.F_OK | fs.R_OK, assert.ifError]);
 
-fs.access(doesNotExist, common.mustCall((err) => {
+common.fsTest('access', [doesNotExist, (err) => {
   assert.notStrictEqual(err, null, 'error should exist');
   assert.strictEqual(err.code, 'ENOENT');
   assert.strictEqual(err.path, doesNotExist);
-}));
+}]);
 
-fs.access(readOnlyFile, fs.W_OK, common.mustCall(function(err) {
+common.fsTest('access', [readOnlyFile, fs.W_OK, function(err) {
   assert.strictEqual(this, undefined);
   if (hasWriteAccessForReadonlyFile) {
     assert.ifError(err);
   } else {
     assert.notStrictEqual(err, null, 'error should exist');
     assert.strictEqual(err.path, readOnlyFile);
   }
-}));
+}]);
 
-common.expectsError(
-  () => {
-    fs.access(100, fs.F_OK, common.mustNotCall());
-  },
-  {
-    code: 'ERR_INVALID_ARG_TYPE',
-    type: TypeError,
-    message: 'The "path" argument must be one of type string, Buffer, or URL.' +
-             ' Received type number'
-  }
-);
+{
+  const expectedError = (e) => {
+    assert.strictEqual(e.code, 'ERR_INVALID_ARG_TYPE');
+    assert(e instanceof TypeError);
+  };
+
+  common.fsTest('access', [100, fs.F_OK, expectedError], { throws: true });
+}
 
+// These tests do not use common.fsTest because the equivalent of a callback
+// is not required in fsPromises.access(). You know, because: Promises.
 common.expectsError(
   () => {
     fs.access(__filename, fs.F_OK);

test/parallel/test-fs-append-file.js

@@ -23,11 +23,12 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
-const join = require('path').join;
+const fsPromises = fs.promises;
+const { join } = require('path');
+const util = require('util');
 
 const tmpdir = require('../common/tmpdir');
-
-const filename = join(tmpdir.path, 'append.txt');
+tmpdir.refresh();
 
 const currentFileData = 'ABCD';
 
@@ -40,122 +41,114 @@ const s = '南越国是前203年至前111年存在于岭南地区的一个国家
           '历经五代君主。南越国是岭南地区的第一个有记载的政权国家，采用封建制和郡县制并存的制度，' +
           '它的建立保证了秦末乱世岭南地区社会秩序的稳定，有效的改善了岭南地区落后的政治、##济现状。\n';
 
-let ncallbacks = 0;
-
-tmpdir.refresh();
-
 // test that empty file will be created and have content added
-fs.appendFile(filename, s, function(e) {
-  assert.ifError(e);
-
-  ncallbacks++;
+{
+  const filename = join(tmpdir.path, 'append.txt');
 
-  fs.readFile(filename, function(e, buffer) {
+  common.fsTest('appendFile', [filename, s, function(e) {
     assert.ifError(e);
-    ncallbacks++;
-    assert.strictEqual(Buffer.byteLength(s), buffer.length);
-  });
-});
 
-// test that appends data to a non empty file
-const filename2 = join(tmpdir.path, 'append2.txt');
-fs.writeFileSync(filename2, currentFileData);
+    const buffer = fs.readFileSync(filename);
+    assert.strictEqual(buffer.length, Buffer.byteLength(s));
+    fs.unlinkSync(filename);
+  }]);
+}
 
-fs.appendFile(filename2, s, function(e) {
-  assert.ifError(e);
+// test that appends data to a non empty file
+{
+  const filename = join(tmpdir.path, 'append2.txt');
 
-  ncallbacks++;
+  const setup = () => {
+    fs.writeFileSync(filename, currentFileData);
+  };
 
-  fs.readFile(filename2, function(e, buffer) {
+  common.fsTest('appendFile', [filename, s, function(e) {
     assert.ifError(e);
-    ncallbacks++;
-    assert.strictEqual(Buffer.byteLength(s) + currentFileData.length,
-                       buffer.length);
-  });
-});
 
-// test that appendFile accepts buffers
-const filename3 = join(tmpdir.path, 'append3.txt');
-fs.writeFileSync(filename3, currentFileData);
+    const buffer = fs.readFileSync(filename);
+    assert.strictEqual(buffer.length,
+                       Buffer.byteLength(s) + currentFileData.length);
+  }], { setup: setup });
+}
 
-const buf = Buffer.from(s, 'utf8');
+// test that appendFile accepts buffers
+{
+  const filename = join(tmpdir.path, 'append3.txt');
 
-fs.appendFile(filename3, buf, function(e) {
-  assert.ifError(e);
+  const setup = () => {
+    fs.writeFileSync(filename, currentFileData);
+  };
 
-  ncallbacks++;
+  const buf = Buffer.from(s, 'utf8');
 
-  fs.readFile(filename3, function(e, buffer) {
+  common.fsTest('appendFile', [filename, buf, function(e) {
     assert.ifError(e);
-    ncallbacks++;
-    assert.strictEqual(buf.length + currentFileData.length, buffer.length);
-  });
-});
-
-// test that appendFile accepts numbers.
-const filename4 = join(tmpdir.path, 'append4.txt');
-fs.writeFileSync(filename4, currentFileData);
 
-const m = 0o600;
-fs.appendFile(filename4, n, { mode: m }, function(e) {
-  assert.ifError(e);
+    const buffer = fs.readFileSync(filename);
+    assert.strictEqual(buffer.length, buf.length + currentFileData.length);
+  }], { setup: setup });
+}
 
-  ncallbacks++;
+// test that appendFile accepts numbers.
+{
+  const filename = join(tmpdir.path, 'append4.txt');
 
-  // windows permissions aren't unix
-  if (!common.isWindows) {
-    const st = fs.statSync(filename4);
-    assert.strictEqual(st.mode & 0o700, m);
-  }
+  const setup = () => { fs.writeFileSync(filename, currentFileData); };
 
-  fs.readFile(filename4, function(e, buffer) {
+  const m = 0o600;
+  common.fsTest('appendFile', [filename, n, { mode: m }, function(e) {
     assert.ifError(e);
-    ncallbacks++;
-    assert.strictEqual(Buffer.byteLength(String(n)) + currentFileData.length,
-                       buffer.length);
-  });
-});
-
-// test that appendFile accepts file descriptors
-const filename5 = join(tmpdir.path, 'append5.txt');
-fs.writeFileSync(filename5, currentFileData);
 
-fs.open(filename5, 'a+', function(e, fd) {
-  assert.ifError(e);
-
-  ncallbacks++;
-
-  fs.appendFile(fd, s, function(e) {
-    assert.ifError(e);
+    // windows permissions aren't unix
+    if (!common.isWindows) {
+      const st = fs.statSync(filename);
+      assert.strictEqual(st.mode & 0o700, m);
+    }
 
-    ncallbacks++;
+    const buffer = fs.readFileSync(filename);
+    assert.strictEqual(buffer.length,
+                       Buffer.byteLength(String(n)) + currentFileData.length);
+  }], { setup: setup });
+}
 
-    fs.close(fd, function(e) {
+// test that appendFile accepts file descriptors
+{
+  const filename = join(tmpdir.path, 'append5a.txt');
+  const otherFilename = join(tmpdir.path, 'append5b.txt');
+
+  fs.writeFileSync(filename, currentFileData);
+  fs.writeFileSync(otherFilename, currentFileData);
+
+  const runTest = (err, fd) => {
+    assert.ifError(err);
+
+    let appendFile, close;
+    if (typeof fd === 'number') {
+      appendFile = fs.appendFile;
+      close = fs.close.bind(fs, fd);
+    } else {
+      appendFile = util.callbackify(fsPromises.appendFile);
+      close = util.callbackify(fd.close.bind(fd));
+    }
+
+    appendFile(fd, s, common.mustCall((e) => {
       assert.ifError(e);
-
-      ncallbacks++;
-
-      fs.readFile(filename5, function(e, buffer) {
+      close(common.mustCall((e) => {
         assert.ifError(e);
-
-        ncallbacks++;
-        assert.strictEqual(Buffer.byteLength(s) + currentFileData.length,
-                           buffer.length);
-      });
-    });
-  });
-});
+        const buffer = fs.readFileSync(filename);
+        assert.strictEqual(buffer.length,
+                           Buffer.byteLength(s) + currentFileData.length);
+      }));
+    }));
+  };
+
+  common.fsTest(
+    'open',
+    ['a+', runTest],
+    { differentFiles: [filename, otherFilename] }
+  );
+}
 
 assert.throws(
   () => fs.appendFile(join(tmpdir.path, 'append6.txt'), console.log),
   { code: 'ERR_INVALID_CALLBACK' });
-
-process.on('exit', function() {
-  assert.strictEqual(12, ncallbacks);
-
-  fs.unlinkSync(filename);
-  fs.unlinkSync(filename2);
-  fs.unlinkSync(filename3);
-  fs.unlinkSync(filename4);
-  fs.unlinkSync(filename5);
-});