-
Notifications
You must be signed in to change notification settings - Fork 49
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
no-commented-tests #85
Conversation
Looks like I am misusing |
Some more ideas for this: SpeedIf I had to guess, parsing every comment is going to incur some significant performance penalties. Especially since I will have to parse most comments twice (see /test(?:\(|\.)/.test(src) AccuracyShould we maybe require valid JS between the opening // can't use test(...) here, because
test.cb(t => { }); Or they might be cutting / pasting code out of the comment to a refactored test (leaving it in a non-parseable state). DisablingSometimes your comments contain example code. Especially if you are using JSDoc to generate documentation. Ideally, you would want to disable this rule near the the offending line in the comment, like this: test.cb('foo', t => {
// My comments might describe test code, like:
//
// We have to use the `test.cb()` form for this test because... // eslint-disable-line
//
doSomething(t.end);
}); Not sure how disable comments are attached / handled, but I would imaging they attach to actual Nodes (not comments). How does it apply to leading / trailing comments? Need to play with it some more. |
|
||
// TODO: Probably should not throw here. Maybe return `null` and modify the message warning the exact location could not be found? | ||
// This should be unreachable code, so would like to encourage bug reports if that ever proves untrue. | ||
throw new Error('unable to translate location'); |
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.
I think throwing is fine if this is meant to be never hit. The error message should improved though. It should say that this should really never happen and tell the user to open a bug report with the link https://github.com/sindresorhus/eslint-plugin-ava/issues/new
.
Looks promising. As for naming, we should probably follow our convention (e.g.
👍
👍
Can't we just ignore JSDoc comments? Comments starting with |
Ah yes - ignoring jsdoc. That should work. |
hi guys! Sorry to pester but I'm really keen to use this feature. Where are we stuck? |
@revelt It's all in the feedback comments. |
Closing for lack of activity. |
It's pity, this would have been a nice rule. |
Prevents commented out tests.
Fixes #84.
Still needs docs.
What should the rule name be? Some ideas:
no-commented-tests
no-commented-out-tests
no-tests-in-comments
no-comment-tests