Skip to content

Add no-ignored-test-files rule (fixes #51) #56

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

Merged
merged 1 commit into from
Apr 4, 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
92 changes: 92 additions & 0 deletions docs/rules/no-ignored-test-files.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Ensure no tests are written in ignored files

When searching for tests, AVA ignores files contained in `node_modules` or folders named `fixtures` or `helpers`. By default, it will search in `test.js test-*.js test/**/*.js`, which you can override by specifying a path when launching AVA or in the [AVA configuration in the `package.json` file](https://github.com/sindresorhus/ava#configuration).

This rule will verify that files which create tests are in the searched files and not in ignored folders. It will consider the root of the project to be the closest folder containing a `package.json` file, and will not do anything if it can't find one. Test files in `node_modules` will not be linted as they are ignored by ESLint.

Note that this rule will not be able to warn correctly if you use AVA by specifying the files in the command line ( `ava "lib/**/*.test.js"` ). Prefer configuring AVA as described in the link above.

## Fail

```js
// File: test/foo/fixtures/bar.js
// Invalid because in `fixtures` folder
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: test/foo/helpers/bar.js
// Invalid because in `helpers` folder
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: lib/foo.test.js
// Invalid because not in the searched files
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: test.js
// with { "files": ["lib/**/*.test.js", "utils/**/*.test.js"] }
// in either `package.json` under 'ava key' or in the rule options
// Invalid because not in the searched files
import test from 'ava';

test('foo', t => {
t.pass();
});
```


## Pass

```js
// File: test/foo/not-fixtures/bar.js
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: test/foo/not-helpers/bar.js
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: test.js
import test from 'ava';

test('foo', t => {
t.pass();
});

// File: lib/foo.test.js
// with { "files": ["lib/**/*.test.js", "utils/**/*.test.js"] }
// in either `package.json` under 'ava key' or in the rule options
import test from 'ava';

test('foo', t => {
t.pass();
});
```

## Options

This rule supports the following options:

`files`: An array of strings representing the files glob that AVA will use to find test files. Overrides the default and the configuration found in the `package.json` file.

You can set the options like this:

```js
"ava/no-ignored-test-files": [2, {"files": ["lib/**/*.test.js", "utils/**/*.test.js"]}]
```
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module.exports = {
'max-asserts': require('./rules/max-asserts'),
'no-cb-test': require('./rules/no-cb-test'),
'no-identical-title': require('./rules/no-identical-title'),
'no-ignored-test-files': require('./rules/no-ignored-test-files'),
'no-invalid-end': require('./rules/no-invalid-end'),
'no-only-test': require('./rules/no-only-test'),
'no-skip-assert': require('./rules/no-skip-assert'),
Expand All @@ -31,6 +32,7 @@ module.exports = {
'ava/max-asserts': [0, 5],
'ava/no-cb-test': 0,
'ava/no-identical-title': 2,
'ava/no-ignored-test-files': 2,
'ava/no-invalid-end': 2,
'ava/no-only-test': 2,
'ava/no-skip-assert': 2,
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@
"dependencies": {
"deep-strict-equal": "^0.1.0",
"espurify": "^1.5.0",
"object-assign": "^4.0.1"
"multimatch": "^2.1.0",
"object-assign": "^4.0.1",
"pkg-up": "^1.0.0"
},
"devDependencies": {
"ava": "*",
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Configure it in `package.json`.
"ava/max-asserts": [2, 5],
"ava/no-cb-test": 0,
"ava/no-identical-title": 2,
"ava/no-ignored-test-files": 2,
"ava/no-invalid-end": 2,
"ava/no-only-test": 2,
"ava/no-skip-assert": 2,
Expand All @@ -57,6 +58,7 @@ The rules will only activate in test files.
- [max-asserts](docs/rules/max-asserts.md) - Limit the number of assertions in a test.
- [no-cb-test](docs/rules/no-cb-test.md) - Ensure no `test.cb()` is used.
- [no-identical-title](docs/rules/no-identical-title.md) - Ensure no tests have the same title.
- [no-ignored-test-files](docs/rules/no-ignored-test-files.md) - Ensure no tests are written in ignored files.
- [no-invalid-end](docs/rules/no-invalid-end.md) - Ensure `t.end()` is only called inside `test.cb()`.
- [no-only-test](docs/rules/no-only-test.md) - Ensure no `test.only()` are present.
- [no-skip-assert](docs/rules/no-skip-assert.md) - Ensure no assertions are skipped.
Expand Down
80 changes: 80 additions & 0 deletions rules/no-ignored-test-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
'use strict';
var path = require('path');
var pkgUp = require('pkg-up');
var multimatch = require('multimatch');
var util = require('../util');
var createAvaRule = require('../create-ava-rule');

var defaultFiles = [
'test.js',
'test-*.js',
'test/**/*.js'
];

var excludedFolders = [
'**/fixtures/**',
'**/helpers/**'
];

function isIgnored(rootDir, files, filepath) {
var relativeFilePath = path.relative(rootDir, filepath);
if (multimatch([relativeFilePath], excludedFolders).length !== 0) {
return 'Test file is ignored because it is in `' + excludedFolders.join(' ') + '`';
}

if (multimatch([relativeFilePath], files).length === 0) {
return 'Test file is ignored because it is not in `' + files.join(' ') + '`';
}

return null;
}

function getPackageInfo() {
var packageFilePath = pkgUp.sync();
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to document the assumption that process.cwd() is the directory of the file being linted?

Some test suites may have nested package.json files. I wonder if you should keep searching until you find AVA config?

Here's an example from AVA itself with a nested package.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change the wording to indicate that it will use the closest package.json. I think in most cases having multiple package.json will be used for examples inside the repo or test fixtures (though not sure I see the value in the one you showed).

I wonder if you should keep searching until you find AVA config?

Why not, but then again, it will not be able to handle some cases. Some projects might have sub-projects as examples (thinking of [Redux](https://github.com/reactjs/redux/tree/master/examples, even though it doesn't use AVA afaik) where it's pretty much the point that they don't depend on any parent dirs. I'm guessing that in most cases, what you want is isolation from the parent "project", so I'd rather not do this. But let me know what you guys think.

return {
rootDir: packageFilePath && path.dirname(packageFilePath),
files: util.getAvaConfig(packageFilePath).files
};
}

/* eslint quote-props: [2, "as-needed"] */
module.exports = function (context) {
var ava = createAvaRule();
var packageInfo = getPackageInfo();
var options = context.options[0] || {};
var files = options.files || packageInfo.files || defaultFiles;
var hasTestCall = false;

if (!packageInfo.rootDir) {
// Could not find a package.json folder
return {};
}

return ava.merge({
CallExpression: function (node) {
if (ava.isTestFile && ava.currentTestNode === node) {
hasTestCall = true;
}
},
'Program:exit': function (node) {
if (!hasTestCall) {
return;
}

var ignoredReason = isIgnored(packageInfo.rootDir, files, context.getFilename());
if (ignoredReason) {
context.report(node, ignoredReason);
}
hasTestCall = false;
}
});
};

module.exports.schema = [{
type: 'object',
properties: {
files: {
type: 'array'
}
}
}];
153 changes: 153 additions & 0 deletions test/no-ignored-test-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import path from 'path';
import test from 'ava';
import {RuleTester} from 'eslint';
import util from '../util';
import rule from '../rules/no-ignored-test-files';

const ruleTester = new RuleTester({
env: {
es6: true
}
});

const header = `const test = require('ava');\n`;
const rootDir = path.dirname(process.cwd());

function toPath(subPath) {
return path.join(rootDir, subPath);
}

function code(hasHeader) {
return (hasHeader ? header : '') + 'test(t => { t.pass(); });';
}

test('without AVA config in package.json', () => {
ruleTester.run('no-ignored-test-files', rule, {
valid: [
{
code: code(true),
filename: toPath('test/foo/bar.js')
},
{
code: code(true),
filename: toPath('test/foo/not-fixtures/bar.js')
},
{
code: code(true),
filename: toPath('test/foo/not-helpers/bar.js')
},
{
code: header + 'foo(t => {});',
filename: toPath('test/foo/fixtures/bar.js')
},
{
code: header + 'foo(t => {});',
filename: toPath('test/foo/helpers/bar.js')
},
{
code: code(false),
filename: toPath('test/foo/fixtures/bar.js')
},
{
code: code(false),
filename: toPath('test/foo/helpers/bar.js')
},
{
code: code(true),
filename: toPath('test.js')
},
{
code: code(true),
filename: toPath('test-foo.js')
},
{
code: code(true),
filename: toPath('lib/foo.test.js'),
options: [{files: ['lib/**/*.test.js']}]
}
],
invalid: [
{
code: code(true),
filename: toPath('test/foo/fixtures/bar.js'),
errors: [{message: 'Test file is ignored because it is in `**/fixtures/** **/helpers/**`'}]
},
{
code: code(true),
filename: toPath('test/foo/helpers/bar.js'),
errors: [{message: 'Test file is ignored because it is in `**/fixtures/** **/helpers/**`'}]
},
{
code: code(true),
filename: toPath('lib/foo.test.js'),
errors: [{message: 'Test file is ignored because it is not in `test.js test-*.js test/**/*.js`'}]
},
{
code: code(true),
filename: toPath('test/foo/bar.js'),
options: [{files: ['lib/**/*.test.js']}],
errors: [{message: 'Test file is ignored because it is not in `lib/**/*.test.js`'}]
},
{
code: code(true),
filename: toPath('lib/foo.not-test.js'),
options: [{files: ['lib/**/*.test.js']}],
errors: [{message: 'Test file is ignored because it is not in `lib/**/*.test.js`'}]
}
]
});
});

test('with AVA config in package.json', () => {
const oldGetAvaConfig = util.getAvaConfig;

util.getAvaConfig = function mockGetAvaConfig() {
return {
files: ['lib/**/*.test.js']
};
};

ruleTester.run('no-ignored-test-files', rule, {
valid: [
{
code: code(true),
filename: toPath('lib/foo.test.js')
},
{
code: code(true),
filename: toPath('bar/foo.test.js'),
options: [{files: ['bar/**/*.test.js']}]
}
],
invalid: [
{
code: code(true),
filename: toPath('lib/foo/fixtures/bar.test.js'),
errors: [{message: 'Test file is ignored because it is in `**/fixtures/** **/helpers/**`'}]
},
{
code: code(true),
filename: toPath('lib/foo/helpers/bar.test.js'),
errors: [{message: 'Test file is ignored because it is in `**/fixtures/** **/helpers/**`'}]
},
{
code: code(true),
filename: toPath('test.js'),
errors: [{message: 'Test file is ignored because it is not in `lib/**/*.test.js`'}]
},
{
code: code(true),
filename: toPath('bar/foo.test.js'),
errors: [{message: 'Test file is ignored because it is not in `lib/**/*.test.js`'}]
},
{
code: code(true),
filename: toPath('lib/foo.test.js'),
options: [{files: ['bar/**/*.test.js']}],
errors: [{message: 'Test file is ignored because it is not in `bar/**/*.test.js`'}]
}
]
});

util.getAvaConfig = oldGetAvaConfig;
});
16 changes: 16 additions & 0 deletions util.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
'use strict';

var fs = require('fs');

exports.nameOfRootObject = function (node) {
if (node.object.type === 'MemberExpression') {
return exports.nameOfRootObject(node.object);
}

return node.object.name;
};

exports.getAvaConfig = function (filepath) {
var defaultResult = {};
if (!filepath) {
return defaultResult;
}

try {
var packageContent = JSON.parse(fs.readFileSync(filepath, 'utf8'));
return packageContent && packageContent.ava || defaultResult;
} catch (e) {
return defaultResult;
}
};