Skip to content

Commit 3f3113f

Browse files
authored
Merge pull request #227 from playwright-community/no-duplicate-hooks
feat: Add `no-duplicate-hooks` rule
2 parents 9dd834b + 0a28d25 commit 3f3113f

File tree

5 files changed

+451
-0
lines changed

5 files changed

+451
-0
lines changed

README.md

+1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ command line option.\
125125
| | | | [no-commented-out-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-commented-out-test.md) | Disallow commented out tests |
126126
|| | | [no-conditional-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally |
127127
|| | | [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 |
128+
| | | | [no-duplicate-hooks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks |
128129
|| | 💡 | [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md) | Disallow usage of element handles |
129130
|| | | [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()` |
130131
|| | 💡 | [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md) | Disallow usage of `.only` annotation |

docs/rules/no-duplicate-hooks.md

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# Disallow duplicate setup and teardown hooks (`no-duplicate-hooks`)
2+
3+
A `describe` block should not contain duplicate hooks.
4+
5+
## Rule details
6+
7+
Examples of **incorrect** code for this rule
8+
9+
```js
10+
/* eslint playwright/no-duplicate-hooks: "error" */
11+
12+
test.describe('foo', () => {
13+
test.beforeEach(() => {
14+
// some setup
15+
});
16+
test.beforeEach(() => {
17+
// some setup
18+
});
19+
test('foo_test', () => {
20+
// some test
21+
});
22+
});
23+
24+
// Nested describe scenario
25+
test.describe('foo', () => {
26+
test.beforeEach(() => {
27+
// some setup
28+
});
29+
test('foo_test', () => {
30+
// some test
31+
});
32+
test.describe('bar', () => {
33+
test('bar_test', () => {
34+
test.afterAll(() => {
35+
// some teardown
36+
});
37+
test.afterAll(() => {
38+
// some teardown
39+
});
40+
});
41+
});
42+
});
43+
```
44+
45+
Examples of **correct** code for this rule
46+
47+
```js
48+
/* eslint playwright/no-duplicate-hooks: "error" */
49+
50+
test.describe('foo', () => {
51+
test.beforeEach(() => {
52+
// some setup
53+
});
54+
test('foo_test', () => {
55+
// some test
56+
});
57+
});
58+
59+
// Nested describe scenario
60+
test.describe('foo', () => {
61+
test.beforeEach(() => {
62+
// some setup
63+
});
64+
test('foo_test', () => {
65+
// some test
66+
});
67+
test.describe('bar', () => {
68+
test('bar_test', () => {
69+
test.beforeEach(() => {
70+
// some setup
71+
});
72+
});
73+
});
74+
});
75+
```

src/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import missingPlaywrightAwait from './rules/missing-playwright-await';
55
import noCommentedOutTests from './rules/no-commented-out-tests';
66
import noConditionalExpect from './rules/no-conditional-expect';
77
import noConditionalInTest from './rules/no-conditional-in-test';
8+
import noDuplicateHooks from './rules/no-duplicate-hooks';
89
import noElementHandle from './rules/no-element-handle';
910
import noEval from './rules/no-eval';
1011
import noFocusedTest from './rules/no-focused-test';
@@ -46,6 +47,7 @@ const index = {
4647
'no-commented-out-tests': noCommentedOutTests,
4748
'no-conditional-expect': noConditionalExpect,
4849
'no-conditional-in-test': noConditionalInTest,
50+
'no-duplicate-hooks': noDuplicateHooks,
4951
'no-element-handle': noElementHandle,
5052
'no-eval': noEval,
5153
'no-focused-test': noFocusedTest,

src/rules/no-duplicate-hooks.ts

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { Rule } from 'eslint';
2+
import { getStringValue, isDescribeCall, isTestHook } from '../utils/ast';
3+
4+
export default {
5+
create(context) {
6+
const hookContexts: Array<Record<string, number>> = [{}];
7+
8+
return {
9+
CallExpression(node) {
10+
if (isDescribeCall(node)) {
11+
hookContexts.push({});
12+
}
13+
14+
if (!isTestHook(context, node)) {
15+
return;
16+
}
17+
18+
const currentLayer = hookContexts[hookContexts.length - 1];
19+
const name =
20+
node.callee.type === 'MemberExpression'
21+
? getStringValue(node.callee.property)
22+
: '';
23+
24+
currentLayer[name] ||= 0;
25+
currentLayer[name] += 1;
26+
27+
if (currentLayer[name] > 1) {
28+
context.report({
29+
data: { hook: name },
30+
messageId: 'noDuplicateHook',
31+
node,
32+
});
33+
}
34+
},
35+
'CallExpression:exit'(node) {
36+
if (isDescribeCall(node)) {
37+
hookContexts.pop();
38+
}
39+
},
40+
};
41+
},
42+
meta: {
43+
docs: {
44+
category: 'Best Practices',
45+
description: 'Disallow duplicate setup and teardown hooks',
46+
recommended: false,
47+
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-duplicate-hooks.md',
48+
},
49+
messages: {
50+
noDuplicateHook: 'Duplicate {{ hook }} in describe block',
51+
},
52+
type: 'suggestion',
53+
},
54+
} as Rule.RuleModule;

0 commit comments

Comments
 (0)