test: fix flaky test for symlinks

If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and splits the test to make it clear that it is a test for links and
symlinks.

Fixes: https://github.com/nodejs/node/issues/3311
PR-URL: https://github.com/nodejs/node/pull/3418
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>

test/parallel/test-fs-link.js

@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const path = require('path');
+const fs = require('fs');
+
+common.refreshTmpDir();
+
+// test creating and reading hard link
+const srcPath = path.join(common.fixturesDir, 'cycles', 'root.js');
+const dstPath = path.join(common.tmpDir, 'link1.js');
+
+const callback = function(err) {
+  if (err) throw err;
+  const srcContent = fs.readFileSync(srcPath, 'utf8');
+  const dstContent = fs.readFileSync(dstPath, 'utf8');
+  assert.strictEqual(srcContent, dstContent);
+};
+
+fs.link(srcPath, dstPath, common.mustCall(callback));

test/parallel/test-fs-symlink.js

@@ -1,77 +1,50 @@
 'use strict';
-var common = require('../common');
+const common = require('../common');
-var assert = require('assert');
+const assert = require('assert');
-var path = require('path');
+const path = require('path');
-var fs = require('fs');
+const fs = require('fs');
-var exec = require('child_process').exec;
+const exec = require('child_process').exec;
-var completed = 0;
+
-var expected_async = 4;
 var linkTime;
 var fileTime;
 
+if (common.isWindows) {
+  // On Windows, creating symlinks requires admin privileges.
+  // We'll only try to run symlink test if we have enough privileges.
+  exec('whoami /priv', function(err, o) {
+    if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) {
+      console.log('1..0 # Skipped: insufficient privileges');
+      return;
+    }
+  });
+}
+
 common.refreshTmpDir();
 
-var runtest = function(skip_symlinks) {
+// test creating and reading symbolic link
-  if (!skip_symlinks) {
+const linkData = path.join(common.fixturesDir, '/cycles/root.js');
-    // test creating and reading symbolic link
+const linkPath = path.join(common.tmpDir, 'symlink1.js');
-    var linkData = path.join(common.fixturesDir, '/cycles/root.js');
-    var linkPath = path.join(common.tmpDir, 'symlink1.js');
 
-    fs.symlink(linkData, linkPath, function(err) {
+fs.symlink(linkData, linkPath, function(err) {
   if (err) throw err;
-      console.log('symlink done');
 
-      fs.lstat(linkPath, function(err, stats) {
+  fs.lstat(linkPath, common.mustCall(function(err, stats) {
     if (err) throw err;
     linkTime = stats.mtime.getTime();
-        completed++;
+  }));
-      });
 
-      fs.stat(linkPath, function(err, stats) {
+  fs.stat(linkPath, common.mustCall(function(err, stats) {
     if (err) throw err;
     fileTime = stats.mtime.getTime();
-        completed++;
+  }));
-      });
 
-      fs.readlink(linkPath, function(err, destination) {
+  fs.readlink(linkPath, common.mustCall(function(err, destination) {
     if (err) throw err;
     assert.equal(destination, linkData);
-        completed++;
+  }));
-      });
+});
-    });
-  }
-
-  // test creating and reading hard link
-  var srcPath = path.join(common.fixturesDir, 'cycles', 'root.js');
-  var dstPath = path.join(common.tmpDir, 'link1.js');
-
-  fs.link(srcPath, dstPath, function(err) {
-    if (err) throw err;
-    console.log('hard link done');
-    var srcContent = fs.readFileSync(srcPath, 'utf8');
-    var dstContent = fs.readFileSync(dstPath, 'utf8');
-    assert.equal(srcContent, dstContent);
-    completed++;
-  });
-};
 
-if (common.isWindows) {
-  // On Windows, creating symlinks requires admin privileges.
-  // We'll only try to run symlink test if we have enough privileges.
-  exec('whoami /priv', function(err, o) {
-    if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) {
-      expected_async = 1;
-      runtest(true);
-    } else {
-      runtest(false);
-    }
-  });
-} else {
-  runtest(false);
-}
 
 process.on('exit', function() {
-  assert.equal(completed, expected_async);
+  assert.notStrictEqual(linkTime, fileTime);
-  assert(linkTime !== fileTime);
 });
-