diff --git a/README.md b/README.md index b738164..0b3c812 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,7 @@ CLI option\ | [no-raw-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-raw-locators.md) | Disallow using raw locators | | | | | [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | | [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-slowed-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-slowed-test.md) | Disallow usage of the `.slow` annotation | ✅ | | 💡 | | [no-standalone-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-standalone-expect.md) | Disallow using expect outside of `test` blocks | ✅ | | | | [no-unsafe-references](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md) | Prevent unsafe variable references in `page.evaluate()` | ✅ | 🔧 | | | [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods | ✅ | 🔧 | | diff --git a/docs/rules/no-slowed-test.md b/docs/rules/no-slowed-test.md new file mode 100644 index 0000000..d872b4b --- /dev/null +++ b/docs/rules/no-slowed-test.md @@ -0,0 +1,56 @@ +# Disallow usage of the `.slow` annotation (`no-slowed-test`) + +## Rule Details + +Examples of **incorrect** code for this rule: + +```javascript +test.slow('slow this test', async ({ page }) => {}) + +test.describe('slow test inside describe', () => { + test.slow() +}) + +test.describe('slow test conditionally', async ({ browserName }) => { + test.slow(browserName === 'firefox', 'Working on it') +}) +``` + +Examples of **correct** code for this rule: + +```javascript +test('this test', async ({ page }) => {}) + +test.describe('two tests', () => { + test('one', async ({ page }) => {}) + test('two', async ({ page }) => {}) +}) +``` + +## Options + +```json +{ + "playwright/no-slowed-test": [ + "error", + { + "allowConditional": false + } + ] +} +``` + +### `allowConditional` + +Setting this option to `true` will allow using `test.slow()` to conditionally +mark a test as slow. This can be helpful if you want to prevent usage of +`test.slow` being added by mistake but still allow slow tests based on +browser/environment setup. + +Example of **correct** code for the `{ "allowConditional": true }` option: + +```javascript +test('foo', ({ browserName }) => { + test.slow(browserName === 'firefox', 'Still working on it') +}) +``` diff --git a/src/index.ts b/src/index.ts index 556a5e7..138545b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,6 +20,7 @@ import noPagePause from './rules/no-page-pause.js' import noRawLocators from './rules/no-raw-locators.js' import noRestrictedMatchers from './rules/no-restricted-matchers.js' import noSkippedTest from './rules/no-skipped-test.js' +import noSlowedTests from './rules/no-slowed-tests.js' import noStandaloneExpect from './rules/no-standalone-expect.js' import noUnsafeReferences from './rules/no-unsafe-references.js' import noUselessAwait from './rules/no-useless-await.js' @@ -72,6 +73,7 @@ const index = { 'no-raw-locators': noRawLocators, 'no-restricted-matchers': noRestrictedMatchers, 'no-skipped-test': noSkippedTest, + 'no-slowed-test': noSlowedTests, 'no-standalone-expect': noStandaloneExpect, 'no-unsafe-references': noUnsafeReferences, 'no-useless-await': noUselessAwait, diff --git a/src/rules/no-slowed-test.test.ts b/src/rules/no-slowed-test.test.ts new file mode 100644 index 0000000..da6249c --- /dev/null +++ b/src/rules/no-slowed-test.test.ts @@ -0,0 +1,188 @@ +import { runRuleTester } from '../utils/rule-tester' +import rule from './no-slowed-test' + +const messageId = 'removeSlowedTestAnnotation' + +runRuleTester('no-slowed-test', rule, { + invalid: [ + { + code: 'test.slow("slow this test", async ({ page }) => {});', + errors: [ + { + column: 6, + endColumn: 10, + line: 1, + messageId: 'noSlowedTest', + suggestions: [ + { + messageId, + output: 'test("slow this test", async ({ page }) => {});', + }, + ], + }, + ], + }, + { + code: 'test["slow"]("slow this test", async ({ page }) => {});', + errors: [ + { + column: 6, + endColumn: 12, + line: 1, + messageId: 'noSlowedTest', + suggestions: [ + { + messageId, + output: 'test("slow this test", async ({ page }) => {});', + }, + ], + }, + ], + }, + { + code: 'test[`slow`]("slow this test", async ({ page }) => {});', + errors: [ + { + column: 6, + endColumn: 12, + line: 1, + messageId: 'noSlowedTest', + suggestions: [ + { + messageId, + output: 'test("slow this test", async ({ page }) => {});', + }, + ], + }, + ], + }, + { + code: 'test.slow("a test", { tag: ["@fast", "@login"] }, () => {})', + errors: [ + { + column: 6, + endColumn: 10, + line: 1, + messageId: 'noSlowedTest', + suggestions: [ + { + messageId, + output: 'test("a test", { tag: ["@fast", "@login"] }, () => {})', + }, + ], + }, + ], + }, + { + code: 'test.slow(browserName === "firefox");', + errors: [ + { + column: 1, + endColumn: 37, + line: 1, + messageId: 'noSlowedTest', + suggestions: [{ messageId, output: '' }], + }, + ], + }, + { + code: 'test.slow(browserName === "firefox", "Still working on it");', + errors: [ + { + column: 1, + endColumn: 60, + line: 1, + messageId: 'noSlowedTest', + suggestions: [{ messageId, output: '' }], + }, + ], + }, + { + code: 'test.slow()', + errors: [ + { + column: 1, + endColumn: 12, + line: 1, + messageId: 'noSlowedTest', + suggestions: [{ messageId, output: '' }], + }, + ], + }, + { + code: 'test["slow"]()', + errors: [ + { + column: 1, + endColumn: 15, + line: 1, + messageId: 'noSlowedTest', + suggestions: [{ messageId, output: '' }], + }, + ], + }, + { + code: 'test[`slow`]()', + errors: [ + { + column: 1, + endColumn: 15, + line: 1, + messageId: 'noSlowedTest', + suggestions: [{ messageId, output: '' }], + }, + ], + }, + // Global aliases + { + code: 'it.slow("slow this test", async ({ page }) => {});', + errors: [ + { + column: 4, + endColumn: 8, + line: 1, + messageId: 'noSlowedTest', + suggestions: [ + { + messageId, + output: 'it("slow this test", async ({ page }) => {});', + }, + ], + }, + ], + settings: { + playwright: { + globalAliases: { test: ['it'] }, + }, + }, + }, + ], + valid: [ + 'test("a test", () => {});', + 'test("a test", { tag: "@fast" }, () => {});', + 'test("a test", { tag: ["@fast", "@report"] }, () => {});', + 'test("one", async ({ page }) => {});', + 'test.only(isMobile, "Settings page does not work in mobile yet");', + 'test.skip();', + 'test["skip"]();', + 'test[`skip`]();', + 'const slow = true;', + 'function slow() { return null };', + 'this.slow();', + 'this["slow"]();', + 'this[`slow`]();', + { + code: 'test.slow(browserName === "firefox", "Still working on it");', + options: [{ allowConditional: true }], + }, + // Global aliases + { + code: 'it("a test", () => {});', + settings: { + playwright: { + globalAliases: { test: ['it'] }, + }, + }, + }, + ], +}) diff --git a/src/rules/no-slowed-test.ts b/src/rules/no-slowed-test.ts new file mode 100644 index 0000000..de6aebc --- /dev/null +++ b/src/rules/no-slowed-test.ts @@ -0,0 +1,77 @@ +import { getStringValue } from '../utils/ast' +import { createRule } from '../utils/createRule' +import { parseFnCall } from '../utils/parseFnCall' + +export default createRule({ + create(context) { + return { + CallExpression(node) { + const options = context.options[0] || {} + const allowConditional = !!options.allowConditional + + const call = parseFnCall(context, node) + if (call?.group !== 'test') { + return + } + + const slowNode = call.members.find((s) => getStringValue(s) === 'slow') + if (!slowNode) return + + // If the call is a standalone `test.slow()` call, and not a test + // annotation, we have to treat it a bit differently. + const isStandalone = call.type === 'config' + + // If allowConditional is enabled and it's not a test function, + // we ignore any `test.slow` calls that have no arguments. + if (isStandalone && allowConditional) { + return + } + + context.report({ + messageId: 'noSlowedTest', + node: isStandalone ? node : slowNode, + suggest: [ + { + fix: (fixer) => { + return isStandalone + ? fixer.remove(node.parent) + : fixer.removeRange([ + slowNode.range![0] - 1, + slowNode.range![1] + + Number(slowNode.type !== 'Identifier'), + ]) + }, + messageId: 'removeSlowedTestAnnotation', + }, + ], + }) + }, + } + }, + meta: { + docs: { + category: 'Best Practices', + description: 'Prevent usage of the `.slow()` slow test annotation.', + recommended: true, + url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-slowed-test.md', + }, + hasSuggestions: true, + messages: { + noSlowedTest: 'Unexpected use of the `.slow()` annotation.', + removeSlowedTestAnnotation: 'Remove the `.slow()` annotation.', + }, + schema: [ + { + additionalProperties: false, + properties: { + allowConditional: { + default: false, + type: 'boolean', + }, + }, + type: 'object', + }, + ], + type: 'suggestion', + }, +})