Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates Electron unit tests to address Vitest mocking pitfalls (mock hoisting / spying on Node builtins) that can cause runtime errors in the test suite.
Changes:
- Adjust
node:fsimport style in the window-state unit test to supportvi.spyOnusage. - Refactor the
electronmodule mock in the tray unit test to avoid Vitest hoisting/TDZ issues and update the related assertion. - Refactor the
electronmodule mock setup in the menu unit test (currently introduces a syntax/shape error).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| electron/tests/unit/main/window-state.test.ts | Switches fs import style used by spies in window-state tests. |
| electron/tests/unit/main/tray.test.ts | Makes Tray mock hoist-safe by defining it inside the vi.mock factory; updates assertion accordingly. |
| electron/tests/unit/main/menu.test.ts | Introduces a shared mockApp, but the vi.mock factory object is currently malformed (parse-breaking). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vi.mock('electron', () => { | ||
| const MockTrayConstructorInline = vi.fn((icon: unknown) => { | ||
| createdTrayIcon = icon; | ||
| mockTrayInstance = createMockTray(icon); | ||
| return mockTrayInstance; | ||
| }); |
There was a problem hiding this comment.
This mock now defines MockTrayConstructorInline inside the vi.mock factory, but the file still keeps the top-level MockTrayConstructor with identical behavior. Since it’s no longer used, it adds duplication and makes it unclear which mock should be asserted against; remove the unused top-level constructor (or reuse a single hoist-safe mock) to keep the test maintainable.
Description
Type of Change
Related Issues
Screenshots (if applicable)
Testing
Checklist