Comfy-Org / Comfy-Org/ComfyUI_frontend

test: lint against module-scope vi.stubGlobal and vi.spyOn

Open
#15,308 1 comment 1 reaction 0 assignees View on GitHub
area:testing developer experience refactor
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Problem

Once #14836 lands, `unstubGlobals` and `restoreMocks` silently disarm any stub or spy installed at module scope. The reset runs in `onBeforeTryTask`, which is before the first test of every file, so a module scope `vi.stubGlobal` or `vi.spyOn` is gone by the time any test body runs.

Most instances fail loudly and get caught. The dangerous ones do not:

- a stub whose only assertion is negative (`expect(fetchMock).not.toHaveBeenCalled()`) becomes vacuously true and passes forever
- a `vi.stubGlobal('fetch', ...)` that dies means the test issues a real network request instead

Both shapes were live in #14836 and were only found by hand (`initHostTelemetry.test.ts`, `LogsTerminal.test.ts`, `authStore.test.ts`). There is nothing stopping the next one.

## Suggested fix

Lint for it. Something like this in `eslint.config.ts`, scoped to test files:

```js
'no-restricted-syntax': [
'error',
{
selector: "Program > ExpressionStatement > CallExpression[callee.object.name='vi'][callee.property.name=/^(stubGlobal|spyOn)$/]",
message: 'Install stubs and spies in beforeEach. unstubGlobals and restoreMocks remove module scope ones before each test.'
}
]
```

Also worth covering `const x = vi.spyOn(...)` at module scope, which is the same hazard with an extra failure mode: the handle survives and can be re-armed in `beforeEach`, but it is detached from the object, so the real method runs and the spy records nothing. That was the `downloadUtil.test.ts` case.

After #14836 the rule should be close to clean, so adoption is cheap.

## Context

Found while reviewing #14836. Not blocking that PR.

Contributor guide

Open the contributing guide

Research direction

Start in eslint.config.ts and inspect how rules are scoped to test files. Review the mentioned cases in initHostTelemetry.test.ts, LogsTerminal.test.ts, authStore.test.ts, and downloadUtil.test.ts, then run the repository lint command. Done means module-scope vi.stubGlobal and vi.spyOn usages, including assigned spy handles, are reported and the existing test files are clean.

Written by the indexing model from the issue text.

Assessment

Tech stack
eslint, typescript
Domain
testing, tooling
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.