Skip to content

Commit 56ebc36

Browse files
hikimochimskelton
authored andcommitted
feat: Add no-nested-step rule
1 parent 743da39 commit 56ebc36

File tree

5 files changed

+168
-0
lines changed

5 files changed

+168
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ command line option.\
6464
|| | | [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` |
6565
|| | 💡 | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md) | Disallow usage of `.only` annotation |
6666
|| | | [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 |
67+
|| | | [no-nested-step](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md) | Disallow nested `test.step()` methods |
6768
|| | | [no-networkidle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-networkidle.md) | Disallow usage of the `networkidle` option |
6869
|| | | [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md) | Disallow using `page.pause` |
6970
|| 🔧 | | [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 |

docs/rules/no-nested-step.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Disallow nested `test.step()` methods (`no-nested-step`)
2+
3+
Nesting `test.step()` methods can make your tests difficult to read.
4+
5+
## Rule Details
6+
7+
Examples of **incorrect** code for this rule:
8+
9+
```javascript
10+
test('foo', async () => {
11+
await test.step('step1', async () => {
12+
await test.step('nest step', async () => {
13+
await expect(true).toBe(true);
14+
});
15+
});
16+
});
17+
```
18+
19+
Examples of **correct** code for this rule:
20+
21+
```javascript
22+
test('foo', async () => {
23+
await test.step('step1', async () => {
24+
await expect(true).toBe(true);
25+
});
26+
await test.step('step2', async () => {
27+
await expect(true).toBe(true);
28+
});
29+
});
30+
```

src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import noElementHandle from './rules/no-element-handle';
55
import noEval from './rules/no-eval';
66
import noFocusedTest from './rules/no-focused-test';
77
import noForceOption from './rules/no-force-option';
8+
import noNestedStep from './rules/no-nested-step';
89
import noNetworkidle from './rules/no-networkidle';
910
import noPagePause from './rules/no-page-pause';
1011
import noRestrictedMatchers from './rules/no-restricted-matchers';
@@ -35,6 +36,7 @@ const recommended = {
3536
'playwright/no-eval': 'warn',
3637
'playwright/no-focused-test': 'error',
3738
'playwright/no-force-option': 'warn',
39+
'playwright/no-nested-step': 'warn',
3840
'playwright/no-networkidle': 'error',
3941
'playwright/no-page-pause': 'warn',
4042
'playwright/no-skipped-test': 'warn',
@@ -91,6 +93,7 @@ export = {
9193
'no-eval': noEval,
9294
'no-focused-test': noFocusedTest,
9395
'no-force-option': noForceOption,
96+
'no-nested-step': noNestedStep,
9497
'no-networkidle': noNetworkidle,
9598
'no-page-pause': noPagePause,
9699
'no-restricted-matchers': noRestrictedMatchers,

src/rules/no-nested-step.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { Rule } from 'eslint';
2+
import * as ESTree from 'estree';
3+
import { isPropertyAccessor } from '../utils/ast';
4+
5+
function isStepCall(node: ESTree.Node): boolean {
6+
const inner = node.type === 'CallExpression' ? node.callee : node;
7+
8+
if (inner.type !== 'MemberExpression') {
9+
return false;
10+
}
11+
12+
return isPropertyAccessor(inner, 'step');
13+
}
14+
15+
export default {
16+
create(context) {
17+
const stack: number[] = [];
18+
19+
function pushStepCallback(node: Rule.Node) {
20+
if (node.parent.type !== 'CallExpression' || !isStepCall(node.parent)) {
21+
return;
22+
}
23+
24+
stack.push(0);
25+
26+
if (stack.length > 1) {
27+
context.report({
28+
messageId: 'noNestedStep',
29+
node: node.parent.callee,
30+
});
31+
}
32+
}
33+
34+
function popStepCallback(node: Rule.Node) {
35+
const { parent } = node;
36+
37+
if (parent.type === 'CallExpression' && isStepCall(parent)) {
38+
stack.pop();
39+
}
40+
}
41+
42+
return {
43+
ArrowFunctionExpression: pushStepCallback,
44+
'ArrowFunctionExpression:exit': popStepCallback,
45+
FunctionExpression: pushStepCallback,
46+
'FunctionExpression:exit': popStepCallback,
47+
};
48+
},
49+
meta: {
50+
docs: {
51+
category: 'Best Practices',
52+
description: 'Disallow nested `test.step()` methods',
53+
recommended: true,
54+
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md',
55+
},
56+
messages: {
57+
noNestedStep: 'Do not nest `test.step()` methods.',
58+
},
59+
schema: [],
60+
type: 'problem',
61+
},
62+
} as Rule.RuleModule;

test/spec/no-nested-step.spec.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import rule from '../../src/rules/no-nested-step';
2+
import { runRuleTester } from '../utils/rule-tester';
3+
import dedent = require('dedent');
4+
5+
const messageId = 'noNestedStep';
6+
7+
runRuleTester('max-nested-step', rule, {
8+
invalid: [
9+
{
10+
code: dedent`
11+
test('foo', async () => {
12+
await test.step("step1", async () => {
13+
await test.step("nested step1", async () => {
14+
await expect(true).toBe(true);
15+
});
16+
});
17+
});
18+
`,
19+
errors: [{ column: 11, endColumn: 20, endLine: 3, line: 3, messageId }],
20+
},
21+
{
22+
code: dedent`
23+
test('foo', async () => {
24+
await test.step("step1", async () => {
25+
await test.step("nested step1", async () => {
26+
await expect(true).toBe(true);
27+
});
28+
await test.step("nested step1", async () => {
29+
await expect(true).toBe(true);
30+
});
31+
});
32+
});
33+
`,
34+
errors: [
35+
{ column: 11, endColumn: 20, endLine: 3, line: 3, messageId },
36+
{ column: 11, endColumn: 20, endLine: 6, line: 6, messageId },
37+
],
38+
},
39+
],
40+
valid: [
41+
'await test.step("step1", () => {});',
42+
'await test.step("step1", async () => {});',
43+
{
44+
code: dedent`
45+
test('foo', async () => {
46+
await expect(true).toBe(true);
47+
});
48+
`,
49+
},
50+
{
51+
code: dedent`
52+
test('foo', async () => {
53+
await test.step("step1", async () => {
54+
await expect(true).toBe(true);
55+
});
56+
});
57+
`,
58+
},
59+
{
60+
code: dedent`
61+
test('foo', async () => {
62+
await test.step("step1", async () => {
63+
await expect(true).toBe(true);
64+
});
65+
await test.step("step2", async () => {
66+
await expect(true).toBe(true);
67+
});
68+
});
69+
`,
70+
},
71+
],
72+
});

0 commit comments

Comments
 (0)