Skip to content

Commit 490f418

Browse files
authored
Merge branch 'main' into copilot/fix-3b6de93e-8dce-49f5-8cd1-5f79d0da7ad8
2 parents 509cc47 + 49e4242 commit 490f418

14 files changed

Lines changed: 823 additions & 98 deletions

File tree

.github/instructions/testing-workflow.instructions.md

Lines changed: 4 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -537,70 +537,17 @@ envConfig.inspect
537537
- ❌ Tests that don't clean up mocks properly
538538
- ❌ Overly complex test setup that's hard to understand
539539

540-
## � Common Pitfalls & Fast Diagnosis
541-
542-
These recurring issues have caused wasted cycles. Follow the quick diagnosis flow to avoid them:
543-
544-
### 1. 0 Tests Detected When Filtering (e.g. with --grep or targeted run)
545-
546-
Likely Causes (in order):
547-
548-
- The TypeScript test file has not been compiled to `out/test/...` yet.
549-
- The file was moved outside the compiler `rootDir` (tsconfig sets `rootDir: src`), so `tsc` will not emit it into `out/`.
550-
- The grep / test name filter does not match the suite or test names.
551-
552-
Diagnosis Checklist:
553-
554-
1. Confirm compile step ran: if not in watch mode, run the compile test script (see below) **before** executing tests.
555-
2. Check for the emitted JS: derive the expected path (replace `src/` with `out/` and ensure the directory pattern matches `out/test/**/*.unit.test.js`). If the JS file is missing, it's a compilation / placement issue.
556-
3. Open the compiled JS and verify the `suite("<Your Suite Name>")` string literally appears; if missing, the source file wasn’t included or was misnamed.
557-
4. Re-run the test using the programmatic test tool (preferred) referencing the file path instead of a grep first; if that succeeds, refine your grep pattern.
558-
559-
Remediation:
560-
561-
- Ensure unit test files live under `src/test/...` (since `rootDir` is `src`) so they are emitted under `out/test/...` and picked up by Mocha’s spec glob.
562-
- Run `watch-tests` once (preferred) or a one-off `compile-tests` before first execution.
563-
- Avoid relocating tests to a top-level `test/` folder unless tsconfig and spec globs are updated accordingly.
564-
565-
### 2. Using Terminal First Instead of Test Tool
566-
567-
Always default to the test execution tool (runTests) for:
568-
569-
- Precise targeting (single file / specific tests via testNames)
570-
- Richer integration & structured output
571-
572-
Use terminal (`npm run unittest`) only as a fallback.
573-
574-
### 3. Infinite Watch Confusion
575-
576-
If you started `npm run watch-tests`, do **not** also run `compile-tests` repeatedly—watch already handles incremental builds. Just re-run the test tool after edits.
577-
578-
### Quick Flowchart
579-
580-
0 tests? → Was TS compiled? (Check for `out/test/...js`) → If no: compile. → If yes: Is file under `src/test`? → If no: relocate. → If yes: Does suite name match grep / filter? → Adjust filter → Use runTests with direct file path.
581-
582-
### Example Programmatic Run
583-
584-
```
585-
await runTests({
586-
files: ['/absolute/path/to/src/test/features/execution/execUtils.unit.test.ts']
587-
});
588-
```
589-
590-
If that returns 0, re-check compilation path mapping.
591-
592-
## Agent Learning Patterns
593-
594-
### Key Implementation Insights
540+
## 🧠 Agent Learnings
595541

596542
- Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility (1)
597543
- Use `runTests` tool for programmatic test execution rather than terminal commands for better integration and result parsing (1)
598-
- Mock wrapper functions (e.g., `workspaceApis.getConfiguration()`) instead of VS Code APIs directly to avoid stubbing issues (1)
544+
- Mock wrapper functions (e.g., `workspaceApis.getConfiguration()`) instead of VS Code APIs directly to avoid stubbing issues (2)
599545
- Start compilation with `npm run watch-tests` before test execution to ensure TypeScript files are built (1)
600-
- Use `sinon.match()` patterns for resilient assertions that don't break on minor output changes (1)
546+
- Use `sinon.match()` patterns for resilient assertions that don't break on minor output changes (2)
601547
- Fix test issues iteratively - run tests, analyze failures, apply fixes, repeat until passing (1)
602548
- When fixing mock environment creation, use `null` to truly omit properties rather than `undefined` (1)
603549
- Always recompile TypeScript after making import/export changes before running tests, as stubs won't work if they're applied to old compiled JavaScript that doesn't have the updated imports (2)
604550
- Create proxy abstraction functions for Node.js APIs like `cp.spawn` to enable clean testing - use function overloads to preserve Node.js's intelligent typing while making the functions mockable (1)
605551
- When a targeted test run yields 0 tests, first verify the compiled JS exists under `out/test` (rootDir is `src`); absence almost always means the test file sits outside `src` or compilation hasn't run yet (1)
606552
- When unit tests fail with VS Code API errors like `TypeError: X is not a constructor` or `Cannot read properties of undefined (reading 'Y')`, check if VS Code APIs are properly mocked in `/src/test/unittests.ts` - add missing Task-related APIs (`Task`, `TaskScope`, `ShellExecution`, `TaskRevealKind`, `TaskPanelKind`) and namespace mocks (`tasks`) following the existing pattern of `mockedVSCode.X = vscodeMocks.vscMockExtHostedTypes.X` (1)
553+
- Create minimal mock objects with only required methods and use TypeScript type assertions (e.g., mockApi as PythonEnvironmentApi) to satisfy interface requirements instead of implementing all interface methods when only specific methods are needed for the test (1)

src/common/childProcess.apis.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import * as cp from 'child_process';
2+
3+
/**
4+
* Spawns a new process using the specified command and arguments.
5+
* This function abstracts cp.spawn to make it easier to mock in tests.
6+
*
7+
* When stdio: 'pipe' is used, returns ChildProcessWithoutNullStreams.
8+
* Otherwise returns the standard ChildProcess.
9+
*/
10+
11+
// Overload for stdio: 'pipe' - guarantees non-null streams
12+
export function spawnProcess(
13+
command: string,
14+
args: string[],
15+
options: cp.SpawnOptions & { stdio: 'pipe' },
16+
): cp.ChildProcessWithoutNullStreams;
17+
18+
// Overload for general case
19+
export function spawnProcess(command: string, args: string[], options?: cp.SpawnOptions): cp.ChildProcess;
20+
21+
// Implementation - delegates to cp.spawn to preserve its typing magic
22+
export function spawnProcess(
23+
command: string,
24+
args: string[],
25+
options?: cp.SpawnOptions,
26+
): cp.ChildProcess | cp.ChildProcessWithoutNullStreams {
27+
return cp.spawn(command, args, options ?? {});
28+
}

src/features/execution/runInBackground.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import * as cp from 'child_process';
21
import { PythonBackgroundRunOptions, PythonEnvironment, PythonProcess } from '../../api';
2+
import { spawnProcess } from '../../common/childProcess.apis';
33
import { traceError, traceInfo, traceWarn } from '../../common/logging';
44
import { quoteStringIfNecessary } from './execUtils';
55

@@ -39,7 +39,11 @@ export async function runInBackground(
3939
traceWarn(`Error checking if executable exists: ${err instanceof Error ? err.message : String(err)}`);
4040
}
4141

42-
const proc = cp.spawn(executable, allArgs, { stdio: 'pipe', cwd: options.cwd, env: options.env });
42+
const proc = spawnProcess(executable, allArgs, {
43+
stdio: 'pipe',
44+
cwd: options.cwd,
45+
env: options.env,
46+
});
4347

4448
return {
4549
pid: proc.pid,

src/features/terminal/shells/bash/bashStartup.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import which from 'which';
55
import { traceError, traceInfo, traceVerbose } from '../../../../common/logging';
66
import { ShellConstants } from '../../../common/shellConstants';
77
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
8-
import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
8+
import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
99
import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
1010
import { BASH_ENV_KEY, BASH_OLD_ENV_KEY, BASH_SCRIPT_VERSION, ZSH_ENV_KEY, ZSH_OLD_ENV_KEY } from './bashConstants';
1111

@@ -69,7 +69,8 @@ async function isStartupSetup(profile: string, key: string): Promise<ShellSetupS
6969
return ShellSetupState.NotSetup;
7070
}
7171
async function setupStartup(profile: string, key: string, name: string): Promise<boolean> {
72-
if (shellIntegrationForActiveTerminal(name, profile) && !isWsl()) {
72+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
73+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(name, profile))) && !isWsl()) {
7374
removeStartup(profile, key);
7475
return true;
7576
}

src/features/terminal/shells/common/shellUtils.ts

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
import { PythonCommandRunConfiguration, PythonEnvironment } from '../../../../api';
22
import { traceInfo } from '../../../../common/logging';
3+
import { getGlobalPersistentState } from '../../../../common/persistentState';
4+
import { sleep } from '../../../../common/utils/asyncUtils';
35
import { isWindows } from '../../../../common/utils/platformUtils';
46
import { activeTerminalShellIntegration } from '../../../../common/window.apis';
7+
import { getConfiguration } from '../../../../common/workspace.apis';
58
import { ShellConstants } from '../../../common/shellConstants';
69
import { quoteArgs } from '../../../execution/execUtils';
10+
import { SHELL_INTEGRATION_POLL_INTERVAL, SHELL_INTEGRATION_TIMEOUT } from '../../utils';
11+
12+
export const SHELL_INTEGRATION_STATE_KEY = 'shellIntegration.enabled';
713

814
function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string {
915
const parts = [];
@@ -98,22 +104,60 @@ export function extractProfilePath(content: string): string | undefined {
98104
return undefined;
99105
}
100106

101-
export function shellIntegrationForActiveTerminal(name: string, profile?: string): boolean {
102-
const hasShellIntegration = activeTerminalShellIntegration();
107+
export async function shellIntegrationForActiveTerminal(name: string, profile?: string): Promise<boolean> {
108+
let hasShellIntegration = activeTerminalShellIntegration();
109+
let timeout = 0;
110+
111+
while (!hasShellIntegration && timeout < SHELL_INTEGRATION_TIMEOUT) {
112+
await sleep(SHELL_INTEGRATION_POLL_INTERVAL);
113+
timeout += SHELL_INTEGRATION_POLL_INTERVAL;
114+
hasShellIntegration = activeTerminalShellIntegration();
115+
}
103116

104117
if (hasShellIntegration) {
105118
traceInfo(
106-
`SHELL: Shell integration is available on your active terminal, with name ${name} and profile ${profile}. Python activate scripts will be evaluated at shell integration level, except in WSL.`
119+
`SHELL: Shell integration is available on your active terminal, with name ${name} and profile ${profile}. Python activate scripts will be evaluated at shell integration level, except in WSL.`,
107120
);
121+
122+
// Update persistent storage to reflect that shell integration is available
123+
const persistentState = await getGlobalPersistentState();
124+
await persistentState.set(SHELL_INTEGRATION_STATE_KEY, true);
125+
108126
return true;
109127
}
110128
return false;
111129
}
112130

113131
export function isWsl(): boolean {
114132
// WSL sets these environment variables
115-
return !!(process.env.WSL_DISTRO_NAME ||
116-
process.env.WSL_INTEROP ||
117-
process.env.WSLENV);
133+
return !!(process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP || process.env.WSLENV);
118134
}
119135

136+
export async function getShellIntegrationEnabledCache(): Promise<boolean> {
137+
const persistentState = await getGlobalPersistentState();
138+
const shellIntegrationInspect =
139+
getConfiguration('terminal.integrated').inspect<boolean>('shellIntegration.enabled');
140+
141+
let shellIntegrationEnabled = true;
142+
if (shellIntegrationInspect) {
143+
// Priority: workspaceFolder > workspace > globalRemoteValue > globalLocalValue > global > default
144+
const inspectValue = shellIntegrationInspect as Record<string, unknown>;
145+
146+
if (shellIntegrationInspect.workspaceFolderValue !== undefined) {
147+
shellIntegrationEnabled = shellIntegrationInspect.workspaceFolderValue;
148+
} else if (shellIntegrationInspect.workspaceValue !== undefined) {
149+
shellIntegrationEnabled = shellIntegrationInspect.workspaceValue;
150+
} else if ('globalRemoteValue' in shellIntegrationInspect && inspectValue.globalRemoteValue !== undefined) {
151+
shellIntegrationEnabled = inspectValue.globalRemoteValue as boolean;
152+
} else if ('globalLocalValue' in shellIntegrationInspect && inspectValue.globalLocalValue !== undefined) {
153+
shellIntegrationEnabled = inspectValue.globalLocalValue as boolean;
154+
} else if (shellIntegrationInspect.globalValue !== undefined) {
155+
shellIntegrationEnabled = shellIntegrationInspect.globalValue;
156+
} else if (shellIntegrationInspect.defaultValue !== undefined) {
157+
shellIntegrationEnabled = shellIntegrationInspect.defaultValue;
158+
}
159+
}
160+
161+
await persistentState.set(SHELL_INTEGRATION_STATE_KEY, shellIntegrationEnabled);
162+
return shellIntegrationEnabled;
163+
}

src/features/terminal/shells/fish/fishStartup.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import which from 'which';
66
import { traceError, traceInfo, traceVerbose } from '../../../../common/logging';
77
import { ShellConstants } from '../../../common/shellConstants';
88
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
9-
import { isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
9+
import { getShellIntegrationEnabledCache, isWsl, shellIntegrationForActiveTerminal } from '../common/shellUtils';
1010
import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
1111
import { FISH_ENV_KEY, FISH_OLD_ENV_KEY, FISH_SCRIPT_VERSION } from './fishConstants';
1212

@@ -58,7 +58,8 @@ async function isStartupSetup(profilePath: string, key: string): Promise<boolean
5858

5959
async function setupStartup(profilePath: string, key: string): Promise<boolean> {
6060
try {
61-
if (shellIntegrationForActiveTerminal('fish', profilePath) && !isWsl()) {
61+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
62+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal('fish', profilePath))) && !isWsl()) {
6263
removeFishStartup(profilePath, key);
6364
return true;
6465
}

src/features/terminal/shells/pwsh/pwshStartup.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { ShellConstants } from '../../../common/shellConstants';
1313
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
1414
import {
1515
extractProfilePath,
16+
getShellIntegrationEnabledCache,
1617
isWsl,
1718
PROFILE_TAG_END,
1819
PROFILE_TAG_START,
@@ -58,6 +59,27 @@ async function isPowerShellInstalled(shell: string): Promise<boolean> {
5859
}
5960
}
6061

62+
/**
63+
* Detects the major version of PowerShell by executing a version query command.
64+
* This helps with debugging activation issues since PowerShell 5.x and 7+ have different behaviors.
65+
* @param shell The PowerShell executable name ('powershell' for Windows PowerShell or 'pwsh' for PowerShell Core/7+)
66+
* @returns Promise resolving to the major version number as a string, or undefined if detection fails
67+
*/
68+
async function getPowerShellVersion(shell: 'powershell' | 'pwsh'): Promise<string | undefined> {
69+
try {
70+
const command = `${shell} -c '\$PSVersionTable.PSVersion.Major'`;
71+
const versionOutput = await runCommand(command);
72+
if (versionOutput && !isNaN(Number(versionOutput))) {
73+
return versionOutput;
74+
}
75+
traceVerbose(`Failed to parse PowerShell version from output: ${versionOutput}`);
76+
return undefined;
77+
} catch (err) {
78+
traceVerbose(`Failed to get PowerShell version for ${shell}`, err);
79+
return undefined;
80+
}
81+
}
82+
6183
async function getProfileForShell(shell: 'powershell' | 'pwsh'): Promise<string> {
6284
const cachedPath = getProfilePathCache(shell);
6385
if (cachedPath) {
@@ -125,7 +147,8 @@ function getActivationContent(): string {
125147
' try {',
126148
` Invoke-Expression $env:${POWERSHELL_ENV_KEY}`,
127149
' } catch {',
128-
` Write-Error "Failed to activate Python environment: $_" -ErrorAction Continue`,
150+
` $psVersion = $PSVersionTable.PSVersion.Major`,
151+
` Write-Error "Failed to activate Python environment (PowerShell $psVersion): $_" -ErrorAction Continue`,
129152
' }',
130153
' }',
131154
'}',
@@ -146,7 +169,9 @@ async function isPowerShellStartupSetup(shell: string, profile: string): Promise
146169
}
147170

148171
async function setupPowerShellStartup(shell: string, profile: string): Promise<boolean> {
149-
if (shellIntegrationForActiveTerminal(shell, profile) && !isWsl()) {
172+
const shellIntegrationEnabled = await getShellIntegrationEnabledCache();
173+
174+
if ((shellIntegrationEnabled || (await shellIntegrationForActiveTerminal(shell, profile))) && !isWsl()) {
150175
removePowerShellStartup(shell, profile, POWERSHELL_OLD_ENV_KEY);
151176
removePowerShellStartup(shell, profile, POWERSHELL_ENV_KEY);
152177
return true;
@@ -220,6 +245,12 @@ export class PwshStartupProvider implements ShellStartupScriptProvider {
220245
results.set(shell, this._isPs5Installed);
221246
} else {
222247
const isInstalled = await isPowerShellInstalled(shell);
248+
if (isInstalled) {
249+
// Log PowerShell version for debugging activation issues
250+
const version = await getPowerShellVersion(shell);
251+
const versionText = version ? ` (version ${version})` : ' (version unknown)';
252+
traceInfo(`SHELL: ${shell} is installed${versionText}`);
253+
}
223254
if (shell === 'pwsh') {
224255
this._isPwshInstalled = isInstalled;
225256
} else {

0 commit comments

Comments
 (0)