Fix Tests to run properly on Windows (RooCodeInc#1963)

* fix: remove -p flag from test script to prevent git operation errors

The -p flag in npm-run-all was causing tests to run in parallel, which led to 'Cannot log after tests are done' errors with git operations. These errors don't appear when running test:extension alone.

The issue occurs because git-based tests create temporary directories and run async operations that can interfere with each other when executed in parallel. Running tests sequentially resolves this cleanly.

While it might increase total test time slightly, it ensures more reliable and consistent test results.

* refactor(terminal): improve mock streams and fix test issues

- Create shell-specific mock streams (bash, cmd, pwsh) with proper line ending handling
- Fix open handles in tests by properly managing timeouts
- Standardize stderr redirection across all shell implementations using stdio option
- Improve test reliability and output cleanliness

* fix(tests): add skipVerification option to PowerShell tests to debug Linux issues

* fix(tests): use explicit variable name in PowerShell test to fix Linux compatibility

* Refactor terminal tests to use purpose-based approach instead of command mapping

* Remove reference to non-existent test file

* fix: use printf instead of echo -e for more consistent behavior across platforms

* fix: use single quotes for PowerShell commands to preserve variables on Linux

* Update code-qa workflow to run tests on both Windows and Ubuntu

* fix: use platform-specific PowerShell command execution for Linux and Windows

* Fix toggleToolAlwaysAllow to handle path normalization for cross-platform compatibility

* Fix McpHub tests to handle normalized paths on Windows

* Suppress console.error messages in McpHub tests

* fix: make Bedrock ARN regex patterns Windows-compatible

Fixed an issue where AWS Bedrock tests were timing out on Windows but passing on Linux. The root cause was path separator handling in regex patterns used for model ID extraction from ARNs.

1. Updated model ID extraction regex to handle both forward slashes (Linux) and backslashes (Windows)
2. Modified ARN matching regex to be platform-independent
3. Ensured consistent region prefix handling for all supported regions

This change maintains functionality while ensuring cross-platform compatibility.

* fix: make WorkspaceTracker test cross-platform compatible

Fixed an issue where the WorkspaceTracker test 'should initialize with workspace files' was failing on Windows but passing on Linux. The problem was in the mock implementation of toRelativePath that only handled forward slashes.

- Updated the toRelativePath mock to use path.relative which properly handles platform-specific path separators
- Ensured all paths are converted to forward slashes for consistency in test assertions
- The fix maintains cross-platform compatibility while preserving the test's intent

* fix: make WorkspaceTracker tests cross-platform compatible

Fixed cross-platform compatibility issues in the WorkspaceTracker tests that were causing failures on Windows but passing on Linux:

1. Updated the toRelativePath mock implementation to:
   - Use path.relative which properly handles platform-specific path separators
   - Convert paths to forward slashes for consistency in test assertions

2. Enhanced the 'should not update file paths' test to be platform-agnostic by:
   - Using more flexible assertions that don't depend on specific path formats
   - Checking file path length and content rather than exact string matches
   - Properly typed the test assertions to fix TypeScript errors

These changes preserve the test intent while ensuring they run successfully across different operating systems.

* fix: make McpHub tests cross-platform compatible

Fixed cross-platform compatibility issues in the McpHub tests that were causing failures on Windows but passing on Linux:

1. Made the toggleToolAlwaysAllow tests more platform-agnostic by:
   - No longer relying on specific path formats which differ between Windows and Linux
   - Using the last write call instead of searching for a specific path string
   - Adding more robust assertions that verify structure instead of exact path matches
   - Properly handling array existence checks

2. These tests would fail on Windows because paths are formatted with backslashes instead of
   forward slashes, causing path equality checks to fail.

The changes maintain test intent while ensuring cross-platform compatibility.

* handle escaping of slash and quote

* fix: ensure consistent line endings in git fallback strategy

Fixed an issue where tests would fail on GitHub Windows runners but pass on local Windows machines due to line ending differences. The fix ensures consistent line ending handling by:

1. Normalizing CRLF to LF when reading files in the git fallback strategy
2. Disabling Git's automatic line ending conversion
3. Maintaining consistent line ending usage throughout text operations

* feat: run tests sequentially on Windows, parallel otherwise

Author: StevenTCramer

.changeset/curvy-cows-cry.md

@@ -0,0 +1,11 @@
+---
+"roo-cline": patch
+---
+
+fix: ensure consistent line endings in git fallback strategy
+
+Fixed a cross-platform issue where tests would fail on GitHub Windows runners but pass on local Windows machines due to line ending differences. The fix ensures consistent line ending handling by:
+
+1. Normalizing CRLF to LF when reading files in the git fallback strategy
+2. Disabling Git's automatic line ending conversion using core.autocrlf setting
+3. Maintaining consistent line ending usage throughout the text operations

.github/workflows/code-qa.yml

@@ -62,7 +62,10 @@ jobs:
         run: npm run knip
 
   test-extension:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest]
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -77,7 +80,10 @@ jobs:
         run: npx jest --silent
 
   test-webview:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest]
     steps:
       - name: Checkout code
         uses: actions/checkout@v4

jest.config.js

@@ -19,6 +19,12 @@ module.exports = {
 		],
 	},
 	testMatch: ["**/__tests__/**/*.test.ts"],
+	// Platform-specific test configuration
+	testPathIgnorePatterns: [
+		// Skip platform-specific tests based on environment
+		...(process.platform === "win32" ? [".*\\.bash\\.test\\.ts$"] : [".*\\.cmd\\.test\\.ts$"]),
+		// PowerShell tests are conditionally skipped in the test files themselves using the setupFilesAfterEnv
+	],
 	moduleNameMapper: {
 		"^vscode$": "<rootDir>/src/__mocks__/vscode.js",
 		"@modelcontextprotocol/sdk$": "<rootDir>/src/__mocks__/@modelcontextprotocol/sdk/index.js",
@@ -39,4 +45,5 @@ module.exports = {
 	modulePathIgnorePatterns: [".vscode-test"],
 	reporters: [["jest-simple-dot-reporter", {}]],
 	setupFiles: ["<rootDir>/src/__mocks__/jest.setup.ts"],
+	setupFilesAfterEnv: ["<rootDir>/src/integrations/terminal/__tests__/setupTerminalTests.ts"],
 }

package.json

@@ -331,7 +331,7 @@
 		"package": "npm-run-all -p build:webview build:esbuild check-types lint",
 		"pretest": "npm run compile",
 		"dev": "cd webview-ui && npm run dev",
-		"test": "npm-run-all -p test:*",
+		"test": "node scripts/run-tests.js",
 		"test:extension": "jest",
 		"test:webview": "cd webview-ui && npm run test",
 		"prepare": "husky",

scripts/run-tests.js

@@ -0,0 +1,8 @@
+// run-tests.js
+const { execSync } = require("child_process")
+
+if (process.platform === "win32") {
+	execSync("npm-run-all test:*", { stdio: "inherit" })
+} else {
+	execSync("npm-run-all -p test:*", { stdio: "inherit" })
+}

src/api/providers/__tests__/bedrock.test.ts

@@ -736,15 +736,39 @@ describe("AwsBedrockHandler", () => {
 				awsRegion: "us-east-1",
 			})
 
-			const mockStreamEvent = {
-				trace: {
-					promptRouter: {
-						invokedModelId: "arn:aws:bedrock:us-east-1:123456789:foundation-model/default-model:0",
+			// Create a spy on the getModelByName method
+			const getModelByNameSpy = jest.spyOn(handler, "getModelByName")
+
+			// Mock the BedrockRuntimeClient.prototype.send method
+			const mockSend = jest.spyOn(BedrockRuntimeClient.prototype, "send").mockImplementationOnce(async () => {
+				return {
+					stream: {
+						[Symbol.asyncIterator]: async function* () {
+							// First yield a trace event with invokedModelId
+							yield {
+								trace: {
+									promptRouter: {
+										invokedModelId:
+											"arn:aws:bedrock:us-east-1:123456789:foundation-model/default-model:0",
+									},
+								},
+							}
+							// Then yield the metadata event (required to finish the stream processing)
+							yield {
+								metadata: {
+									usage: {
+										inputTokens: 10,
+										outputTokens: 20,
+									},
+								},
+							}
+						},
 					},
-				},
-			}
+				}
+			})
 
-			jest.spyOn(handler, "getModel").mockReturnValue({
+			// Mock getModel to provide a test model config
+			const getModelSpy = jest.spyOn(handler, "getModel").mockReturnValue({
 				id: "default-model",
 				info: {
 					maxTokens: 4096,
@@ -754,7 +778,24 @@ describe("AwsBedrockHandler", () => {
 				},
 			})
 
-			await handler.createMessage("system prompt", [{ role: "user", content: "user message" }]).next()
+			// Collect all yielded events to ensure completion
+			const events = []
+			const messageGenerator = handler.createMessage("system prompt", [{ role: "user", content: "user message" }])
+
+			// Use a timeout to prevent test hanging
+			const timeout = 1000
+			const startTime = Date.now()
+
+			while (true) {
+				if (Date.now() - startTime > timeout) {
+					throw new Error("Test timed out waiting for stream to complete")
+				}
+
+				const result = await messageGenerator.next()
+				events.push(result.value)
+
+				if (result.done) break
+			}
 
 			expect(handler.getModel()).toEqual({
 				id: "default-model",

src/api/providers/bedrock.ts

@@ -289,15 +289,25 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
 				if (streamEvent?.trace?.promptRouter?.invokedModelId) {
 					try {
 						const invokedModelId = streamEvent.trace.promptRouter.invokedModelId
-						const modelMatch = invokedModelId.match(/\/([^\/]+)(?::|$)/)
+						// Create a platform-independent regex that doesn't use forward slash as both delimiter and matcher
+						const modelMatch = invokedModelId.match(new RegExp("[\\/\\\\]([^\\/\\\\]+)(?::|$)"))
 						if (modelMatch && modelMatch[1]) {
 							let modelName = modelMatch[1]
 
 							// Get a new modelConfig from getModel() using invokedModelId.. remove the region first
 							let region = modelName.slice(0, 3)
 
+							// Check for all region prefixes (us., eu., and apac prefix)
 							if (region === "us." || region === "eu.") modelName = modelName.slice(3)
+							else if (modelName.startsWith("apac.")) modelName = modelName.slice(5)
 							this.costModelConfig = this.getModelByName(modelName)
+
+							// Log successful model extraction to help with debugging
+							logger.debug("Successfully extracted model from invokedModelId", {
+								ctx: "bedrock",
+								invokedModelId,
+								extractedModelName: modelName,
+							})
 						}
 
 						// Handle metadata events for the promptRouter.
@@ -513,15 +523,19 @@ Please check:
 
 		// If custom ARN is provided, use it
 		if (this.options.awsCustomArn) {
-			// Extract the model name from the ARN
+			// Extract the model name from the ARN using platform-independent regex
 			const arnMatch = this.options.awsCustomArn.match(
-				/^arn:aws:bedrock:([^:]+):(\d+):(inference-profile|foundation-model|provisioned-model)\/(.+)$/,
+				new RegExp(
+					"^arn:aws:bedrock:([^:]+):(\\d+):(inference-profile|foundation-model|provisioned-model|default-prompt-router|prompt-router)[/\\\\](.+)$",
+				),
 			)
 
 			let modelName = arnMatch ? arnMatch[4] : ""
 			if (modelName) {
 				let region = modelName.slice(0, 3)
+				// Check for all region prefixes (us., eu., and apac prefix)
 				if (region === "us." || region === "eu.") modelName = modelName.slice(3)
+				else if (modelName.startsWith("apac.")) modelName = modelName.slice(5)
 
 				let modelData = this.getModelByName(modelName)
 				modelData.id = this.options.awsCustomArn

src/core/diff/strategies/new-unified/edit-strategies.ts

@@ -157,6 +157,8 @@ export async function applyGitFallback(hunk: Hunk, content: string[]): Promise<E
 		await git.init()
 		await git.addConfig("user.name", "Temp")
 		await git.addConfig("user.email", "[email protected]")
+		// Prevent Git from automatically converting line endings
+		await git.addConfig("core.autocrlf", "false")
 
 		const filePath = path.join(tmpDir.name, "file.txt")
 
@@ -168,6 +170,7 @@ export async function applyGitFallback(hunk: Hunk, content: string[]): Promise<E
 			.filter((change) => change.type === "context" || change.type === "add")
 			.map((change) => change.originalLine || change.indent + change.content)
 
+		// Ensure consistent line endings (LF only) in all text operations
 		const searchText = searchLines.join("\n")
 		const replaceText = replaceLines.join("\n")
 		const originalText = content.join("\n")
@@ -195,7 +198,9 @@ export async function applyGitFallback(hunk: Hunk, content: string[]): Promise<E
 				await git.raw(["cherry-pick", "--minimal", replaceCommit.commit])
 
 				const newText = fs.readFileSync(filePath, "utf-8")
-				const newLines = newText.split("\n")
+				// Normalize line endings to LF before splitting
+				const normalizedText = newText.replace(/\r\n/g, "\n")
+				const newLines = normalizedText.split("\n")
 				return {
 					confidence: 1,
 					result: newLines,
@@ -212,6 +217,8 @@ export async function applyGitFallback(hunk: Hunk, content: string[]): Promise<E
 			await git.init()
 			await git.addConfig("user.name", "Temp")
 			await git.addConfig("user.email", "[email protected]")
+			// Prevent Git from automatically converting line endings
+			await git.addConfig("core.autocrlf", "false")
 
 			fs.writeFileSync(filePath, searchText)
 			await git.add("file.txt")
@@ -237,7 +244,9 @@ export async function applyGitFallback(hunk: Hunk, content: string[]): Promise<E
 				await git.raw(["cherry-pick", "--minimal", replaceHash])
 
 				const newText = fs.readFileSync(filePath, "utf-8")
-				const newLines = newText.split("\n")
+				// Normalize line endings to LF before splitting
+				const normalizedText = newText.replace(/\r\n/g, "\n")
+				const newLines = normalizedText.split("\n")
 				return {
 					confidence: 1,
 					result: newLines,

src/extension.ts

@@ -116,7 +116,7 @@ export async function activate(context: vscode.ExtensionContext) {
 	registerTerminalActions(context)
 
 	// Allows other extensions to activate once Roo is ready.
-	vscode.commands.executeCommand('roo-cline.activationCompleted');
+	vscode.commands.executeCommand("roo-cline.activationCompleted")
 
 	// Implements the `RooCodeAPI` interface.
 	return new API(outputChannel, provider)