Always set shell to cmd.exe on Windows

Shopify · Oct 25, 2024 · ffcb4b4 · ffcb4b4
1 parent 4afc68b
commit ffcb4b4
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 17 deletions.
diff --git a/vscode/src/ruby/versionManager.ts b/vscode/src/ruby/versionManager.ts
@@ -88,7 +88,9 @@ export abstract class VersionManager {
     // If the user has configured a default shell, we use that one since they are probably sourcing their version
     // manager scripts in that shell's configuration files. On Windows, we never set the shell no matter what to ensure
     // that activation runs on `cmd.exe` and not PowerShell, which avoids complex quoting and escaping issues.
-    if (vscode.env.shell.length > 0 && os.platform() !== "win32") {
+    if (os.platform() === "win32") {
+      shell = "cmd.exe";
+    } else if (vscode.env.shell.length > 0) {
       shell = vscode.env.shell;
     }
 

diff --git a/vscode/src/test/suite/ruby/custom.test.ts b/vscode/src/test/suite/ruby/custom.test.ts
@@ -42,7 +42,7 @@ suite("Custom", () => {
     const { env, version, yjit } = await custom.activate();
 
     // We must not set the shell on Windows
-    const shell = os.platform() === "win32" ? undefined : vscode.env.shell;
+    const shell = os.platform() === "win32" ? "cmd.exe" : vscode.env.shell;
 
     assert.ok(
       execStub.calledOnceWithExactly(

diff --git a/vscode/src/test/suite/ruby/none.test.ts b/vscode/src/test/suite/ruby/none.test.ts
@@ -39,7 +39,7 @@ suite("None", () => {
     const { env, version, yjit } = await none.activate();
 
     // We must not set the shell on Windows
-    const shell = os.platform() === "win32" ? undefined : vscode.env.shell;
+    const shell = os.platform() === "win32" ? "cmd.exe" : vscode.env.shell;
 
     assert.ok(
       execStub.calledOnceWithExactly(

diff --git a/vscode/src/test/suite/ruby/rubyInstaller.test.ts b/vscode/src/test/suite/ruby/rubyInstaller.test.ts
@@ -12,7 +12,6 @@ import { RubyInstaller } from "../../../ruby/rubyInstaller";
 import { WorkspaceChannel } from "../../../workspaceChannel";
 import { LOG_CHANNEL } from "../../../common";
 import { RUBY_VERSION } from "../../rubyVersion";
-import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager";
 
 suite("RubyInstaller", () => {
   if (os.platform() !== "win32") {
@@ -104,7 +103,7 @@ suite("RubyInstaller", () => {
     });
   });
 
-  test("Doesn't set the shell when invoking activation script", async () => {
+  test("Sets shell to cmd.exe when invoking activation script", async () => {
     const [major, minor, _patch] = RUBY_VERSION.split(".").map(Number);
     fs.symlinkSync(
       path.join(
@@ -121,20 +120,18 @@ suite("RubyInstaller", () => {
     fs.writeFileSync(path.join(workspacePath, ".ruby-version"), RUBY_VERSION);
 
     const windows = new RubyInstaller(workspaceFolder, outputChannel);
-    const result = ["/fake/dir", "/other/fake/dir", true, RUBY_VERSION].join(
-      ACTIVATION_SEPARATOR,
-    );
-    const execStub = sinon.stub(common, "asyncExec").resolves({
-      stdout: "",
-      stderr: result,
-    });
+    const execSpy = sinon.spy(common, "asyncExec");
+    const { env, version, yjit } = await windows.activate();
+    execSpy.restore();
 
-    await windows.activate();
-    execStub.restore();
+    assert.strictEqual(execSpy.callCount, 1);
+    const callArgs = execSpy.getCall(0).args;
+    assert.strictEqual(callArgs[1]?.shell, "cmd.exe");
 
-    assert.strictEqual(execStub.callCount, 1);
-    const callArgs = execStub.getCall(0).args;
-    assert.strictEqual(callArgs[1]?.shell, undefined);
+    assert.match(env.GEM_PATH!, /ruby\/3\.3\.0/);
+    assert.match(env.GEM_PATH!, /lib\/ruby\/gems\/3\.3\.0/);
+    assert.strictEqual(version, RUBY_VERSION);
+    assert.notStrictEqual(yjit, undefined);
 
     fs.rmSync(path.join(os.homedir(), `Ruby${major}${minor}-${os.arch()}`), {
       recursive: true,