Skip to content

Commit 7e95328

Browse files
zimegClaude
andcommitted
fix(cli-test): use shell:false on all platforms
Using shell:true causes issues with special characters like # being interpreted as comments. Spawning the CLI binary directly with shell:false avoids both the Windows Docker hang and the need for outer-quote hacks to protect shell metacharacters. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
1 parent 9bbce7a commit 7e95328

2 files changed

Lines changed: 29 additions & 61 deletions

File tree

packages/cli-test/src/cli/shell.test.ts

Lines changed: 28 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('shell module', () => {
6565
const fakeArgs = ['"hi there"'];
6666
shell.runCommandSync(fakeCmd, fakeArgs);
6767
sandbox.assert.calledOnce(assembleSpy);
68-
const expectedShell = process.platform !== 'win32';
68+
const expectedShell = false;
6969
sandbox.assert.calledWithMatch(
7070
runSpy,
7171
sinon.match.string,
@@ -86,35 +86,19 @@ describe('shell module', () => {
8686
shell.runCommandSync('about to explode', []);
8787
}, /this is bat country/);
8888
});
89-
if (process.platform === 'win32') {
90-
it('on Windows, should spawn the command directly without cmd.exe', () => {
91-
const fakeEnv = { HEY: 'yo' };
92-
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
93-
const fakeCmd = 'echo';
94-
const fakeArgs = ['"hi there"'];
95-
shell.runCommandSync(fakeCmd, fakeArgs);
96-
sandbox.assert.calledWithMatch(
97-
runSpy,
98-
fakeCmd,
99-
sinon.match.array.contains(fakeArgs),
100-
sinon.match({ shell: false, env: fakeEnv }),
101-
);
102-
});
103-
} else {
104-
it('on non-Windows, should shell out to provided command directly', () => {
105-
const fakeEnv = { HEY: 'yo' };
106-
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
107-
const fakeCmd = 'echo';
108-
const fakeArgs = ['"hi there"'];
109-
shell.runCommandSync(fakeCmd, fakeArgs);
110-
sandbox.assert.calledWithMatch(
111-
runSpy,
112-
fakeCmd,
113-
sinon.match.array.contains(fakeArgs),
114-
sinon.match({ shell: true, env: fakeEnv }),
115-
);
116-
});
117-
}
89+
it('should spawn the command directly without a shell', () => {
90+
const fakeEnv = { HEY: 'yo' };
91+
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
92+
const fakeCmd = 'echo';
93+
const fakeArgs = ['"hi there"'];
94+
shell.runCommandSync(fakeCmd, fakeArgs);
95+
sandbox.assert.calledWithMatch(
96+
runSpy,
97+
fakeCmd,
98+
sinon.match.array.contains(fakeArgs),
99+
sinon.match({ shell: false, env: fakeEnv }),
100+
);
101+
});
118102
});
119103

120104
describe('spawnProcess method', () => {
@@ -125,7 +109,7 @@ describe('shell module', () => {
125109
const fakeArgs = ['"hi there"'];
126110
shell.spawnProcess(fakeCmd, fakeArgs);
127111
sandbox.assert.calledOnce(assembleSpy);
128-
const expectedShell = process.platform !== 'win32';
112+
const expectedShell = false;
129113
sandbox.assert.calledWithMatch(
130114
spawnSpy,
131115
sinon.match.string,
@@ -148,35 +132,19 @@ describe('shell module', () => {
148132
shell.spawnProcess('about to explode', []);
149133
}, /this is bat country/);
150134
});
151-
if (process.platform === 'win32') {
152-
it('on Windows, should spawn the command directly without cmd.exe', () => {
153-
const fakeEnv = { HEY: 'yo' };
154-
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
155-
const fakeCmd = 'echo';
156-
const fakeArgs = ['"hi there"'];
157-
shell.spawnProcess(fakeCmd, fakeArgs);
158-
sandbox.assert.calledWithMatch(
159-
spawnSpy,
160-
fakeCmd,
161-
sinon.match.array.contains(fakeArgs),
162-
sinon.match({ shell: false, env: fakeEnv }),
163-
);
164-
});
165-
} else {
166-
it('on non-Windows, should shell out to provided command directly', () => {
167-
const fakeEnv = { HEY: 'yo' };
168-
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
169-
const fakeCmd = 'echo';
170-
const fakeArgs = ['"hi there"'];
171-
shell.spawnProcess(fakeCmd, fakeArgs);
172-
sandbox.assert.calledWithMatch(
173-
spawnSpy,
174-
fakeCmd,
175-
sinon.match.array.contains(fakeArgs),
176-
sinon.match({ shell: true, env: fakeEnv }),
177-
);
178-
});
179-
}
135+
it('should spawn the command directly without a shell', () => {
136+
const fakeEnv = { HEY: 'yo' };
137+
sandbox.stub(shell, 'assembleShellEnv').returns(fakeEnv);
138+
const fakeCmd = 'echo';
139+
const fakeArgs = ['"hi there"'];
140+
shell.spawnProcess(fakeCmd, fakeArgs);
141+
sandbox.assert.calledWithMatch(
142+
spawnSpy,
143+
fakeCmd,
144+
sinon.match.array.contains(fakeArgs),
145+
sinon.match({ shell: false, env: fakeEnv }),
146+
);
147+
});
180148
});
181149

182150
describe('waitForOutput method', () => {

packages/cli-test/src/cli/shell.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ function getSpawnArguments(
277277
command,
278278
args,
279279
{
280-
shell: true,
280+
shell: false,
281281
env,
282282
...shellOpts,
283283
},

0 commit comments

Comments
 (0)