feat(code-quality): add test-discipline rules - #53
Merged
Merged
Conversation
Reports toThrow() / toThrowError() with no argument, or with only a string, template or regex, sync or after .rejects: any error passes, so a TypeError from a broken mock satisfies the assertion. A message-only assertion is accepted when the same test pins the class of the same subject. Options: throwMatchers, allowMessageOnly, trustErrorInstances (false under Jest) and assertionHelpers. Opt-in, left out of recommended.
Reports a real sleep in a unit test file: a promise whose executor resolves from setTimeout, setTimeout from timers/promises, and promisify(setTimeout). A zero or omitted delay (allowZeroDelay, the act() flush), a reject-only timeout guard, a deadline whose handle is kept for clearTimeout, and any file that installs fake timers are left alone. Opt-in, left out of recommended. Adds a visitor-key walkSome helper to the plugin's ast utils.
Reports Date.now(), an argless new Date() and Date() in a unit test file that never fakes the clock (useFakeTimers, setSystemTime, a Date.now or global Date spy, an assignment to Date.now, or a mocked module matching clockModules). An offset from now (Date.now() + 60_000, new Date().getTime() - 1000) is robust against the real clock and allowed. Opt-in, left out of recommended.
Reports an object literal containing jest.fn() / vi.fn() (at any depth) cast `as unknown as T` or `as any as T`: the double cast switches type checking off, so a mock of a renamed or removed method keeps passing. Suggests jest.Mocked<Pick<T, ...>> or satisfies. Options: mockFactories, allowTargets for types too wide to Pick from. Opt-in, left out of recommended.
…-expect
A test whose only assertion is that the render root is present
(expect(container).not.toBeEmptyDOMElement(), container.firstChild
toBeInTheDocument, container.innerHTML not.toBe('') and the like) now
reports soleRenderRootExpect: it passes for anything that renders, an
error fallback included. On by default for container and baseElement;
the new renderRoots option names other roots, and [] turns it off.
skipped-tests-need-tracking now also reads node:test skips from the syntax: a skip or todo option that is true or a non-empty string on test / it / describe / suite (and a t.test subtest), and t.skip() / t.todo() as a statement of the test callback's own body. A computed option value or a t.skip() inside an if is a platform guard and stays silent. The marker lookup is the same lookback window as for .skip(.
…able throw check In no-message-only-throw-assertion, a class pinned on the same subject no longer excuses a bare toThrow() (a later call on that subject is often a different scenario that any error satisfies), and it only excuses a message check when the pin sits in the same or an enclosing block, so a pin in one if branch no longer covers the other branch or the code after the if.
…t checks more than the message
no-message-only-throw-assertion paired a message check with any
.rejects.toEqual / toStrictEqual / toMatchObject / toHaveProperty on the
same subject. Jest's equals() compares two errors by message only, so
under trustErrorInstances: false an error-instance argument to toEqual,
toStrictEqual or toMatchObject no longer pins the class; toMatchObject
with only a message key and toHaveProperty('message') never do. A throw
matcher given expect.objectContaining({ message }) is now a message check.
…rs it no-sleep-in-unit-tests exempted any setTimeout whose handle was stored, so `const t = setTimeout(resolve, 100)` with no clearTimeout(t) was a silent sleep. A kept handle now counts as a cancellable deadline only when the same file passes it to clearTimeout or clearInterval. The doc also says a sleep helper is seen in its own file, not where an import of it is called.
…cuous-expect The render-root check took any `container` or `<x>.container`, so a sole `expect(container).not.toBeNull()` on a docker inspection reported under recommended. A root now has to come from a call: destructured from one, read off one (or off a binding initialised from one), or returned by a render* function.
no-real-clock-in-unit-tests reported new Date() in a file that fakes
the clock with vi.stubGlobal('Date', FakeDate). A stubGlobal whose first
argument is 'Date' now counts, next to the existing spy, replaceProperty
and fake-timer forms. The doc says how to handle a clock library such as
MockDate that the rule does not know.
…hrow-assertion toThrowErrorMatchingSnapshot and toThrowErrorMatchingInlineSnapshot record only the error's message, so a mock's TypeError with the same wording passes them. They are now in the default throwMatchers and are always a message check, paired with a class pin like any other.
…f mocks
typed-mock-over-double-cast only saw `as unknown as T` and `as any as T`.
It now also reports a double cast through `never`, and the angle-bracket
forms `<T><unknown>{ ... }` and `<T>({ ... } as unknown)`.
skipped-tests-need-tracking missed `await t.skip()` at the top of a
node:test callback, and treated only `true` and non-empty strings as an
unconditional skip option. An awaited context skip is now the same
statement as a plain one, and any truthy literal (`{ skip: 1 }`) counts.
Shironex
added
new rule
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Four new opt-in test-discipline rules in
@noctcore/eslint-plugin-code-quality, and two existingrecommendedrules that now report more:no-message-only-throw-assertion(opt-in): a throw assertion with no argument, or with only a message (string, template, regex, message snapshot), sync or after.rejects. Any error satisfies it, including aTypeErrorfrom a broken mock. A class argument, an asymmetric matcher and a class pin on the same subject in the same or an enclosing block are accepted; a baretoThrow()is never excused.no-sleep-in-unit-tests(opt-in): a real sleep in a unit test (new Promise(r => setTimeout(r, n)),timers/promises,promisify(setTimeout)). Zero delays, reject-only timeout guards, deadlines whose handle the file clears, and files that install fake timers are left alone.no-real-clock-in-unit-tests(opt-in):Date.now()/ arglessnew Date()in a unit test file that never fakes the clock; offsets from now are allowed.typed-mock-over-double-cast(opt-in): an object literal containingjest.fn()/vi.fn()cast throughunknown,anyornever.no-vacuous-expect(inrecommended): now reports a test whose only assertion is that the render root is present.renderRoots: []restores the old behaviour.skipped-tests-need-tracking(inrecommended): now reads unconditionalnode:testskips ({ skip: true },t.skip()); computed platform guards stay silent.Each rule was measured read-only on a large consumer monorepo (936 test files) before and after an adversarial review; the review's proven evasions (a class pin excusing a bare throw, Jest's message-only
toEqualon errors, a kept timer handle that is never cleared) are fixed with a failing case written first.Checklist
.changeset/(code-quality-test-discipline.md, one minor), written for a consumer reading the changelog, that says whether a project spreadingrecommendedwill see new errors.bun run build && bun run typecheck && bun run testis green locally (testruns on ESLint 10 and then 9; code-quality 393 cases).ts/tsxfence in the new and changeddocs/rules/*.mdis labelledbad,goodorprose, and they ran (bunx vitest run tests/docs/plugins.test.ts -t <rule>inpackages/eslint-test-utils).EXPECTED_RULE_COUNTinsite/scripts/parity.test.tsis 117,bun run docs:readmesregenerated the README table and doc headers, and the four new rules are named inOMITTED_FROM_PRESETSwith a reason.