Skip to content

Commit

Permalink
feat: Add no-slow-tests rule
Browse files Browse the repository at this point in the history
Fixes #296
  • Loading branch information
mskelton committed Dec 2, 2024
1 parent 35e37a1 commit 02f037b
Show file tree
Hide file tree
Showing 5 changed files with 324 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 || 🔧 | |
Expand Down
56 changes: 56 additions & 0 deletions docs/rules/no-slowed-test.md
Original file line number Diff line number Diff line change
@@ -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')
})
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down
188 changes: 188 additions & 0 deletions src/rules/no-slowed-test.test.ts
Original file line number Diff line number Diff line change
@@ -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'] },
},
},
},
],
})
77 changes: 77 additions & 0 deletions src/rules/no-slowed-test.ts
Original file line number Diff line number Diff line change
@@ -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',
},
})

0 comments on commit 02f037b

Please sign in to comment.