mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-22 21:38:52 +00:00
d96be3d668
## Summary Add a shared `assert(condition, message)` utility in `src/base/` that centralizes DEV-throw / Desktop-Sentry / nightly-toast / `console.error` policy for invariant reporting across the codebase. ## Changes - **`src/base/assert.ts`**: New `assert()` utility with `setAssertReporter()` registration pattern - `console.error` always fires on failure - Throws `Error` in DEV mode (surfaces bugs immediately) - Delegates to registered reporter otherwise (Sentry, toast, etc.) - No imports from `platform/` — respects layer architecture (`base → platform → workbench → renderer`) - **`src/main.ts`**: Registers Sentry + nightly-toast reporter after `Sentry.init()` - **`src/scripts/changeTracker.ts`**: Migrates `reportInactiveTrackerCall()` to use `assert()`, removing inline `Sentry.captureMessage` + `console.warn` calls - **`src/scripts/changeTracker.test.ts`**: Mocks `@/base/assert` to prevent DEV-mode throws in existing no-op tests ## Testing ### Automated - `src/base/assert.test.ts` — 6 tests covering: no-op on true, console.error on false, DEV throw, non-DEV no-throw, reporter invocation, reporter not called on true - `src/scripts/changeTracker.test.ts` — 16 tests all pass (pre-existing) - Coverage: 100% for assert.ts ### E2E Verification Steps 1. Run `pnpm test:unit` — all tests pass 2. Build the app and open browser devtools 3. In DEV mode: trigger a lifecycle violation (call an inactive tracker method) — should see error thrown in console 4. In production build: same trigger — should see `console.error` only, no throw ## Review Focus - `setAssertReporter()` is called in `main.ts` once at startup — appropriate for a singleton reporter. In tests that import `assert`, the reporter is reset to a no-op in `afterEach`. - Layer architecture respected: `base/assert.ts` has zero imports, upper layers wire in side effects via `setAssertReporter()`. Fixes #11373 <!-- Pipeline-Ticket: pick-issue-3410 --> ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11824-feat-3410-add-centralized-assert-utility-in-src-base-3546d73d3650819d96afdf4018161c26) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Connor Byrne <c.byrne@comfy.org>
92 lines
2.7 KiB
TypeScript
92 lines
2.7 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import { assert, setAssertReporter } from '@/base/assert'
|
|
|
|
describe('assert', () => {
|
|
let consoleErrorSpy: ReturnType<typeof vi.spyOn>
|
|
|
|
beforeEach(() => {
|
|
consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
})
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks()
|
|
vi.unstubAllEnvs()
|
|
setAssertReporter(null)
|
|
})
|
|
|
|
it('does nothing when condition is true', () => {
|
|
expect(() => assert(true, 'should not throw')).not.toThrow()
|
|
expect(consoleErrorSpy).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('logs console.error when condition is false', () => {
|
|
vi.stubEnv('DEV', false)
|
|
assert(false, 'test message')
|
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
|
'[Assertion failed]: test message'
|
|
)
|
|
})
|
|
|
|
it('throws in DEV mode when condition is false', () => {
|
|
vi.stubEnv('DEV', true)
|
|
const reporter = vi.fn()
|
|
setAssertReporter(reporter)
|
|
expect(() => assert(false, 'dev error')).toThrow(
|
|
'[Assertion failed]: dev error'
|
|
)
|
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
|
'[Assertion failed]: dev error'
|
|
)
|
|
expect(reporter).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('does not throw in non-DEV mode when condition is false', () => {
|
|
vi.stubEnv('DEV', false)
|
|
expect(() => assert(false, 'non-dev error')).not.toThrow()
|
|
})
|
|
|
|
it('calls registered reporter in non-DEV mode with formatted message', () => {
|
|
vi.stubEnv('DEV', false)
|
|
const reporter = vi.fn()
|
|
setAssertReporter(reporter)
|
|
assert(false, 'reporter message')
|
|
expect(reporter).toHaveBeenCalledWith(
|
|
'[Assertion failed]: reporter message'
|
|
)
|
|
})
|
|
|
|
it('does not call reporter when condition is true', () => {
|
|
vi.stubEnv('DEV', false)
|
|
const reporter = vi.fn()
|
|
setAssertReporter(reporter)
|
|
assert(true, 'no call')
|
|
expect(reporter).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('handles null reporter gracefully in non-DEV mode', () => {
|
|
vi.stubEnv('DEV', false)
|
|
setAssertReporter(null)
|
|
expect(() => assert(false, 'null reporter')).not.toThrow()
|
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
|
'[Assertion failed]: null reporter'
|
|
)
|
|
})
|
|
|
|
it('swallows reporter exceptions in non-DEV mode', () => {
|
|
vi.stubEnv('DEV', false)
|
|
const reporter = vi.fn(() => {
|
|
throw new Error('reporter blew up')
|
|
})
|
|
setAssertReporter(reporter)
|
|
expect(() => assert(false, 'safe under reporter failure')).not.toThrow()
|
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
|
'[Assertion failed]: safe under reporter failure'
|
|
)
|
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
|
'[Assertion reporter failed]',
|
|
expect.any(Error)
|
|
)
|
|
})
|
|
})
|