fix(watcher): improve watching efficiency

This fix significantly improves watching performance. It tells chokidar to ignore all paths that do not match any of the watched patterns. Before we were only ignoring paths that match some of the excludes patterns. That means that even files that didn't match any of the watched patterns were still watched by chokidar and ignored on the level of `fileList`. Note, this fix requires chokidar 0.7.0 Closes #616
karma-runner · Oct 22, 2013 · 6a272aa · 6a272aa
1 parent ee8303c
commit 6a272aa
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 48 deletions.
diff --git a/lib/watcher.js b/lib/watcher.js
@@ -17,13 +17,7 @@ var watchPatterns = function(patterns, watcher) {
   var uniqueMap = {};
   var path;
 
-  patterns.forEach(function(patternObject) {
-    var pattern = patternObject.pattern;
-
-    if (!patternObject.watched) {
-      return;
-    }
-
+  patterns.forEach(function(pattern) {
     path = baseDirFromPattern(pattern);
     if (!uniqueMap[path]) {
       uniqueMap[path] = true;
@@ -42,27 +36,50 @@ var watchPatterns = function(patterns, watcher) {
   });
 };
 
-// Function to test if an item is on the exclude list
-// and therefore should not be watched by chokidar
-// TODO(vojta): ignore non-matched files as well
-var createIgnore = function(excludes) {
-  return function(item) {
-    var matchExclude = function(pattern) {
-      log.debug('Excluding %s', pattern);
-      return mm(item, pattern, {dot: true});
-    };
-    return excludes.some(matchExclude);
+// Function to test if a path should be ignored by chokidar.
+var createIgnore = function(patterns, excludes) {
+  return function(path, stat) {
+    if (!stat || stat.isDirectory()) {
+      return false;
+    }
+
+    // Check if the path matches any of the watched patterns.
+    if (!patterns.some(function(pattern) {
+      return mm(path, pattern, {dot: true});
+    })) {
+      return true;
+    }
+
+    // Check if the path matches any of the exclude patterns.
+    if (excludes.some(function(pattern) {
+      return mm(path, pattern, {dot: true});
+    })) {
+      return true;
+    }
+
+    return false;
   };
 };
 
+var onlyWatchedTrue = function(pattern) {
+  return pattern.watched;
+};
+
+var getWatchedPatterns = function(patternObjects) {
+  return patternObjects.filter(onlyWatchedTrue).map(function(patternObject) {
+    return patternObject.pattern;
+  });
+};
+
 exports.watch = function(patterns, excludes, fileList) {
+  var watchedPatterns = getWatchedPatterns(patterns);
   var options = {
     ignorePermissionErrors: true,
-    ignored: createIgnore(excludes)
+    ignored: createIgnore(watchedPatterns, excludes)
   };
   var chokidarWatcher = new chokidar.FSWatcher(options);
 
-  watchPatterns(patterns, chokidarWatcher);
+  watchPatterns(watchedPatterns, chokidarWatcher);
 
   var bind = function(fn) {
     return function(path) {

diff --git a/package.json b/package.json
@@ -90,7 +90,7 @@
   "dependencies": {
     "di": "~0.0.1",
     "socket.io": "~0.9.13",
-    "chokidar": "~0.6",
+    "chokidar": "~0.7.0",
     "glob": "~3.1.21",
     "minimatch": "~0.2",
     "http-proxy": "~0.10",

diff --git a/test/unit/watcher.spec.coffee b/test/unit/watcher.spec.coffee
@@ -6,10 +6,6 @@ describe 'watcher', ->
   config = require '../../lib/config'
   m = null
 
-  # create an array of pattern objects from given strings
-  patterns = (strings...) ->
-    new config.createPatternObject(str) for str in strings
-
   beforeEach ->
     mocks_ = chokidar: mocks.chokidar
     m = mocks.loadFile __dirname + '/../../lib/watcher.js', mocks_
@@ -43,55 +39,76 @@ describe 'watcher', ->
       chokidarWatcher = new mocks.chokidar.FSWatcher
 
     it 'should watch all the patterns', ->
-      m.watchPatterns patterns('/some/*.js', '/a/*'), chokidarWatcher
+      m.watchPatterns ['/some/*.js', '/a/*'], chokidarWatcher
       expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some', '/a']
 
 
-    it 'should not watch urls', ->
-      m.watchPatterns patterns('http://some.com', '/a.*'), chokidarWatcher
-      expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/']
-
-
     it 'should not watch the same path twice', ->
-      m.watchPatterns patterns('/some/a*.js', '/some/*.txt'), chokidarWatcher
+      m.watchPatterns ['/some/a*.js', '/some/*.txt'], chokidarWatcher
       expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some']
 
 
     it 'should not watch subpaths that are already watched', ->
-      m.watchPatterns patterns('/some/sub/*.js', '/some/a*.*'), chokidarWatcher
+      m.watchPatterns ['/some/sub/*.js', '/some/a*.*'], chokidarWatcher
       expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some']
 
 
     it 'should watch a file matching subpath directory', ->
       # regression #521
-      m.watchPatterns patterns('/some/test-file.js', '/some/test/**'), chokidarWatcher
+      m.watchPatterns ['/some/test-file.js', '/some/test/**'], chokidarWatcher
       expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some/test-file.js', '/some/test']
 
 
-    it 'should not watch if watched false', ->
-      m.watchPatterns [
-        new config.Pattern('/some/*.js', true, true, false)
-        new config.Pattern('/some/sub/*.js')
-      ], chokidarWatcher
+  describe 'getWatchedPatterns', ->
 
-      expect(chokidarWatcher.watchedPaths_).to.deep.equal ['/some/sub']
+    it 'should return list of watched patterns (strings)', ->
+      watchedPatterns = m.getWatchedPatterns [
+        config.createPatternObject('/watched.js')
+        config.createPatternObject(pattern: 'non/*.js', watched: false)
+      ]
+      expect(watchedPatterns).to.deep.equal ['/watched.js']
 
 
   #============================================================================
   # ignore() [PRIVATE]
   #============================================================================
   describe 'ignore', ->
+    FILE_STAT =
+      isDirectory: -> false
+      isFile: -> true
+    DIRECTORY_STAT =
+      isDirectory: -> true
+      isFile: -> false
 
     it 'should ignore all files', ->
-      ignore = m.createIgnore ['**/*']
-      expect(ignore '/some/files/deep/nested.js').to.equal true
-      expect(ignore '/some/files').to.equal true
+      ignore = m.createIgnore ['**/*'], ['**/*']
+      expect(ignore '/some/files/deep/nested.js', FILE_STAT).to.equal true
+      expect(ignore '/some/files', FILE_STAT).to.equal true
 
 
     it 'should ignore .# files', ->
-      ignore = m.createIgnore ['**/.#*']
-      expect(ignore '/some/files/deep/nested.js').to.equal false
-      expect(ignore '/some/files').to.equal false
-      expect(ignore '/some/files/deep/.npm').to.equal false
-      expect(ignore '.#files.js').to.equal true
-      expect(ignore '/some/files/deeper/nested/.#files.js').to.equal true
+      ignore = m.createIgnore ['**/*'], ['**/.#*']
+      expect(ignore '/some/files/deep/nested.js', FILE_STAT).to.equal false
+      expect(ignore '/some/files', FILE_STAT).to.equal false
+      expect(ignore '/some/files/deep/.npm', FILE_STAT).to.equal false
+      expect(ignore '.#files.js', FILE_STAT).to.equal true
+      expect(ignore '/some/files/deeper/nested/.#files.js', FILE_STAT).to.equal true
+
+
+    it 'should ignore files that do not match any pattern', ->
+      ignore = m.createIgnore ['/some/*.js'], []
+      expect(ignore '/a.js', FILE_STAT).to.equal true
+      expect(ignore '/some.js', FILE_STAT).to.equal true
+      expect(ignore '/some/a.js', FILE_STAT).to.equal false
+
+
+    it 'should not ignore directories', ->
+      ignore = m.createIgnore ['**/*'], ['**/*']
+      expect(ignore '/some/dir', DIRECTORY_STAT).to.equal false
+
+
+    it 'should not ignore items without stat', ->
+      # before we know whether it's a directory or file, we can't ignore
+      ignore = m.createIgnore ['**/*'], ['**/*']
+      expect(ignore '/some.js', undefined).to.equal false
+      expect(ignore '/what/ever', undefined).to.equal false