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

New: disallow identical tests #11

Merged
merged 8 commits into from
Jul 1, 2017
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Name | ✔️ | 🛠 | Description
[prefer-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-placeholders.md) | | | Disallows template literals as report messages
[no-useless-token-range](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md) | ✔️ | 🛠 | Disallows unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken
[consistent-output](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/consistent-output.md) | | | Enforces consistent use of output assertions in rule tests
[no-identical-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-identical-tests.md) | | | Disallows identical tests

## Supported Presets

Expand Down
39 changes: 39 additions & 0 deletions docs/rules/no-identical-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Disallow identical tests (no-identical-tests)

When a rule has a lot of tests, it's sometimes difficult to tell if any tests are duplicates. This rule would warn if any test cases have the same properties.

## Rule Details

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

```js
/* eslint eslint-plugin/consistent-output: error */
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: This should say /* eslint eslint-plugin/no-identical-tests: error */


new RuleTester().run('foo', bar, {
valid: [
{ code: 'foo' },
{ code: 'foo' }
],
invalid: []
});

```

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

```js
/* eslint eslint-plugin/consistent-output: error */
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: This should say /* eslint eslint-plugin/no-identical-tests: error */


new RuleTester().run('foo', bar, {
valid: [
{ code: 'foo' },
{ code: 'bar' }
],
invalid: []
});

```

## When Not To Use It

If you want to allow identical tests, do not enable this rule.
54 changes: 54 additions & 0 deletions lib/rules/no-identical-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* @fileoverview disallow identical tests
* @author 薛定谔的猫<[email protected]>
*/

'use strict';

const utils = require('../utils');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'disallow identical tests',
category: 'Tests',
recommended: false,
},
fixable: null, // or "code" or "whitespace"
schema: [],
},

create (context) {
// ----------------------------------------------------------------------
// Public
// ----------------------------------------------------------------------
const message = 'This test case is identical to another case.';
const sourceCode = context.getSourceCode();

return {
Program (ast) {
utils.getTestInfo(context, ast).forEach(testRun => {
[testRun.valid, testRun.invalid].forEach(tests => {
const cache = Object.create(null);
// to avoid tests being null
(tests || []).forEach(test => {
const testCode = sourceCode.getText(test);
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually, I think it would also be useful to also catch cases like this:

{
  code: 'foo',
  options: ['bar']
},
{
  options: ['bar'],
  code: 'foo'
}

But it's probably not necessary to change that as part of this PR.

Copy link
Contributor Author

@aladdin-add aladdin-add Jul 1, 2017

Choose a reason for hiding this comment

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

yes, it is an enhancement, I will finish it in another PR.:(

if (cache[testCode]) {
context.report({
node: test,
message,
});
} else {
cache[testCode] = true;
}
});
});
});
},
};
},
};
17 changes: 0 additions & 17 deletions tests/lib/rules/no-deprecated-report-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,6 @@ ruleTester.run('no-deprecated-report-api', rule, {
}
};
`,
`
module.exports = {
create(context) {
context.report({
node,
message: "Foo."
});
}
};
`,
`
module.exports = {
create(context) {
foo.report(bar, baz);
}
};
`,
`
module.exports = {
create(context) {
Expand Down
64 changes: 64 additions & 0 deletions tests/lib/rules/no-identical-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* @fileoverview disallow identical tests
* @author 薛定谔的猫<[email protected]>
*/

'use strict';

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

const rule = require('../../../lib/rules/no-identical-tests');
const RuleTester = require('eslint').RuleTester;

const ERROR = { message: 'This test case is identical to another case.' };

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

const ruleTester = new RuleTester();
ruleTester.run('no-identical-tests', rule, {
valid: [
`
new RuleTester().run('foo', bar, {
valid: [
{ code: 'foo' },
{ code: 'bar' }
],
invalid: []
});
`,
],

invalid: [
{
code: `
new RuleTester().run('foo', bar, {
valid: [
{ code: 'foo' },
{ code: 'foo' }
],
invalid: []
});
`,
errors: [ERROR],
},
{
code: `
new RuleTester().run('foo', bar, {
valid: [
{ code: 'foo' },
{ code: 'foo' },
],
invalid: [
{ code: 'foo', errors: ['bar'] },
{ code: 'foo', errors: ['bar'] },
]
});
`,
errors: [ERROR, ERROR],
},
],
});
20 changes: 0 additions & 20 deletions tests/lib/rules/report-message-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,26 +63,6 @@ ruleTester.run('report-message-format', rule, {
`,
options: ['^foo$'],
},
{
code: `
module.exports = {
create(context) {
context.report(node, 'not foo' + message);
}
};
`,
options: ['^foo$'],
},
{
code: `
module.exports = {
create(context) {
context.report({node, message: 'foo'});
}
};
`,
options: ['^foo$'],
},
{
code: `
module.exports = {
Expand Down
8 changes: 0 additions & 8 deletions tests/lib/rules/require-meta-fixable.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ ruleTester.run('require-meta-fixable', rule, {
}
};
`,
`
module.exports = {
meta: { fixable: 'code' },
create(context) {
context.report({node, message, fix: foo});
}
};
`,
`
module.exports = {
meta: { fixable: 'whitespace' },
Expand Down