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-in-order rule #225

Merged
merged 2 commits 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-in-order](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order |
| | | | [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 |
Expand Down
129 changes: 129 additions & 0 deletions docs/rules/prefer-hooks-in-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# Prefer having hooks in a consistent order (`prefer-hooks-in-order`)

While hooks can be setup in any order, they're always called by `playwright` in
this specific order:

1. `beforeAll`
1. `beforeEach`
1. `afterEach`
1. `afterAll`

This rule aims to make that more obvious by enforcing grouped hooks be setup in
that order within tests.

## Rule details

Examples of **incorrect** code for this rule

```js
/* eslint playwright/prefer-hooks-in-order: "error" */

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

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

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

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-in-order: "error" */

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

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

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

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.beforeEach(() => {
mockLogger();
});

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

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

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

## Also See

- [`prefer-hooks-on-top`](prefer-hooks-on-top.md)
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 preferHooksInOrder from './rules/prefer-hooks-in-order';
import preferHooksOnTop from './rules/prefer-hooks-on-top';
import preferLowercaseTitle from './rules/prefer-lowercase-title';
import preferStrictEqual from './rules/prefer-strict-equal';
Expand Down Expand Up @@ -61,6 +62,7 @@ const index = {
'no-useless-not': noUselessNot,
'no-wait-for-selector': noWaitForSelector,
'no-wait-for-timeout': noWaitForTimeout,
'prefer-hooks-in-order': preferHooksInOrder,
'prefer-hooks-on-top': preferHooksOnTop,
'prefer-lowercase-title': preferLowercaseTitle,
'prefer-strict-equal': preferStrictEqual,
Expand Down
70 changes: 70 additions & 0 deletions src/rules/prefer-hooks-in-order.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { Rule } from 'eslint';
import { getStringValue, isTestHook } from '../utils/ast';

const HooksOrder = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'];

export default {
create(context) {
let previousHookIndex = -1;
let inHook = false;

return {
CallExpression(node) {
// Ignore everything that is passed into a hook
if (inHook) return;

// Reset the previousHookIndex when encountering something different from a hook
if (!isTestHook(context, node)) {
previousHookIndex = -1;
return;
}

inHook = true;
const currentHook =
node.callee.type === 'MemberExpression'
? getStringValue(node.callee.property)
: '';
const currentHookIndex = HooksOrder.indexOf(currentHook);

if (currentHookIndex < previousHookIndex) {
return context.report({
data: {
currentHook,
previousHook: HooksOrder[previousHookIndex],
},
messageId: 'reorderHooks',
node,
});
}

previousHookIndex = currentHookIndex;
},
'CallExpression:exit'(node) {
if (isTestHook(context, node)) {
inHook = false;
return;
}

if (inHook) {
return;
}

// Reset the previousHookIndex when encountering something different from a hook
previousHookIndex = -1;
},
};
},
meta: {
docs: {
category: 'Best Practices',
description: 'Prefer having hooks in a consistent order',
recommended: false,
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md',
},
messages: {
reorderHooks:
'`{{ currentHook }}` hooks should be before any `{{ previousHook }}` hooks',
},
type: 'suggestion',
},
} as Rule.RuleModule;
Loading
Loading