diff --git a/README.md b/README.md index 13625b3e..9621c3d3 100644 --- a/README.md +++ b/README.md @@ -260,6 +260,7 @@ export default defineConfig({ | [require-hook](docs/rules/require-hook.md) | require setup and teardown to be within a hook | | 🌐 | | | | | | | [require-local-test-context-for-concurrent-snapshots](docs/rules/require-local-test-context-for-concurrent-snapshots.md) | require local Test Context for concurrent snapshot tests | ✅ | 🌐 | | | | | | | [require-mock-type-parameters](docs/rules/require-mock-type-parameters.md) | enforce using type parameters with vitest mock functions | | 🌐 | | 🔧 | | | | +| [require-test-timeout](docs/rules/require-test-timeout.md) | require tests to declare a timeout | | | 🌐 | | | | | | [require-to-throw-message](docs/rules/require-to-throw-message.md) | require toThrow() to be called with an error message | | 🌐 | | | | | | | [require-top-level-describe](docs/rules/require-top-level-describe.md) | enforce that all tests are in a top-level describe | | 🌐 | | | | | | | [valid-describe-callback](docs/rules/valid-describe-callback.md) | enforce valid describe callback | ✅ | 🌐 | | | | | | diff --git a/docs/rules/require-test-timeout.md b/docs/rules/require-test-timeout.md new file mode 100644 index 00000000..f6614829 --- /dev/null +++ b/docs/rules/require-test-timeout.md @@ -0,0 +1,33 @@ +# vitest/require-test-timeout + +📝 Require tests to declare a timeout. + +🚫 This rule is _disabled_ in the 🌐 `all` config. + + + +This rule ensures tests explicitly declare a timeout so long-running tests don't hang silently. + +```ts +// bad +it('slow test', async () => { + await doSomethingSlow() +}) + +// good (numeric timeout) +test('slow test', async () => { + await doSomethingSlow() +}, 1000) + +// good (options object) +test('slow test', { timeout: 1000 }, async () => { + await doSomethingSlow() +}) + +// good (file-level) +vi.setConfig({ testTimeout: 1000 }) + +test('slow test', async () => { + await doSomethingSlow() +}) +``` diff --git a/src/index.ts b/src/index.ts index e4c8a5aa..c39eb721 100644 --- a/src/index.ts +++ b/src/index.ts @@ -100,6 +100,7 @@ const allRules = { 'valid-expect': 'warn', 'valid-title': 'warn', 'require-awaited-expect-poll': 'warn', + 'require-test-timeout': 'off', } as const satisfies RuleList const recommendedRules = { diff --git a/src/rules/expect-expect.ts b/src/rules/expect-expect.ts index 4e67e589..467b13bc 100644 --- a/src/rules/expect-expect.ts +++ b/src/rules/expect-expect.ts @@ -39,6 +39,12 @@ export default createEslintRule({ additionalProperties: false, }, ], + defaultOptions: [ + { + assertFunctionNames: ['expect', 'assert'], + additionalTestBlockFunctions: [], + }, + ], messages: { noAssertions: 'Test has no assertions', }, diff --git a/src/rules/index.ts b/src/rules/index.ts index 04afc505..0dc180c9 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -68,6 +68,7 @@ import preferTodo from './prefer-todo' import preferViMocked from './prefer-vi-mocked' import requireAwaitedExpectPoll from './require-awaited-expect-poll' import requireHook from './require-hook' +import requireTestTimeout from './require-test-timeout' import requireLocalTestContextForConcurrentSnapshots from './require-local-test-context-for-concurrent-snapshots' import requireMockTypeParameters from './require-mock-type-parameters' import requireToThrowMessage from './require-to-throw-message' @@ -152,6 +153,7 @@ export const rules = { 'prefer-vi-mocked': preferViMocked, 'require-awaited-expect-poll': requireAwaitedExpectPoll, 'require-hook': requireHook, + 'require-test-timeout': requireTestTimeout, 'require-local-test-context-for-concurrent-snapshots': requireLocalTestContextForConcurrentSnapshots, 'require-mock-type-parameters': requireMockTypeParameters, diff --git a/src/rules/no-restricted-matchers.ts b/src/rules/no-restricted-matchers.ts index 81ab23c2..b66b7689 100644 --- a/src/rules/no-restricted-matchers.ts +++ b/src/rules/no-restricted-matchers.ts @@ -32,6 +32,7 @@ export default createEslintRule({ }, }, ], + defaultOptions: [], messages: { restrictedChain: 'use of {{ restriction }} is disallowed', restrictedChainWithMessage: '{{ message }}', diff --git a/src/rules/no-standalone-expect.ts b/src/rules/no-standalone-expect.ts index db01efa6..6f31b430 100644 --- a/src/rules/no-standalone-expect.ts +++ b/src/rules/no-standalone-expect.ts @@ -69,6 +69,7 @@ export default createEslintRule({ additionalProperties: false, }, ], + defaultOptions: [], }, defaultOptions: [{ additionalTestBlockFunctions: [] }], create(context, [{ additionalTestBlockFunctions = [] }]) { diff --git a/src/rules/prefer-expect-assertions.ts b/src/rules/prefer-expect-assertions.ts index 2b42ebac..6d725cb2 100644 --- a/src/rules/prefer-expect-assertions.ts +++ b/src/rules/prefer-expect-assertions.ts @@ -90,6 +90,7 @@ export default createEslintRule({ additionalProperties: false, }, ], + defaultOptions: [], }, defaultOptions: [ { diff --git a/src/rules/prefer-snapshot-hint.ts b/src/rules/prefer-snapshot-hint.ts index 16e63fce..636402f1 100644 --- a/src/rules/prefer-snapshot-hint.ts +++ b/src/rules/prefer-snapshot-hint.ts @@ -49,6 +49,7 @@ export default createEslintRule({ enum: ['always', 'multi'], }, ], + defaultOptions: [], }, defaultOptions: ['multi'], create(context, [mode]) { diff --git a/src/rules/require-mock-type-parameters.ts b/src/rules/require-mock-type-parameters.ts index e3ce8104..83609cb3 100644 --- a/src/rules/require-mock-type-parameters.ts +++ b/src/rules/require-mock-type-parameters.ts @@ -30,6 +30,7 @@ export default createEslintRule({ additionalProperties: false, }, ], + defaultOptions: [], }, defaultOptions: [ { diff --git a/src/rules/require-test-timeout.ts b/src/rules/require-test-timeout.ts new file mode 100644 index 00000000..7b6f5dee --- /dev/null +++ b/src/rules/require-test-timeout.ts @@ -0,0 +1,152 @@ +import { createEslintRule, getAccessorValue } from '../utils' +import { parseVitestFnCall } from '../utils/parse-vitest-fn-call' +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils' +export const RULE_NAME = 'require-test-timeout' +export type MESSAGE_ID = 'missingTimeout' +export type Options = [] +export default createEslintRule({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: 'require tests to declare a timeout', + recommended: false, + }, + messages: { + missingTimeout: 'Test is missing a timeout. Add an explicit timeout.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + /** + * Track positions (character offsets) of vi.setConfig({ testTimeout }) + * calls so we only exempt tests that appear _after_ the call. We use the + * call's end offset and the test's start offset so that a setConfig on the + * same line but before the test still exempts it. + */ + const setConfigPositions: number[] = [] + + return { + CallExpression(node: TSESTree.CallExpression) { + const vitestFnCall = parseVitestFnCall(node, context) + + // detect vi.setConfig({ testTimeout: ... }) + if (vitestFnCall && vitestFnCall.type === 'vi') { + const firstMember = vitestFnCall.members[0] + + if (firstMember && getAccessorValue(firstMember) === 'setConfig') { + const arg = node.arguments[0] + + if (arg && arg.type === AST_NODE_TYPES.ObjectExpression) { + for (const prop of arg.properties) { + if (prop.type === AST_NODE_TYPES.Property) { + const key = prop.key + + // Only accept a numeric literal >= 0 for testTimeout + if ( + ((key.type === AST_NODE_TYPES.Identifier && + key.name === 'testTimeout') || + (key.type === AST_NODE_TYPES.Literal && + key.value === 'testTimeout')) && + prop.value.type === AST_NODE_TYPES.Literal && + typeof prop.value.value === 'number' && + prop.value.value >= 0 + ) { + const endOffset = node.range ? node.range[1] : 0 + + setConfigPositions.push(endOffset) + + break + } + } + } + } + } + } + + if (!vitestFnCall) return + if (vitestFnCall.type !== 'test' && vitestFnCall.type !== 'it') return + if ( + // skip todo/skip/xit/etc. + vitestFnCall.members.some((m) => { + const v = getAccessorValue(m) + return v === 'todo' || v === 'skip' + }) || + vitestFnCall.name.startsWith('x') + ) + return + + // check if test has a function callback + const args = node.arguments + const hasFunctionArg = args.some( + (a) => + a.type === AST_NODE_TYPES.FunctionExpression || + a.type === AST_NODE_TYPES.ArrowFunctionExpression, + ) + + if (!hasFunctionArg) return + + /** + * If there is a setConfig call *before* this test that sets testTimeout, + * exempt this test. Note: we compare source positions (character offsets) + * so that a later setConfig call does not affect earlier tests. + */ + const testPos = node.range ? node.range[0] : 0 + + if (setConfigPositions.some((p) => p <= testPos)) return + + let foundNumericTimeout = false + let foundObjectTimeout = false + + // numeric literal timeout as argument (third-arg style) + for (const a of args) { + if ( + a.type === AST_NODE_TYPES.Literal && + typeof a.value === 'number' + ) { + if (a.value >= 0) { + foundNumericTimeout = true + } else { + // negative numeric timeouts are explicitly invalid + context.report({ node, messageId: 'missingTimeout' }) + return + } + } + + // object literal with timeout property + if (a.type === AST_NODE_TYPES.ObjectExpression) { + for (const prop of a.properties) { + if (prop.type !== AST_NODE_TYPES.Property) continue + + const key = prop.key + + if ( + (key.type === AST_NODE_TYPES.Identifier && + key.name === 'timeout') || + (key.type === AST_NODE_TYPES.Literal && key.value === 'timeout') + ) { + if ( + prop.value.type === AST_NODE_TYPES.Literal && + typeof prop.value.value === 'number' && + prop.value.value >= 0 + ) { + foundObjectTimeout = true + } else { + // any explicitly provided non-numeric or negative timeout is invalid + context.report({ node, messageId: 'missingTimeout' }) + + return + } + } + } + } + } + + if (foundNumericTimeout || foundObjectTimeout) return + + context.report({ node, messageId: 'missingTimeout' }) + }, + } + }, +}) diff --git a/src/rules/valid-title.ts b/src/rules/valid-title.ts index 9cffb9bf..875a64a8 100644 --- a/src/rules/valid-title.ts +++ b/src/rules/valid-title.ts @@ -182,6 +182,7 @@ export default createEslintRule({ additionalProperties: false, }, ], + defaultOptions: [], fixable: 'code', }, defaultOptions: [ diff --git a/tests/require-test-timeout.test.ts b/tests/require-test-timeout.test.ts new file mode 100644 index 00000000..6ffda232 --- /dev/null +++ b/tests/require-test-timeout.test.ts @@ -0,0 +1,113 @@ +import rule, { RULE_NAME } from '../src/rules/require-test-timeout' +import { ruleTester } from './ruleTester' + +ruleTester.run(RULE_NAME, rule, { + valid: [ + 'test.todo("a")', + 'xit("a", () => {})', + 'test("a", () => {}, 0)', + 'it("a", () => {}, 500)', + 'it.skip("a", () => {})', + 'test.skip("a", () => {})', + 'test("a", () => {}, 1000)', + 'it.only("a", () => {}, 1234)', + 'test.only("a", () => {}, 1234)', + 'it.concurrent("a", () => {}, 400)', + 'test("a", () => {}, { timeout: 0 })', + 'test.concurrent("a", () => {}, 400)', + 'test("a", () => {}, { timeout: 500 })', + 'test("a", { timeout: 500 }, () => {})', + 'vi.setConfig({ testTimeout: 1000 }); test("a", () => {})', + // multiple object args where one contains timeout + 'test("a", { foo: 1 }, { timeout: 500 }, () => {})', + // both object and numeric timeout present + 'test("a", { timeout: 500 }, 1000, () => {})', + 'test("a", () => {}, 1000, { extra: true })', + ], + invalid: [ + { + code: 'test("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'it("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test.only("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test.concurrent("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'it.concurrent("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'vi.setConfig({}); test("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'const t = 500; test("a", { timeout: t }, () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + // null/undefined/identifier/negative cases + { + code: 'test("a", () => {}, { timeout: null })', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, { timeout: undefined })', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, -100)', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, { timeout: -1 })', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'vi.setConfig({ testTimeout: null }); test("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'vi.setConfig({ testTimeout: undefined }); test("a", () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + // setConfig after test should NOT exempt the earlier test + code: 'test("a", () => {}); vi.setConfig({ testTimeout: 1000 })', + errors: [{ messageId: 'missingTimeout' }], + }, + // spread/complex object cases (spread should not be treated as literal timeout) + { + code: 'const opts = { timeout: 1000 }; test("a", { ...opts }, () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'const opts = { timeout: 1000 }; test("a", { ...opts }, { foo: 1 }, () => {})', + errors: [{ messageId: 'missingTimeout' }], + }, + // mixed valid/invalid timeout specs: any explicit invalid timeout should fail + { + code: 'test("a", () => {}, { timeout: -1 }, { timeout: 500 })', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, { timeout: 500 }, { timeout: -1 })', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, { timeout: -1 }, 1000)', + errors: [{ messageId: 'missingTimeout' }], + }, + { + code: 'test("a", () => {}, 1000, { timeout: -1 })', + errors: [{ messageId: 'missingTimeout' }], + }, + ], +})