From 8d5fd33eed633f0c0bbdcb9e86bd2d8d7de79c4b Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 16 Jan 2022 09:28:52 +1300 Subject: [PATCH] feat(prefer-expect-assertions): support requiring only if `expect` is used in a callback (#1028) --- docs/rules/prefer-expect-assertions.md | 66 +++ .../prefer-expect-assertions.test.ts | 451 ++++++++++++++++++ src/rules/prefer-expect-assertions.ts | 31 +- 3 files changed, 545 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-expect-assertions.md b/docs/rules/prefer-expect-assertions.md index 6f338b862..144b7b290 100644 --- a/docs/rules/prefer-expect-assertions.md +++ b/docs/rules/prefer-expect-assertions.md @@ -157,3 +157,69 @@ describe('getNumbers', () => { }); }); ``` + +#### `onlyFunctionsWithExpectInCallback` + +When `true`, this rule will only warn for tests that have `expect` calls within +a callback. + +```json +{ + "rules": { + "jest/prefer-expect-assertions": [ + "warn", + { "onlyFunctionsWithExpectInCallback": true } + ] + } +} +``` + +Examples of **incorrect** code when `'onlyFunctionsWithExpectInCallback'` is +`true`: + +```js +describe('getNumbers', () => { + it('only returns numbers that are greater than zero', () => { + const numbers = getNumbers(); + + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(0); + }); + }); +}); + +describe('/users', () => { + it.each([1, 2, 3])('returns ok', id => { + client.get(`/users/${id}`, response => { + expect(response.status).toBe(200); + }); + }); +}); +``` + +Examples of **correct** code when `'onlyFunctionsWithExpectInCallback'` is +`true`: + +```js +describe('getNumbers', () => { + it('only returns numbers that are greater than zero', () => { + expect.hasAssertions(); + + const numbers = getNumbers(); + + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(0); + }); + }); +}); + +describe('/users', () => { + it.each([1, 2, 3])('returns ok', id => { + expect.assertions(3); + + client.get(`/users/${id}`, response => { + expect(response.status).toBe(200); + }); + }); +}); +``` diff --git a/src/rules/__tests__/prefer-expect-assertions.test.ts b/src/rules/__tests__/prefer-expect-assertions.test.ts index 58ee5d179..e0860c427 100644 --- a/src/rules/__tests__/prefer-expect-assertions.test.ts +++ b/src/rules/__tests__/prefer-expect-assertions.test.ts @@ -718,6 +718,457 @@ ruleTester.run('prefer-expect-assertions (loops)', rule, { ], }); +ruleTester.run('prefer-expect-assertions (callbacks)', rule, { + valid: [ + { + code: dedent` + const expectNumbersToBeGreaterThan = (numbers, value) => { + numbers.forEach(number => { + expect(number).toBeGreaterThan(value); + }); + }; + + it('returns numbers that are greater than two', function () { + expectNumbersToBeGreaterThan(getNumbers(), 2); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it('returns numbers that are greater than two', function () { + expect.assertions(2); + + const expectNumbersToBeGreaterThan = (numbers, value) => { + for (let number of numbers) { + expect(number).toBeGreaterThan(value); + } + }; + + expectNumbersToBeGreaterThan(getNumbers(), 2); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it("returns numbers that are greater than five", function () { + expect.assertions(2); + + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(5); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it("returns things that are less than ten", function () { + expect.hasAssertions(); + + things.forEach(thing => { + expect(thing).toBeLessThan(10); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it('sends the data as a string', () => { + expect.hasAssertions(); + + const stream = openStream(); + + stream.on('data', data => { + expect(data).toBe(expect.any(String)); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it('responds ok', function () { + expect.assertions(1); + + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + { + code: dedent` + it.each([1, 2, 3])("returns ok", id => { + expect.assertions(3); + + client.get(\`/users/$\{id}\`, response => { + expect(response.status).toBe(200); + }); + }); + + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + }, + ], + invalid: [ + { + code: dedent` + it('sends the data as a string', () => { + const stream = openStream(); + + stream.on('data', data => { + expect(data).toBe(expect.any(String)); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + client.get('/user', response => { + expect.assertions(1); + + expect(response.status).toBe(200); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + const expectOkResponse = response => { + expect.assertions(1); + + expect(response.status).toBe(200); + }; + + client.get('/user', expectOkResponse); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('returns numbers that are greater than two', function () { + const expectNumberToBeGreaterThan = (number, value) => { + expect(number).toBeGreaterThan(value); + }; + + expectNumberToBeGreaterThan(1, 2); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: 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: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('only returns numbers that are greater than six', () => { + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(6); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("is wrong"); + + it('responds ok', function () { + const expectOkResponse = response => { + expect.assertions(1); + + expect(response.status).toBe(200); + }; + + client.get('/user', expectOkResponse); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 3, + }, + ], + }, + { + code: dedent` + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + + it('responds ok', function () { + const expectOkResponse = response => { + expect(response.status).toBe(200); + }; + + client.get('/user', expectOkResponse); + }); + + it("returns numbers that are greater than five", () => { + expect(number).toBeGreaterThan(5); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 5, + }, + ], + }, + { + code: dedent` + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + + it("returns numbers that are greater than four", () => { + getNumbers().map(number => { + expect(number).toBeGreaterThan(0); + }); + }); + + it("returns numbers that are greater than five", () => { + expect(number).toBeGreaterThan(5); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 5, + }, + ], + }, + { + code: dedent` + it.each([1, 2, 3])("returns ok", id => { + client.get(\`/users/$\{id}\`, response => { + expect(response.status).toBe(200); + }); + }); + + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + + it("is a number that is greater than four", () => { + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + + it("is a number that is greater than four", () => { + expect.hasAssertions(); + + expect(number).toBeGreaterThan(4); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 1, + }, + ], + }, + { + code: dedent` + it("it1", () => { + expect.hasAssertions(); + + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(0); + }); + }); + + it("it1", () => { + getNumbers().forEach(number => { + expect(number).toBeGreaterThan(0); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it('responds ok', function () { + expect.hasAssertions(); + + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + + it('responds not found', function () { + client.get('/user', response => { + expect(response.status).toBe(404); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + { + code: dedent` + it.skip.each\`\`("it1", async () => { + expect.hasAssertions(); + + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + + it("responds ok", () => { + client.get('/user', response => { + expect(response.status).toBe(200); + }); + }); + `, + options: [{ onlyFunctionsWithExpectInCallback: true }], + errors: [ + { + messageId: 'haveExpectAssertions', + column: 1, + line: 9, + }, + ], + }, + ], +}); + ruleTester.run('prefer-expect-assertions (mixed)', rule, { valid: [ { diff --git a/src/rules/prefer-expect-assertions.ts b/src/rules/prefer-expect-assertions.ts index 125f4f8dd..c96c96612 100644 --- a/src/rules/prefer-expect-assertions.ts +++ b/src/rules/prefer-expect-assertions.ts @@ -46,6 +46,7 @@ const suggestRemovingExtraArguments = ( interface RuleOptions { onlyFunctionsWithAsyncKeyword?: boolean; onlyFunctionsWithExpectInLoop?: boolean; + onlyFunctionsWithExpectInCallback?: boolean; } type MessageIds = @@ -93,6 +94,7 @@ export default createRule<[RuleOptions], MessageIds>({ properties: { onlyFunctionsWithAsyncKeyword: { type: 'boolean' }, onlyFunctionsWithExpectInLoop: { type: 'boolean' }, + onlyFunctionsWithExpectInCallback: { type: 'boolean' }, }, additionalProperties: false, }, @@ -102,9 +104,12 @@ export default createRule<[RuleOptions], MessageIds>({ { onlyFunctionsWithAsyncKeyword: false, onlyFunctionsWithExpectInLoop: false, + onlyFunctionsWithExpectInCallback: false, }, ], create(context, [options]) { + let expressionDepth = 0; + let hasExpectInCallback = false; let hasExpectInLoop = false; let inTestCaseCall = false; let inForLoop = false; @@ -112,7 +117,8 @@ export default createRule<[RuleOptions], MessageIds>({ const shouldCheckFunction = (testFunction: TSESTree.FunctionLike) => { if ( !options.onlyFunctionsWithAsyncKeyword && - !options.onlyFunctionsWithExpectInLoop + !options.onlyFunctionsWithExpectInLoop && + !options.onlyFunctionsWithExpectInCallback ) { return true; } @@ -129,13 +135,25 @@ export default createRule<[RuleOptions], MessageIds>({ } } + if (options.onlyFunctionsWithExpectInCallback) { + if (hasExpectInCallback) { + return true; + } + } + return false; }; + const enterExpression = () => inTestCaseCall && expressionDepth++; + const exitExpression = () => inTestCaseCall && expressionDepth--; const enterForLoop = () => (inForLoop = true); const exitForLoop = () => (inForLoop = false); return { + FunctionExpression: enterExpression, + 'FunctionExpression:exit': exitExpression, + ArrowFunctionExpression: enterExpression, + 'ArrowFunctionExpression:exit': exitExpression, ForStatement: enterForLoop, 'ForStatement:exit': exitForLoop, ForInStatement: enterForLoop, @@ -149,8 +167,14 @@ export default createRule<[RuleOptions], MessageIds>({ return; } - if (isExpectCall(node) && inTestCaseCall && inForLoop) { - hasExpectInLoop = true; + if (isExpectCall(node) && inTestCaseCall) { + if (inForLoop) { + hasExpectInLoop = true; + } + + if (expressionDepth > 1) { + hasExpectInCallback = true; + } } }, 'CallExpression:exit'(node: TSESTree.CallExpression) { @@ -176,6 +200,7 @@ export default createRule<[RuleOptions], MessageIds>({ } hasExpectInLoop = false; + hasExpectInCallback = false; const testFuncBody = testFn.body.body;