Skip to content
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

No slow tests #302

Merged
merged 1 commit into from
Dec 2, 2024
Merged
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 @@ -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-test.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.js'
import rule from './no-slowed-test.js'

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.js'
import { createRule } from '../utils/createRule.js'
import { parseFnCall } from '../utils/parseFnCall.js'

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