Merge pull request #63 from voltrevo/master

Fix issue 62; duplicate delete for _rmdirRecursiveSync

lib/tmp.js

@@ -262,7 +262,7 @@ function _rmdirRecursiveSync(root) {
   do {
     var
       dir = dirs.pop(),
-      canRemove = true,
+      deferred = false,
       files = fs.readdirSync(dir);
 
     for (var i = 0, length = files.length; i < length; i++) {
@@ -271,15 +271,17 @@ function _rmdirRecursiveSync(root) {
         stat = fs.lstatSync(file); // lstat so we don't recurse into symlinked directories
 
       if (stat.isDirectory()) {
-        canRemove = false;
-        dirs.push(dir);
+        if (!deferred) {
+          deferred = true;
+          dirs.push(dir);
+        }  
         dirs.push(file);
       } else {
         fs.unlinkSync(file);
       }
     }
 
-    if (canRemove) {
+    if (!deferred) {
       fs.rmdirSync(dir);
     }
   } while (dirs.length !== 0);

test/base.js

@@ -88,7 +88,7 @@ function _testGracefulSync(type, graceful, cb) {
 
 function _assertName(err, name) {
   assert.isString(name);
-  assert.isNotZero(name.length);
+  assert.isNotZero(name.length, 'an empty string is not a valid name');
 }
 
 function _assertNameSync(result) {
@@ -118,10 +118,18 @@ function _testUnsafeCleanup(unsafe, cb) {
   _spawnTestWithoutError('unsafe.js', [ 'dir', unsafe ], cb);
 }
 
+function _testIssue62(cb) {
+  _spawnTestWithoutError('issue62.js', [], cb);
+}
+
 function _testUnsafeCleanupSync(unsafe, cb) {
   _spawnTestWithoutError('unsafe-sync.js', [ 'dir', unsafe ], cb);
 }
 
+function _testIssue62Sync(cb) {
+  _spawnTestWithoutError('issue62-sync.js', [], cb);
+}
+
 module.exports.testStat = _testStat;
 module.exports.testPrefix = _testPrefix;
 module.exports.testPrefixSync = _testPrefixSync;
@@ -136,5 +144,6 @@ module.exports.assertNameSync = _assertNameSync;
 module.exports.testName = _testName;
 module.exports.testNameSync = _testNameSync;
 module.exports.testUnsafeCleanup = _testUnsafeCleanup;
+module.exports.testIssue62 = _testIssue62;
 module.exports.testUnsafeCleanupSync = _testUnsafeCleanupSync;
-
+module.exports.testIssue62Sync = _testIssue62Sync;

test/dir-sync-test.js

@@ -188,6 +188,18 @@ vows.describe('Synchronous directory creation').addBatch({
     }
   },
 
+  'unsafeCleanup === true with issue62 structure': {
+    topic: function () {
+      Test.testIssue62Sync(this.callback);
+    },
+
+    'should not return with an error': assert.isNull,
+    'should return with a name': Test.assertName,
+    'should not exist': function (err, name) {
+      assert.ok(!existsSync(name), 'Directory should be removed');
+    }
+  },
+
   'unsafeCleanup === false': {
     topic: function () {
       Test.testUnsafeCleanupSync('0', this.callback);

test/dir-test.js

@@ -182,6 +182,18 @@ vows.describe('Directory creation').addBatch({
     }
   },
 
+  'unsafeCleanup === true with issue62 structure': {
+    topic: function () {
+      Test.testIssue62(this.callback);
+    },
+
+    'should not return with an error': assert.isNull,
+    'should return with a name': Test.assertName,
+    'should not exist': function (err, name) {
+      assert.ok(!existsSync(name), 'Directory should be removed');
+    }
+  },
+
   'unsafeCleanup === false': {
     topic: function () {
       Test.testUnsafeCleanup('0', this.callback);

test/issue62-sync.js

@@ -0,0 +1,27 @@
+
+var
+  fs    = require('fs'),
+  join  = require('path').join,
+  spawn = require('./spawn-sync');
+
+try {
+  var result = spawn.tmpFunction({ unsafeCleanup: true });
+  try {
+    // creates structure from issue 62
+    // /~https://github.com/raszi/node-tmp/issues/62
+
+    fs.mkdirSync(join(result.name, 'issue62'));
+
+    ['foo', 'bar'].forEach(function(subdir) {
+      fs.mkdirSync(join(result.name, 'issue62', subdir));
+      fs.writeFileSync(join(result.name, 'issue62', subdir, 'baz.txt'), '');
+    });
+
+    spawn.out(result.name, spawn.exit);
+  } catch (e) {
+    spawn.err(e.toString(), spawn.exit);
+  }
+}
+catch (e) {
+  spawn.err(e, spawn.exit);
+}

test/issue62.js

@@ -0,0 +1,27 @@
+var
+  fs    = require('fs'),
+  join  = require('path').join,
+  spawn = require('./spawn');
+
+spawn.tmpFunction({ unsafeCleanup: true }, function (err, name) {
+  if (err) {
+    spawn.err(err, spawn.exit);
+    return;
+  }
+
+  try {
+    // creates structure from issue 62
+    // /~https://github.com/raszi/node-tmp/issues/62
+
+    fs.mkdirSync(join(name, 'issue62'));
+
+    ['foo', 'bar'].forEach(function(subdir) {
+      fs.mkdirSync(join(name, 'issue62', subdir));
+      fs.writeFileSync(join(name, 'issue62', subdir, 'baz.txt'), '');
+    });
+
+    spawn.out(name, spawn.exit);
+  } catch (e) {
+    spawn.err(e.toString(), spawn.exit);
+  }
+});

test/unsafe-sync.js

@@ -28,4 +28,3 @@ try {
 catch (e) {
   spawn.err(err, spawn.exit);
 }
-