Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve source pattern handling #730

Merged
merged 3 commits into from
Apr 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions docs/recipes/watch-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.

Expand All @@ -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
Expand All @@ -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
72 changes: 52 additions & 20 deletions lib/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
};
}

Expand Down
66 changes: 48 additions & 18 deletions test/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]);
Expand All @@ -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
}
]);
Expand Down Expand Up @@ -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();

Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we land the API refactor, I would like to move all the logic of building include/exclude patterns from the config into lib/ava-files, and rewrite many of these tests so they are just:

t.true(avaFiles.shouldWatch('some/path'));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea would be good to centralize the pattern matching logic.

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}]);
});
});

Expand Down