Add require-top-level-describe rule

README.md

@@ -49,18 +49,19 @@ command line option.\
 💡: Some problems reported by this rule are manually fixable by editor
 [suggestions](https://eslint.org/docs/latest/developer-guide/working-with-rules#providing-suggestions).
 
-|  ✔  | 🔧  | 💡  | Rule                                                                                                                                          | Description                                                     |
-| :-: | :-: | :-: | --------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- |
-|  ✔  |     |     | [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md)           | Enforces a maximum depth to nested describe calls               |
-|  ✔  | 🔧  |     | [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md) | Enforce Playwright APIs to be awaited                           |
-|  ✔  |     |     | [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md)     | Disallow conditional logic in tests                             |
-|  ✔  |     | 💡  | [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md)               | Disallow usage of element handles                               |
-|  ✔  |     |     | [no-eval](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-eval.md)                                   | Disallow usage of `page.$eval` and `page.$$eval`                |
-|  ✔  |     | 💡  | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md)                   | Disallow usage of `.only` annotation                            |
-|  ✔  |     |     | [no-force-option](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-force-option.md)                   | Disallow usage of the `{ force: true }` option                  |
-|  ✔  |     |     | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md)                       | Disallow using `page.pause`                                     |
-|  ✔  |     | 💡  | [no-skipped-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-skipped-test.md)                   | Disallow usage of the `.skip` annotation                        |
-|  ✔  |     |     | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md)                     | Disallow usage of `not` matchers when a specific matcher exists |
-|  ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)           | Disallow usage of `page.waitForTimeout`                         |
-|     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)     | Enforce lowercase test names                                    |
-|  ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                         | Enforce valid `expect()` usage                                  |
+|  ✔  | 🔧  | 💡  | Rule                                                                                                                                              | Description                                                       |
+| :-: | :-: | :-: | ------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- |
+|  ✔  |     |     | [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md)               | Enforces a maximum depth to nested describe calls                 |
+|  ✔  | 🔧  |     | [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md)     | Enforce Playwright APIs to be awaited                             |
+|  ✔  |     |     | [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md)         | Disallow conditional logic in tests                               |
+|  ✔  |     | 💡  | [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md)                   | Disallow usage of element handles                                 |
+|  ✔  |     |     | [no-eval](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-eval.md)                                       | Disallow usage of `page.$eval` and `page.$$eval`                  |
+|  ✔  |     | 💡  | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md)                       | Disallow usage of `.only` annotation                              |
+|  ✔  |     |     | [no-force-option](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-force-option.md)                       | Disallow usage of the `{ force: true }` option                    |
+|  ✔  |     |     | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md)                           | Disallow using `page.pause`                                       |
+|  ✔  |     | 💡  | [no-skipped-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-skipped-test.md)                       | Disallow usage of the `.skip` annotation                          |
+|  ✔  | 🔧  |     | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md)                         | Disallow usage of `not` matchers when a specific matcher exists   |
+|  ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)               | Disallow usage of `page.waitForTimeout`                           |
+|     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)         | Enforce lowercase test names                                      |
+|     |     |     | [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-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                             | Enforce valid `expect()` usage                                    |

docs/rules/require-top-level-describe.md

@@ -0,0 +1,74 @@
+# Require test cases and hooks to be inside a `test.describe` block (`require-top-level-describe`)
+
+Playwright allows you to organise your test files the way you want it. However,
+the more your codebase grows, the more it becomes hard to navigate in your test
+files. This rule makes sure you provide at least a top-level `describe` block in
+your test file.
+
+## Rule Details
+
+This rule triggers a warning if a test case (`test`) or a hook
+(`test.beforeAll`, `test.beforeEach`, `test.afterEach`, `test.afterAll`) is not
+located in a top-level `test.describe` block.
+
+The following patterns are considered warnings:
+
+```javascript
+// Above a describe block
+test('my test', () => {});
+test.describe('test suite', () => {
+  test('test', () => {});
+});
+
+// Below a describe block
+test.describe('test suite', () => {});
+test('my test', () => {});
+
+// Same for hooks
+test.beforeAll('my beforeAll', () => {});
+test.describe('test suite', () => {});
+test.afterEach('my afterEach', () => {});
+```
+
+The following patterns are **not** considered warnings:
+
+```javascript
+// In a describe block
+test.describe('test suite', () => {
+  test('my test', () => {});
+});
+
+// In a nested describe block
+test.describe('test suite', () => {
+  test('my test', () => {});
+
+  test.describe('another test suite', () => {
+    test('my other test', () => {});
+  });
+});
+```
+
+You can also enforce a limit on the number of describes allowed at the top-level
+using the `maxTopLevelDescribes` option:
+
+```json
+{
+  "playwright/require-top-level-describe": [
+    "error",
+    { "maxTopLevelDescribes": 2 }
+  ]
+}
+```
+
+Examples of **incorrect** code with the above config:
+
+```js
+test.describe('test suite', () => {
+  test('test', () => {});
+});
+
+test.describe('test suite', () => {});
+test.describe('test suite', () => {});
+```
+
+This option defaults to `Infinity`, allowing any number of top-level describes.

src/index.ts

@@ -11,6 +11,7 @@ import noConditionalInTest from './rules/no-conditional-in-test';
 import noUselessNot from './rules/no-useless-not';
 import validExpect from './rules/valid-expect';
 import preferLowercaseTitle from './rules/prefer-lowercase-title';
+import requireTopLevelDescribe from './rules/require-top-level-describe';
 
 export = {
   configs: {
@@ -82,5 +83,6 @@ export = {
     'no-useless-not': noUselessNot,
     'valid-expect': validExpect,
     'prefer-lowercase-title': preferLowercaseTitle,
+    'require-top-level-describe': requireTopLevelDescribe,
   },
 };

src/rules/max-nested-describe.ts

@@ -48,7 +48,7 @@ export default {
     docs: {
       category: 'Best Practices',
       description: 'Enforces a maximum depth to nested describe calls',
-      recommended: false,
+      recommended: true,
       url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md',
     },
     messages: {

src/rules/no-conditional-in-test.ts

@@ -22,7 +22,7 @@ export default {
     docs: {
       category: 'Best Practices',
       description: 'Disallow conditional logic in tests',
-      recommended: false,
+      recommended: true,
       url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md',
     },
     messages: {

src/rules/prefer-lowercase-title.ts

@@ -95,7 +95,7 @@ export default {
     docs: {
       category: 'Best Practices',
       description: 'Enforce lowercase test names',
-      recommended: true,
+      recommended: false,
       url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md',
     },
     messages: {

src/rules/require-top-level-describe.ts

@@ -0,0 +1,75 @@
+import { Rule } from 'eslint';
+import { getAmountData } from '../utils/misc';
+import * as ESTree from 'estree';
+import { isDescribeCall, isTest, isTestHook } from '../utils/ast';
+
+export default {
+  create(context) {
+    const { maxTopLevelDescribes } = {
+      maxTopLevelDescribes: Infinity,
+      ...((context.options?.[0] as {}) ?? {}),
+    };
+
+    let topLevelDescribeCount = 0;
+    let describeCount = 0;
+
+    return {
+      CallExpression(node) {
+        if (isDescribeCall(node)) {
+          describeCount++;
+
+          if (describeCount === 1) {
+            topLevelDescribeCount++;
+
+            if (topLevelDescribeCount > maxTopLevelDescribes) {
+              context.report({
+                node,
+                messageId: 'tooManyDescribes',
+                data: getAmountData(maxTopLevelDescribes),
+              });
+            }
+          }
+        } else if (!describeCount) {
+          if (isTest(node)) {
+            context.report({ node, messageId: 'unexpectedTest' });
+          } else if (isTestHook(node)) {
+            context.report({ node, messageId: 'unexpectedHook' });
+          }
+        }
+      },
+      'CallExpression:exit'(node: ESTree.CallExpression) {
+        if (isDescribeCall(node)) {
+          describeCount--;
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description:
+        'Require test cases and hooks to be inside a `test.describe` block',
+      recommended: false,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md',
+    },
+    messages: {
+      tooManyDescribes:
+        'There should not be more than {{max}} describe{{s}} at the top level',
+      unexpectedTest: 'All test cases must be wrapped in a describe block.',
+      unexpectedHook: 'All hooks must be wrapped in a describe block.',
+    },
+    type: 'suggestion',
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          maxTopLevelDescribes: {
+            type: 'number',
+            minimum: 1,
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
+  },
+} as Rule.RuleModule;

src/rules/valid-expect.ts

@@ -2,6 +2,7 @@ import { Rule } from 'eslint';
 import { isExpectCall, isIdentifier } from '../utils/ast';
 import { NodeWithParent } from '../utils/types';
 import * as ESTree from 'estree';
+import { getAmountData } from '../utils/misc';
 
 function isMatcherFound(node: NodeWithParent) {
   if (node.parent.type !== 'MemberExpression') {
@@ -39,11 +40,6 @@ function isMatcherCalled(node: NodeWithParent): {
   return isMatcherCalled(node.parent);
 }
 
-const getAmountData = (amount: number) => ({
-  amount: amount.toString(),
-  s: amount === 1 ? '' : 's',
-});
-
 export default {
   create(context) {
     const options = {

src/utils/ast.ts

@@ -121,6 +121,16 @@ export function isTest(node: ESTree.CallExpression) {
   );
 }
 
+const testHooks = new Set(['afterAll', 'afterEach', 'beforeAll', 'beforeEach']);
+export function isTestHook(node: ESTree.CallExpression) {
+  return (
+    node.callee.type === 'MemberExpression' &&
+    isIdentifier(node.callee.object, 'test') &&
+    node.callee.property.type === 'Identifier' &&
+    testHooks.has(node.callee.property.name)
+  );
+}
+
 const expectSubCommands = new Set(['soft', 'poll']);
 export function isExpectCall(node: ESTree.CallExpression) {
   return (

src/utils/misc.ts

@@ -0,0 +1,4 @@
+export const getAmountData = (amount: number) => ({
+  amount: amount.toString(),
+  s: amount === 1 ? '' : 's',
+});

test/spec/require-top-level-describe.spec.ts

@@ -0,0 +1,127 @@
+import rule from '../../src/rules/require-top-level-describe';
+import * as dedent from 'dedent';
+import { runRuleTester } from '../utils/rule-tester';
+
+const invalid = (code: string, messageId: string) => ({
+  code,
+  errors: [{ messageId }],
+});
+
+runRuleTester('require-top-level-describe', rule, {
+  valid: [
+    'foo()',
+    'test.info()',
+    'test.describe("suite", () => { test("foo") });',
+    'test.describe.only("suite", () => { test.beforeAll("my beforeAll") });',
+    'test.describe.parallel("suite", () => { test.beforeEach("my beforeAll") });',
+    'test.describe.serial("suite", () => { test.afterAll("my afterAll") });',
+    'test.describe.parallel.fixme("suite", () => { test.afterEach("my afterEach") });',
+    dedent`
+      test.describe("suite", () => {
+        test.beforeEach("a", () => {});
+        test.describe("b", () => {});
+        test("c", () => {})
+      });
+    `,
+    dedent`
+      test.describe("suite", () => {
+        test("foo", () => {})
+        test.describe("another suite", () => {});
+        test("my other test", () => {})
+      });
+    `,
+    dedent`
+      test.describe('one', () => {});
+      test.describe('two', () => {});
+      test.describe('three', () => {});
+    `,
+    {
+      code: dedent`
+        test.describe('one', () => {
+          test.describe('two', () => {});
+          test.describe('three', () => {});
+        });
+      `,
+      options: [{ maxTopLevelDescribes: 1 }],
+    },
+  ],
+  invalid: [
+    // Top level hooks
+    invalid('test.beforeAll(() => {})', 'unexpectedHook'),
+    invalid('test.beforeEach(() => {})', 'unexpectedHook'),
+    invalid('test.afterAll(() => {})', 'unexpectedHook'),
+    invalid('test.afterEach(() => {})', 'unexpectedHook'),
+    {
+      code: dedent`
+        test.describe("suite", () => {});
+        test.afterAll(() => {})
+      `,
+      errors: [{ messageId: 'unexpectedHook' }],
+    },
+    // Top level tests
+    invalid('test("foo", () => {})', 'unexpectedTest'),
+    invalid('test.skip("foo", () => {})', 'unexpectedTest'),
+    invalid('test.fixme("foo", () => {})', 'unexpectedTest'),
+    invalid('test.only("foo", () => {})', 'unexpectedTest'),
+    {
+      code: dedent`
+        test("foo", () => {})
+        test.describe("suite", () => {});
+      `,
+      errors: [{ messageId: 'unexpectedTest' }],
+    },
+    {
+      code: dedent`
+        test("foo", () => {})
+        test.describe("suite", () => {
+          test("bar", () => {})
+        });
+      `,
+      errors: [{ messageId: 'unexpectedTest' }],
+    },
+    // Too many describes
+    {
+      code: dedent`
+        test.describe('one', () => {});
+        test.describe.only('two', () => {});
+        test.describe.parallel('three', () => {});
+      `,
+      options: [{ maxTopLevelDescribes: 2 }],
+      errors: [{ messageId: 'tooManyDescribes', line: 3 }],
+    },
+    {
+      code: dedent`
+        test.describe('one', () => {
+          test.describe('one (nested)', () => {});
+          test.describe('two (nested)', () => {});
+        });
+
+        test.describe.only('two', () => {
+          test.describe('one (nested)', () => {});
+          test.describe.serial.only('two (nested)', () => {});
+          test.describe.fixme('three (nested)', () => {});
+        });
+
+        test.describe.fixme('three', () => {
+          test.describe('one (nested)', () => {});
+          test.describe.serial('two (nested)', () => {});
+          test.describe.parallel('three (nested)', () => {});
+        });
+      `,
+      options: [{ maxTopLevelDescribes: 2 }],
+      errors: [{ messageId: 'tooManyDescribes', line: 12 }],
+    },
+    {
+      code: dedent`
+        test.describe('one', () => {});
+        test.describe.fixme.only('two', () => {});
+        test.describe.fixme('three', () => {});
+      `,
+      options: [{ maxTopLevelDescribes: 1 }],
+      errors: [
+        { messageId: 'tooManyDescribes', line: 2 },
+        { messageId: 'tooManyDescribes', line: 3 },
+      ],
+    },
+  ],
+});