From 193c6b239e10db6ca31a5f326926b05c51924831 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 16 Oct 2015 21:07:21 -0700 Subject: [PATCH 1/3] 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 renames the test to make it clear that it is a test for links and not just symlinks. Fixes: https://github.com/nodejs/node/issues/3311 --- .../{test-fs-symlink.js => test-fs-link.js} | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) rename test/parallel/{test-fs-symlink.js => test-fs-link.js} (65%) diff --git a/test/parallel/test-fs-symlink.js b/test/parallel/test-fs-link.js similarity index 65% rename from test/parallel/test-fs-symlink.js rename to test/parallel/test-fs-link.js index 199add4a1ba724..082c39a7752ba2 100644 --- a/test/parallel/test-fs-symlink.js +++ b/test/parallel/test-fs-link.js @@ -1,21 +1,21 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var path = require('path'); -var fs = require('fs'); -var exec = require('child_process').exec; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const exec = require('child_process').exec; var completed = 0; -var expected_async = 4; +var expected_async; var linkTime; var fileTime; common.refreshTmpDir(); -var runtest = function(skip_symlinks) { +const runTest = function() { if (!skip_symlinks) { // test creating and reading symbolic link - var linkData = path.join(common.fixturesDir, '/cycles/root.js'); - var linkPath = path.join(common.tmpDir, 'symlink1.js'); + const linkData = path.join(common.fixturesDir, '/cycles/root.js'); + const linkPath = path.join(common.tmpDir, 'symlink1.js'); fs.symlink(linkData, linkPath, function(err) { if (err) throw err; @@ -42,36 +42,36 @@ var runtest = function(skip_symlinks) { } // test creating and reading hard link - var srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); - var dstPath = path.join(common.tmpDir, 'link1.js'); + const srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); + const 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'); + const srcContent = fs.readFileSync(srcPath, 'utf8'); + const dstContent = fs.readFileSync(dstPath, 'utf8'); assert.equal(srcContent, dstContent); completed++; }); }; +var skip_symlinks = false; +var expected_async = 4; 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); + skip_symlinks = true; } }); -} else { - runtest(false); } +runTest(); process.on('exit', function() { assert.equal(completed, expected_async); - assert(linkTime !== fileTime); + if (! skip_symlinks) { + assert.notStrictEqual(linkTime, fileTime); + } }); - From 2475e1b71ccffe3a8e4d5a8448ca7a44680dc8a8 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 17 Oct 2015 22:49:28 -0700 Subject: [PATCH 2/3] fixup --- test/parallel/test-fs-link.js | 75 +++++------------------------------ 1 file changed, 9 insertions(+), 66 deletions(-) diff --git a/test/parallel/test-fs-link.js b/test/parallel/test-fs-link.js index 082c39a7752ba2..4e95d20f7b6959 100644 --- a/test/parallel/test-fs-link.js +++ b/test/parallel/test-fs-link.js @@ -3,75 +3,18 @@ const common = require('../common'); const assert = require('assert'); const path = require('path'); const fs = require('fs'); -const exec = require('child_process').exec; -var completed = 0; -var expected_async; -var linkTime; -var fileTime; common.refreshTmpDir(); -const runTest = function() { - if (!skip_symlinks) { - // test creating and reading symbolic link - const linkData = path.join(common.fixturesDir, '/cycles/root.js'); - const linkPath = path.join(common.tmpDir, 'symlink1.js'); +// test creating and reading hard link +const srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); +const dstPath = path.join(common.tmpDir, 'link1.js'); - fs.symlink(linkData, linkPath, function(err) { - if (err) throw err; - console.log('symlink done'); - - fs.lstat(linkPath, function(err, stats) { - if (err) throw err; - linkTime = stats.mtime.getTime(); - completed++; - }); - - fs.stat(linkPath, function(err, stats) { - if (err) throw err; - fileTime = stats.mtime.getTime(); - completed++; - }); - - fs.readlink(linkPath, function(err, destination) { - if (err) throw err; - assert.equal(destination, linkData); - completed++; - }); - }); - } - - // test creating and reading hard link - const srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); - const dstPath = path.join(common.tmpDir, 'link1.js'); - - fs.link(srcPath, dstPath, function(err) { - if (err) throw err; - console.log('hard link done'); - const srcContent = fs.readFileSync(srcPath, 'utf8'); - const dstContent = fs.readFileSync(dstPath, 'utf8'); - assert.equal(srcContent, dstContent); - completed++; - }); +const callback = function(err) { + if (err) throw err; + const srcContent = fs.readFileSync(srcPath, 'utf8'); + const dstContent = fs.readFileSync(dstPath, 'utf8'); + assert.strictEqual(srcContent, dstContent); }; -var skip_symlinks = false; -var expected_async = 4; -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; - skip_symlinks = true; - } - }); -} -runTest(); - -process.on('exit', function() { - assert.equal(completed, expected_async); - if (! skip_symlinks) { - assert.notStrictEqual(linkTime, fileTime); - } -}); +fs.link(srcPath, dstPath, common.mustCall(callback)); From 5e12a3180d25590ef4cb9fc259e376d4f34dac27 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 17 Oct 2015 22:49:37 -0700 Subject: [PATCH 3/3] fixup --- test/parallel/test-fs-symlink.js | 50 ++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/parallel/test-fs-symlink.js diff --git a/test/parallel/test-fs-symlink.js b/test/parallel/test-fs-symlink.js new file mode 100644 index 00000000000000..b506013b0a23f5 --- /dev/null +++ b/test/parallel/test-fs-symlink.js @@ -0,0 +1,50 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const exec = require('child_process').exec; + +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(); + +// test creating and reading symbolic link +const linkData = path.join(common.fixturesDir, '/cycles/root.js'); +const linkPath = path.join(common.tmpDir, 'symlink1.js'); + +fs.symlink(linkData, linkPath, function(err) { + if (err) throw err; + + fs.lstat(linkPath, common.mustCall(function(err, stats) { + if (err) throw err; + linkTime = stats.mtime.getTime(); + })); + + fs.stat(linkPath, common.mustCall(function(err, stats) { + if (err) throw err; + fileTime = stats.mtime.getTime(); + })); + + fs.readlink(linkPath, common.mustCall(function(err, destination) { + if (err) throw err; + assert.equal(destination, linkData); + })); +}); + + +process.on('exit', function() { + assert.notStrictEqual(linkTime, fileTime); +});