Skip to content

Enable the five deferred eslint-plugin-jest test-quality rules (follow-up to #2970) #3129

Description

@acoliver

Context

Issue #2970 replaced @vitest/eslint-plugin with eslint-plugin-jest (configured with
settings.jest.globalPackage = 'bun:test') after every test moved to bun:test in #2969.

Two facts came out of that work:

  1. @vitest/eslint-plugin had been silently inert since Rewrite all vitest imports to bun:test and delete the Vitest compatibility shim (#2578) #2969. It identifies test blocks by
    their import source, and it does not recognise bun:test. Verified with a controlled
    experiment — identical file and rules, only the import changed:

    import { describe, it, expect } from 'vitest';    -> 3 errors
    import { describe, it, expect } from 'bun:test';  -> 0 errors, exit 0
    
  2. eslint-plugin-jest implements several of the same rules more strictly than
    @vitest/eslint-plugin ever did. This is not a regression introduced by Rewrite all vitest imports to bun:test and delete the Vitest compatibility shim (#2578) #2969: running the
    jest plugin against the tree immediately BEFORE Rewrite all vitest imports to bun:test and delete the Vitest compatibility shim (#2578) #2969 (commit a805a21), with
    globalPackage: 'vitest' so it could see those files, reported 728 violations in
    packages/core alone.

#2970 enabled every rule that was free or cheap, fixing 36 real violations to do so.

Remaining work

Five rules are currently off in eslint.config.js, each with its options preserved so
re-enabling is a one-word change:

Rule Violations
jest/no-conditional-in-test 3059
jest/no-standalone-expect 617
jest/prefer-strict-equal 199
jest/require-top-level-describe 154
jest/valid-expect 154

Total: 4183.

jest/valid-expect is the highest value of these — an un-awaited async assertion silently
passes, so those 154 sites are tests that may not be asserting anything. Recommend taking it
first, then no-standalone-expect, then the rest.

Acceptance criteria

  • Each rule above is enabled at error in eslint.config.js
  • Every violation is fixed by correcting the test, never by weakening or deleting it
  • No eslint-disable directives are introduced
  • npm run lint passes at 0 warnings

Out of scope

Reintroducing Vitest in any form. See the lint:no-vitest guard added by #2970.

Metadata

Metadata

Assignees

Labels

Code Quality / ModularizationIssues to do with the quality of llxprt code and its maintainability.ci/cdIssues with github and the workflow scripts and CI CD environment.

Type

Projects

Status
In Progress

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions