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

Author: gustavo-meilus

README.md

@@ -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                                | ✅  |     |     |

docs/rules/require-to-pass-timeout.md

@@ -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,
+})
+```

src/index.ts

@@ -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'
@@ -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,

src/rules/require-to-pass-timeout.test.ts

@@ -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 });",
+    },
+  ],
+})

src/rules/require-to-pass-timeout.ts

@@ -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',
+  },
+})