Skip to content

Conversation

@davidenke
Copy link

@davidenke davidenke commented Aug 25, 2025

An ESLint rule that ensures consistent spacing between test blocks (e.g. test, test.step, test.beforeEach, etc.). This rule helps improve the readability and maintainability of test code by ensuring that test blocks are clearly separated from each other.


Note

Adds a new rule to enforce a blank line between Playwright test blocks, with docs, tests, utils, and plugin export.

  • Rules:
    • New consistent-spacing-between-blocks: enforces a blank line before test, test.step, hooks; auto-fixable; recommended; error message missingWhitespace.
    • Implementation in src/rules/consistent-spacing-between-blocks.ts; tests in src/rules/consistent-spacing-between-blocks.test.ts.
  • Utils:
    • Add src/utils/test-expression.ts to unwrap and detect test-related expressions.
  • Plugin wiring:
    • Export rule in src/index.ts under rules.
  • Docs:
    • Add docs/rules/consistent-spacing-between-blocks.md with examples.
    • Update README.md rules table to include consistent-spacing-between-blocks (✅, 🔧).

Written by Cursor Bugbot for commit 9374973. This will update automatically on new commits. Configure here.

@mskelton mskelton force-pushed the rule/consistent-spacing-between-blocks branch from 3b6154f to 9374973 Compare September 30, 2025 00:56
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Bug: Recommended Rule Not Enabled

The consistent-spacing-between-blocks rule is marked as recommended in the README and its metadata, but it's not included in sharedConfig.rules. This means the rule won't be enabled in the recommended configuration, contradicting its documented status.

src/index.ts#L57-L58

rules: {
'consistent-spacing-between-blocks': consistentSpacingBetweenBlocks,

src/index.ts#L97-L134

'prefer-to-contain': preferToContain,
'prefer-to-have-count': preferToHaveCount,
'prefer-to-have-length': preferToHaveLength,
'prefer-web-first-assertions': preferWebFirstAssertions,
'require-hook': requireHook,
'require-soft-assertions': requireSoftAssertions,
'require-to-throw-message': requireToThrowMessage,
'require-top-level-describe': requireTopLevelDescribe,
'valid-describe-callback': validDescribeCallback,
'valid-expect': validExpect,
'valid-expect-in-promise': validExpectInPromise,
'valid-test-tags': validTestTags,
'valid-title': validTitle,
},
}
const sharedConfig = {
rules: {
'no-empty-pattern': 'off',
'playwright/expect-expect': 'warn',
'playwright/max-nested-describe': 'warn',
'playwright/missing-playwright-await': 'error',
'playwright/no-conditional-expect': 'warn',
'playwright/no-conditional-in-test': 'warn',
'playwright/no-element-handle': 'warn',
'playwright/no-eval': 'warn',
'playwright/no-focused-test': 'error',
'playwright/no-force-option': 'warn',
'playwright/no-nested-step': 'warn',
'playwright/no-networkidle': 'error',
'playwright/no-page-pause': 'warn',
'playwright/no-skipped-test': 'warn',
'playwright/no-standalone-expect': 'error',
'playwright/no-unsafe-references': 'error',
'playwright/no-useless-await': 'warn',
'playwright/no-useless-not': 'warn',
'playwright/no-wait-for-navigation': 'error',
'playwright/no-wait-for-selector': 'warn',

README.md#L120-L121

| --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | :-: | :-: | :-: |
| [consistent-spacing-between-blocks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/consistent-spacing-between-blocks.md) | Enforce consistent spacing between test blocks || 🔧 | |

Fix in Cursor Fix in Web


call.type === 'step' ||
call.type === 'hook' ||
(call.type === 'test' && methods.includes('test'))
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Function Ignores Methods for Non-Test Calls

The isTestExpression function incorrectly handles the methods parameter. When methods is provided, it unconditionally returns true for step and hook call types, even if they are not included in the methods array. It only correctly filters for test call types.

Fix in Cursor Fix in Web

@davidenke davidenke requested a review from mskelton October 1, 2025 13:39
@davidenke davidenke force-pushed the rule/consistent-spacing-between-blocks branch from 24a0d16 to b3373f0 Compare October 1, 2025 15:26
Comment on lines +116 to +145
const nodeToCheck = getRealNodeToCheck(node)

if (isFirstNode(nodeToCheck)) return
if (hasNewlineBefore(nodeToCheck)) return

const leadingComments = sourceCode.getCommentsBefore(nodeToCheck)
const firstComment = leadingComments[0]
const reportLoc = firstComment?.loc ?? nodeToCheck.loc

context.report({
data: {
source: sourceCode.getText(nodeToCheck).split('\n')[0],
},
fix(fixer) {
const tokenBefore = sourceCode.getTokenBefore(nodeToCheck)
if (!tokenBefore) return null

const newlines =
nodeToCheck.loc?.start.line === tokenBefore.loc.end.line
? '\n\n'
: '\n'
const targetNode = firstComment ?? nodeToCheck
const nodeStart = targetNode.range?.[0] ?? 0
const textBeforeNode = sourceCode.text.substring(0, nodeStart)
const lastNewlineIndex = textBeforeNode.lastIndexOf('\n')
const insertPosition = lastNewlineIndex + 1

return fixer.insertTextBeforeRange(
[insertPosition, nodeStart],
newlines,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels oddly complex. Why not just this:

  • Get the line number
  • Using context.sourceCode, get the previous line (or bail if the test is on the first line of the file)
  • If the previously line is all whitespace, it's good, otherwise it's not.

Comment on lines +33 to +43
if (parentType === 'IfStatement') {
return isPrecededByTokens(node, ['else', ')'])
}

if (parentType === 'DoWhileStatement') {
return isPrecededByTokens(node, ['do'])
}

if (parentType === 'SwitchCase') {
return isPrecededByTokens(node, [':'])
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just check if it's in a BlockStatement, doesn't matter what type of block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants