From e6f4f8aaf7664bcf9d9d5549c3c43b1b09f49461 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 16 Jan 2022 09:24:54 +1300 Subject: [PATCH] feat(prefer-expect-assertions): support requiring only if `expect` is used in a loop (#1013) * feat(prefer-expect-assertions): support requiring only if `expect` is used in a loop * test(prefer-expect-assertions): add cases for each * chore(prefer-expect-assertions): add default value for `onlyFunctionsWithExpectInLoop` option * test(prefer-expect-assertions): adjust some cases * fix(prefer-expect-assertions): report expects in loops in functions in tests * test(prefer-expect-assertions): format cases a bit --- docs/rules/prefer-expect-assertions.md | 60 ++ .../prefer-expect-assertions.test.ts | 606 +++++++++++++++++- src/rules/prefer-expect-assertions.ts | 72 ++- 3 files changed, 722 insertions(+), 16 deletions(-) diff --git a/docs/rules/prefer-expect-assertions.md b/docs/rules/prefer-expect-assertions.md index b80b63dc3..6f338b862 100644 --- a/docs/rules/prefer-expect-assertions.md +++ b/docs/rules/prefer-expect-assertions.md @@ -58,6 +58,16 @@ test('my test', () => { ## Options +This rule can be configured to only check tests that match certain patterns that +typically look like `expect` calls might be missed, such as in promises or +loops. + +By default, none of these options are enabled meaning the rule checks _every_ +test for a call to either `expect.hasAssertions` or `expect.assertions`. If any +of the options are enabled the rule checks any test that matches _at least one_ +of the patterns represented by the enabled options (think "OR" rather than +"AND"). + #### `onlyFunctionsWithAsyncKeyword` When `true`, this rule will only warn for tests that use the `async` keyword. @@ -97,3 +107,53 @@ test('my test', async () => { expect(result).toBe('foo'); }); ``` + +#### `onlyFunctionsWithExpectInLoop` + +When `true`, this rule will only warn for tests that have `expect` calls within +a native loop. + +```json +{ + "rules": { + "jest/prefer-expect-assertions": [ + "warn", + { "onlyFunctionsWithAsyncKeyword": true } + ] + } +} +``` + +Examples of **incorrect** code when `'onlyFunctionsWithExpectInLoop'` is `true`: + +```js +describe('getNumbers', () => { + it('only returns numbers that are greater than zero', () => { + const numbers = getNumbers(); + + for (const number in numbers) { + expect(number).toBeGreaterThan(0); + } + }); +}); +``` + +Examples of **correct** code when `'onlyFunctionsWithExpectInLoop'` is `true`: + +```js +describe('getNumbers', () => { + it('only returns numbers that are greater than zero', () => { + expect.hasAssertions(); + + const numbers = getNumbers(); + + for (const number in numbers) { + expect(number).toBeGreaterThan(0); + } + }); + + it('returns more than one number', () => { + expect(getNumbers().length).toBeGreaterThan(1); + }); +}); +``` diff --git a/src/rules/__tests__/prefer-expect-assertions.test.ts b/src/rules/__tests__/prefer-expect-assertions.test.ts index f2f5a45bc..58ee5d179 100644 --- a/src/rules/__tests__/prefer-expect-assertions.test.ts +++ b/src/rules/__tests__/prefer-expect-assertions.test.ts @@ -21,18 +21,36 @@ ruleTester.run('prefer-expect-assertions', rule, { it("it1", function() { expect.assertions(1); expect(someValue).toBe(true) - }) + }); `, 'test("it1")', 'itHappensToStartWithIt("foo", function() {})', 'testSomething("bar", function() {})', 'it(async () => {expect.assertions(0);})', + dedent` + it("returns numbers that are greater than four", function() { + expect.assertions(2); + + for(let thing in things) { + expect(number).toBeGreaterThan(4); + } + }); + `, + dedent` + it("returns numbers that are greater than four", function() { + expect.hasAssertions(); + + for (let i = 0; i < things.length; i++) { + expect(number).toBeGreaterThan(4); + } + }); + `, { code: dedent` it("it1", async () => { expect.assertions(1); expect(someValue).toBe(true) - }) + }); `, options: [{ onlyFunctionsWithAsyncKeyword: true }], }, @@ -40,7 +58,7 @@ ruleTester.run('prefer-expect-assertions', rule, { code: dedent` it("it1", function() { expect(someValue).toBe(true) - }) + }); `, options: [{ onlyFunctionsWithAsyncKeyword: true }], }, @@ -48,6 +66,28 @@ ruleTester.run('prefer-expect-assertions', rule, { code: 'it("it1", () => {})', options: [{ onlyFunctionsWithAsyncKeyword: true }], }, + { + code: dedent` + it("returns numbers that are greater than four", async () => { + expect.assertions(2); + + for(let thing in things) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithAsyncKeyword: true }], + }, + { + code: dedent` + it("returns numbers that are greater than four", () => { + for(let thing in things) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithAsyncKeyword: true }], + }, ], invalid: [ { @@ -95,7 +135,7 @@ ruleTester.run('prefer-expect-assertions', rule, { it("it1", function() { someFunctionToDo(); someFunctionToDo2(); - }) + }); `, errors: [ { @@ -109,7 +149,7 @@ ruleTester.run('prefer-expect-assertions', rule, { it("it1", function() { expect.hasAssertions();someFunctionToDo(); someFunctionToDo2(); - }) + }); `, }, { @@ -118,7 +158,7 @@ ruleTester.run('prefer-expect-assertions', rule, { it("it1", function() { expect.assertions();someFunctionToDo(); someFunctionToDo2(); - }) + }); `, }, ], @@ -223,7 +263,7 @@ ruleTester.run('prefer-expect-assertions', rule, { someFunctionToDo(); someFunctionToDo2(); }); - }) + }); `, errors: [ { @@ -236,7 +276,7 @@ ruleTester.run('prefer-expect-assertions', rule, { output: dedent` it("it1", function() { expect.hasAssertions(); - }) + }); `, }, ], @@ -247,7 +287,64 @@ ruleTester.run('prefer-expect-assertions', rule, { code: dedent` it("it1", async function() { expect(someValue).toBe(true); - }) + }); + `, + options: [{ onlyFunctionsWithAsyncKeyword: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", async () => { + for(let thing in things) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithAsyncKeyword: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", async () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithAsyncKeyword: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", async () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("returns numbers that are greater than five", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(5); + } + }); `, options: [{ onlyFunctionsWithAsyncKeyword: true }], errors: [ @@ -261,6 +358,497 @@ ruleTester.run('prefer-expect-assertions', rule, { ], }); +ruleTester.run('prefer-expect-assertions (loops)', rule, { + valid: [ + { + code: dedent` + const expectNumbersToBeGreaterThan = (numbers, value) => { + for (let number of numbers) { + expect(number).toBeGreaterThan(value); + } + }; + + it('returns numbers that are greater than two', function () { + expectNumbersToBeGreaterThan(getNumbers(), 2); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + }, + { + code: dedent` + it("returns numbers that are greater than five", function () { + expect.assertions(2); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(5); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + }, + { + code: dedent` + it("returns things that are less than ten", function () { + expect.hasAssertions(); + + for (const thing in things) { + expect(thing).toBeLessThan(10); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + }, + ], + invalid: [ + { + code: dedent` + it('only returns numbers that are greater than six', () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(6); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('returns numbers that are greater than two', function () { + const expectNumbersToBeGreaterThan = (numbers, value) => { + for (let number of numbers) { + expect(number).toBeGreaterThan(value); + } + }; + + expectNumbersToBeGreaterThan(getNumbers(), 2); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("only returns numbers that are greater than seven", function () { + const numbers = getNumbers(); + + for (let i = 0; i < numbers.length; i++) { + expect(numbers[i]).toBeGreaterThan(7); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('has the number two', () => { + expect(number).toBe(2); + }); + + it('only returns numbers that are less than twenty', () => { + for (const number of getNumbers()) { + expect(number).toBeLessThan(20); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 5, + }, + ], + }, + { + code: dedent` + it("is wrong"); + + it("is a test", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 3, + }, + ], + }, + { + code: dedent` + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + + it("returns numbers that are greater than four", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("returns numbers that are greater than five", () => { + expect(number).toBeGreaterThan(5); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 5, + }, + ], + }, + { + code: dedent` + it.each([1, 2, 3])("returns numbers that are greater than four", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("is a number that is greater than four", () => { + expect.hasAssertions(); + + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("it1", () => { + expect.hasAssertions(); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + + it("it1", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it("returns numbers that are greater than four", async () => { + for (const number of await getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("returns numbers that are greater than five", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(5); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + { + messageId: 'haveExpectAssertions', + column: 1, + line: 7, + }, + ], + }, + { + code: dedent` + it("it1", async () => { + expect.hasAssertions(); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("it1", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it.skip.each\`\`("it1", async () => { + expect.hasAssertions(); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("it1", () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it("it1", async () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + + it("it1", () => { + expect.hasAssertions(); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(4); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + ], +}); + +ruleTester.run('prefer-expect-assertions (mixed)', rule, { + valid: [ + { + code: dedent` + it('only returns numbers that are greater than zero', async () => { + expect.hasAssertions(); + + for (const number of await getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + `, + options: [ + { + onlyFunctionsWithAsyncKeyword: true, + onlyFunctionsWithExpectInLoop: true, + }, + ], + }, + { + code: dedent` + it('only returns numbers that are greater than zero', async () => { + expect.assertions(2); + + for (const number of await getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + `, + options: [ + { + onlyFunctionsWithAsyncKeyword: true, + onlyFunctionsWithExpectInLoop: true, + }, + ], + }, + ], + invalid: [ + { + code: dedent` + it('only returns numbers that are greater than zero', () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + + it("is zero", () => { + expect.hasAssertions(); + + expect(0).toBe(0); + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('only returns numbers that are greater than zero', () => { + expect.hasAssertions(); + + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + + it('only returns numbers that are less than 100', () => { + for (const number of getNumbers()) { + expect(number).toBeLessThan(0); + } + }); + `, + options: [{ onlyFunctionsWithExpectInLoop: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it("to be true", async function() { + expect(someValue).toBe(true); + }); + `, + options: [ + { + onlyFunctionsWithAsyncKeyword: true, + onlyFunctionsWithExpectInLoop: true, + }, + ], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('only returns numbers that are greater than zero', async () => { + for (const number of getNumbers()) { + expect(number).toBeGreaterThan(0); + } + }); + `, + options: [ + { + onlyFunctionsWithAsyncKeyword: true, + onlyFunctionsWithExpectInLoop: true, + }, + ], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + ], +}); + ruleTester.run('.each support', rule, { valid: [ 'test.each()("is fine", () => { expect.assertions(0); })', diff --git a/src/rules/prefer-expect-assertions.ts b/src/rules/prefer-expect-assertions.ts index f3dd081fa..125f4f8dd 100644 --- a/src/rules/prefer-expect-assertions.ts +++ b/src/rules/prefer-expect-assertions.ts @@ -8,6 +8,7 @@ import { createRule, getAccessorValue, hasOnlyOneArgument, + isExpectCall, isFunction, isSupportedAccessor, isTestCaseCall, @@ -44,6 +45,7 @@ const suggestRemovingExtraArguments = ( interface RuleOptions { onlyFunctionsWithAsyncKeyword?: boolean; + onlyFunctionsWithExpectInLoop?: boolean; } type MessageIds = @@ -89,18 +91,69 @@ export default createRule<[RuleOptions], MessageIds>({ { type: 'object', properties: { - onlyFunctionsWithAsyncKeyword: { - type: 'boolean', - }, + onlyFunctionsWithAsyncKeyword: { type: 'boolean' }, + onlyFunctionsWithExpectInLoop: { type: 'boolean' }, }, additionalProperties: false, }, ], }, - defaultOptions: [{ onlyFunctionsWithAsyncKeyword: false }], + defaultOptions: [ + { + onlyFunctionsWithAsyncKeyword: false, + onlyFunctionsWithExpectInLoop: false, + }, + ], create(context, [options]) { + let hasExpectInLoop = false; + let inTestCaseCall = false; + let inForLoop = false; + + const shouldCheckFunction = (testFunction: TSESTree.FunctionLike) => { + if ( + !options.onlyFunctionsWithAsyncKeyword && + !options.onlyFunctionsWithExpectInLoop + ) { + return true; + } + + if (options.onlyFunctionsWithAsyncKeyword) { + if (testFunction.async) { + return true; + } + } + + if (options.onlyFunctionsWithExpectInLoop) { + if (hasExpectInLoop) { + return true; + } + } + + return false; + }; + + const enterForLoop = () => (inForLoop = true); + const exitForLoop = () => (inForLoop = false); + return { - CallExpression(node: TSESTree.CallExpression) { + ForStatement: enterForLoop, + 'ForStatement:exit': exitForLoop, + ForInStatement: enterForLoop, + 'ForInStatement:exit': exitForLoop, + ForOfStatement: enterForLoop, + 'ForOfStatement:exit': exitForLoop, + CallExpression(node) { + if (isTestCaseCall(node)) { + inTestCaseCall = true; + + return; + } + + if (isExpectCall(node) && inTestCaseCall && inForLoop) { + hasExpectInLoop = true; + } + }, + 'CallExpression:exit'(node: TSESTree.CallExpression) { if (!isTestCaseCall(node)) { return; } @@ -113,12 +166,17 @@ export default createRule<[RuleOptions], MessageIds>({ if ( !isFunction(testFn) || - testFn.body.type !== AST_NODE_TYPES.BlockStatement || - (options.onlyFunctionsWithAsyncKeyword && !testFn.async) + testFn.body.type !== AST_NODE_TYPES.BlockStatement ) { return; } + if (!shouldCheckFunction(testFn)) { + return; + } + + hasExpectInLoop = false; + const testFuncBody = testFn.body.body; if (!isFirstLineExprStmt(testFuncBody)) {