Skip to content

fix: improve Windows compatibility in tests#646

Merged
TabishB merged 1 commit intoFission-AI:mainfrom
wenxiaoyu:fix/windows-test-compatibility
Feb 2, 2026
Merged

fix: improve Windows compatibility in tests#646
TabishB merged 1 commit intoFission-AI:mainfrom
wenxiaoyu:fix/windows-test-compatibility

Conversation

@wenxiaoyu
Copy link
Contributor

@wenxiaoyu wenxiaoyu commented Feb 2, 2026

  • Replace Unix 'cat' command with fs.readFile in spec.test.ts

  • Replace 'mkdir -p' and 'bash' commands with fs.mkdir/writeFile in validate.enriched-output.test.ts

  • Skip symlink test on Windows (requires admin privileges) in file-system.test.ts

Summary by CodeRabbit

  • Tests
    • Updated test infrastructure with asynchronous file operations for improved reliability and maintainability.
    • Enhanced Windows platform compatibility by adding conditional test skipping for platform-specific scenarios.

- Replace Unix 'cat' command with fs.readFile in spec.test.ts

- Replace 'mkdir -p' and 'bash' commands with fs.mkdir/writeFile in validate.enriched-output.test.ts

- Skip symlink test on Windows (requires admin privileges) in file-system.test.ts
@wenxiaoyu wenxiaoyu requested a review from TabishB as a code owner February 2, 2026 06:13
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

This pull request converts synchronous file operations in test files to asynchronous ones using Promise-based APIs and adds a Windows platform-specific skip condition to a symbolic link test.

Changes

Cohort / File(s) Summary
Async test conversions
test/commands/spec.test.ts, test/commands/validate.enriched-output.test.ts
Converted test functions from synchronous to async, replacing execSync file reads and shell-based setup with Promise-based fs operations (fs.readFile, fs.mkdir, fs.writeFile).
Platform-specific test skipping
test/utils/file-system.test.ts
Added skipIf wrapper to skip symbolic link test on Windows platform using process.platform check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Async hops through test files with grace,
No more sync blocks clogging up the place,
Windows gets a gentle skip with care,
Promises await, floating through the air! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective of the PR: improving Windows compatibility by replacing Unix-specific commands and skipping platform-specific tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@vibe-kanban-cloud
Copy link

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

@greptile-apps
Copy link

greptile-apps bot commented Feb 2, 2026

Greptile Overview

Greptile Summary

Improved Windows compatibility in test suite by replacing Unix-specific shell commands with cross-platform Node.js filesystem APIs.

  • Replaced cat command with fs.readFile in spec.test.ts
  • Replaced mkdir -p and bash heredoc with fs.mkdir and fs.writeFile in validate.enriched-output.test.ts
  • Conditionally skipped symlink test on Windows in file-system.test.ts (creating symlinks requires admin privileges or Developer Mode on Windows)

All changes maintain test functionality while ensuring cross-platform compatibility.

Confidence Score: 5/5

  • Safe to merge - all changes improve test portability without affecting functionality
  • Changes are isolated to test files, use well-established Node.js APIs, and maintain equivalent functionality while improving cross-platform compatibility
  • No files require special attention

Important Files Changed

Filename Overview
test/commands/spec.test.ts Replaced Unix cat command with cross-platform fs.readFile for file reading
test/commands/validate.enriched-output.test.ts Replaced mkdir -p and bash commands with Node.js fs.mkdir and fs.writeFile APIs
test/utils/file-system.test.ts Conditionally skipped symlink test on Windows due to privilege requirements

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant FS as fs.promises
    participant OS as Operating System
    
    Note over Test,OS: spec.test.ts (file reading)
    Test->>FS: readFile(path, 'utf-8')
    FS->>OS: Read file content
    OS-->>FS: File content
    FS-->>Test: UTF-8 string
    
    Note over Test,OS: validate.enriched-output.test.ts (file creation)
    Test->>FS: mkdir(changePath, {recursive: true})
    FS->>OS: Create directory
    OS-->>FS: Success
    FS-->>Test: Directory created
    Test->>FS: writeFile(filePath, content)
    FS->>OS: Write file
    OS-->>FS: Success
    FS-->>Test: File written
    
    Note over Test,OS: file-system.test.ts (Windows check)
    Test->>Test: Check process.platform === 'win32'
    alt Windows platform
        Test->>Test: Skip symlink test
    else Non-Windows
        Test->>FS: symlink(target, link)
        FS->>OS: Create symbolic link
        OS-->>FS: Success
        FS-->>Test: Symlink created
    end
Loading

Copy link
Contributor

@TabishB TabishB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@TabishB TabishB merged commit 62d4391 into Fission-AI:main Feb 2, 2026
10 checks passed
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.

2 participants