From babf03799d1eb6288914a1b9381b8e059d7dc5d9 Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 17:32:23 +0200 Subject: [PATCH 1/5] feat(rules): add require-top-level-describe rule --- docs/rules/require-top-level-describe.md | 47 +++++++++++++++ src/__tests__/rules.test.ts | 2 +- .../require-top-level-describe.test.ts | 58 +++++++++++++++++++ src/rules/require-top-level-describe.ts | 37 ++++++++++++ 4 files changed, 143 insertions(+), 1 deletion(-) 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/docs/rules/require-top-level-describe.md b/docs/rules/require-top-level-describe.md new file mode 100644 index 000000000..1e5a78638 --- /dev/null +++ b/docs/rules/require-top-level-describe.md @@ -0,0 +1,47 @@ +# 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', () => {}); +``` + +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..f5e15878e --- /dev/null +++ b/src/rules/__tests__/require-top-level-describe.test.ts @@ -0,0 +1,58 @@ +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", () => {}) + }); + `, + ], + invalid: [ + { + code: ` + test("my test", () => {}) + describe("test suite", () => {}); + `, + errors: [{ messageId: 'unexpectedMethod' }], + }, + { + code: ` + test("my test", () => {}) + describe("test suite", () => { + it("test", () => {}) + }); + `, + errors: [{ messageId: 'unexpectedMethod' }], + }, + { + code: ` + describe("test suite", () => {}); + afterAll("my test", () => {}) + `, + errors: [{ messageId: 'unexpectedMethod' }], + }, + ], +}); diff --git a/src/rules/require-top-level-describe.ts b/src/rules/require-top-level-describe.ts new file mode 100644 index 000000000..08d62cbef --- /dev/null +++ b/src/rules/require-top-level-describe.ts @@ -0,0 +1,37 @@ +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: { + unexpectedMethod: 'There must be a top-level describe block.', + }, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + let numberOfDescribeBlock = 0; + return { + CallExpression(node) { + isDescribe(node) && numberOfDescribeBlock++; + if ((isTestCase(node) || isHook(node)) && numberOfDescribeBlock === 0) { + context.report({ node, messageId: 'unexpectedMethod' }); + } + }, + 'CallExpression:exit'(node: TSESTree.CallExpression) { + if (isDescribe(node)) { + numberOfDescribeBlock--; + } + }, + }; + }, +}); From 3e3415a6c9991df5c7d850f37eb5189018916683 Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 18:13:06 +0200 Subject: [PATCH 2/5] chore(require-top-level-describe): improve code readability --- src/rules/require-top-level-describe.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/rules/require-top-level-describe.ts b/src/rules/require-top-level-describe.ts index 08d62cbef..ddfa59f87 100644 --- a/src/rules/require-top-level-describe.ts +++ b/src/rules/require-top-level-describe.ts @@ -12,24 +12,31 @@ export default createRule({ recommended: false, }, messages: { - unexpectedMethod: 'There must be a top-level describe block.', + unexpectedMethod: 'All test cases must be wrapped in a describe block.', }, type: 'suggestion', schema: [], }, defaultOptions: [], create(context) { - let numberOfDescribeBlock = 0; + let numberOfDescribeBlocks = 0; return { CallExpression(node) { - isDescribe(node) && numberOfDescribeBlock++; - if ((isTestCase(node) || isHook(node)) && numberOfDescribeBlock === 0) { + if (isDescribe(node)) { + numberOfDescribeBlocks++; + return; + } + + if ( + (numberOfDescribeBlocks === 0 && isTestCase(node)) || + isHook(node) + ) { context.report({ node, messageId: 'unexpectedMethod' }); } }, 'CallExpression:exit'(node: TSESTree.CallExpression) { if (isDescribe(node)) { - numberOfDescribeBlock--; + numberOfDescribeBlocks--; } }, }; From 9ad6cfe4c3cf0775478d937f93b20ca59036644d Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 18:20:59 +0200 Subject: [PATCH 3/5] docs(require-top-level-describe): add rule details in main README --- README.md | 82 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 40 deletions(-) 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 From 83794bc3e7cada09da50e886125e23ace4a0c0f6 Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 18:53:06 +0200 Subject: [PATCH 4/5] feat(require-top-level-describe): separate test and hook error message --- docs/rules/require-top-level-describe.md | 5 +++++ .../require-top-level-describe.test.ts | 6 +++--- src/rules/require-top-level-describe.ts | 18 ++++++++++++------ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/rules/require-top-level-describe.md b/docs/rules/require-top-level-describe.md index 1e5a78638..4e43bbc07 100644 --- a/docs/rules/require-top-level-describe.md +++ b/docs/rules/require-top-level-describe.md @@ -23,6 +23,11 @@ describe('test suite', () => { // 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: diff --git a/src/rules/__tests__/require-top-level-describe.test.ts b/src/rules/__tests__/require-top-level-describe.test.ts index f5e15878e..625671ef3 100644 --- a/src/rules/__tests__/require-top-level-describe.test.ts +++ b/src/rules/__tests__/require-top-level-describe.test.ts @@ -36,7 +36,7 @@ ruleTester.run('no-standalone-hook', rule, { test("my test", () => {}) describe("test suite", () => {}); `, - errors: [{ messageId: 'unexpectedMethod' }], + errors: [{ messageId: 'unexpectedTestCase' }], }, { code: ` @@ -45,14 +45,14 @@ ruleTester.run('no-standalone-hook', rule, { it("test", () => {}) }); `, - errors: [{ messageId: 'unexpectedMethod' }], + errors: [{ messageId: 'unexpectedTestCase' }], }, { code: ` describe("test suite", () => {}); afterAll("my test", () => {}) `, - errors: [{ messageId: 'unexpectedMethod' }], + errors: [{ messageId: 'unexpectedHook' }], }, ], }); diff --git a/src/rules/require-top-level-describe.ts b/src/rules/require-top-level-describe.ts index ddfa59f87..a7a11341e 100644 --- a/src/rules/require-top-level-describe.ts +++ b/src/rules/require-top-level-describe.ts @@ -12,7 +12,8 @@ export default createRule({ recommended: false, }, messages: { - unexpectedMethod: 'All test cases must be wrapped in a describe block.', + unexpectedTestCase: 'All test cases must be wrapped in a describe block.', + unexpectedHook: 'All hooks must be wrapped in a describe block.', }, type: 'suggestion', schema: [], @@ -27,11 +28,16 @@ export default createRule({ return; } - if ( - (numberOfDescribeBlocks === 0 && isTestCase(node)) || - isHook(node) - ) { - context.report({ node, messageId: 'unexpectedMethod' }); + 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) { From 503a488eb64bbe857da0f8ece8279b493b7b6a68 Mon Sep 17 00:00:00 2001 From: Thomas Lombart Date: Thu, 29 Aug 2019 19:35:17 +0200 Subject: [PATCH 5/5] test(require-top-level-describe): add test case simple CallExpression --- src/rules/__tests__/require-top-level-describe.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/rules/__tests__/require-top-level-describe.test.ts b/src/rules/__tests__/require-top-level-describe.test.ts index 625671ef3..72347bb0c 100644 --- a/src/rules/__tests__/require-top-level-describe.test.ts +++ b/src/rules/__tests__/require-top-level-describe.test.ts @@ -29,8 +29,13 @@ ruleTester.run('no-standalone-hook', rule, { test("my other test", () => {}) }); `, + 'foo()', ], invalid: [ + { + code: 'beforeEach("my test", () => {})', + errors: [{ messageId: 'unexpectedHook' }], + }, { code: ` test("my test", () => {})