Improve source pattern handling (#730)

* watcher: correctly test if source is in the cwd

Make sure the path starts with ../ (after platform-specific slashes have been
corrected). Clarify existing test and add a case where the source starts with
two dots but is still in the current working directory.

* change handling of negated source patterns

Fixes #614. Allow negated source patterns to be specified without unsetting the
default negation patterns.

Allow source patterns to override the default negation patterns if they start
with one of the ignored directories.

* update watch mode docs

* Suggest `watch:test` as the npm script
* Document how to always enable watch mode using the ava section in package.json
* Recommend source patterns are configured through the ava section in package.json
* Suggest using the verbose reporter when debugging

Author: novemberborn

docs/recipes/watch-mode.md

@@ -34,15 +34,25 @@ You could also set up a special script:
 {
   "scripts": {
     "test": "ava",
-    "test:watch": "ava --watch"
+    "watch:test": "ava --watch"
   }
 }
 ```
 
 And then use:
 
 ```console
-$ npm run test:watch
+$ npm run watch:test
+```
+
+Finally you could configure AVA to *always* run in watch mode by setting the `watch` key in the [`ava` section of your `package.json`]:
+
+```json
+{
+  "ava": {
+    "watch": true
+  }
+}
 ```
 
 Please note that the TAP reporter is unavailable when using watch mode.
@@ -61,7 +71,9 @@ In AVA there's a distinction between *source files* and *test files*. As you can
 
 By default AVA watches for changes to the test files, `package.json`, and any other `.js` files. It'll ignore files in [certain directories](https://github.com/novemberborn/ignore-by-default/blob/master/index.js) as provided by the [`ignore-by-default`] package.
 
-You can configure patterns for the source files using the [`--source` CLI flag] or in the `ava` section of your `package.json` file. Note that if you specify a negative pattern the directories from [`ignore-by-default`] will no longer be ignored, so you may want to repeat these in your config.
+You can configure patterns for the source files in the [`ava` section of your `package.json`] file, using the `source` key. This is the recommended way, though you could also use the [`--source` CLI flag].
+
+You can specify patterns to match files in the folders that would otherwise be ignored, e.g. use `node_modules/some-dependency/*.js` to specify all `.js` files in `node_modules/some-dependency` as a source, even though normally all files in `node_modules` are ignored. Note that you need to specify an exact directory; `{bower_components,node_modules}/**/*.js` won't work.
 
 If your tests write to disk they may trigger the watcher to rerun your tests. If this occurs you will need to use the `--source` flag.
 
@@ -81,17 +93,17 @@ You can quickly rerun all tests by typing <kbd>r</kbd> on the console, followed
 
 ## Debugging
 
-Sometimes watch mode does something surprising like rerunning all tests when you thought only a single test would be run. To see its reasoning you can enable a debug mode:
+Sometimes watch mode does something surprising like rerunning all tests when you thought only a single test would be run. To see its reasoning you can enable a debug mode. This will work best with the verbose reporter:
 
 ```console
-$ DEBUG=ava:watcher npm test -- --watch
+$ DEBUG=ava:watcher npm test -- --watch --verbose
 ```
 
 On Windows use:
 
 ```console
 $ set DEBUG=ava:watcher
-$ npm test -- --watch
+$ npm test -- --watch --verbose
 ```
 
 ## Help us make watch mode better
@@ -103,3 +115,4 @@ Watch mode is relatively new and there might be some rough edges. Please [report
 [`--require` CLI flag]: https://github.com/sindresorhus/ava#cli
 [`--source` CLI flag]: https://github.com/sindresorhus/ava#cli
 [`.only` modifier]: https://github.com/sindresorhus/ava#running-specific-tests
+[`ava` section of your `package.json`]: https://github.com/sindresorhus/ava#configuration

lib/watcher.js

@@ -27,6 +27,12 @@ function rethrowAsync(err) {
 	});
 }
 
+function getDefaultIgnorePatterns() {
+	return defaultIgnore.map(function (dir) {
+		return dir + '/**/*';
+	});
+}
+
 // Used on paths before they're passed to multimatch to Harmonize matching
 // across platforms.
 var matchable = process.platform === 'win32' ? slash : function (path) {
@@ -289,52 +295,78 @@ function getChokidarPatterns(files, sources) {
 		}
 	});
 
+	// Allow source patterns to override the default ignore patterns. Chokidar
+	// ignores paths that match the list of ignored patterns. It uses anymatch
+	// under the hood, which supports negation patterns. For any source pattern
+	// that starts with an ignored directory, ensure the corresponding negation
+	// pattern is added to the ignored paths.
+	var overrideDefaultIgnorePatterns = paths.filter(function (pattern) {
+		return defaultIgnore.indexOf(pattern.split('/')[0]) >= 0;
+	}).map(function (pattern) {
+		return '!' + pattern;
+	});
+	ignored = getDefaultIgnorePatterns().concat(ignored, overrideDefaultIgnorePatterns);
+
 	if (paths.length === 0) {
 		paths = ['package.json', '**/*.js'];
 	}
-
 	paths = paths.concat(files);
 
-	if (ignored.length === 0) {
-		ignored = defaultIgnore;
-	}
-
 	return {
 		paths: paths,
 		ignored: ignored
 	};
 }
 
 function makeSourceMatcher(sources) {
-	var patterns = sources;
+	var mixedPatterns = [];
+	var defaultIgnorePatterns = getDefaultIgnorePatterns();
+	var overrideDefaultIgnorePatterns = [];
 
-	var hasPositivePattern = patterns.some(function (pattern) {
-		return pattern[0] !== '!';
-	});
+	var hasPositivePattern = false;
+	sources.forEach(function (pattern) {
+		mixedPatterns.push(pattern);
+		if (!hasPositivePattern && pattern[0] !== '!') {
+			hasPositivePattern = true;
+		}
 
-	var hasNegativePattern = patterns.some(function (pattern) {
-		return pattern[0] === '!';
+		// Extract patterns that start with an ignored directory. These need to be
+		// rematched separately.
+		if (defaultIgnore.indexOf(pattern.split('/')[0]) >= 0) {
+			overrideDefaultIgnorePatterns.push(pattern);
+		}
 	});
 
 	// Same defaults as used for Chokidar.
 	if (!hasPositivePattern) {
-		patterns = ['package.json', '**/*.js'].concat(patterns);
-	}
-
-	if (!hasNegativePattern) {
-		patterns = patterns.concat(defaultIgnore.map(function (dir) {
-			return '!' + dir + '/**/*';
-		}));
+		mixedPatterns = ['package.json', '**/*.js'].concat(mixedPatterns);
 	}
 
 	return function (path) {
+		path = matchable(path);
+
 		// Ignore paths outside the current working directory. They can't be matched
 		// to a pattern.
-		if (/^\.\./.test(path)) {
+		if (/^\.\.\//.test(path)) {
+			return false;
+		}
+
+		var isSource = multimatch(path, mixedPatterns).length === 1;
+		if (!isSource) {
 			return false;
 		}
 
-		return multimatch(matchable(path), patterns).length === 1;
+		var isIgnored = multimatch(path, defaultIgnorePatterns).length === 1;
+		if (!isIgnored) {
+			return true;
+		}
+
+		var isErroneouslyIgnored = multimatch(path, overrideDefaultIgnorePatterns).length === 1;
+		if (isErroneouslyIgnored) {
+			return true;
+		}
+
+		return false;
 	};
 }
 

test/watcher.js

@@ -174,7 +174,9 @@ group('chokidar is installed', function (beforeEach, test, group) {
 		t.same(chokidar.watch.firstCall.args, [
 			['package.json', '**/*.js'].concat(files),
 			{
-				ignored: defaultIgnore,
+				ignored: defaultIgnore.map(function (dir) {
+					return dir + '/**/*';
+				}),
 				ignoreInitial: true
 			}
 		]);
@@ -188,21 +190,25 @@ group('chokidar is installed', function (beforeEach, test, group) {
 		t.same(chokidar.watch.firstCall.args, [
 			['foo.js', 'baz.js'].concat(files),
 			{
-				ignored: ['bar.js', 'qux.js'],
+				ignored: defaultIgnore.map(function (dir) {
+					return dir + '/**/*';
+				}).concat('bar.js', 'qux.js'),
 				ignoreInitial: true
 			}
 		]);
 	});
 
-	test('default set of ignored files if configured sources does not contain exclusion patterns', function (t) {
+	test('configured sources can override default ignore patterns', function (t) {
 		t.plan(2);
-		start(['foo.js', 'baz.js']);
+		start(['node_modules/foo/*.js']);
 
 		t.ok(chokidar.watch.calledOnce);
 		t.same(chokidar.watch.firstCall.args, [
-			['foo.js', 'baz.js'].concat(files),
+			['node_modules/foo/*.js'].concat(files),
 			{
-				ignored: defaultIgnore,
+				ignored: defaultIgnore.map(function (dir) {
+					return dir + '/**/*';
+				}).concat('!node_modules/foo/*.js'),
 				ignoreInitial: true
 			}
 		]);
@@ -814,7 +820,7 @@ group('chokidar is installed', function (beforeEach, test, group) {
 			});
 		});
 
-		test('uses default patterns', function (t) {
+		test('uses default source patterns', function (t) {
 			t.plan(4);
 			seed();
 
@@ -839,16 +845,11 @@ group('chokidar is installed', function (beforeEach, test, group) {
 			});
 		});
 
-		test('uses default exclusion patterns if no exclusion pattern is given', function (t) {
+		test('uses default exclusion patterns', function (t) {
 			t.plan(2);
 
-			// Ensure each directory is treated as containing sources, but rely on
-			// the default exclusion patterns, also based on these directories, to
-			// exclude them again.
-			var sources = defaultIgnore.map(function (dir) {
-				return dir + '/**/*';
-			});
-			seed(sources);
+			// Ensure each directory is treated as containing sources.
+			seed(['**/*']);
 
 			// Synthesize an excluded file for each directory that's ignored by
 			// default. Apply deeper nesting for each file.
@@ -857,7 +858,7 @@ group('chokidar is installed', function (beforeEach, test, group) {
 				for (var i = index; i >= 0; i--) {
 					relPath = path.join(relPath, String(i));
 				}
-				return relPath;
+				return relPath + '.js';
 			});
 
 			// Ensure test/1.js also depends on the excluded files.
@@ -876,17 +877,46 @@ group('chokidar is installed', function (beforeEach, test, group) {
 			});
 		});
 
-		test('ignores dependencies outside of the current working directory', function (t) {
+		test('allows default exclusion patterns to be overriden', function (t) {
 			t.plan(2);
-			seed();
+			seed(['node_modules/foo/*.js']);
+
+			var dep = path.join('node_modules', 'foo', 'index.js');
+			emitDependencies(path.join('test', '1.js'), [path.resolve(dep)]);
+			change(dep);
+
+			return debounce(1).then(function () {
+				t.ok(api.run.calledTwice);
+				t.same(api.run.secondCall.args, [[path.join('test', '1.js')], {runOnlyExclusive: false}]);
+			});
+		});
+
+		test('ignores dependencies outside of the current working directory', function (t) {
+			t.plan(4);
+			seed(['**/*.js', '..foo.js']);
 
 			emitDependencies(path.join('test', '1.js'), [path.resolve('../outside.js')]);
+			emitDependencies(path.join('test', '2.js'), [path.resolve('..foo.js')]);
 			// Pretend Chokidar detected a change to verify (normally Chokidar would
 			// also be ignoring this file but hey).
 			change(path.join('..', 'outside.js'));
+
+			api.run.returns(Promise.resolve());
 			return debounce().then(function () {
 				t.ok(api.run.calledTwice);
+				// If ../outside.js was tracked as a dependency of test/1.js this would
+				// have caused test/1.js to be rerun. Instead expect all tests to be
+				// rerun. This is somewhat artifical: normally changes to ../outside.js
+				// wouldn't even be picked up. However this lets us test dependency
+				// tracking without directly inspecting the internal state of the
+				// watcher.
 				t.same(api.run.secondCall.args, [files, {runOnlyExclusive: false}]);
+
+				change('..foo.js');
+				return debounce();
+			}).then(function () {
+				t.ok(api.run.calledThrice);
+				t.same(api.run.thirdCall.args, [[path.join('test', '2.js')], {runOnlyExclusive: false}]);
 			});
 		});