From 14d83ef1c70bc1a75b60bb96a9b5cbe78efd7dc7 Mon Sep 17 00:00:00 2001 From: Juriy Zaytsev Date: Wed, 22 May 2019 09:06:28 -0400 Subject: [PATCH] feat(rules): add no-commented-out rule (#262) --- README.md | 2 + docs/rules/no-commented-out-tests.md | 61 +++++ src/__tests__/rules.test.js | 2 +- .../__tests__/no-commented-out-tests.test.js | 215 ++++++++++++++++++ src/rules/no-commented-out-tests.js | 39 ++++ 5 files changed, 318 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-commented-out-tests.md create mode 100644 src/rules/__tests__/no-commented-out-tests.test.js create mode 100644 src/rules/no-commented-out-tests.js diff --git a/README.md b/README.md index 33ccde9b3..351b7674c 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ for more information about extending configuration files. | [lowercase-name][] | Disallow capitalized test names | | ![fixable-green][] | | [no-alias-methods][] | Disallow alias methods | ![recommended][] | ![fixable-green][] | | [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | | +| [no-commented-out-tests][] | Disallow commented out tests | | | | [no-empty-title][] | Disallow empty titles | | | | [no-focused-tests][] | Disallow focused tests | ![recommended][] | | | [no-hooks][] | Disallow setup and teardown hooks | | | @@ -142,6 +143,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting [lowercase-name]: docs/rules/lowercase-name.md [no-alias-methods]: docs/rules/no-alias-methods.md [no-disabled-tests]: docs/rules/no-disabled-tests.md +[no-commented-out-tests]: docs/rules/no-commented-out-tests.md [no-empty-title]: docs/rules/no-empty-title.md [no-focused-tests]: docs/rules/no-focused-tests.md [no-hooks]: docs/rules/no-hooks.md diff --git a/docs/rules/no-commented-out-tests.md b/docs/rules/no-commented-out-tests.md new file mode 100644 index 000000000..829c44291 --- /dev/null +++ b/docs/rules/no-commented-out-tests.md @@ -0,0 +1,61 @@ +# Disallow commented out tests (no-commented-out-tests) + +This rule raises a warning about commented out tests. It's similar to +no-disabled-tests rule. + +## Rule Details + +The rule uses fuzzy matching to do its best to determine what constitutes a +commented out test, checking for a presence of `it(`, `describe(`, `it.skip(`, +etc. in code comments. + +The following patterns are considered warnings: + +```js +// describe('foo', () => {}); +// it('foo', () => {}); +// test('foo', () => {}); + +// describe.skip('foo', () => {}); +// it.skip('foo', () => {}); +// test.skip('foo', () => {}); + +// describe['skip']('bar', () => {}); +// it['skip']('bar', () => {}); +// test['skip']('bar', () => {}); + +// xdescribe('foo', () => {}); +// xit('foo', () => {}); +// xtest('foo', () => {}); + +/* +describe('foo', () => {}); +*/ +``` + +These patterns would not be considered warnings: + +```js +describe('foo', () => {}); +it('foo', () => {}); +test('foo', () => {}); + +describe.only('bar', () => {}); +it.only('bar', () => {}); +test.only('bar', () => {}); + +// foo('bar', () => {}); +``` + +### Limitations + +The plugin looks at the literal function names within test code, so will not +catch more complex examples of commented out tests, such as: + +```js +// const testSkip = test.skip; +// testSkip('skipped test', () => {}); + +// const myTest = test; +// myTest('does not have function body'); +``` diff --git a/src/__tests__/rules.test.js b/src/__tests__/rules.test.js index 687352c36..defda6016 100644 --- a/src/__tests__/rules.test.js +++ b/src/__tests__/rules.test.js @@ -5,7 +5,7 @@ const path = require('path'); const { rules } = require('../'); const ruleNames = Object.keys(rules); -const numberOfRules = 31; +const numberOfRules = 32; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/no-commented-out-tests.test.js b/src/rules/__tests__/no-commented-out-tests.test.js new file mode 100644 index 000000000..6d9a41e6f --- /dev/null +++ b/src/rules/__tests__/no-commented-out-tests.test.js @@ -0,0 +1,215 @@ +'use strict'; + +const { RuleTester } = require('eslint'); +const rule = require('../no-commented-out-tests'); + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + }, +}); + +ruleTester.run('no-commented-out-tests', rule, { + valid: [ + '// foo("bar", function () {})', + 'describe("foo", function () {})', + 'it("foo", function () {})', + 'describe.only("foo", function () {})', + 'it.only("foo", function () {})', + 'test("foo", function () {})', + 'test.only("foo", function () {})', + 'var appliedSkip = describe.skip; appliedSkip.apply(describe)', + 'var calledSkip = it.skip; calledSkip.call(it)', + '({ f: function () {} }).f()', + '(a || b).f()', + 'itHappensToStartWithIt()', + 'testSomething()', + [ + 'import { pending } from "actions"', + '', + 'test("foo", () => {', + ' expect(pending()).toEqual({})', + '})', + ].join('\n'), + [ + 'const { pending } = require("actions")', + '', + 'test("foo", () => {', + ' expect(pending()).toEqual({})', + '})', + ].join('\n'), + [ + 'test("foo", () => {', + ' const pending = getPending()', + ' expect(pending()).toEqual({})', + '})', + ].join('\n'), + [ + 'test("foo", () => {', + ' expect(pending()).toEqual({})', + '})', + '', + 'function pending() {', + ' return {}', + '}', + ].join('\n'), + ], + + invalid: [ + { + code: '// describe("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// describe["skip"]("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// describe[\'skip\']("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// it.skip("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// it.only("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// it["skip"]("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// test.skip("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// test["skip"]("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// xdescribe("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// xit("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// fit("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// xtest("foo", function () {})', + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: `// test( + // "foo", function () {} + // )`, + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: `/* test + ( + "foo", function () {} + ) + */`, + errors: [ + { message: 'Some tests seem to be commented', column: 1, line: 1 }, + ], + }, + { + code: '// it("has title but no callback")', + errors: [ + { + message: 'Some tests seem to be commented', + column: 1, + line: 1, + }, + ], + }, + { + code: '// it()', + errors: [ + { + message: 'Some tests seem to be commented', + column: 1, + line: 1, + }, + ], + }, + { + code: '// test.someNewMethodThatMightBeAddedInTheFuture()', + errors: [ + { + message: 'Some tests seem to be commented', + column: 1, + line: 1, + }, + ], + }, + { + code: '// test["someNewMethodThatMightBeAddedInTheFuture"]()', + errors: [ + { + message: 'Some tests seem to be commented', + column: 1, + line: 1, + }, + ], + }, + { + code: '// test("has title but no callback")', + errors: [ + { + message: 'Some tests seem to be commented', + column: 1, + line: 1, + }, + ], + }, + { + code: ` + foo() + /* + describe("has title but no callback", () => {}) + */ + bar()`, + errors: [ + { + message: 'Some tests seem to be commented', + column: 7, + line: 3, + }, + ], + }, + ], +}); diff --git a/src/rules/no-commented-out-tests.js b/src/rules/no-commented-out-tests.js new file mode 100644 index 000000000..013dc08cf --- /dev/null +++ b/src/rules/no-commented-out-tests.js @@ -0,0 +1,39 @@ +'use strict'; + +const { getDocsUrl } = require('./util'); + +const message = 'Some tests seem to be commented'; + +function hasTests(node) { + return /(x|f)?(test|it|describe)(\.\w+|\[['"]\w+['"]\])?\s*\(/m.test( + node.value, + ); +} + +module.exports = { + meta: { + docs: { + url: getDocsUrl(__filename), + }, + }, + create(context) { + const sourceCode = context.getSourceCode(); + + function checkNode(node) { + if (!hasTests(node)) return; + + context.report({ + message, + node, + }); + } + + return { + Program() { + const comments = sourceCode.getAllComments(); + + comments.filter(token => token.type !== 'Shebang').forEach(checkNode); + }, + }; + }, +};