From 83bb4bbc56a8cf8604e29c35a8a2c4ebf85d8ee7 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Wed, 23 Jun 2021 21:24:57 -0400 Subject: [PATCH] feat: add new rule `no-only-tests` --- README.md | 1 + docs/rules/no-only-tests.md | 49 ++++++++++++++++++ lib/rules/no-only-tests.js | 55 +++++++++++++++++++++ tests/lib/rules/no-only-tests.js | 85 ++++++++++++++++++++++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 docs/rules/no-only-tests.md create mode 100644 lib/rules/no-only-tests.js create mode 100644 tests/lib/rules/no-only-tests.js diff --git a/README.md b/README.md index e7c17ba2..01f72963 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ Name | ✔️ | 🛠 | Description [no-deprecated-report-api](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-deprecated-report-api.md) | ✔️ | 🛠 | disallow use of the deprecated context.report() API [no-identical-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-identical-tests.md) | ✔️ | 🛠 | disallow identical tests [no-missing-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-missing-placeholders.md) | ✔️ | | disallow missing placeholders in rule report messages +[no-only-tests](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-only-tests.md) | | | disallow the test case property `only` [no-unused-placeholders](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-unused-placeholders.md) | ✔️ | | disallow unused placeholders in rule 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) | ✔️ | 🛠 | disallow unnecessary calls to sourceCode.getFirstToken and sourceCode.getLastToken [prefer-object-rule](https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/prefer-object-rule.md) | | 🛠 | disallow rule exports where the export is a function. diff --git a/docs/rules/no-only-tests.md b/docs/rules/no-only-tests.md new file mode 100644 index 00000000..e82b3fe5 --- /dev/null +++ b/docs/rules/no-only-tests.md @@ -0,0 +1,49 @@ +# Disallow the test case property `only` (no-only-tests) + +The [`only` property](https://eslint.org/docs/developer-guide/unit-tests#running-individual-tests) can be used as of [ESLint 7.29](https://eslint.org/blog/2021/06/eslint-v7.29.0-released#highlights) for running individual rule test cases with less-noisy debugging. This feature should be only used in development, as it prevents all the tests from running. Mistakenly checking-in a test case with this property can cause CI tests to incorrectly pass. + +## 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. + +Examples of **incorrect** code for this rule: + +```js +/* eslint eslint-plugin/no-only-tests: error */ + +ruleTester.run('my-rule', myRule, { + valid: [ + { + code: 'const valid = 42;', + only: true, + }, + RuleTester.only('const valid = 42;'), + ], + invalid: [ + { + code: 'const invalid = 42;', + only: true, + errors: [/* ... */], + }, + ], +}); +``` + +Examples of **correct** code for this rule: + +```js +/* eslint eslint-plugin/no-only-tests: error */ + +ruleTester.run('my-rule', myRule, { + valid: [ + 'const valid = 42;', + { code: 'const valid = 42;' } + ], + invalid: [ + { + code: 'const invalid = 42;', + errors: [/* ... */], + }, + ], +}); +``` diff --git a/lib/rules/no-only-tests.js b/lib/rules/no-only-tests.js new file mode 100644 index 00000000..8c1a739c --- /dev/null +++ b/lib/rules/no-only-tests.js @@ -0,0 +1,55 @@ +'use strict'; + +const utils = require('../utils'); + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'disallow the test case property `only`', + category: 'Tests', + recommended: false, + }, + schema: [], + messages: { + foundOnly: + 'The test case property `only` can be used during development, but should not be checked-in, since it prevents all the tests from running.', + }, + }, + + create (context) { + return { + Program (ast) { + for (const testRun of utils.getTestInfo(context, ast)) { + for (const test of [...testRun.valid, ...testRun.invalid]) { + if (test.type === 'ObjectExpression') { + // Test case object: { code: 'const x = 123;', ... } + + const onlyProperty = test.properties.find( + property => + property.key.type === 'Identifier' && + property.key.name === 'only' && + property.value.type === 'Literal' && + property.value.value + ); + + if (onlyProperty) { + context.report({ node: onlyProperty, messageId: 'foundOnly' }); + } + } else if ( + test.type === 'CallExpression' && + test.callee.type === 'MemberExpression' && + test.callee.object.type === 'Identifier' && + test.callee.object.name === 'RuleTester' && + test.callee.property.type === 'Identifier' && + test.callee.property.name === 'only' + ) { + // RuleTester.only('const x = 123;'); + context.report({ node: test.callee, messageId: 'foundOnly' }); + } + } + } + }, + }; + }, +}; diff --git a/tests/lib/rules/no-only-tests.js b/tests/lib/rules/no-only-tests.js new file mode 100644 index 00000000..c66c9264 --- /dev/null +++ b/tests/lib/rules/no-only-tests.js @@ -0,0 +1,85 @@ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/no-only-tests'); +const RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester(); +ruleTester.run('no-only-tests', rule, { + valid: [ + // No test cases with `only` + ` + new RuleTester().run('foo', bar, { + valid: [ + 'foo', + { code: 'foo', foo: true }, + RuleTester.somethingElse(), + notRuleTester.only() + ], + invalid: [ + { code: 'bar', foo: true }, + ] + }); + `, + // `only` set to `false` + ` + new RuleTester().run('foo', bar, { + valid: [ + { code: 'foo', only: false }, + ], + invalid: [ + { code: 'bar', only: false }, + ] + }); + `, + ], + + invalid: [ + { + // Valid test case with `only` + code: ` + new RuleTester().run('foo', bar, { + valid: [ + { code: 'foo', only: true }, + ], + invalid: [] + }); + `, + output: null, + errors: [{ messageId: 'foundOnly', type: 'Property', line: 4, endLine: 4, column: 28, endColumn: 38 }], + }, + { + // Valid test case using `RuleTester.only` + code: ` + new RuleTester().run('foo', bar, { + valid: [ + RuleTester.only('foo'), + ], + invalid: [] + }); + `, + output: null, + errors: [{ messageId: 'foundOnly', type: 'MemberExpression', line: 4, endLine: 4, column: 13, endColumn: 28 }], + }, + { + // Invalid test case with `only` + code: ` + new RuleTester().run('foo', bar, { + valid: [], + invalid: [ + { code: 'foo', only: true }, + ] + }); + `, + output: null, + errors: [{ messageId: 'foundOnly', type: 'Property', line: 5, endLine: 5, column: 28, endColumn: 38 }], + }, + ], +});