Skip to content

test(desktop): fix ~55 ambient failing suites (electron node:test TS imports, jsdom harness, packaging scripts) - #274

Merged
Kyzcreig merged 4 commits into
mainfrom
fix/desktop-test-suite-cleanup
Jul 10, 2026
Merged

Kyzcreig merged 4 commits into
mainfrom
fix/desktop-test-suite-cleanup

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Cleans up the pre-existing desktop test debt (all reproduce on pristine fork/main; NOT recent regressions). Baseline: node:test platform suite fully red (ERR_MODULE_NOT_FOUND on extensionless TS imports), vitest 54 failed files / 44 failed tests.

Classes fixed

  1. electron node:test TS import resolution (37 suites): explicit .ts extensions on relative imports in electron/ + allowImportingTsExtensions in tsconfig.electron.json. Node's type-stripping requires explicit extensions for ESM resolution. Verified the esbuild bundler (bundle-electron-main.mjs) still resolves them — 470KB main bundle builds clean.
  2. vitest jsdom harness (10 suites): shared src/test/setup.ts (canvas getContext stub, localStorage/state isolation cleanup), assertion drift fixes, vitest config block in vite.config.ts.
  3. packaging script tests (3): brought scripts/*.test.mjs into vitest discovery.

Verification (independent re-run, not worker narration)

  • npm run test:desktop:platforms: 320 passed / 0 failed / 1 skipped (platform-conditional)
  • npx vitest run --environment jsdom: 156 files, 1231 tests, 0 failed
  • npm run typecheck: clean
  • node scripts/bundle-electron-main.mjs: bundles clean (product import-specifier change is bundler-safe)

No tests deleted; only pre-existing platform-conditional skips remain.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR fixes ~55 pre-existing desktop test failures across three categories: electron node:test TS import resolution (37 suites), vitest jsdom harness (10 suites), and packaging script test discovery (3 suites). No tests were deleted; only assertion drift and harness wiring were corrected.

  • Electron import resolution: All relative imports in electron/main.ts and sibling modules now carry explicit .ts extensions, and tsconfig.electron.json gains rewriteRelativeImportExtensions: true (TS 5.7) so the emitted JS rewrites them back to .js — compatible with composite: true and declaration: true.
  • jsdom harness: A new src/test/setup.ts wires per-test localStorage isolation, a canvas getContext stub, requestAnimationFrame/CSS.escape polyfills, and registers via setupFiles in vite.config.ts; scripts/**/*.test.mjs files are added to the vitest include glob to bring packaging tests into discovery.
  • Assertion drift: Several vitest test assertions are corrected to match current implementation behavior (added explicit_only, source: 'desktop', PROMPT_SUBMIT_TIMEOUT_MS, etc.).

Confidence Score: 5/5

Safe to merge — all changes are test infrastructure with no production code paths touched beyond import specifier strings.

The tsconfig fix (rewriteRelativeImportExtensions) correctly addresses the prior concern about emit compatibility; the assertion drift corrections mirror actual implementation behavior that was independently verified by the author; the new setup.ts properly isolates tests between runs. Nothing in this PR alters runtime behavior.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/tsconfig.electron.json Adds rewriteRelativeImportExtensions: true (TS 5.7) — correctly rewrites .ts → .js on emit, compatible with composite: true/declaration: true, no TS5096 conflict.
apps/desktop/electron/main.ts All relative imports updated to explicit .ts extensions; esbuild bundler resolves both forms, so the production bundle is unaffected.
apps/desktop/src/test/setup.ts New shared vitest setup: per-test localStorage/sessionStorage clear, canvas mock with per-test call-count reset, and browser polyfills; correctly addresses the isolation concerns from the previous review round.
apps/desktop/vite.config.ts Adds vitest test block with jsdom URL, setupFiles, and an extended include glob that pulls in scripts/**/*.test.mjs.
apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx Adds PROMPT_SUBMIT_TIMEOUT_MS as a third argument to requestGateway in all prompt.submit assertions, and updates session.resume expectation to include source: 'desktop' — both match current implementation signatures.
apps/desktop/src/app/settings/model-settings.test.tsx Wraps ModelSettings in QueryClientProvider (now required) and stubs the new setApiRequestProfile hermes export — correct for the current component API.
apps/desktop/scripts/assert-dist-built.test.mjs Migrated from node:assert/node:test to vitest expect API; logic is unchanged, vitest discovery now picks it up via the new include glob.
apps/desktop/scripts/before-pack.test.mjs Same node:test → vitest migration as assert-dist-built; doesNotReject replaced by resolves.toBeUndefined() which is slightly stricter but correct given the hook's contract.
apps/desktop/src/lib/model-options.test.ts Adds explicit_only: true to expected gateway and REST call arguments, tracking the new parameter added to the production requestModelOptions implementation.
apps/desktop/src/store/onboarding.test.ts Switches /api/model/options path matching from strict equality to startsWith to tolerate query parameters appended by the updated implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test Run Entry] --> B{Runner?}
    B -->|node:test| C[electron/*.test.ts]
    B -->|vitest| D[src/**/*.test.tsx\nscripts/**/*.test.mjs]

    C --> E[Import with .ts extension\ne.g. './backend-command.ts']
    E --> F[Node type-stripping ESM\nresolves explicit extension ✓]

    D --> G[vite.config.ts test block\nsetupFiles: src/test/setup.ts]
    G --> H[setup.ts]
    H --> H1[localStorage.clear per test]
    H --> H2[canvasContext.mockClear per test]
    H --> H3[requestAnimationFrame polyfill]
    H --> H4[CSS.escape polyfill]

    subgraph tsconfig.electron.json
        I[rewriteRelativeImportExtensions: true]
        I --> J['.ts' imports rewritten to '.js' on emit\ncompatible with composite: true]
    end

    C --> I
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Test Run Entry] --> B{Runner?}
    B -->|node:test| C[electron/*.test.ts]
    B -->|vitest| D[src/**/*.test.tsx\nscripts/**/*.test.mjs]

    C --> E[Import with .ts extension\ne.g. './backend-command.ts']
    E --> F[Node type-stripping ESM\nresolves explicit extension ✓]

    D --> G[vite.config.ts test block\nsetupFiles: src/test/setup.ts]
    G --> H[setup.ts]
    H --> H1[localStorage.clear per test]
    H --> H2[canvasContext.mockClear per test]
    H --> H3[requestAnimationFrame polyfill]
    H --> H4[CSS.escape polyfill]

    subgraph tsconfig.electron.json
        I[rewriteRelativeImportExtensions: true]
        I --> J['.ts' imports rewritten to '.js' on emit\ncompatible with composite: true]
    end

    C --> I
Loading

Reviews (4): Last reviewed commit: "fix(desktop): TS5096 (rewriteRelativeImp..." | Re-trigger Greptile

Comment thread apps/desktop/tsconfig.electron.json Outdated
Comment thread apps/desktop/src/test/setup.ts
Comment thread apps/desktop/src/test/setup.ts
@Kyzcreig
Kyzcreig force-pushed the fix/desktop-test-suite-cleanup branch from eddb516 to 64317a2 Compare July 10, 2026 22:12
@Kyzcreig
Kyzcreig force-pushed the fix/desktop-test-suite-cleanup branch from 64317a2 to ce4325c Compare July 10, 2026 22:22
@Kyzcreig
Kyzcreig merged commit 1171388 into main Jul 10, 2026
24 checks passed
@Kyzcreig
Kyzcreig deleted the fix/desktop-test-suite-cleanup branch July 10, 2026 22:31
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.

1 participant