Skip to content

test(server): skip the pathspec-magic filename test on Windows - #15651

Open
OhadC wants to merge 1 commit into
pingdotgg:mainfrom
OhadC:skip-pathspec-test-on-windows
Open

OhadC wants to merge 1 commit into
pingdotgg:mainfrom
OhadC:skip-pathspec-test-on-windows

Conversation

@OhadC

@OhadC OhadC commented Oct 4, 2026

Copy link
Copy Markdown

Problem

GitVcsDriverCore.test.ts › "keeps untracked filenames with pathspec magic in the review" (added in #8086) always fails on Windows. It writes a file named :(exclude)after.ts, and NTFS never allows : in a file name, so the setup step fails with ENOENT before any assertion runs:

PlatformError: NotFound: FileSystem.writeFile (…\git-vcs-driver-test-…\:(exclude)after.ts)

Change

Skip the test on Windows with it.effect.skipIf(HostProcessPlatform.defaultValue() === "win32"), the same guard this file already uses for the newline-in-path test. The diff is mostly re-indentation from wrapping the test.

Scope and approval

A very small, focused fix of an obvious test bug: the file name the test needs can't exist on Windows, so there is no Windows behavior for it to check. The other platforms still run it unchanged.

Verification

On Windows 11, Node 24:

  • Before: vp test run src/vcs/GitVcsDriverCore.test.ts (from apps/server) fails this test with the error above.
  • After: vp test run src/vcs/GitVcsDriverCore.test.ts -t "review diff previews" gives 29 passed and the rest skipped, including this test. No failures.

I couldn't run it on macOS or Linux, but nothing changes there.

Done with Claude Opus 5.5 in Claude Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at db6a886

Macroscope's review found this PR approvable — This is a single test-file change confined to an ignored path, adjusting only Windows test execution without affecting production behavior, product defaults, or static-analysis settings. Its narrowly scoped test-harness impact makes it low risk.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/internals/effect-services.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4954a008-4fea-4487-adb9-a51319ea2db5
📥 Commits

Reviewing files that changed from the base of the PR and between 4ee6bfd and db6a886.

📒 Files selected for processing (1)
  • apps/server/src/vcs/GitVcsDriverCore.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The untracked-pathspec review test now skips on Windows. On other platforms, it checks preview contents and verifies that previewing leaves the Git index unchanged.

Changes

Untracked pathspec test

Layer / File(s) Summary
Platform-aware pathspec assertions
apps/server/src/vcs/GitVcsDriverCore.test.ts
The test skips on Windows. On other platforms, it checks full and file-scoped previews and confirms that previewing does not change the Git index.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to db6a8

This test-only change skips a filename case unsupported on Windows and retains its checks on other platforms. No production behavior changes, so no actionable merge-blocking risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to db6a8

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/vcs/GitVcsDriverCore.test.ts: The pathspec-magic untracked-filename test now uses skipIf when the host platform is Windows; on other platforms it retains its full and file-scoped preview assertions and verifies the index remains unchanged.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: skipping the pathspec-magic filename test on Windows.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and verification. It also states that macOS and Linux testing was not performed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant