-
-
Notifications
You must be signed in to change notification settings - Fork 29
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 new rule no-only-tests
#145
Conversation
👍 agreed! |
|
||
## Rule Details | ||
|
||
This rule flags a violation when a test case is using `only`. Note that this rule is not autofixable since automatically deleting the property would prevent developers from being able to use it during development. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can add a suggestion to fix it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a suggestion to remove the only: true
property.
In a follow-up PR, I will update the README to make it more clear which rules provide suggestions.
`, | ||
output: null, | ||
errors: [{ messageId: 'foundOnly', type: 'Property', line: 5, endLine: 5, column: 28, endColumn: 38 }], | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add some more tests like:
const {RuleTester} = require("eslint");
const tester = new RuleTester();
tester.run('foo', bar, {...});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added imports to tests/docs.
2342d55
to
d8e1f1b
Compare
Most other test frameworks have a similar rule to this: * [ava/no-only-test](https://github.com/avajs/eslint-plugin-ava/blob/main/docs/rules/no-only-test.md) * [jest/no-focused-tests](https://github.com/jest-community/eslint-plugin-jest/blob/main/docs/rules/no-focused-tests.md) * [mocha/no-exclusive-tests](https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-exclusive-tests.md) * [qunit/no-only](https://github.com/platinumazure/eslint-plugin-qunit/blob/master/docs/rules/no-only.md)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
* @param {Token} token The token to check. | ||
* @returns {boolean} `true` if the token is a comma token. | ||
*/ | ||
function isCommaToken (token) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, didn't realize they came from there, fixed #156
Fixes #118.
I named it
no-only-tests
for consistency with the no-identical-tests rule name. I would be equally happy to name itno-test-only
or something else. Let me know if you want me to change the name.We should enable this as a
recommended
rule in v4 (#120).Most other test frameworks have a similar rule to this: