Skip to content

Commit 521578b

Browse files
committed
fix: address review feedback - use two commands for conda.sh and add tests
1 parent c9b09b9 commit 521578b

File tree

2 files changed

+195
-5
lines changed

2 files changed

+195
-5
lines changed

src/managers/conda/condaUtils.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,12 @@ async function generateShellActivationMapFromConfig(
582582
return { shellActivation, shellDeactivation };
583583
}
584584

585-
async function windowsExceptionGenerateConfig(
585+
/**
586+
* Generates shell-specific activation configuration for Windows.
587+
* Handles PowerShell, CMD, and Git Bash with appropriate scripts.
588+
* @internal Exported for testing
589+
*/
590+
export async function windowsExceptionGenerateConfig(
586591
sourceInitPath: string,
587592
prefix: string,
588593
condaFolder: string,
@@ -600,10 +605,16 @@ async function windowsExceptionGenerateConfig(
600605
const pwshActivate = [{ executable: activation }, { executable: 'conda', args: ['activate', quotedPrefix] }];
601606
const cmdActivate = [{ executable: sourceInitPath }, { executable: 'conda', args: ['activate', quotedPrefix] }];
602607

603-
// Use conda.sh for bash-based shells (Git Bash) instead of activate.bat
604-
// conda.sh is the proper initialization script for bash shells
605-
const bashSourcePath = condaShPath ?? sourceInitPath;
606-
const bashActivate = [{ executable: 'source', args: [bashSourcePath.replace(/\\/g, '/'), quotedPrefix] }];
608+
// When condaShPath is available, it is an initialization script (conda.sh) and does not
609+
// itself activate an environment. In that case, first source conda.sh, then
610+
// run "conda activate <envIdentifier>". When falling back to sourceInitPath,
611+
// retain the existing single "source <activate-script> <env>" behavior.
612+
const bashActivate: PythonCommandRunConfiguration[] = condaShPath
613+
? [
614+
{ executable: 'source', args: [condaShPath.replace(/\\/g, '/')] },
615+
{ executable: 'conda', args: ['activate', quotedPrefix] },
616+
]
617+
: [{ executable: 'source', args: [sourceInitPath.replace(/\\/g, '/'), quotedPrefix] }];
607618
traceVerbose(
608619
`Windows activation commands:
609620
PowerShell: ${JSON.stringify(pwshActivate)},
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import assert from 'assert';
2+
import * as sinon from 'sinon';
3+
import { ShellConstants } from '../../../features/common/shellConstants';
4+
import * as condaSourcingUtils from '../../../managers/conda/condaSourcingUtils';
5+
import { windowsExceptionGenerateConfig } from '../../../managers/conda/condaUtils';
6+
7+
/**
8+
* Tests for windowsExceptionGenerateConfig - Windows shell activation commands.
9+
*
10+
* Key behavior tested:
11+
* - Git Bash uses conda.sh (initialization script) + conda activate when condaShPath is available
12+
* - Git Bash falls back to source <activate-script> <env> when condaShPath is not available
13+
* - PowerShell uses ps1 hook + conda activate
14+
* - CMD uses activate.bat + conda activate
15+
*/
16+
suite('Conda Utils - windowsExceptionGenerateConfig', () => {
17+
let getCondaHookPs1PathStub: sinon.SinonStub;
18+
19+
setup(() => {
20+
// Mock getCondaHookPs1Path to avoid filesystem access
21+
getCondaHookPs1PathStub = sinon.stub(condaSourcingUtils, 'getCondaHookPs1Path');
22+
});
23+
24+
teardown(() => {
25+
sinon.restore();
26+
});
27+
28+
suite('Git Bash activation with conda.sh', () => {
29+
test('Uses source conda.sh + conda activate when condaShPath is provided', async () => {
30+
// Arrange
31+
getCondaHookPs1PathStub.resolves('C:\\conda\\shell\\condabin\\conda-hook.ps1');
32+
const sourceInitPath = 'C:\\conda\\Scripts\\activate.bat';
33+
const prefix = 'myenv';
34+
const condaFolder = 'C:\\conda';
35+
const condaShPath = 'C:\\conda\\etc\\profile.d\\conda.sh';
36+
37+
// Act
38+
const result = await windowsExceptionGenerateConfig(sourceInitPath, prefix, condaFolder, condaShPath);
39+
40+
// Assert
41+
const gitBashActivation = result.shellActivation.get(ShellConstants.GITBASH);
42+
assert.ok(gitBashActivation, 'Git Bash activation should be defined');
43+
assert.strictEqual(gitBashActivation.length, 2, 'Should have 2 commands: source conda.sh + conda activate');
44+
45+
// First command: source conda.sh (no env arg - it's an initialization script)
46+
assert.strictEqual(gitBashActivation[0].executable, 'source');
47+
assert.deepStrictEqual(gitBashActivation[0].args, ['C:/conda/etc/profile.d/conda.sh']);
48+
49+
// Second command: conda activate <env>
50+
assert.strictEqual(gitBashActivation[1].executable, 'conda');
51+
assert.deepStrictEqual(gitBashActivation[1].args, ['activate', 'myenv']);
52+
});
53+
54+
test('Falls back to single source command when condaShPath is undefined', async () => {
55+
// Arrange
56+
getCondaHookPs1PathStub.resolves(undefined);
57+
const sourceInitPath = 'C:\\conda\\Scripts\\activate.bat';
58+
const prefix = 'myenv';
59+
const condaFolder = 'C:\\conda';
60+
const condaShPath = undefined; // Not available
61+
62+
// Act
63+
const result = await windowsExceptionGenerateConfig(sourceInitPath, prefix, condaFolder, condaShPath);
64+
65+
// Assert
66+
const gitBashActivation = result.shellActivation.get(ShellConstants.GITBASH);
67+
assert.ok(gitBashActivation, 'Git Bash activation should be defined');
68+
assert.strictEqual(gitBashActivation.length, 1, 'Should have 1 command when condaShPath not available');
69+
70+
// Single command: source <activate-script> <env>
71+
assert.strictEqual(gitBashActivation[0].executable, 'source');
72+
assert.deepStrictEqual(gitBashActivation[0].args, ['C:/conda/Scripts/activate.bat', 'myenv']);
73+
});
74+
75+
test('Converts Windows backslashes to forward slashes for bash', async () => {
76+
// Arrange
77+
getCondaHookPs1PathStub.resolves(undefined);
78+
const condaShPath = 'C:\\Tools\\miniforge3\\etc\\profile.d\\conda.sh';
79+
80+
// Act
81+
const result = await windowsExceptionGenerateConfig(
82+
'C:\\Tools\\miniforge3\\Scripts\\activate.bat',
83+
'pipes',
84+
'C:\\Tools\\miniforge3',
85+
condaShPath,
86+
);
87+
88+
// Assert
89+
const gitBashActivation = result.shellActivation.get(ShellConstants.GITBASH);
90+
assert.ok(gitBashActivation, 'Git Bash activation should be defined');
91+
// Verify forward slashes are used
92+
const sourcePath = gitBashActivation[0].args?.[0];
93+
assert.ok(sourcePath, 'Source path should be defined');
94+
assert.ok(!sourcePath.includes('\\'), 'Path should not contain backslashes');
95+
assert.ok(sourcePath.includes('/'), 'Path should contain forward slashes');
96+
});
97+
});
98+
99+
suite('PowerShell activation', () => {
100+
test('Uses ps1 hook when available', async () => {
101+
// Arrange
102+
const ps1HookPath = 'C:\\conda\\shell\\condabin\\conda-hook.ps1';
103+
getCondaHookPs1PathStub.resolves(ps1HookPath);
104+
105+
// Act
106+
const result = await windowsExceptionGenerateConfig(
107+
'C:\\conda\\Scripts\\activate.bat',
108+
'myenv',
109+
'C:\\conda',
110+
undefined,
111+
);
112+
113+
// Assert
114+
const pwshActivation = result.shellActivation.get(ShellConstants.PWSH);
115+
assert.ok(pwshActivation, 'PowerShell activation should be defined');
116+
assert.strictEqual(pwshActivation.length, 2, 'Should have 2 commands');
117+
assert.strictEqual(pwshActivation[0].executable, ps1HookPath);
118+
assert.strictEqual(pwshActivation[1].executable, 'conda');
119+
assert.deepStrictEqual(pwshActivation[1].args, ['activate', 'myenv']);
120+
});
121+
122+
test('Falls back to sourceInitPath when ps1 hook not found', async () => {
123+
// Arrange
124+
getCondaHookPs1PathStub.resolves(undefined);
125+
const sourceInitPath = 'C:\\conda\\Scripts\\activate.bat';
126+
127+
// Act
128+
const result = await windowsExceptionGenerateConfig(sourceInitPath, 'myenv', 'C:\\conda', undefined);
129+
130+
// Assert
131+
const pwshActivation = result.shellActivation.get(ShellConstants.PWSH);
132+
assert.ok(pwshActivation, 'PowerShell activation should be defined');
133+
assert.strictEqual(pwshActivation[0].executable, sourceInitPath);
134+
});
135+
});
136+
137+
suite('CMD activation', () => {
138+
test('Uses activate.bat + conda activate', async () => {
139+
// Arrange
140+
getCondaHookPs1PathStub.resolves(undefined);
141+
const sourceInitPath = 'C:\\conda\\Scripts\\activate.bat';
142+
143+
// Act
144+
const result = await windowsExceptionGenerateConfig(sourceInitPath, 'myenv', 'C:\\conda', undefined);
145+
146+
// Assert
147+
const cmdActivation = result.shellActivation.get(ShellConstants.CMD);
148+
assert.ok(cmdActivation, 'CMD activation should be defined');
149+
assert.strictEqual(cmdActivation.length, 2, 'Should have 2 commands');
150+
assert.strictEqual(cmdActivation[0].executable, sourceInitPath);
151+
assert.strictEqual(cmdActivation[1].executable, 'conda');
152+
assert.deepStrictEqual(cmdActivation[1].args, ['activate', 'myenv']);
153+
});
154+
});
155+
156+
suite('Deactivation commands', () => {
157+
test('All shells use conda deactivate', async () => {
158+
// Arrange
159+
getCondaHookPs1PathStub.resolves(undefined);
160+
161+
// Act
162+
const result = await windowsExceptionGenerateConfig(
163+
'C:\\conda\\Scripts\\activate.bat',
164+
'myenv',
165+
'C:\\conda',
166+
undefined,
167+
);
168+
169+
// Assert: All shells should have conda deactivate
170+
for (const shell of [ShellConstants.GITBASH, ShellConstants.CMD, ShellConstants.PWSH]) {
171+
const deactivation = result.shellDeactivation.get(shell);
172+
assert.ok(deactivation, `${shell} deactivation should be defined`);
173+
assert.strictEqual(deactivation.length, 1, `${shell} should have 1 deactivation command`);
174+
assert.strictEqual(deactivation[0].executable, 'conda');
175+
assert.deepStrictEqual(deactivation[0].args, ['deactivate']);
176+
}
177+
});
178+
});
179+
});

0 commit comments

Comments
 (0)