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

Add prefer-hooks-on-top rule #224

Merged
merged 1 commit into from
Feb 18, 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 @@ -142,6 +142,7 @@ command line option.\
|| 🔧 | | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists |
|| | 💡 | [no-wait-for-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md) | Disallow usage of `page.waitForSelector()` |
|| | 💡 | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md) | Disallow usage of `page.waitForTimeout()` |
| | | | [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases |
| | | 💡 | [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` |
| | 🔧 | | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names |
| | 🔧 | | [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` |
Expand Down
121 changes: 121 additions & 0 deletions docs/rules/prefer-hooks-on-top.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# Suggest having hooks before any test cases (`prefer-hooks-on-top`)

While hooks can be setup anywhere in a test file, they are always called in a
specific order, which means it can be confusing if they're intermixed with test
cases.

This rule helps to ensure that hooks are always defined before test cases.

## Rule details

Examples of **incorrect** code for this rule

```js
/* eslint playwright/prefer-hooks-on-top: "error" */

test.describe('foo', () => {
test.beforeEach(() => {
seedMyDatabase();
});

test('accepts this input', () => {
// ...
});

test.beforeAll(() => {
createMyDatabase();
});

test('returns that value', () => {
// ...
});

test.describe('when the database has specific values', () => {
const specificValue = '...';

test.beforeEach(() => {
seedMyDatabase(specificValue);
});

test('accepts that input', () => {
// ...
});

test('throws an error', () => {
// ...
});

test.afterEach(() => {
clearLogger();
});

test.beforeEach(() => {
mockLogger();
});

test('logs a message', () => {
// ...
});
});

test.afterAll(() => {
removeMyDatabase();
});
});
```

Examples of **correct** code for this rule

```js
/* eslint playwright/prefer-hooks-on-top: "error" */

test.describe('foo', () => {
test.beforeAll(() => {
createMyDatabase();
});

test.beforeEach(() => {
seedMyDatabase();
});

test.afterAll(() => {
clearMyDatabase();
});

test('accepts this input', () => {
// ...
});

test('returns that value', () => {
// ...
});

test.describe('when the database has specific values', () => {
const specificValue = '...';

beforeEach(() => {
seedMyDatabase(specificValue);
});

beforeEach(() => {
mockLogger();
});

afterEach(() => {
clearLogger();
});

test('accepts that input', () => {
// ...
});

test('throws an error', () => {
// ...
});

test('logs a message', () => {
// ...
});
});
});
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import noUselessAwait from './rules/no-useless-await';
import noUselessNot from './rules/no-useless-not';
import noWaitForSelector from './rules/no-wait-for-selector';
import noWaitForTimeout from './rules/no-wait-for-timeout';
import preferHooksOnTop from './rules/prefer-hooks-on-top';
import preferLowercaseTitle from './rules/prefer-lowercase-title';
import preferStrictEqual from './rules/prefer-strict-equal';
import preferToBe from './rules/prefer-to-be';
Expand Down Expand Up @@ -60,6 +61,7 @@ const index = {
'no-useless-not': noUselessNot,
'no-wait-for-selector': noWaitForSelector,
'no-wait-for-timeout': noWaitForTimeout,
'prefer-hooks-on-top': preferHooksOnTop,
'prefer-lowercase-title': preferLowercaseTitle,
'prefer-strict-equal': preferStrictEqual,
'prefer-to-be': preferToBe,
Expand Down
40 changes: 40 additions & 0 deletions src/rules/prefer-hooks-on-top.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { Rule } from 'eslint';
import { isTestCall, isTestHook } from '../utils/ast';

export default {
create(context) {
const stack = [false];

return {
CallExpression(node) {
if (isTestCall(context, node)) {
stack[stack.length - 1] = true;
}

if (stack.at(-1) && isTestHook(context, node)) {
context.report({
messageId: 'noHookOnTop',
node,
});
}

stack.push(false);
},
'CallExpression:exit'() {
stack.pop();
},
};
},
meta: {
docs: {
category: 'Best Practices',
description: 'Suggest having hooks before any test cases',
recommended: false,
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md',
},
messages: {
noHookOnTop: 'Hooks should come before test cases',
},
type: 'suggestion',
},
} as Rule.RuleModule;
Loading
Loading