Skip to content

fix(desktop): fix electron import error in bounds-validation test#687

Merged
Kitenite merged 2 commits into
mainfrom
fix-test
Jan 9, 2026
Merged

fix(desktop): fix electron import error in bounds-validation test#687
Kitenite merged 2 commits into
mainfrom
fix-test

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Jan 9, 2026

Summary

  • Fixed bounds-validation.test.ts failing with "Export named 'screen' not found in module electron"
  • The issue was caused by static imports being resolved before mock.module could intercept them
  • Changed to use mock.module for electron before dynamic import, following the same pattern used in terminal manager tests

Test plan

  • Verified all 973 tests pass after the fix
  • Confirmed the bounds-validation test suite (31 tests) runs successfully

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests

  • Enhanced test setup and coverage for window bounds validation across multiple display configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

The test was failing with "Export named 'screen' not found" because
static imports are resolved before mock.module can intercept them.

Changed to use mock.module for electron before dynamic import, following
the same pattern used in terminal manager tests. This ensures the mock
is in place before the bounds-validation module (which imports electron)
is loaded.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 9, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The test file for bounds-validation is reworked to properly mock Electron's screen API before module import. The setup defers importing the tested module using dynamic imports and establishes mockScreen with display methods, uses mock.module for Electron injection, and introduces a MockType alias for improved type safety in test casts.

Changes

Cohort / File(s) Summary
Test Infrastructure Refactoring
apps/desktop/src/main/lib/window-state/bounds-validation.test.ts
Restructures test setup to mock Electron screen API before module import. Introduces dynamic import deferral, mockScreen object with getPrimaryDisplay/getAllDisplays methods, MockType alias for type casts, and updated mock.module injection pattern. Changes: +52/-20

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Mocks are set before we test,
Deferring imports, doing best,
Screen displays now properly mocked,
Test gates securely locked!
Types precise with MockType's might

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0e24f8 and 27bd67a.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • apps/desktop/src/main/lib/window-state/bounds-validation.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite Kitenite merged commit 5089a77 into main Jan 9, 2026
5 of 6 checks passed
@Kitenite Kitenite deleted the fix-test branch January 9, 2026 04:06
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 9, 2026

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Fly.io Electric (Fly.io) View App
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

saddlepaddle pushed a commit that referenced this pull request Jan 10, 2026
* fix(desktop): use dynamic import for electron in bounds-validation test

The test was failing with "Export named 'screen' not found" because
static imports are resolved before mock.module can intercept them.

Changed to use mock.module for electron before dynamic import, following
the same pattern used in terminal manager tests. This ensures the mock
is in place before the bounds-validation module (which imports electron)
is loaded.
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