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

Add no-inline-assertions rule #262

Merged
merged 19 commits into from Jul 5, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions docs/rules/no-inline-assertions.md
@@ -0,0 +1,33 @@
# Ensure no assertions are called from inline functions
JLHwung marked this conversation as resolved.
Show resolved Hide resolved

Prevent assertions being called from an inline function, to make it clear that it does not return.
Copy link
Member

Choose a reason for hiding this comment

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

This is repeating the title too much. It should also use the wording "test implementation".


## Fail
```js
import test from 'ava';

test('foo', t => t.true(fn()));
```

```js
import test from 'ava';

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

```js
import test from 'ava';

test('foo', t =>
t.true(fn())
);
```

## Pass
```js
import test from 'ava';

test('foo', t => {
t.true(fn())
});
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -28,6 +28,7 @@ module.exports = {
'ava/no-identical-title': 'error',
'ava/no-ignored-test-files': 'error',
'ava/no-import-test-files': 'error',
'ava/no-inline-assertions': 'error',
'ava/no-invalid-end': 'error',
'ava/no-nested-tests': 'error',
'ava/no-only-test': 'error',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -46,6 +46,7 @@ Configure it in `package.json`.
"ava/no-identical-title": "error",
"ava/no-ignored-test-files": "error",
"ava/no-import-test-files": "error",
"ava/no-inline-assertions": "error",
"ava/no-invalid-end": "error",
"ava/no-nested-tests": "error",
"ava/no-only-test": "error",
Expand Down Expand Up @@ -83,6 +84,7 @@ The rules will only activate in test files.
- [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-import-test-files](docs/rules/no-import-test-files.md) - Ensure no test files are imported anywhere.
- [no-inline-assertions](docs/rules/no-inline-assertions.md) - Ensure no assertions are called from inline functions.
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
- [no-invalid-end](docs/rules/no-invalid-end.md) - Ensure `t.end()` is only called inside `test.cb()`.
- [no-nested-tests](docs/rules/no-nested-tests.md) - Ensure no tests are nested.
- [no-only-test](docs/rules/no-only-test.md) - Ensure no `test.only()` are present. *(fixable)*
Expand Down
61 changes: 61 additions & 0 deletions rules/no-inline-assertions.js
@@ -0,0 +1,61 @@
'use strict';
const {visitIf} = require('enhance-visitors');
const createAvaRule = require('../create-ava-rule');
const util = require('../util');

function report({node, context}) {
context.report({
node,
message: 'Assertions should not be called from an inline function.'
Copy link
Member

Choose a reason for hiding this comment

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

This would be more correct:

Suggested change
message: 'Assertions should not be called from an inline function.'
message: 'The test implementation should not be just an assertion in an inline arrow function.'

But would be nice to find a way to shorten the above slightly.

});
}

const create = context => {
const ava = createAvaRule();

return ava.merge({
CallExpression: visitIf([
ava.isInTestFile,
ava.isTestNode
])(node => {
const functionArgIndex = node.arguments.length - 1;
if (functionArgIndex > 1) {
return;
}

const functionArg = node.arguments[functionArgIndex];

if (!util.isFunctionExpression(functionArg)) {
return;
}

const {body} = functionArg;
if (body.type === 'CallExpression') {
report({node, context});
return;
}

if (body.type === 'BlockStatement') {
if (body.loc.start.line !== body.loc.end.line) {
return;
}

if (body.body.length === 0) {
return;
}

report({node, context});
}
})
});
};

module.exports = {
create,
meta: {
docs: {
url: util.getDocsUrl(__filename)
},
type: 'suggestion'
}
};
58 changes: 58 additions & 0 deletions test/no-inline-assertions.js
@@ -0,0 +1,58 @@
import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import rule from '../rules/no-inline-assertions';

const ruleTester = avaRuleTester(test, {
env: {
es6: true
}
});

const errors = [{ruleId: 'no-inline-assertions'}];
const header = 'const test = require(\'ava\');\n';

ruleTester.run('no-todo-test', rule, {
valid: [
header + 'test("my test name", t => {\n t.true(fn()); \n});',
// Shouldn't be triggered since test body is empty
header + 'test("my test name", () => {});',
header + 'test("my test name", (t) => {});',
// Shouldn't be triggered since test body is ill-defined
header + 'test("my test name", (t) => "foo");',
// Shouldn't be triggered since it's not a test file
'test.todo("my test name");',
// Shouldn't be triggered since the signature is incorrect
header + 'test.todo("my test name", "bar");',
header + 'test.todo("my test name", undefined, t => {})'
],
invalid: [
{
code: header + 'test("my test name", t => { t.skip(); });',
errors
},
{
code: header + 'test("my test name", t => t.skip());',
errors
},
{
code: header + 'test("my test name", t => t.true(fn()));',
errors
},
{
code: header + 'test("my test name", t => \n t.true(fn()));',
errors
},
{
code: header + 'test("my test name", t => { t.true(fn()) });',
errors
},
{
code: header + 'test("my test name", function(t) { foo(); });',
errors
},
{
code: header + 'test("my test name", t => { return t.true(fn()) });',
errors
}
]
});