fix(tests): resolve Windows test failures and upgrade prek by GeorgeDong32 · Pull Request #13619 · CherryHQ/cherry-studio

GeorgeDong32 · 2026-03-19T05:39:26Z

Summary

Fix Windows test failures and upgrade prek dependency to resolve git hook issues on Windows.

Problem

On Windows, the pre-commit hook was failing due to a bug in prek v0.2.28 where command lookup didn't work correctly with Windows paths. This caused but commit and regular git commit to fail on Windows, while working fine on macOS and Linux.

Changes

Dependencies

@j178/prek: v0.2.28 → v0.3.4

This upgrade fixes the Windows command lookup bug (#1383) that caused pre-commit hooks to fail.

Test Fixes

Use path.join() for platform-independent path construction in DxtService tests
Skip path-matching tests on Windows due to path format differences
Adjust configManager.set call count expectations based on platform
Fix execFileSync mock expectation to use expect.any(Object)
Use path.normalize() for cross-platform path comparison in BaseService test

Test Plan

pnpm test:main - 33 test files passed, 575 tests passed
pnpm lint - passed
Pre-commit hook working on Windows

Refs

Fix command lookup on Windows j178/prek#1383 (Windows command lookup bug fix)

Fix cross-platform test compatibility issues: - Use path.join() for platform-independent path construction in DxtService tests - Skip path-matching tests on Windows due to path format differences - Adjust configManager.set call count expectations based on platform - Fix execFileSync mock expectation to use expect.any(Object) Affected test files: - src/main/services/__tests__/DxtService.test.ts - src/main/services/__tests__/BackupManager.deleteTempBackup.test.ts - src/main/utils/__tests__/process.test.ts

Fix Windows command lookup bug and improve cross-platform compatibility. Changes: - Upgrade @j178/prek from v0.2.28 to v0.3.4 - This fixes Windows-specific pre-commit hook issues Refs: j178/prek#1383

Fix BaseService test that failed on Windows due to path.normalize converting forward slashes to backslashes on Windows. The test now uses path.normalize() in the expected value to ensure cross-platform compatibility.

GeorgeDong32

Note

This review was translated by Claude.

Code Review: Skipped Tests in BackupManager.test.ts

🔍 Problem Analysis

There is a logic error in the current test file that prevents tests from being skipped correctly on Windows:

// Problematic code (lines 103-104, 243-244)
describe('Normal Operations', () => {
  const itFn = process.platform === 'win32' ? it.skip : it  // ❌ Evaluated at definition time
  itFn('should delete valid file in allowed directory', async () => { ... })
})

Root Cause: itFn is evaluated during the test definition phase, while the process.platform mock in beforeEach only takes effect during the test execution phase. Therefore, itFn always makes judgments based on the actual platform, causing tests to be incorrectly skipped on Windows.

💡 Suggested Solutions

Solution 1: Mock the `path` Module (Recommended)

Force all tests to use POSIX path format to completely eliminate platform differences:

vi.mock('path', () => ({
  ...jest.requireActual('path'),
  posix: jest.requireActual('path').posix,
  // Force use of POSIX methods
  join: (...args) => args.join('/'),
  normalize: (p) => p.replace(/\\/+/g, '/'),
  resolve: (...args) => args.join('/'),
  sep: '/'
}))

Solution 2: Use `describe.skipIf` + Platform-Agnostic Assertions

Refer to the correct usage in src/main/utils/__tests__/process.test.ts:

describe.skipIf(process.platform === 'win32')('POSIX-only path tests', () => {
  it('should handle path correctly', () => {
    // Use path.join() to construct cross-platform paths
    const testPath = path.join('/tmp', 'cherry-studio', 'lan-transfer', 'file.zip')
    expect(result).toBe(true)
  })
})

Solution 3: Platform-Agnostic Test Assertions

it('should delete valid file in allowed directory', async () => {
  // Use path.join to construct test path
  const validPath = path.join(app.getPath('temp'), 'cherry-studio', 'lan-transfer', 'backup.zip')
  
  vi.mocked(fs.pathExists).mockResolvedValue(true as never)
  vi.mocked(fs.remove).mockResolvedValue(undefined as never)
  
  const result = await backupManager.deleteLanTransferBackup({} as any, validPath)
  expect(result).toBe(true)
})

📋 Other Findings

File	Status	Notes
`DxtService.test.ts`	✅ Correctly uses `path.join()`	No skipped tests
`BaseService.test.ts`	✅ Correctly uses `path.normalize()`	Cross-platform compatible
`process.test.ts`	✅ Correctly uses `describe.skipIf`	Windows-specific tests

🎯 Suggested Actions

Short-term: Current skip logic can be retained (tests will still be skipped on Windows CI)
Medium-term: Refactor tests using Solution 1 or Solution 3 to enable running on all platforms
Long-term: Establish CI platform matrix to ensure test coverage for macOS/Linux/Windows

Overall Assessment: Comprehensive safety test coverage, only need to improve cross-platform testing strategy.

Original Content

Code Review: Skipped Tests in BackupManager.test.ts

🔍 问题分析

当前测试文件中存在一个逻辑错误，导致 Windows 上的测试被 skip 后无法正确执行：

// 问题代码 (lines 103-104, 243-244)
describe('Normal Operations', () => {
  const itFn = process.platform === 'win32' ? it.skip : it  // ❌ 在定义时求值
  itFn('should delete valid file in allowed directory', async () => { ... })
})

根本原因：itFn 在测试定义阶段就被求值，而 beforeEach 中的 process.platform mock 只在测试执行阶段生效。因此 itFn 始终基于真实平台做判断，导致 Windows 上测试被错误地 skip。

💡 建议方案

方案一：Mock `path` 模块（推荐）

强制所有测试使用 POSIX 路径格式，彻底消除平台差异：

vi.mock('path', () => ({
  ...jest.requireActual('path'),
  posix: jest.requireActual('path').posix,
  // 强制使用 POSIX 方法
  join: (...args) => args.join('/'),
  normalize: (p) => p.replace(/\\/+/g, '/'),
  resolve: (...args) => args.join('/'),
  sep: '/'
}))

方案二：使用 `describe.skipIf` + 平台无关断言

参考 src/main/utils/__tests__/process.test.ts 的正确用法：

describe.skipIf(process.platform === 'win32')('POSIX-only path tests', () => {
  it('should handle path correctly', () => {
    // 使用 path.join() 构造跨平台路径
    const testPath = path.join('/tmp', 'cherry-studio', 'lan-transfer', 'file.zip')
    expect(result).toBe(true)
  })
})

方案三：平台无关的测试断言

it('should delete valid file in allowed directory', async () => {
  // 使用 path.join 构造测试路径
  const validPath = path.join(app.getPath('temp'), 'cherry-studio', 'lan-transfer', 'backup.zip')
  
  vi.mocked(fs.pathExists).mockResolvedValue(true as never)
  vi.mocked(fs.remove).mockResolvedValue(undefined as never)
  
  const result = await backupManager.deleteLanTransferBackup({} as any, validPath)
  expect(result).toBe(true)
})

📋 其他发现

文件	状态	备注
`DxtService.test.ts`	✅ 正确使用 `path.join()`	无 skipped tests
`BaseService.test.ts`	✅ 正确使用 `path.normalize()`	跨平台兼容
`process.test.ts`	✅ 正确使用 `describe.skipIf`	Windows 专用测试

🎯 建议行动

短期：当前 skip 逻辑可以保留（测试在 Windows CI 上仍会 skip）
中期：采用方案一或方案三重构测试，使所有平台都能运行
长期：建立 CI 平台矩阵，确保 macOS/Linux/Windows 都有测试覆盖

总体评价：安全测试覆盖全面，仅需改进跨平台测试策略。

Mock path module in BackupManager tests to force POSIX path behavior across all platforms. This eliminates the need to skip tests on Windows. Changes: - Replace process.platform mocking with comprehensive path module mock - Force all path operations to use forward slashes (POSIX style) - Handle path.resolve specially to avoid Windows drive letter prefix - Remove all it.skip for path-matching tests This approach (suggested in code review) ensures tests run on all platforms without skipping any functionality.

Fix lint error: import() type annotations are forbidden. Use type-only import syntax instead.

### What this PR does Release Cherry Studio v1.8.2 Before this PR: - Version 1.8.1 is the latest release After this PR: - Version 1.8.2 with bug fixes and model updates  Fixes # ### Why we need it and why it was done in this way This is a patch release that includes: - Bug fixes for knowledge base, message search, and backup restoration - New model support for Xiaomi MiMo-V2-Pro and MiMo-V2-Omni - MiniMax default model upgrade to M2.7 The following tradeoffs were made: - N/A — this is a scheduled patch release The following alternatives were considered: - N/A — standard release workflow Links to places where the discussion took place:  ### Breaking changes  None. This is a patch release with no breaking changes. ### Special notes for your reviewer  **Release Review Checklist:** - [ ] Review generated release notes in `electron-builder.yml` - [ ] Verify version bump in `package.json` (1.8.1 → 1.8.2) - [ ] CI passes - [ ] Merge to trigger release build **Included Commits:** - 8124236 fix(config): update app upgrade segments and include new gateway version - 62c1eb2 fix: correct parameter order in knowledgeSearchTool call (#13635) - bb3dec9 fix(MessageHeader): crash when clicking topic in message search (#13627) - 24645d3 fix(tests): resolve Windows test failures and upgrade prek (#13619) - 0b08fd9 refactor: remove manual install update logic and related API calls - f517a07 fix(BackupManager): update data destination path for backup restoration - f658484 refactor(ConfigManager): remove legacy config migration logic - c1c1b34 chore: Add CI check scripts to package.json (#13564) - 78decc4 feat(model): add support for MiMo-V2-Pro and MiMo-V2-Omni (#13613) - d081b05 fix: Format provider API hosts in API server (#13198) - e4112ba feat: upgrade MiniMax default model to M2.7 (#13593) ### Checklist This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR. Approvers are expected to review this list. - [x] PR: The PR description is expressive enough and will help future contributors - [ ] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [ ] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html) - [ ] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [ ] Self-review: I have reviewed my own code (e.g., via [`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`, or GitHub UI) before requesting review from others ### Release note  ```release-note Cherry Studio 1.8.2 - Bug Fixes and Model Updates 🐛 Bug Fixes - [Knowledge Base] Fix knowledge base content not being delivered to the model when selected in conversation - [Search] Fix crash when clicking topic title in message search results - [Backup] Fix backup restoration path issue ✨ New Features - [Models] Add support for Xiaomi MiMo-V2-Pro and MiMo-V2-Omni models with reasoning control and tool use capabilities 💄 Improvements - [Models] Upgrade MiniMax default model to M2.7 with enhanced reasoning and coding capabilities ``` Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

GeorgeDong32 added 3 commits March 18, 2026 22:02

chore(deps): upgrade prek to v0.3.4

ad13abb

Fix Windows command lookup bug and improve cross-platform compatibility. Changes: - Upgrade @j178/prek from v0.2.28 to v0.3.4 - This fixes Windows-specific pre-commit hook issues Refs: j178/prek#1383

fix(tests): use path.normalize for cross-platform path comparison

39c33d9

Fix BaseService test that failed on Windows due to path.normalize converting forward slashes to backslashes on Windows. The test now uses path.normalize() in the expected value to ensure cross-platform compatibility.

DeJeune approved these changes Mar 19, 2026

View reviewed changes

GeorgeDong32 commented Mar 19, 2026

View reviewed changes

GeorgeDong32 added 2 commits March 19, 2026 14:33

fix: use type-only import for path module mock

6ba875a

Fix lint error: import() type annotations are forbidden. Use type-only import syntax instead.

kangfenmao approved these changes Mar 19, 2026

View reviewed changes

kangfenmao merged commit 24645d3 into main Mar 19, 2026
7 checks passed

kangfenmao deleted the fix/windows-test-failures branch March 19, 2026 08:21

kangfenmao mentioned this pull request Mar 19, 2026

chore: release v1.8.2 #13639

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tests): resolve Windows test failures and upgrade prek#13619

fix(tests): resolve Windows test failures and upgrade prek#13619
kangfenmao merged 5 commits intomainfrom
fix/windows-test-failures

GeorgeDong32 commented Mar 19, 2026

Uh oh!

GeorgeDong32 left a comment •

edited by kangfenmao

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

GeorgeDong32 commented Mar 19, 2026

Summary

Problem

Changes

Dependencies

Test Fixes

Test Plan

Refs

Uh oh!

GeorgeDong32 left a comment • edited by kangfenmao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review: Skipped Tests in BackupManager.test.ts

🔍 Problem Analysis

💡 Suggested Solutions

Solution 1: Mock the path Module (Recommended)

Solution 2: Use describe.skipIf + Platform-Agnostic Assertions

Solution 3: Platform-Agnostic Test Assertions

📋 Other Findings

🎯 Suggested Actions

Code Review: Skipped Tests in BackupManager.test.ts

🔍 问题分析

💡 建议方案

方案一：Mock path 模块（推荐）

方案二：使用 describe.skipIf + 平台无关断言

方案三：平台无关的测试断言

📋 其他发现

🎯 建议行动

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GeorgeDong32 left a comment •

edited by kangfenmao

Loading

Solution 1: Mock the `path` Module (Recommended)

Solution 2: Use `describe.skipIf` + Platform-Agnostic Assertions

方案一：Mock `path` 模块（推荐）

方案二：使用 `describe.skipIf` + 平台无关断言