Skip to content

feat: Add require-to-pass-timeout rule (#345) #346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ CLI option\
| [prefer-web-first-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-web-first-assertions.md) | Suggest using web first assertions | ✅ | 🔧 | |
| [require-hook](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | |
| [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md) | Require assertions to use `expect.soft()` | | 🔧 | |
| [require-to-pass-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-to-pass-timeout.md) | Require a timeout for `toPass()` | | | |
| [require-to-throw-message](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | |
| [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `test.describe` block | | | |
| [valid-describe-callback](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | ✅ | | |
Expand Down
31 changes: 31 additions & 0 deletions docs/rules/require-to-pass-timeout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Require a timeout for `toPass()` (`require-to-pass-timeout`)

`toPass()` is used to retry blocks of code until they pass successfully, such as
in `await expect(async () => { ... }).toPass()`. However, if no timeout is
defined, the test may run indefinitely until the test timeout. Requiring a
timeout ensures that the test will fail early if it does not pass within the
specified time.

## Rule details

This rule triggers a warning if `toPass()` is used without a timeout.

The following patterns are considered warnings:

```js
await expect(async () => {
const response = await page.request.get('https://api.example.com')
expect(response.status()).toBe(200)
}).toPass()
```

The following patterns are not considered warnings:

```js
await expect(async () => {
const response = await page.request.get('https://api.example.com')
expect(response.status()).toBe(200)
}).toPass({
timeout: 60000,
})
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import preferToHaveLength from './rules/prefer-to-have-length.js'
import preferWebFirstAssertions from './rules/prefer-web-first-assertions.js'
import requireHook from './rules/require-hook.js'
import requireSoftAssertions from './rules/require-soft-assertions.js'
import requireToPassTimeout from './rules/require-to-pass-timeout.js'
import requireToThrowMessage from './rules/require-to-throw-message.js'
import requireTopLevelDescribe from './rules/require-top-level-describe.js'
import validDescribeCallback from './rules/valid-describe-callback.js'
Expand Down Expand Up @@ -95,6 +96,7 @@ const index = {
'prefer-web-first-assertions': preferWebFirstAssertions,
'require-hook': requireHook,
'require-soft-assertions': requireSoftAssertions,
'require-to-pass-timeout': requireToPassTimeout,
'require-to-throw-message': requireToThrowMessage,
'require-top-level-describe': requireTopLevelDescribe,
'valid-describe-callback': validDescribeCallback,
Expand Down
39 changes: 39 additions & 0 deletions src/rules/require-to-pass-timeout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { runRuleTester } from '../utils/rule-tester.js'
import rule from './require-to-pass-timeout.js'

runRuleTester('require-to-pass-timeout', rule, {
invalid: [
// toPass without timeout
{
code: "await expect(async () => { const response = await page.request.get('https://api.example.com'); expect(response.status()).toBe(200); }).toPass();",
errors: [
{
column: 136,
line: 1,
messageId: 'addTimeoutOption',
},
],
},
// toPass with empty object
{
code: "await expect(async () => { const response = await page.request.get('https://api.example.com'); expect(response.status()).toBe(200); }).toPass({});",
errors: [
{
column: 136,
line: 1,
messageId: 'addTimeoutOption',
},
],
},
],
valid: [
// toPass with timeout
{
code: "await expect(async () => { const response = await page.request.get('https://api.example.com'); expect(response.status()).toBe(200); }).toPass({ timeout: 60000 });",
},
// toPass with other options including timeout
{
code: "await expect(async () => { const response = await page.request.get('https://api.example.com'); expect(response.status()).toBe(200); }).toPass({ intervals: [1000, 2000], timeout: 60000 });",
},
],
})
48 changes: 48 additions & 0 deletions src/rules/require-to-pass-timeout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { createRule } from '../utils/createRule.js'
import { parseFnCall } from '../utils/parseFnCall.js'

export default createRule({
create(context) {
return {
CallExpression(node) {
const call = parseFnCall(context, node)
if (call?.type !== 'expect') return

if (call.matcherName !== 'toPass') return

const objectArg = call.matcherArgs.find(
(arg) => arg.type === 'ObjectExpression',
)

if (
!objectArg?.properties.find(
(prop) =>
prop.type === 'Property' &&
prop.key.type === 'Identifier' &&
prop.key.name === 'timeout',
)
) {
// Look for `toPass` calls without `timeout` in the arguments.
context.report({
data: { matcherName: call.matcherName },
messageId: 'addTimeoutOption',
node: call.matcher,
})
}
},
}
},
meta: {
docs: {
category: 'Best Practices',
description: 'Require a timeout option for `toPass()`',
recommended: false,
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-to-pass-timeout.md',
},
messages: {
addTimeoutOption: 'Add a timeout option to {{ matcherName }}()',
},
schema: [],
type: 'suggestion',
},
})