From 1c40d1a06ae5212c9559049ea0e8d6c9a7c55dcd Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 21:51:00 +0200 Subject: [PATCH] feat(rules): add `require-top-level-describe` rule (#407) Closes #401 --- README.md | 82 ++++++++++--------- docs/rules/require-top-level-describe.md | 52 ++++++++++++ src/__tests__/rules.test.ts | 2 +- .../require-top-level-describe.test.ts | 63 ++++++++++++++ src/rules/require-top-level-describe.ts | 50 +++++++++++ 5 files changed, 208 insertions(+), 41 deletions(-) create mode 100644 docs/rules/require-top-level-describe.md create mode 100644 src/rules/__tests__/require-top-level-describe.test.ts create mode 100644 src/rules/require-top-level-describe.ts diff --git a/README.md b/README.md index cb8ee5a72..34d21478d 100644 --- a/README.md +++ b/README.md @@ -104,46 +104,47 @@ installations requiring long-term consistency. ## Rules -| Rule | Description | Recommended | Fixable | -| ---------------------------- | ----------------------------------------------------------------- | ---------------- | ------------------- | -| [consistent-test-it][] | Enforce consistent test or it keyword | | ![fixable-green][] | -| [expect-expect][] | Enforce assertion to be made in a test body | | | -| [lowercase-name][] | Disallow capitalized test names | | ![fixable-green][] | -| [no-alias-methods][] | Disallow alias methods | ![recommended][] | ![fixable-green][] | -| [no-commented-out-tests][] | Disallow commented out tests | | | -| [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | | -| [no-duplicate-hooks][] | Disallow duplicate hooks within a `describe` block | | | -| [no-empty-title][] | Disallow empty titles | | | -| [no-expect-resolves][] | Disallow using `expect().resolves` | | | -| [no-export][] | Disallow export from test files | | | -| [no-focused-tests][] | Disallow focused tests | ![recommended][] | | -| [no-hooks][] | Disallow setup and teardown hooks | | | -| [no-identical-title][] | Disallow identical titles | ![recommended][] | | -| [no-if][] | Disallow conditional logic | | | -| [no-jasmine-globals][] | Disallow Jasmine globals | ![recommended][] | ![fixable-yellow][] | -| [no-jest-import][] | Disallow importing `jest` | ![recommended][] | | -| [no-large-snapshots][] | Disallow large snapshots | | | -| [no-mocks-import][] | Disallow manually importing from `__mocks__` | | | -| [no-standalone-expect][] | Prevents `expect` statements outside of a `test` or `it` block | | | -| [no-test-callback][] | Using a callback in asynchronous tests | | ![fixable-green][] | -| [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][] | -| [no-test-return-statement][] | Disallow explicitly returning from tests | | | -| [no-truthy-falsy][] | Disallow using `toBeTruthy()` & `toBeFalsy()` | | | -| [no-try-expect][] | Prevent `catch` assertions in tests | | | -| [prefer-called-with][] | Suggest using `toBeCalledWith()` OR `toHaveBeenCalledWith()` | | | -| [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | -| [prefer-inline-snapshots][] | Suggest using `toMatchInlineSnapshot()` | | ![fixable-green][] | -| [prefer-spy-on][] | Suggest using `jest.spyOn()` | | ![fixable-green][] | -| [prefer-strict-equal][] | Suggest using `toStrictEqual()` | | ![fixable-green][] | -| [prefer-to-be-null][] | Suggest using `toBeNull()` | | ![fixable-green][] | -| [prefer-to-be-undefined][] | Suggest using `toBeUndefined()` | | ![fixable-green][] | -| [prefer-to-contain][] | Suggest using `toContain()` | | ![fixable-green][] | -| [prefer-to-have-length][] | Suggest using `toHaveLength()` | | ![fixable-green][] | -| [prefer-todo][] | Suggest using `test.todo()` | | ![fixable-green][] | -| [require-tothrow-message][] | Require that `toThrow()` and `toThrowError` includes a message | | | -| [valid-describe][] | Enforce valid `describe()` callback | ![recommended][] | | -| [valid-expect-in-promise][] | Enforce having return statement when testing with promises | ![recommended][] | | -| [valid-expect][] | Enforce valid `expect()` usage | ![recommended][] | | +| Rule | Description | Recommended | Fixable | +| ------------------------------ | ----------------------------------------------------------------- | ---------------- | ------------------- | +| [consistent-test-it][] | Enforce consistent test or it keyword | | ![fixable-green][] | +| [expect-expect][] | Enforce assertion to be made in a test body | | | +| [lowercase-name][] | Disallow capitalized test names | | ![fixable-green][] | +| [no-alias-methods][] | Disallow alias methods | ![recommended][] | ![fixable-green][] | +| [no-commented-out-tests][] | Disallow commented out tests | | | +| [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | | +| [no-duplicate-hooks][] | Disallow duplicate hooks within a `describe` block | | | +| [no-empty-title][] | Disallow empty titles | | | +| [no-expect-resolves][] | Disallow using `expect().resolves` | | | +| [no-export][] | Disallow export from test files | | | +| [no-focused-tests][] | Disallow focused tests | ![recommended][] | | +| [no-hooks][] | Disallow setup and teardown hooks | | | +| [no-identical-title][] | Disallow identical titles | ![recommended][] | | +| [no-if][] | Disallow conditional logic | | | +| [no-jasmine-globals][] | Disallow Jasmine globals | ![recommended][] | ![fixable-yellow][] | +| [no-jest-import][] | Disallow importing `jest` | ![recommended][] | | +| [no-large-snapshots][] | Disallow large snapshots | | | +| [no-mocks-import][] | Disallow manually importing from `__mocks__` | | | +| [no-standalone-expect][] | Prevents `expect` statements outside of a `test` or `it` block | | | +| [no-test-callback][] | Using a callback in asynchronous tests | | ![fixable-green][] | +| [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][] | +| [no-test-return-statement][] | Disallow explicitly returning from tests | | | +| [no-truthy-falsy][] | Disallow using `toBeTruthy()` & `toBeFalsy()` | | | +| [no-try-expect][] | Prevent `catch` assertions in tests | | | +| [prefer-called-with][] | Suggest using `toBeCalledWith()` OR `toHaveBeenCalledWith()` | | | +| [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | +| [prefer-inline-snapshots][] | Suggest using `toMatchInlineSnapshot()` | | ![fixable-green][] | +| [prefer-spy-on][] | Suggest using `jest.spyOn()` | | ![fixable-green][] | +| [prefer-strict-equal][] | Suggest using `toStrictEqual()` | | ![fixable-green][] | +| [prefer-to-be-null][] | Suggest using `toBeNull()` | | ![fixable-green][] | +| [prefer-to-be-undefined][] | Suggest using `toBeUndefined()` | | ![fixable-green][] | +| [prefer-to-contain][] | Suggest using `toContain()` | | ![fixable-green][] | +| [prefer-to-have-length][] | Suggest using `toHaveLength()` | | ![fixable-green][] | +| [prefer-todo][] | Suggest using `test.todo()` | | ![fixable-green][] | +| [require-top-level-describe][] | Require a top-level `describe` block | | | +| [require-tothrow-message][] | Require that `toThrow()` and `toThrowError` includes a message | | | +| [valid-describe][] | Enforce valid `describe()` callback | ![recommended][] | | +| [valid-expect-in-promise][] | Enforce having return statement when testing with promises | ![recommended][] | | +| [valid-expect][] | Enforce valid `expect()` usage | ![recommended][] | | ## Credit @@ -193,6 +194,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting [prefer-to-contain]: docs/rules/prefer-to-contain.md [prefer-to-have-length]: docs/rules/prefer-to-have-length.md [prefer-todo]: docs/rules/prefer-todo.md +[require-top-level-describe]: docs/rules/require-top-level-describe.md [require-tothrow-message]: docs/rules/require-tothrow-message.md [valid-describe]: docs/rules/valid-describe.md [valid-expect-in-promise]: docs/rules/valid-expect-in-promise.md diff --git a/docs/rules/require-top-level-describe.md b/docs/rules/require-top-level-describe.md new file mode 100644 index 000000000..4e43bbc07 --- /dev/null +++ b/docs/rules/require-top-level-describe.md @@ -0,0 +1,52 @@ +# Require top-level describe block (require-top-level-describe) + +Jest allows you to organise your test files the way you want it. However, the +more your codebase grows, the more it becomes hard to navigate in your test +files. This rule makes sure that you provide at least a top-level describe block +in your test file. + +## Rule Details + +This rule triggers a warning if a test case (`test` and `it`) or a hook +(`beforeAll`, `beforeEach`, `afterEach`, `afterAll`) is not located in a +top-level describe block. + +The following patterns are considered warnings: + +```js +// Above a describe block +test('my test', () => {}); +describe('test suite', () => { + it('test', () => {}); +}); + +// Below a describe block +describe('test suite', () => {}); +test('my test', () => {}); + +// Same for hooks +beforeAll('my beforeAll', () => {}); +describe('test suite', () => {}); +afterEach('my afterEach', () => {}); +``` + +The following patterns are **not** considered warnings: + +```js +// In a describe block +describe('test suite', () => { + test('my test', () => {}); +}); + +// In a nested describe block +describe('test suite', () => { + test('my test', () => {}); + describe('another test suite', () => { + test('my other test', () => {}); + }); +}); +``` + +## When Not To Use It + +Don't use this rule on non-jest test files. diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index b7659cc24..787c3c4cb 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -3,7 +3,7 @@ import { resolve } from 'path'; import plugin from '../'; const ruleNames = Object.keys(plugin.rules); -const numberOfRules = 38; +const numberOfRules = 39; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/require-top-level-describe.test.ts b/src/rules/__tests__/require-top-level-describe.test.ts new file mode 100644 index 000000000..72347bb0c --- /dev/null +++ b/src/rules/__tests__/require-top-level-describe.test.ts @@ -0,0 +1,63 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import rule from '../require-top-level-describe'; + +const ruleTester = new TSESLint.RuleTester({ + parserOptions: { + ecmaVersion: 2015, + }, +}); + +ruleTester.run('no-standalone-hook', rule, { + valid: [ + 'describe("test suite", () => { test("my test") });', + 'describe("test suite", () => { it("my test") });', + ` + describe("test suite", () => { + beforeEach("a", () => {}); + describe("b", () => {}); + test("c", () => {}) + }); + `, + 'describe("test suite", () => { beforeAll("my beforeAll") });', + 'describe("test suite", () => { afterEach("my afterEach") });', + 'describe("test suite", () => { afterAll("my afterAll") });', + ` + describe("test suite", () => { + it("my test", () => {}) + describe("another test suite", () => { + }); + test("my other test", () => {}) + }); + `, + 'foo()', + ], + invalid: [ + { + code: 'beforeEach("my test", () => {})', + errors: [{ messageId: 'unexpectedHook' }], + }, + { + code: ` + test("my test", () => {}) + describe("test suite", () => {}); + `, + errors: [{ messageId: 'unexpectedTestCase' }], + }, + { + code: ` + test("my test", () => {}) + describe("test suite", () => { + it("test", () => {}) + }); + `, + errors: [{ messageId: 'unexpectedTestCase' }], + }, + { + code: ` + describe("test suite", () => {}); + afterAll("my test", () => {}) + `, + errors: [{ messageId: 'unexpectedHook' }], + }, + ], +}); diff --git a/src/rules/require-top-level-describe.ts b/src/rules/require-top-level-describe.ts new file mode 100644 index 000000000..a7a11341e --- /dev/null +++ b/src/rules/require-top-level-describe.ts @@ -0,0 +1,50 @@ +import { TSESTree } from '@typescript-eslint/experimental-utils'; + +import { createRule, isDescribe, isHook, isTestCase } from './utils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: + 'Prevents test cases and hooks to be outside of a describe block', + recommended: false, + }, + messages: { + unexpectedTestCase: 'All test cases must be wrapped in a describe block.', + unexpectedHook: 'All hooks must be wrapped in a describe block.', + }, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + let numberOfDescribeBlocks = 0; + return { + CallExpression(node) { + if (isDescribe(node)) { + numberOfDescribeBlocks++; + return; + } + + if (numberOfDescribeBlocks === 0) { + if (isTestCase(node)) { + context.report({ node, messageId: 'unexpectedTestCase' }); + return; + } + + if (isHook(node)) { + context.report({ node, messageId: 'unexpectedHook' }); + return; + } + } + }, + 'CallExpression:exit'(node: TSESTree.CallExpression) { + if (isDescribe(node)) { + numberOfDescribeBlocks--; + } + }, + }; + }, +});