Skip to content

Commit

Permalink
Introduce no-noop-setup-on-error-in-before rule for tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Viktar Korshun committed Aug 26, 2020
1 parent a66a983 commit 7baf741
Show file tree
Hide file tree
Showing 5 changed files with 379 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Expand Up @@ -183,6 +183,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
| :white_check_mark: | [no-ember-testing-in-module-scope](./docs/rules/no-ember-testing-in-module-scope.md) | disallow use of `Ember.testing` in module scope |
| | [no-invalid-test-waiters](./docs/rules/no-invalid-test-waiters.md) | disallow incorrect usage of test waiter APIs |
| :white_check_mark: | [no-legacy-test-waiters](./docs/rules/no-legacy-test-waiters.md) | disallow the use of the legacy test waiter APIs |
| :wrench: | [no-noop-setup-on-error-in-before](./docs/rules/no-noop-setup-on-error-in-before.md) | disallows using no-op setupOnerror in `before` or `beforeEach` |
| :white_check_mark: | [no-pause-test](./docs/rules/no-pause-test.md) | disallow usage of the `pauseTest` helper in tests |
| | [no-replace-test-comments](./docs/rules/no-replace-test-comments.md) | disallow 'Replace this with your real tests' comments in test files |
| :white_check_mark: | [no-restricted-resolver-tests](./docs/rules/no-restricted-resolver-tests.md) | disallow the use of patterns that use the restricted resolver in tests |
Expand Down
46 changes: 46 additions & 0 deletions docs/rules/no-noop-setup-on-error-in-before.md
View file
@@ -0,0 +1,46 @@
# no-noop-setup-on-error-in-before

:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

Disallows use of no-op `setupOnerror` in `before`/`beforeEach` since it could mask errors or rejections in tests unintentionally

## Examples

Examples of **incorrect** code for this rule:

```js
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';

module('foo', function (hooks) {
hooks.beforeEach(function () {
setupOnerror(() => {});
});
});
```

```js
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';

module('foo', function (hooks) {
hooks.before(function () {
setupOnerror(() => {});
});
});
```

Examples of **correct** code for this rule:

```js
import { setupOnerror } from '@ember/test-helpers';
import { module, test } from 'qunit';

module('foo', function (hooks) {
test('something', function () {
setupOnerror((error) => {
assert.equal(error.message, 'test', 'Should have message');
});
});
});
```
1 change: 1 addition & 0 deletions lib/index.js
View file
Expand Up @@ -42,6 +42,7 @@ module.exports = {
'no-legacy-test-waiters': require('./rules/no-legacy-test-waiters'),
'no-mixins': require('./rules/no-mixins'),
'no-new-mixins': require('./rules/no-new-mixins'),
'no-noop-setup-on-error-in-before': require('./rules/no-noop-setup-on-error-in-before'),
'no-observers': require('./rules/no-observers'),
'no-old-shims': require('./rules/no-old-shims'),
'no-on-calls-in-components': require('./rules/no-on-calls-in-components'),
Expand Down
146 changes: 146 additions & 0 deletions lib/rules/no-noop-setup-on-error-in-before.js
View file
@@ -0,0 +1,146 @@
'use strict';

const { getImportIdentifier } = require('../utils/import');
const types = require('../utils/types');

//------------------------------------------------------------------------------
// General rule - Disallows no-op `setupOnError` in `before` or `beforeEach`.
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'disallows using no-op setupOnerror in `before` or `beforeEach`',
category: 'Testing',
recommended: false,
url:
'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-noop-setup-on-error-in-before.md',
},
fixable: 'code',
schema: [],
messages: {
main: 'Using no-op setupOnerror in `before` or `beforeEach` is not allowed',
},
},

create(context) {
let importedSetupOnerrorName, importedModuleName;
let isInModule = false;
let isInBeforeEachHook = false;
let isInBeforeHook = false;
let hooksName;
const sourceCode = context.getSourceCode();

function reportErrorForNodeIfInBefore(node) {
const isInBeforeOrBeforeEach =
(isInBeforeEachHook || isInBeforeHook) &&
types.isIdentifier(node.callee) &&
node.callee.name === importedSetupOnerrorName;

const callback = node.arguments[0];
const isFunction =
types.isArrowFunctionExpression(callback) ||
types.isFunctionDeclaration(callback) ||
types.isFunctionExpression(callback);

const isNoop =
callback &&
isFunction &&
callback.body &&
callback.body.type === 'BlockStatement' &&
callback.body.body.length === 0;

if (isInBeforeOrBeforeEach && isNoop) {
context.report({
node,
messageId: 'main',
*fix(fixer) {
yield fixer.remove(node);
const semicolon = sourceCode.getTokenAfter(node);
if (semicolon && semicolon.value === ';') {
yield fixer.remove(semicolon);
}
},
});
}
}

return {
ImportDeclaration(node) {
if (node.source.value === '@ember/test-helpers') {
importedSetupOnerrorName =
importedSetupOnerrorName ||
getImportIdentifier(node, '@ember/test-helpers', 'setupOnerror');
}
if (node.source.value === 'qunit') {
importedModuleName = importedModuleName || getImportIdentifier(node, 'qunit', 'module');
}
},

CallExpression(node) {
if (types.isIdentifier(node.callee) && node.callee.name === importedModuleName) {
isInModule = true;
if (node.arguments.length > 1 && node.arguments[1]) {
const moduleCallback = node.arguments[1];
hooksName =
moduleCallback.params &&
moduleCallback.params.length > 0 &&
types.isIdentifier(moduleCallback.params[0]) &&
moduleCallback.params[0].name;
}
}
if (types.isIdentifier(node.callee) && node.callee.name === importedSetupOnerrorName) {
reportErrorForNodeIfInBefore(node);
}
},

'CallExpression:exit'(node) {
if (types.isIdentifier(node.callee) && node.callee.name === importedModuleName) {
isInModule = false;
hooksName = undefined;
}
},

// potentially entering a `beforeEach` hook
'CallExpression[callee.property.name="beforeEach"]'(node) {
if (
isInModule &&
hooksName &&
types.isIdentifier(node.callee.object) &&
node.callee.object.name === hooksName
) {
isInBeforeEachHook = true;
}
},

// potentially exiting a `beforeEach` hook
'CallExpression[callee.property.name="beforeEach"]:exit'() {
if (isInBeforeEachHook) {
isInBeforeEachHook = false;
hooksName = undefined;
}
},

// potentially entering a `before` hook
'CallExpression[callee.property.name="before"]'(node) {
if (
isInModule &&
hooksName &&
types.isIdentifier(node.callee.object) &&
node.callee.object.name === hooksName
) {
isInBeforeHook = true;
}
},

// potentially exiting a `before` hook
'CallExpression[callee.property.name="before"]:exit'() {
if (isInBeforeHook) {
isInBeforeHook = false;
hooksName = undefined;
}
},
};
},
};
185 changes: 185 additions & 0 deletions tests/lib/rules/no-noop-setup-on-error-in-before.js
View file
@@ -0,0 +1,185 @@
//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-noop-setup-on-error-in-before');
const RuleTester = require('eslint').RuleTester;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
},
});

ruleTester.run('no-noop-setup-on-error-in-before', rule, {
valid: [
`
import { setupOnerror } from '@ember/test-helpers';
import { module, test } from 'qunit';
module('foo', function(hooks) {
test('something', function() {
setupOnerror((error) => {
assert.equal(error.message, 'test', 'Should have message');
});
})
});
`,
`
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.beforeEach(function() {
setupOnerror((error) => {
assert.equal(error.message, 'test', 'Should have message');
});
});
});
`,
`
import { setupOnerror } from '@ember/test-helpers';
import { module, test } from 'qunit';
module('foo', function(hooks) {
test('something', function() {
setupOnerror(() => {
});
})
});
`,
`
import { setupOnerror } from '@ember/test-helpers';
import { module, test } from 'qunit';
import moduleBody from 'somewhere';
module('foo', moduleBody());
`,
`
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function() {
});
`,
],
invalid: [
{
code: `
import { setupOnerror } from '@ember/test-helpers';
import { module as moduleVariable } from 'qunit';
moduleVariable('foo', function(hooks) {
hooks.beforeEach(function() {
setupOnerror(() => {});
});
});
`,
output: `
import { setupOnerror } from '@ember/test-helpers';
import { module as moduleVariable } from 'qunit';
moduleVariable('foo', function(hooks) {
hooks.beforeEach(function() {
});
});
`,
errors: [
{
messageId: 'main',
type: 'CallExpression',
},
],
},
{
code: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.beforeEach(function() {
setupOnerror(function(){});
});
});
`,
output: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.beforeEach(function() {
});
});
`,
errors: [
{
messageId: 'main',
type: 'CallExpression',
},
],
},
{
code: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.beforeEach(function() {
setupOnerror(function noop(){});
});
});
`,
output: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.beforeEach(function() {
});
});
`,
errors: [
{
messageId: 'main',
type: 'CallExpression',
},
],
},
{
code: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.before(function() {
setupOnerror(() => {});
});
});
`,
output: `
import { setupOnerror } from '@ember/test-helpers';
import { module } from 'qunit';
module('foo', function(hooks) {
hooks.before(function() {
});
});
`,
errors: [
{
messageId: 'main',
type: 'CallExpression',
},
],
},
],
});

0 comments on commit 7baf741

Please sign in to comment.