Skip to content

Commit 3bf491e

Browse files
committed
feat: add new rule for consistent spacing
between test blocks, like `test()`, `test.step()` and so on
1 parent 38a559e commit 3bf491e

File tree

4 files changed

+380
-0
lines changed

4 files changed

+380
-0
lines changed

src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import globals from 'globals'
2+
import consistentSpacingBetweenBlocks from './rules/consistent-spacing-between-blocks.js'
23
import expectExpect from './rules/expect-expect.js'
34
import maxExpects from './rules/max-expects.js'
45
import maxNestedDescribe from './rules/max-nested-describe.js'
@@ -54,6 +55,7 @@ import validTitle from './rules/valid-title.js'
5455
const index = {
5556
configs: {},
5657
rules: {
58+
'consistent-spacing-between-blocks': consistentSpacingBetweenBlocks,
5759
'expect-expect': expectExpect,
5860
'max-expects': maxExpects,
5961
'max-nested-describe': maxNestedDescribe,
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { javascript, runRuleTester } from '../utils/rule-tester.js'
2+
// some tests import from `../src/`, looks like a tooling issue; here,
3+
// the template used is `prefer-lowercase-title` and its tests
4+
import rule from './consistent-spacing-between-blocks.js'
5+
6+
runRuleTester('consistent-spacing-between-blocks', rule, {
7+
invalid: [
8+
{
9+
code: javascript`
10+
test.beforeEach('Test 1', () => {});
11+
test('Test 2', async () => {
12+
await test.step('Step 1', () => {});
13+
// a comment
14+
test.step('Step 2', () => {});
15+
test.step('Step 3', () => {});
16+
const foo = await test.step('Step 4', () => {});
17+
foo = await test.step('Step 5', () => {});
18+
});
19+
/**
20+
* another comment
21+
*/
22+
test('Test 6', () => {});
23+
`,
24+
errors: [
25+
{ messageId: 'missingWhitespace' },
26+
{ messageId: 'missingWhitespace' },
27+
{ messageId: 'missingWhitespace' },
28+
{ messageId: 'missingWhitespace' },
29+
{ messageId: 'missingWhitespace' },
30+
{ messageId: 'missingWhitespace' },
31+
],
32+
name: 'missing blank lines before test blocks',
33+
output: javascript`
34+
test.beforeEach('Test 1', () => {});
35+
36+
test('Test 2', async () => {
37+
await test.step('Step 1', () => {});
38+
39+
// a comment
40+
test.step('Step 2', () => {});
41+
42+
test.step('Step 3', () => {});
43+
44+
const foo = await test.step('Step 4', () => {});
45+
46+
foo = await test.step('Step 5', () => {});
47+
});
48+
49+
/**
50+
* another comment
51+
*/
52+
test('Test 6', () => {});
53+
`,
54+
},
55+
],
56+
valid: [
57+
{
58+
code: javascript`
59+
test('Test 1', () => {});
60+
61+
test('Test 2', () => {});
62+
`,
63+
name: 'blank line between simple test blocks',
64+
},
65+
{
66+
code: javascript`
67+
test.beforeEach(() => {});
68+
69+
test.skip('Test 2', () => {});
70+
`,
71+
name: 'blank line between test modifiers',
72+
},
73+
{
74+
code: javascript`
75+
test('Test', async () => {
76+
await test.step('Step 1', () => {});
77+
78+
await test.step('Step 2', () => {});
79+
});
80+
`,
81+
name: 'blank line between nested steps in async test',
82+
},
83+
{
84+
code: javascript`
85+
test('Test', async () => {
86+
await test.step('Step 1', () => {});
87+
88+
// some comment
89+
await test.step('Step 2', () => {});
90+
});
91+
`,
92+
name: 'nested steps with a line comment in between',
93+
},
94+
{
95+
code: javascript`
96+
test('Test', async () => {
97+
await test.step('Step 1', () => {});
98+
99+
/**
100+
* another comment
101+
*/
102+
await test.step('Step 2', () => {});
103+
});
104+
`,
105+
name: 'nested steps with a block comment in between',
106+
},
107+
{
108+
code: javascript`
109+
test('assign', async () => {
110+
let foo = await test.step('Step 1', () => {});
111+
112+
foo = await test.step('Step 2', () => {});
113+
});
114+
`,
115+
name: 'assignments initialized by test.step',
116+
},
117+
{
118+
code: javascript`
119+
test('assign', async () => {
120+
let { foo } = await test.step('Step 1', () => {});
121+
122+
({ foo } = await test.step('Step 2', () => {}));
123+
});
124+
`,
125+
name: 'destructuring assignments initialized by test.step',
126+
},
127+
],
128+
})
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import type { AST } from 'eslint'
2+
import type { Comment, Expression, Node } from 'estree'
3+
import { createRule } from '../utils/createRule.js'
4+
import { isTestExpression, unwrapExpression } from '../utils/test-expression.js'
5+
6+
/**
7+
* An ESLint rule that ensures consistent spacing between test blocks (e.g.
8+
* `test`, `test.step`, `test.beforeEach`, etc.). This rule helps improve the
9+
* readability and maintainability of test code by ensuring that test blocks are
10+
* clearly separated from each other.
11+
*/
12+
export default createRule({
13+
create(context) {
14+
/**
15+
* Recursively determines the previous token (if present) and, if necessary,
16+
* a stand-in token to check spacing against. Therefore, the current start
17+
* token can optionally be passed through and used as the comparison token.
18+
*
19+
* Returns the previous token that is not a comment or a grouping expression
20+
* (`previous`), the first token to compare (`start`), and the actual token
21+
* being examined (`origin`).
22+
*
23+
* If there is no previous token for the expression, `null` is returned for
24+
* it. Ideally, the first comparable token is the same as the actual token.
25+
*
26+
* | 1 | test('foo', async () => {
27+
*
28+
* Previous > | 2 | await test.step(...); | 3 | start > | 4 | // first
29+
* comment | 5 | // another comment origin > | 6 | await test.step(...);
30+
*/
31+
function getPreviousToken(
32+
node: AST.Token | Node,
33+
start?: AST.Token | Comment | Node,
34+
): {
35+
/** The token actually being checked */
36+
origin: AST.Token | Node
37+
38+
/**
39+
* The previous token that is neither a comment nor a grouping expression,
40+
* if present
41+
*/
42+
previous: AST.Token | null
43+
44+
/**
45+
* The first token used for comparison, e.g. the start of the test
46+
* expression
47+
*/
48+
start: AST.Token | Comment | Node
49+
} {
50+
const current = start ?? node
51+
const previous = context.sourceCode.getTokenBefore(current, {
52+
includeComments: true,
53+
})
54+
55+
// no predecessor present
56+
if (
57+
previous === null ||
58+
previous === undefined ||
59+
previous.value === '{'
60+
) {
61+
return {
62+
origin: node,
63+
previous: null,
64+
start: current,
65+
}
66+
}
67+
68+
// Recursively traverse comments and determine a stand-in
69+
// and unwrap parenthesized expressions
70+
if (
71+
previous.type === 'Line' || // line comment
72+
previous.type === 'Block' || // block comment
73+
previous.value === '(' // grouping operator
74+
) {
75+
return getPreviousToken(node, previous)
76+
}
77+
78+
// Return result
79+
return {
80+
origin: node,
81+
previous: previous as AST.Token,
82+
start: current,
83+
}
84+
}
85+
86+
/**
87+
* Checks whether the spacing before the given test block meets
88+
* expectations. Optionally an offset token can be provided to check
89+
* against, for example in the case of an assignment.
90+
*
91+
* @param node - The node to be checked.
92+
* @param offset - Optional offset token to check spacing against.
93+
*/
94+
function checkSpacing(node: Expression, offset?: AST.Token | Node) {
95+
const { previous, start } = getPreviousToken(node, offset)
96+
97+
// First expression or no previous token
98+
if (previous === null) return
99+
100+
// Ignore when there is one or more blank lines between
101+
if (previous.loc.end.line < start.loc!.start.line - 1) {
102+
return
103+
}
104+
105+
// Since the hint in the IDE may not appear on the affected test expression
106+
// but possibly on the preceding comment, include the test expression in the message
107+
const source = context.sourceCode.getText(unwrapExpression(node))
108+
109+
context.report({
110+
data: { source },
111+
fix(fixer) {
112+
return fixer.insertTextAfter(previous, '\n')
113+
},
114+
loc: {
115+
end: {
116+
column: start.loc!.start.column,
117+
line: start.loc!.start.line,
118+
},
119+
start: {
120+
column: 0,
121+
line: previous.loc.end.line + 1,
122+
},
123+
},
124+
messageId: 'missingWhitespace',
125+
node,
126+
})
127+
}
128+
129+
return {
130+
// Checks call expressions that could be test steps,
131+
// e.g. `test(...)`, `test.step(...)`, or `await test.step(...)`, but also `foo = test(...)`
132+
ExpressionStatement(node) {
133+
if (isTestExpression(context, node.expression)) {
134+
checkSpacing(node.expression)
135+
}
136+
},
137+
// Checks declarations that might be initialized from return values of test steps,
138+
// e.g. `let result = await test(...)` or `const result = await test.step(...)`
139+
VariableDeclaration(node) {
140+
node.declarations.forEach((declaration) => {
141+
if (declaration.init && isTestExpression(context, declaration.init)) {
142+
// When declaring a variable, our examined test expression is used for initialization.
143+
// Therefore, to check spacing we use the keyword token (let, const, var) before it:
144+
// 1 | const foo = test('foo', () => {});
145+
// 2 | ^
146+
const offset = context.sourceCode.getTokenBefore(declaration)
147+
checkSpacing(declaration.init, offset ?? undefined)
148+
}
149+
})
150+
},
151+
}
152+
},
153+
meta: {
154+
docs: {
155+
description:
156+
'Enforces a blank line between Playwright test blocks (e.g., test, test.step, test.beforeEach, etc.).',
157+
recommended: true,
158+
},
159+
fixable: 'whitespace',
160+
messages: {
161+
missingWhitespace:
162+
"A blank line is required before the test block '{{source}}'.",
163+
},
164+
schema: [],
165+
type: 'layout',
166+
},
167+
})

src/utils/test-expression.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import type { Rule } from 'eslint'
2+
import type { Expression } from 'estree'
3+
import { parseFnCall } from '../utils/parseFnCall.js'
4+
5+
/**
6+
* Unwraps a given expression to get the actual expression being called. This is
7+
* useful when checking the final expression specifically.
8+
*
9+
* This handles:
10+
*
11+
* - Assignment expressions like `result = test(...)`
12+
* - Async calls like `await test(...)`
13+
* - Chained function calls like `test.step().then(...)`
14+
*
15+
* @param node - The expression to unwrap
16+
* @returns The unwrapped expression
17+
*/
18+
export function unwrapExpression(node: Expression): Expression {
19+
// Resolve assignments to get the actual expression
20+
if (node.type === 'AssignmentExpression') {
21+
return unwrapExpression(node.right)
22+
}
23+
24+
// Resolve async await expressions
25+
if (node.type === 'AwaitExpression') {
26+
return unwrapExpression(node.argument)
27+
}
28+
29+
// Traverse chains recursively to find the actual call
30+
if (
31+
node.type === 'CallExpression' &&
32+
node.callee.type === 'MemberExpression' &&
33+
node.callee.object.type === 'CallExpression'
34+
) {
35+
return unwrapExpression(node.callee.object)
36+
}
37+
38+
return node
39+
}
40+
41+
/**
42+
* Checks if a given expression is a test-related call. A test call is a call to
43+
* `test(...)` or one of its methods like `test.step(...)`.
44+
*
45+
* If the expression is chained, the calls are recursively traced back to find
46+
* the actual call. This also handles assignments and async calls with `await`.
47+
*
48+
* @param context - The ESLint rule context
49+
* @param node - The expression to check
50+
* @param methods - Optional list of specific methods to check for
51+
* @returns Whether it's a test block call
52+
*/
53+
export function isTestExpression(
54+
context: Rule.RuleContext,
55+
node: Expression,
56+
methods?: string[],
57+
): boolean {
58+
// Unwrap the actual expression to check the call
59+
const unwrapped = unwrapExpression(node)
60+
61+
// Must be a call expression to be a test call
62+
if (unwrapped.type !== 'CallExpression') {
63+
return false
64+
}
65+
66+
// Use the existing parseFnCall to identify test-related calls
67+
const call = parseFnCall(context, unwrapped)
68+
if (!call) {
69+
return false
70+
}
71+
72+
// If specific methods are requested, check if it's one of them
73+
if (methods !== undefined) {
74+
return (
75+
call.type === 'step' ||
76+
call.type === 'hook' ||
77+
(call.type === 'test' && methods.includes('test'))
78+
)
79+
}
80+
81+
// Check if it's any test-related call
82+
return ['test', 'step', 'hook'].includes(call.type)
83+
}

0 commit comments

Comments
 (0)