From eb0a2a2b82992f6a44b662c9601dba122d0e971e Mon Sep 17 00:00:00 2001 From: Ran Yitzhaki Date: Sun, 11 Feb 2018 22:51:05 +0200 Subject: [PATCH] feat(rules): add consistent-test-it rule Fixes #12 --- README.md | 1 + docs/rules/consistent-test-it.md | 89 +++++ index.js | 2 + rules/__tests__/consistent-test-it.test.js | 416 +++++++++++++++++++++ rules/consistent-test-it.js | 124 ++++++ rules/no-identical-title.js | 25 +- rules/util.js | 19 + 7 files changed, 652 insertions(+), 24 deletions(-) create mode 100644 docs/rules/consistent-test-it.md create mode 100644 rules/__tests__/consistent-test-it.test.js create mode 100644 rules/consistent-test-it.js diff --git a/README.md b/README.md index 1515c89a9..269e908ec 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ for more information about extending configuration files. | Rule | Description | Recommended | Fixable | | ------------------------------------------------------------------ | --------------------------------------------------------------- | ----------------------------------------------------------------------- | ----------------------------------------------------------- | +| [consistent-test-it](docs/rules/consistent-test-it.md) | Enforce consistent test or it keyword | | ![fixable](https://img.shields.io/badge/-fixable-green.svg) | | [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) | | | [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) | | | [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) | | diff --git a/docs/rules/consistent-test-it.md b/docs/rules/consistent-test-it.md new file mode 100644 index 000000000..b4cefe1e0 --- /dev/null +++ b/docs/rules/consistent-test-it.md @@ -0,0 +1,89 @@ +# Have control over `test` and `it` usages (consistent-test-it) + +Jest allows you to choose how you want to define your tests, using the `it` or +the `test` keywords, with multiple permutations for each: + +* **it:** `it`, `xit`, `fit`, `it.only`, `it.skip`. +* **test:** `test`, `xtest`, `test.only`, `test.skip`. + +This rule gives you control over the usage of these keywords in your codebase. + +## Rule Details + +This rule can be configured as follows + +```js +{ + type: 'object', + properties: { + fn: { + enum: ['it', 'test'], + }, + withinDescribe: { + enum: ['it', 'test'], + }, + }, + additionalProperties: false, +} +``` + +#### fn + +Decides whether to use `test` or `it`. + +#### withinDescribe + +Decides whether to use `test` or `it` within a describe scope. + +```js +/*eslint jest/consistent-test-it: ["error", {"fn": "test"}]*/ + +test('foo'); // valid +test.only('foo'); // valid + +it('foo'); // invalid +it.only('foo'); // invalid +``` + +```js +/*eslint jest/consistent-test-it: ["error", {"fn": "it"}]*/ + +it('foo'); // valid +it.only('foo'); // valid + +test('foo'); // invalid +test.only('foo'); // invalid +``` + +```js +/*eslint jest/consistent-test-it: ["error", {"fn": "it", "withinDescribe": "test"}]*/ + +it('foo'); // valid +describe('foo', function() { + test('bar'); // valid +}); + +test('foo'); // invalid +describe('foo', function() { + it('bar'); // invalid +}); +``` + +### Default configuration + +The default configuration forces top level test to use `test` and all tests +nested within describe to use `it`. + +```js +/*eslint jest/consistent-test-it: ["error"]*/ + +test('foo'); // valid +describe('foo', function() { + it('bar'); // valid +}); + +it('foo'); // invalid +describe('foo', function() { + test('bar'); // invalid +}); +``` diff --git a/index.js b/index.js index cd30b2e3d..4d60cdf07 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,6 @@ 'use strict'; +const consistentTestIt = require('./rules/consistent-test-it'); const noDisabledTests = require('./rules/no-disabled-tests'); const noFocusedTests = require('./rules/no-focused-tests'); const noIdenticalTitle = require('./rules/no-identical-title'); @@ -57,6 +58,7 @@ module.exports = { '.snap': snapshotProcessor, }, rules: { + 'consistent-test-it': consistentTestIt, 'no-disabled-tests': noDisabledTests, 'no-focused-tests': noFocusedTests, 'no-identical-title': noIdenticalTitle, diff --git a/rules/__tests__/consistent-test-it.test.js b/rules/__tests__/consistent-test-it.test.js new file mode 100644 index 000000000..094ac1bbe --- /dev/null +++ b/rules/__tests__/consistent-test-it.test.js @@ -0,0 +1,416 @@ +'use strict'; + +const RuleTester = require('eslint').RuleTester; +const rules = require('../..').rules; +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6, + }, +}); + +ruleTester.run('consistent-test-it with fn=test', rules['consistent-test-it'], { + valid: [ + { + code: 'test("foo")', + options: [{ fn: 'test' }], + }, + { + code: 'test.only("foo")', + options: [{ fn: 'test' }], + }, + { + code: 'test.skip("foo")', + options: [{ fn: 'test' }], + }, + { + code: 'xtest("foo")', + options: [{ fn: 'test' }], + }, + { + code: 'describe("suite", () => { test("foo") })', + options: [{ fn: 'test' }], + }, + ], + invalid: [ + { + code: 'it("foo")', + options: [{ fn: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test("foo")', + }, + { + code: 'xit("foo")', + options: [{ fn: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'xtest("foo")', + }, + { + code: 'fit("foo")', + options: [{ fn: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test.only("foo")', + }, + { + code: 'it.skip("foo")', + options: [{ fn: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test.skip("foo")', + }, + { + code: 'it.only("foo")', + options: [{ fn: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test.only("foo")', + }, + { + code: 'describe("suite", () => { it("foo") })', + options: [{ fn: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test("foo") })', + }, + ], +}); + +ruleTester.run('consistent-test-it with fn=it', rules['consistent-test-it'], { + valid: [ + { + code: 'it("foo")', + options: [{ fn: 'it' }], + }, + { + code: 'fit("foo")', + options: [{ fn: 'it' }], + }, + { + code: 'xit("foo")', + options: [{ fn: 'it' }], + }, + { + code: 'it.only("foo")', + options: [{ fn: 'it' }], + }, + { + code: 'it.skip("foo")', + options: [{ fn: 'it' }], + }, + { + code: 'describe("suite", () => { it("foo") })', + options: [{ fn: 'it' }], + }, + ], + invalid: [ + { + code: 'test("foo")', + options: [{ fn: 'it' }], + errors: [{ message: "Prefer using 'it' instead of 'test'" }], + output: 'it("foo")', + }, + { + code: 'xtest("foo")', + options: [{ fn: 'it' }], + errors: [{ message: "Prefer using 'it' instead of 'test'" }], + output: 'xit("foo")', + }, + { + code: 'test.skip("foo")', + options: [{ fn: 'it' }], + errors: [{ message: "Prefer using 'it' instead of 'test'" }], + output: 'it.skip("foo")', + }, + { + code: 'test.only("foo")', + options: [{ fn: 'it' }], + errors: [{ message: "Prefer using 'it' instead of 'test'" }], + output: 'it.only("foo")', + }, + { + code: 'describe("suite", () => { test("foo") })', + options: [{ fn: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it("foo") })', + }, + ], +}); + +ruleTester.run( + 'consistent-test-it with fn=test and withinDescribe=it ', + rules['consistent-test-it'], + { + valid: [ + { + code: 'test("foo")', + options: [{ fn: 'test', withinDescribe: 'it' }], + }, + { + code: 'test.only("foo")', + options: [{ fn: 'test', withinDescribe: 'it' }], + }, + { + code: 'test.skip("foo")', + options: [{ fn: 'test', withinDescribe: 'it' }], + }, + { + code: 'xtest("foo")', + options: [{ fn: 'test', withinDescribe: 'it' }], + }, + { + code: '[1,2,3].forEach(() => { test("foo") })', + options: [{ fn: 'test', withinDescribe: 'it' }], + }, + ], + invalid: [ + { + code: 'describe("suite", () => { test("foo") })', + options: [{ fn: 'test', withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it("foo") })', + }, + { + code: 'describe("suite", () => { test.only("foo") })', + options: [{ fn: 'test', withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it.only("foo") })', + }, + { + code: 'describe("suite", () => { xtest("foo") })', + options: [{ fn: 'test', withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { xit("foo") })', + }, + { + code: 'describe("suite", () => { test.skip("foo") })', + options: [{ fn: 'test', withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it.skip("foo") })', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it with fn=it and withinDescribe=test ', + rules['consistent-test-it'], + { + valid: [ + { + code: 'it("foo")', + options: [{ fn: 'it', withinDescribe: 'test' }], + }, + { + code: 'it.only("foo")', + options: [{ fn: 'it', withinDescribe: 'test' }], + }, + { + code: 'it.skip("foo")', + options: [{ fn: 'it', withinDescribe: 'test' }], + }, + { + code: 'xit("foo")', + options: [{ fn: 'it', withinDescribe: 'test' }], + }, + { + code: '[1,2,3].forEach(() => { it("foo") })', + options: [{ fn: 'it', withinDescribe: 'test' }], + }, + ], + invalid: [ + { + code: 'describe("suite", () => { it("foo") })', + options: [{ fn: 'it', withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test("foo") })', + }, + { + code: 'describe("suite", () => { it.only("foo") })', + options: [{ fn: 'it', withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test.only("foo") })', + }, + { + code: 'describe("suite", () => { xit("foo") })', + options: [{ fn: 'it', withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { xtest("foo") })', + }, + { + code: 'describe("suite", () => { it.skip("foo") })', + options: [{ fn: 'it', withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test.skip("foo") })', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it with fn=test and withinDescribe=test ', + rules['consistent-test-it'], + { + valid: [ + { + code: 'describe("suite", () => { test("foo") })', + options: [{ fn: 'test', withinDescribe: 'test' }], + }, + { + code: 'test("foo");', + options: [{ fn: 'test', withinDescribe: 'test' }], + }, + ], + invalid: [ + { + code: 'describe("suite", () => { it("foo") })', + options: [{ fn: 'test', withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test("foo") })', + }, + { + code: 'it("foo")', + options: [{ fn: 'test', withinDescribe: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test("foo")', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it with fn=it and withinDescribe=it ', + rules['consistent-test-it'], + { + valid: [ + { + code: 'describe("suite", () => { it("foo") })', + options: [{ fn: 'it', withinDescribe: 'it' }], + }, + { + code: 'it("foo")', + options: [{ fn: 'it', withinDescribe: 'it' }], + }, + ], + invalid: [ + { + code: 'describe("suite", () => { test("foo") })', + options: [{ fn: 'it', withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it("foo") })', + }, + { + code: 'test("foo")', + options: [{ fn: 'it', withinDescribe: 'it' }], + errors: [{ message: "Prefer using 'it' instead of 'test'" }], + output: 'it("foo")', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it defaults without config object', + rules['consistent-test-it'], + { + valid: [ + { + code: 'test("foo")', + }, + ], + invalid: [ + { + code: 'describe("suite", () => { test("foo") })', + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it("foo") })', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it with withinDescribe=it', + rules['consistent-test-it'], + { + valid: [ + { + code: 'test("foo")', + options: [{ withinDescribe: 'it' }], + }, + { + code: 'describe("suite", () => { it("foo") })', + options: [{ withinDescribe: 'it' }], + }, + ], + invalid: [ + { + code: 'it("foo")', + options: [{ withinDescribe: 'it' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test("foo")', + }, + { + code: 'describe("suite", () => { test("foo") })', + options: [{ withinDescribe: 'it' }], + errors: [ + { message: "Prefer using 'it' instead of 'test' within describe" }, + ], + output: 'describe("suite", () => { it("foo") })', + }, + ], + } +); + +ruleTester.run( + 'consistent-test-it with withinDescribe=test', + rules['consistent-test-it'], + { + valid: [ + { + code: 'test("foo")', + options: [{ withinDescribe: 'test' }], + }, + { + code: 'describe("suite", () => { test("foo") })', + options: [{ withinDescribe: 'test' }], + }, + ], + invalid: [ + { + code: 'it("foo")', + options: [{ withinDescribe: 'test' }], + errors: [{ message: "Prefer using 'test' instead of 'it'" }], + output: 'test("foo")', + }, + { + code: 'describe("suite", () => { it("foo") })', + options: [{ withinDescribe: 'test' }], + errors: [ + { message: "Prefer using 'test' instead of 'it' within describe" }, + ], + output: 'describe("suite", () => { test("foo") })', + }, + ], + } +); diff --git a/rules/consistent-test-it.js b/rules/consistent-test-it.js new file mode 100644 index 000000000..5f39ba975 --- /dev/null +++ b/rules/consistent-test-it.js @@ -0,0 +1,124 @@ +'use strict'; + +const getNodeName = require('./util').getNodeName; +const isTestCase = require('./util').isTestCase; +const isDescribe = require('./util').isDescribe; + +module.exports = { + meta: { + docs: { + url: + 'https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/consistent-test-it.md', + }, + fixable: 'code', + schema: [ + { + type: 'object', + properties: { + fn: { + enum: ['it', 'test'], + }, + withinDescribe: { + enum: ['it', 'test'], + }, + }, + additionalProperties: false, + }, + ], + }, + create(context) { + const configObj = context.options[0] || {}; + const testKeyword = configObj.fn || 'test'; + const testKeywordWithinDescribe = + configObj.withinDescribe || configObj.fn || 'it'; + + let describeNestingLevel = 0; + + return { + CallExpression(node) { + const nodeName = getNodeName(node.callee); + + if (isDescribe(node)) { + describeNestingLevel++; + } + + if ( + isTestCase(node) && + describeNestingLevel === 0 && + nodeName.indexOf(testKeyword) === -1 + ) { + const oppositeTestKeyword = getOppositeTestKeyword(testKeyword); + + context.report({ + message: + "Prefer using '{{ testKeyword }}' instead of '{{ oppositeTestKeyword }}'", + node: node.callee, + data: { testKeyword, oppositeTestKeyword }, + fix(fixer) { + const nodeToReplace = + node.callee.type === 'MemberExpression' + ? node.callee.object + : node.callee; + + const fixedNodeName = getPreferredNodeName(nodeName, testKeyword); + return [fixer.replaceText(nodeToReplace, fixedNodeName)]; + }, + }); + } + + if ( + isTestCase(node) && + describeNestingLevel > 0 && + nodeName.indexOf(testKeywordWithinDescribe) === -1 + ) { + const oppositeTestKeyword = getOppositeTestKeyword( + testKeywordWithinDescribe + ); + + context.report({ + message: + "Prefer using '{{ testKeywordWithinDescribe }}' instead of '{{ oppositeTestKeyword }}' within describe", + node: node.callee, + data: { testKeywordWithinDescribe, oppositeTestKeyword }, + fix(fixer) { + const nodeToReplace = + node.callee.type === 'MemberExpression' + ? node.callee.object + : node.callee; + + const fixedNodeName = getPreferredNodeName( + nodeName, + testKeywordWithinDescribe + ); + return [fixer.replaceText(nodeToReplace, fixedNodeName)]; + }, + }); + } + }, + 'CallExpression:exit'(node) { + if (isDescribe(node)) { + describeNestingLevel--; + } + }, + }; + }, +}; + +function getPreferredNodeName(nodeName, preferredTestKeyword) { + switch (nodeName) { + case 'fit': + return 'test.only'; + default: + return nodeName.startsWith('f') || nodeName.startsWith('x') + ? nodeName.charAt(0) + preferredTestKeyword + : preferredTestKeyword; + } +} + +function getOppositeTestKeyword(test) { + if (test === 'test') { + return 'it'; + } + + return 'test'; +} diff --git a/rules/no-identical-title.js b/rules/no-identical-title.js index 194ad4e6b..09f34e488 100644 --- a/rules/no-identical-title.js +++ b/rules/no-identical-title.js @@ -1,30 +1,7 @@ 'use strict'; const isDescribe = require('./util').isDescribe; - -const testCaseNames = Object.assign(Object.create(null), { - fit: true, - it: true, - 'it.only': true, - 'it.skip': true, - test: true, - 'test.only': true, - 'test.skip': true, - xit: true, - xtest: true, -}); - -const getNodeName = node => { - if (node.type === 'MemberExpression') { - return node.object.name + '.' + node.property.name; - } - return node.name; -}; - -const isTestCase = node => - node && - node.type === 'CallExpression' && - testCaseNames[getNodeName(node.callee)]; +const isTestCase = require('./util').isTestCase; const newDescribeContext = () => ({ describeTitles: [], diff --git a/rules/util.js b/rules/util.js index a9e9da0ee..e555e78a7 100644 --- a/rules/util.js +++ b/rules/util.js @@ -88,6 +88,18 @@ const describeAliases = Object.assign(Object.create(null), { xdescribe: true, }); +const testCaseNames = Object.assign(Object.create(null), { + fit: true, + it: true, + 'it.only': true, + 'it.skip': true, + test: true, + 'test.only': true, + 'test.skip': true, + xit: true, + xtest: true, +}); + const getNodeName = node => { if (node.type === 'MemberExpression') { return node.object.name + '.' + node.property.name; @@ -95,6 +107,11 @@ const getNodeName = node => { return node.name; }; +const isTestCase = node => + node && + node.type === 'CallExpression' && + testCaseNames[getNodeName(node.callee)]; + const isDescribe = node => node.type === 'CallExpression' && describeAliases[getNodeName(node.callee)]; @@ -116,6 +133,8 @@ module.exports = { expectNotToEqualCase: expectNotToEqualCase, expectToBeUndefinedCase: expectToBeUndefinedCase, expectNotToBeUndefinedCase: expectNotToBeUndefinedCase, + getNodeName: getNodeName, isDescribe: isDescribe, isFunction: isFunction, + isTestCase: isTestCase, };