Skip to content

chore(ci): run tests on windows#319

Merged
binoy14 merged 9 commits into
mainfrom
01-21-chore_ci_run_tests_on_windows
Jan 28, 2026
Merged

chore(ci): run tests on windows#319
binoy14 merged 9 commits into
mainfrom
01-21-chore_ci_run_tests_on_windows

Conversation

@binoy14
Copy link
Copy Markdown
Contributor

@binoy14 binoy14 commented Jan 22, 2026

TL;DR

Add Windows support to the Sanity CLI by fixing path handling and test compatibility issues.

What changed?

  • Added cross-platform path handling throughout the codebase
  • Created a normalizePath utility function to standardize path formats
  • Updated tests to work on both Windows and Unix systems
  • Added Windows to the CI test matrix to ensure compatibility
  • Fixed path-specific issues in file operations and test assertions
  • Updated snapshot serialization to handle Windows line endings
  • Fixed CLI output formatting for Windows command line

How to test?

  • Run the test suite on a Windows machine
  • Verify CLI commands work correctly on Windows
  • Check that path handling works properly across platforms
  • Confirm that the GitHub Actions workflow runs successfully on both Ubuntu and Windows

Why make this change?

The Sanity CLI previously had issues on Windows due to path format differences between operating systems. This change ensures that the CLI works consistently across platforms by properly handling path separators, line endings, and other OS-specific behaviors. Adding Windows to the CI test matrix helps catch platform-specific issues early in the development process.

Copy link
Copy Markdown
Contributor Author

binoy14 commented Jan 22, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown

claude Bot commented Jan 22, 2026

Claude finished @binoy14's task in 3m 26s —— View job


Windows Path Handling Issues - Patch Ready

I've identified 3 critical Windows path handling bugs and created a patch file to fix them.


Issues Found

1. execScript.ts:46 - URL .pathname creates invalid Windows paths
2. getViteConfig.ts:188 - Incomplete Windows node_modules path checking
3. tsxWorkerTask.ts:43 - Environment variable uses invalid Windows path format

All three issues stem from using .pathname on file URLs, which produces /C:/path on Windows instead of C:\path. The fix uses Node's fileURLToPath() for correct cross-platform conversion.


Patch File: windows-fixes.patch

The patch file has been created in the repository root. You can copy the content below and save it as windows-fixes.patch, then apply:

git apply windows-fixes.patch

Patch contents:

diff --git a/packages/@sanity/cli/src/actions/exec/execScript.ts b/packages/@sanity/cli/src/actions/exec/execScript.ts
index existing..new 100644
--- a/packages/@sanity/cli/src/actions/exec/execScript.ts
+++ b/packages/@sanity/cli/src/actions/exec/execScript.ts
@@ -1,7 +1,7 @@
 import {spawn} from 'node:child_process'
 import fs from 'node:fs/promises'
 import path from 'node:path'
-import {pathToFileURL} from 'node:url'
+import {pathToFileURL, fileURLToPath} from 'node:url'

 import {packageDirectory} from 'package-directory'

@@ -43,7 +43,7 @@ export async function execScript(options: ExecScriptOptions): Promise<void> {
   try {
     // Resolve the tsx loader using Node's module resolution relative to package.json
     const tsxPackageUrl = import.meta.resolve('tsx/package.json', import.meta.url)
-    tsxLoaderPath = new URL('dist/loader.mjs', tsxPackageUrl).pathname
+    tsxLoaderPath = fileURLToPath(new URL('dist/loader.mjs', tsxPackageUrl))
   } catch {
     throw new Error('@sanity/cli not able to resolve tsx loader')
   }
diff --git a/packages/@sanity/cli/src/actions/build/getViteConfig.ts b/packages/@sanity/cli/src/actions/build/getViteConfig.ts
index existing..new 100644
--- a/packages/@sanity/cli/src/actions/build/getViteConfig.ts
+++ b/packages/@sanity/cli/src/actions/build/getViteConfig.ts
@@ -185,7 +185,9 @@ function suppressUnusedImport(warning: Rollup.RollupLog & {ids?: string[]}): bo
   }

   // If some library does something unexpected, we suppress since it isn't actionable
-  if (warning.ids?.every((id) => id.includes('/node_modules/') || id.includes('\\node_modules\\')))
+  if (warning.ids?.every((id) => {
+    const normalizedId = id.replaceAll('\\', '/')
+    return normalizedId.includes('/node_modules/')
+  }))
     return true

   return false
diff --git a/packages/@sanity/cli-core/src/loaders/tsx/tsxWorkerTask.ts b/packages/@sanity/cli-core/src/loaders/tsx/tsxWorkerTask.ts
index existing..new 100644
--- a/packages/@sanity/cli-core/src/loaders/tsx/tsxWorkerTask.ts
+++ b/packages/@sanity/cli-core/src/loaders/tsx/tsxWorkerTask.ts
@@ -1,4 +1,4 @@
-import {URL} from 'node:url'
+import {URL, fileURLToPath} from 'node:url'
 import {Worker, type WorkerOptions} from 'node:worker_threads'

 import {getTsconfig} from 'get-tsconfig'
@@ -40,7 +40,7 @@ export function tsxWorkerTask<T = unknown>(
   const env = {
     ...(isRecord(options.env) ? options.env : process.env),
     ...(tsconfig?.path ? {TSX_TSCONFIG_PATH: tsconfig.path} : {}),
-    TSX_WORKER_TASK_SCRIPT: filePath.pathname,
+    TSX_WORKER_TASK_SCRIPT: fileURLToPath(filePath),
   }

   const worker = new Worker(new URL('tsxWorkerLoader.worker.js', import.meta.url), {

What These Fixes Do

  • Import fileURLToPath from node:url in all three files
  • Replace .pathname with fileURLToPath() for proper Windows path conversion
  • Normalize paths in getViteConfig before checking for node_modules

These changes ensure proper Windows path handling while maintaining Unix compatibility. The existing normalizePath utility and test infrastructure are solid.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 22, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 73.82% 5973 / 8091
🔵 Statements 73.13% 6125 / 8375
🔵 Functions 67.25% 908 / 1350
🔵 Branches 61.02% 2654 / 4349
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/@sanity/cli-core/src/loaders/tsx/tsxWorkerTask.ts 85% 66.66% 87.5% 84.21% 63-65
packages/@sanity/cli-core/src/util/normalizePath.ts 100% 100% 100% 100%
packages/@sanity/cli/src/actions/build/getViteConfig.ts 100% 97.61% 100% 100%
packages/@sanity/cli/src/actions/exec/execScript.ts 0% 0% 0% 0% 19-74
packages/@sanity/cli/src/telemetry/store/findTelemetryFiles.ts 66.66% 0% 100% 66.66% 33-37
Generated in workflow #1784 for commit 6737036 by the Vitest Coverage Report Action

@binoy14 binoy14 changed the base branch from main to graphite-base/319 January 23, 2026 20:36
@binoy14 binoy14 force-pushed the 01-21-chore_ci_run_tests_on_windows branch from 59452b4 to 8d7b10c Compare January 23, 2026 20:36
@binoy14 binoy14 changed the base branch from graphite-base/319 to sdk-735 January 23, 2026 20:36
Base automatically changed from sdk-735 to main January 23, 2026 22:43
@binoy14 binoy14 force-pushed the 01-21-chore_ci_run_tests_on_windows branch from 8d7b10c to 1512644 Compare January 24, 2026 03:16
@binoy14
Copy link
Copy Markdown
Contributor Author

binoy14 commented Jan 24, 2026

@claude can you compile a list of tests that are failing on windows and create a plan on how to fix them?

@claude

This comment was marked as outdated.

@binoy14
Copy link
Copy Markdown
Contributor Author

binoy14 commented Jan 24, 2026

@claude Start implementing the fix. Maybe think if you can make shared utils for anything for ease

@claude

This comment was marked as resolved.

@binoy14
Copy link
Copy Markdown
Contributor Author

binoy14 commented Jan 24, 2026

@claude no commit is pushed, can you just create .patch that I can copy paste and apply

@claude

This comment was marked as resolved.

@binoy14 binoy14 force-pushed the 01-21-chore_ci_run_tests_on_windows branch 7 times, most recently from 01a4988 to 0b2f4ab Compare January 27, 2026 03:30
@binoy14 binoy14 marked this pull request as ready for review January 27, 2026 04:53
@binoy14 binoy14 requested a review from a team as a code owner January 27, 2026 04:53
@binoy14 binoy14 requested review from rexxars and somerandomdude and removed request for a team January 27, 2026 04:53
@binoy14 binoy14 force-pushed the 01-21-chore_ci_run_tests_on_windows branch from ea9bc97 to 6737036 Compare January 27, 2026 20:59
Copy link
Copy Markdown
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

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

Kudos for persisting on this, had no idea it would take this amount of changes!

const {stdout} = await runCommand('init --help')

expect(stdout).toMatchInlineSnapshot(String.raw`
expect(stdout).toMatchInlineSnapshot(`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are fun - what caused these changes? Will they survive a snapshot update, I wonder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There was a change to lint rule which caused autofix adding String.raw to all snapshot tests but the problem with using that is --u or watch mode update didn't fix these. I removed most of them but I think these were missed in rebase. It should be okay 🤞

@binoy14 binoy14 merged commit df7e276 into main Jan 28, 2026
56 of 62 checks passed
@binoy14 binoy14 deleted the 01-21-chore_ci_run_tests_on_windows branch January 28, 2026 17:10
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