references #378, more tests, also, yay tests finding issue with refac…

…tor, async directory deletion bad idea
dbashford · Feb 7, 2015 · 6e7ef5b · 6e7ef5b
1 parent 7aef4d4
commit 6e7ef5b
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 76 deletions.
diff --git a/lib/modules/file/clean.js b/lib/modules/file/clean.js
@@ -11,30 +11,22 @@ var fs = require( "fs" )
 
 // if directory exists then attempt to remove it
 // need to scope dirPath
-var _removeDirectoryIfExists = function( config, dirPath, done ) {
-  return function( exists ) {
-    if( exists ) {
-      // remove the directory
-      fs.rmdir( dirPath, function( err ) {
-        if ( err ) {
-
-          // directory not empty, this is ok
-          if ( err.code === "ENOTEMPTY" ) {
-            config.log.info( "Unable to delete directory [[ " + dirPath + " ]] because directory not empty." );
-          } else {
-            // unknown error, this is not ok
-            config.log.error( "Unable to delete directory, [[ " + dirPath + " ]]" );
-            config.log.error( err );
-          }
-        } else {
-          config.log.success( "Deleted empty directory [[ " + dirPath + " ]]" );
-        }
-        done();
-      });
+var _removeDirectory = function( config, dirPath ) {
+  // remove the directory
+  try {
+    fs.rmdirSync( dirPath );
+    config.log.success( "Deleted empty directory [[ " + dirPath + " ]]" );
+  } catch ( err ) {
+    // directory not empty, this is ok
+    if ( err.code === "ENOTEMPTY" ) {
+      config.log.info( "Unable to delete directory [[ " + dirPath + " ]] because directory not empty." );
     } else {
-      done();
+      // unknown error, this is not ok
+      config.log.error( "Unable to delete directory, [[ " + dirPath + " ]]" );
+      config.log.error( err );
     }
-  };
+
+  }
 };
 
 var _clean = function( config, options, next ) {
@@ -47,21 +39,18 @@ var _clean = function( config, options, next ) {
     return next();
   }
 
-  var doneCount = 0;
-  var done = function(){
-    if ( ++doneCount === directories.length ) {
-      next();
-    }
-  };
-
   // sorted directories so deleting those directories with longer
   // names first, if longer names deleted first you'll never
   // attempt to delete a folder that has a deletable folder inside of it
   var sortedDirectories = _.sortBy( directories, "length" ).reverse();
   for ( var i = 0; i < sortedDirectories.length; i++ ) {
     var dirPath = path.join( config.watch.compiledDir, sortedDirectories[i] );
-    fs.exists( dirPath, _removeDirectoryIfExists( config, dirPath, done ) );
+    if ( fs.existsSync( dirPath ) ) {
+      _removeDirectory( config, dirPath );
+    }
   }
+
+  next();
 };
 
 exports.registration = function( config, register ) {

diff --git a/src/modules/file/clean.js b/src/modules/file/clean.js
@@ -11,30 +11,22 @@ var fs = require( "fs" )
 
 // if directory exists then attempt to remove it
 // need to scope dirPath
-var _removeDirectoryIfExists = function( config, dirPath, done ) {
-  return function( exists ) {
-    if( exists ) {
-      // remove the directory
-      fs.rmdir( dirPath, function( err ) {
-        if ( err ) {
-
-          // directory not empty, this is ok
-          if ( err.code === "ENOTEMPTY" ) {
-            config.log.info( "Unable to delete directory [[ " + dirPath + " ]] because directory not empty." );
-          } else {
-            // unknown error, this is not ok
-            config.log.error( "Unable to delete directory, [[ " + dirPath + " ]]" );
-            config.log.error( err );
-          }
-        } else {
-          config.log.success( "Deleted empty directory [[ " + dirPath + " ]]" );
-        }
-        done();
-      });
+var _removeDirectory = function( config, dirPath ) {
+  // remove the directory
+  try {
+    fs.rmdirSync( dirPath );
+    config.log.success( "Deleted empty directory [[ " + dirPath + " ]]" );
+  } catch ( err ) {
+    // directory not empty, this is ok
+    if ( err.code === "ENOTEMPTY" ) {
+      config.log.info( "Unable to delete directory [[ " + dirPath + " ]] because directory not empty." );
     } else {
-      done();
+      // unknown error, this is not ok
+      config.log.error( "Unable to delete directory, [[ " + dirPath + " ]]" );
+      config.log.error( err );
     }
-  };
+
+  }
 };
 
 var _clean = function( config, options, next ) {
@@ -47,21 +39,18 @@ var _clean = function( config, options, next ) {
     return next();
   }
 
-  var doneCount = 0;
-  var done = function(){
-    if ( ++doneCount === directories.length ) {
-      next();
-    }
-  };
-
   // sorted directories so deleting those directories with longer
   // names first, if longer names deleted first you'll never
   // attempt to delete a folder that has a deletable folder inside of it
   var sortedDirectories = _.sortBy( directories, "length" ).reverse();
   for ( var i = 0; i < sortedDirectories.length; i++ ) {
     var dirPath = path.join( config.watch.compiledDir, sortedDirectories[i] );
-    fs.exists( dirPath, _removeDirectoryIfExists( config, dirPath, done ) );
+    if ( fs.existsSync( dirPath ) ) {
+      _removeDirectory( config, dirPath );
+    }
   }
+
+  next();
 };
 
 exports.registration = function( config, register ) {

diff --git a/test/harness/configs/cleaner/exclude.js b/test/harness/configs/cleaner/exclude.js
@@ -1,5 +1,8 @@
 exports.config = {
   modules: ['copy'],
+  watch: {
+    exclude:[/main.js$/]
+  },
   logger: {
     growl: {
       enabled: false

diff --git a/test/harness/configs/cleaner/interval.js b/test/harness/configs/cleaner/interval.js
@@ -1,4 +1,7 @@
 exports.config = {
+  watch: {
+    interval:300
+  },
   modules: ['copy'],
   logger: {
     growl: {

diff --git a/test/integrations/util/cleaner-test.js b/test/integrations/util/cleaner-test.js
@@ -37,28 +37,42 @@ var filesDirectoriesInFolder = function(dir){
   return wrench.readdirSyncRecursive(dir).length;
 }
 
+var basicRun = function(sout, projectData, done) {
+  var assetCount = filesDirectoriesInFolder(projectData.publicDir);
+  expect(sout.indexOf("has been cleaned.")).to.above(900);
+  expect(assetCount).to.eql(0);
+  done();
+}
 
 describe("Mimosa's cleaner", function() {
 
   runTest(
     "when processing completes will remove all code and call the finish callback",
     "cleaner/clean",
     "basic",
+    basicRun
+  );
+
+  runTest(
+    "will ignore files when configured to ignore files",
+    "cleaner/exclude",
+    "basic",
     function(sout, projectData, done) {
       var assetCount = filesDirectoriesInFolder(projectData.publicDir);
-      expect(sout.indexOf("has been cleaned.")).to.above(900);
+      expect(sout.indexOf("has been cleaned.")).to.be.above(900);
+      expect(sout.indexOf("requirejs/require.js")).to.be.above(300);
+      expect(sout.indexOf("main.js")).to.eql(-1);
       expect(assetCount).to.eql(0);
       done();
     }
-  )
-
-  it("will ignore files when configured to ignore files", function(){
-    var projectData = utils.setupProjectData( "cleaner/clean" );
-  })
+  );
 
-  it("works when setting interval", function(){
-    var projectData = utils.setupProjectData( "cleaner/interval" );
-  });
+  // runTest(
+  //   "works when setting interval",
+  //   "cleaner/interval",
+  //   "basic",
+  //   basicRun
+  // );
 
   it("works when setting binaryInterval", function() {
     var projectData = utils.setupProjectData( "cleaner/binaryInterval" );

diff --git a/test/units/modules/file/clean-test.js b/test/units/modules/file/clean-test.js
@@ -40,16 +40,16 @@ describe( "Mimosa file cleaning workflow module", function(){
 
   var testCallbackWithFiles = function( files, exists ) {
     createWrenchStub(files);
-    var rmdirStub = sinon.stub( fs, "rmdir", function(p, cb){
+    var rmdirStub = sinon.stub( fs, "rmdirSync", function(p, cb){
       cb();
     });
-    var existsStub = sinon.stub( fs, "exists", function(p, cb){
+    var existsStub = sinon.stub( fs, "existsSync", function(p, cb){
       if( exists === true ) {
-        cb(true);
+        return true;
       } else if( exists === false ) {
-        cb(false);
+        return false;
       } else {
-        cb(Math.random() >= 0.5);
+        return Math.random() >= 0.5;
       }
     });
     var statSyncStub = sinon.stub( fs, "statSync", function() {
@@ -72,9 +72,9 @@ describe( "Mimosa file cleaning workflow module", function(){
     expect(spy.calledOnce).to.be.true;
     expect(existsStub.callCount).to.eql(files.length);
     expect(statSyncStub.callCount).to.eql(files.length);
-    fs.exists.restore();
+    fs.existsSync.restore();
     fs.statSync.restore();
-    fs.rmdir.restore();
+    fs.rmdirSync.restore();
   };
 
   it("will call lifecycle callback if one directory in project", function() {