Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions .github/instructions/generic.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,40 @@ Provide project context and coding guidelines that AI should follow when generat

## Localization

- Localize all user-facing messages using VS Code’s `l10n` API.
- Internal log messages do not require localization.
- Localize all user-facing messages using VS Code’s `l10n` API.
- Internal log messages do not require localization.

## Logging

- Use the extension’s logging utilities (`traceLog`, `traceVerbose`) for internal logs.
- Do not use `console.log` or `console.warn` for logging.
- Use the extension’s logging utilities (`traceLog`, `traceVerbose`) for internal logs.
- Do not use `console.log` or `console.warn` for logging.

## Settings Precedence

- Always consider VS Code settings precedence:
- Always consider VS Code settings precedence:
1. Workspace folder
2. Workspace
3. User/global
- Remove or update settings from the highest precedence scope first.
- Remove or update settings from the highest precedence scope first.

## Error Handling & User Notifications

- Avoid showing the same error message multiple times in a session; track state with a module-level variable.
- Use clear, actionable error messages and offer relevant buttons (e.g., "Open settings", "Close").
- Avoid showing the same error message multiple times in a session; track state with a module-level variable.
- Use clear, actionable error messages and offer relevant buttons (e.g., "Open settings", "Close").

## Documentation

- Add clear docstrings to public functions, describing their purpose, parameters, and behavior.
- Add clear docstrings to public functions, describing their purpose, parameters, and behavior.

## Cross-Platform Path Handling

**CRITICAL**: This extension runs on Windows, macOS, and Linux. NEVER hardcode POSIX-style paths.

- Use `path.join()` or `path.resolve()` instead of string concatenation with `/`
- Use `Uri.file(path).fsPath` when comparing paths (Windows uses `\`, POSIX uses `/`)
- When asserting path equality, compare `fsPath` to `fsPath`, not raw strings

## Learnings

- When using `getConfiguration().inspect()`, always pass a scope/Uri to `getConfiguration(section, scope)` — otherwise `workspaceFolderValue` will be `undefined` because VS Code doesn't know which folder to inspect (1)
- **path.normalize() vs path.resolve()**: On Windows, `path.normalize('\test')` keeps it as `\test`, but `path.resolve('\test')` adds the current drive → `C:\test`. When comparing paths, use `path.resolve()` on BOTH sides or they won't match (2)
183 changes: 102 additions & 81 deletions .github/instructions/testing-workflow.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,53 @@ This guide covers the full testing lifecycle:

**User Requests Testing:**

- "Write tests for this function"
- "Run the tests"
- "Fix the failing tests"
- "Test this code"
- "Add test coverage"
- "Write tests for this function"
- "Run the tests"
- "Fix the failing tests"
- "Test this code"
- "Add test coverage"

**File Context Triggers:**

- Working in `**/test/**` directories
- Files ending in `.test.ts` or `.unit.test.ts`
- Test failures or compilation errors
- Coverage reports or test output analysis
- Working in `**/test/**` directories
- Files ending in `.test.ts` or `.unit.test.ts`
- Test failures or compilation errors
- Coverage reports or test output analysis

## 🚨 CRITICAL: Common Mistakes (Read First!)

These mistakes have occurred REPEATEDLY. Check this list BEFORE writing any test code:

| Mistake | Fix |
| ---------------------------------------------- | ------------------------------------------------------------------ |
| Hardcoded POSIX paths like `'/test/workspace'` | Use `'.'` for relative paths, `Uri.file(x).fsPath` for comparisons |
| Stubbing `workspace.getConfiguration` directly | Stub the wrapper `workspaceApis.getConfiguration` instead |
| Stubbing `workspace.workspaceFolders` property | Stub wrapper function `workspaceApis.getWorkspaceFolders()` |
| Comparing `fsPath` to raw string | Compare `fsPath` to `Uri.file(expected).fsPath` |

**Pre-flight checklist before completing test work:**

- [ ] All paths use `Uri.file().fsPath` (no hardcoded `/path/to/x`)
- [ ] All VS Code API stubs use wrapper modules, not `vscode.*` directly
- [ ] Tests pass on both Windows and POSIX

## Test Types

When implementing tests as an AI agent, choose between two main types:

### Unit Tests (`*.unit.test.ts`)

- **Fast isolated testing** - Mock all external dependencies
- **Use for**: Pure functions, business logic, data transformations
- **Execute with**: `runTests` tool with specific file patterns
- **Mock everything** - VS Code APIs automatically mocked via `/src/test/unittests.ts`
- **Fast isolated testing** - Mock all external dependencies
- **Use for**: Pure functions, business logic, data transformations
- **Execute with**: `runTests` tool with specific file patterns
- **Mock everything** - VS Code APIs automatically mocked via `/src/test/unittests.ts`

### Extension Tests (`*.test.ts`)

- **Full VS Code integration** - Real environment with actual APIs
- **Use for**: Command registration, UI interactions, extension lifecycle
- **Execute with**: VS Code launch configurations or `runTests` tool
- **Slower but comprehensive** - Tests complete user workflows
- **Full VS Code integration** - Real environment with actual APIs
- **Use for**: Command registration, UI interactions, extension lifecycle
- **Execute with**: VS Code launch configurations or `runTests` tool
- **Slower but comprehensive** - Tests complete user workflows

## 🤖 Agent Tool Usage for Test Execution

Expand Down Expand Up @@ -172,17 +189,17 @@ function analyzeFailure(failure: TestFailure): TestFailureAnalysis {

**Choose Unit Tests (`*.unit.test.ts`) when analyzing:**

- Functions with clear inputs/outputs and no VS Code API dependencies
- Data transformation, parsing, or utility functions
- Business logic that can be isolated with mocks
- Error handling scenarios with predictable inputs
- Functions with clear inputs/outputs and no VS Code API dependencies
- Data transformation, parsing, or utility functions
- Business logic that can be isolated with mocks
- Error handling scenarios with predictable inputs

**Choose Extension Tests (`*.test.ts`) when analyzing:**

- Functions that register VS Code commands or use `vscode.*` APIs
- UI components, tree views, or command palette interactions
- File system operations requiring workspace context
- Extension lifecycle events (activation, deactivation)
- Functions that register VS Code commands or use `vscode.*` APIs
- UI components, tree views, or command palette interactions
- File system operations requiring workspace context
- Extension lifecycle events (activation, deactivation)

**Agent Implementation Pattern:**

Expand Down Expand Up @@ -300,22 +317,22 @@ function generateTestScenarios(analysis: FunctionAnalysis): TestScenario[] {

#### Main Flows

- ✅ **Happy path scenarios** - normal expected usage
- ✅ **Alternative paths** - different configuration combinations
- ✅ **Integration scenarios** - multiple features working together
- ✅ **Happy path scenarios** - normal expected usage
- ✅ **Alternative paths** - different configuration combinations
- ✅ **Integration scenarios** - multiple features working together

#### Edge Cases

- 🔸 **Boundary conditions** - empty inputs, missing data
- 🔸 **Error scenarios** - network failures, permission errors
- 🔸 **Data validation** - invalid inputs, type mismatches
- 🔸 **Boundary conditions** - empty inputs, missing data
- 🔸 **Error scenarios** - network failures, permission errors
- 🔸 **Data validation** - invalid inputs, type mismatches

#### Real-World Scenarios

- ✅ **Fresh install** - clean slate
- ✅ **Existing user** - migration scenarios
- ✅ **Power user** - complex configurations
- 🔸 **Error recovery** - graceful degradation
- ✅ **Fresh install** - clean slate
- ✅ **Existing user** - migration scenarios
- ✅ **Power user** - complex configurations
- 🔸 **Error recovery** - graceful degradation

### Example Test Plan Structure

Expand All @@ -324,30 +341,30 @@ function generateTestScenarios(analysis: FunctionAnalysis): TestScenario[] {

### 1. Configuration Migration Tests

- No legacy settings exist
- Legacy settings already migrated
- Fresh migration needed
- Partial migration required
- Migration failures
- No legacy settings exist
- Legacy settings already migrated
- Fresh migration needed
- Partial migration required
- Migration failures

### 2. Configuration Source Tests

- Global search paths
- Workspace search paths
- Settings precedence
- Configuration errors
- Global search paths
- Workspace search paths
- Settings precedence
- Configuration errors

### 3. Path Resolution Tests

- Absolute vs relative paths
- Workspace folder resolution
- Path validation and filtering
- Absolute vs relative paths
- Workspace folder resolution
- Path validation and filtering

### 4. Integration Scenarios

- Combined configurations
- Deduplication logic
- Error handling flows
- Combined configurations
- Deduplication logic
- Error handling flows
```

## 🔧 Step 4: Set Up Your Test Infrastructure
Expand Down Expand Up @@ -514,47 +531,47 @@ envConfig.inspect

### Configuration Tests

- Test different setting combinations
- Test setting precedence (workspace > user > default)
- Test configuration errors and recovery
- Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility
- Test different setting combinations
- Test setting precedence (workspace > user > default)
- Test configuration errors and recovery
- Always use dynamic path construction with Node.js `path` module when testing functions that resolve paths against workspace folders to ensure cross-platform compatibility

### Data Flow Tests

- Test how data moves through the system
- Test transformations (path resolution, filtering)
- Test state changes (migrations, updates)
- Test how data moves through the system
- Test transformations (path resolution, filtering)
- Test state changes (migrations, updates)

### Error Handling Tests

- Test graceful degradation
- Test error logging
- Test fallback behaviors
- Test graceful degradation
- Test error logging
- Test fallback behaviors

### Integration Tests

- Test multiple features together
- Test real-world scenarios
- Test edge case combinations
- Test multiple features together
- Test real-world scenarios
- Test edge case combinations

## 📊 Step 8: Review and Refine

### Test Quality Checklist

- [ ] **Clear naming** - test names describe the scenario and expected outcome
- [ ] **Good coverage** - main flows, edge cases, error scenarios
- [ ] **Resilient assertions** - won't break due to minor changes
- [ ] **Readable structure** - follows Mock → Run → Assert pattern
- [ ] **Isolated tests** - each test is independent
- [ ] **Fast execution** - tests run quickly with proper mocking
- [ ] **Clear naming** - test names describe the scenario and expected outcome
- [ ] **Good coverage** - main flows, edge cases, error scenarios
- [ ] **Resilient assertions** - won't break due to minor changes
- [ ] **Readable structure** - follows Mock → Run → Assert pattern
- [ ] **Isolated tests** - each test is independent
- [ ] **Fast execution** - tests run quickly with proper mocking

### Common Anti-Patterns to Avoid

- ❌ Testing implementation details instead of behavior
- ❌ Brittle assertions that break on cosmetic changes
- ❌ Order-dependent tests that fail due to processing changes
- ❌ Tests that don't clean up mocks properly
- ❌ Overly complex test setup that's hard to understand
- ❌ Testing implementation details instead of behavior
- ❌ Brittle assertions that break on cosmetic changes
- ❌ Order-dependent tests that fail due to processing changes
- ❌ Tests that don't clean up mocks properly
- ❌ Overly complex test setup that's hard to understand

## 🔄 Reviewing and Improving Existing Tests

Expand All @@ -567,13 +584,17 @@ envConfig.inspect

### Common Fixes

- Over-complex mocks → Minimal mocks with only needed methods
- Brittle assertions → Behavior-focused with error messages
- Vague test names → Clear scenario descriptions (transform "should return X when Y" into "should [expected behavior] when [scenario context]")
- Missing structure → Mock → Run → Assert pattern
- Untestable Node.js APIs → Create proxy abstraction functions (use function overloads to preserve intelligent typing while making functions mockable)
- Over-complex mocks → Minimal mocks with only needed methods
- Brittle assertions → Behavior-focused with error messages
- Vague test names → Clear scenario descriptions (transform "should return X when Y" into "should [expected behavior] when [scenario context]")
- Missing structure → Mock → Run → Assert pattern
- Untestable Node.js APIs → Create proxy abstraction functions (use function overloads to preserve intelligent typing while making functions mockable)

## 🧠 Agent Learnings
- Avoid testing exact error messages or log output - assert only that errors are thrown or rejection occurs to prevent brittle tests (1)
- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1)

- Avoid testing exact error messages or log output - assert only that errors are thrown or rejection occurs to prevent brittle tests (1)
- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1)
- Always compile tests (`npm run compile-tests`) before running them after adding new test cases - test counts will be wrong if running against stale compiled output (1)
- Never create "documentation tests" that just `assert.ok(true)` — if mocking limitations prevent testing, either test a different layer that IS mockable, or skip the test entirely with a clear explanation (1)
- **REPEATED**: When stubbing vscode APIs in tests via wrapper modules (e.g., `workspaceApis`), the production code must also use those wrappers — sinon cannot stub properties directly on the vscode namespace like `workspace.workspaceFolders`, so both production and test code must reference the same stubbable wrapper functions (3)
- **REPEATED**: Use OS-agnostic path handling in tests: use `'.'` for relative paths in configs (NOT `/test/workspace`), compare `fsPath` to `Uri.file(expected).fsPath` (NOT raw strings). This breaks on Windows every time! (5)
2 changes: 2 additions & 0 deletions src/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export const EXTENSION_ROOT_DIR = path.dirname(__dirname);

export const DEFAULT_PACKAGE_MANAGER_ID = 'ms-python.python:pip';
export const DEFAULT_ENV_MANAGER_ID = 'ms-python.python:venv';
export const VENV_MANAGER_ID = 'ms-python.python:venv';
export const SYSTEM_MANAGER_ID = 'ms-python.python:system';

export const KNOWN_FILES = [
'requirements.txt',
Expand Down
18 changes: 11 additions & 7 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ import {
} from './features/envCommands';
import { PythonEnvironmentManagers } from './features/envManagers';
import { EnvVarManager, PythonEnvVariableManager } from './features/execution/envVariableManager';
import {
applyInitialEnvironmentSelection,
registerInterpreterSettingsChangeListener,
} from './features/interpreterSelection';
import { PythonProjectManagerImpl } from './features/projectManager';
import { getPythonApi, setPythonApi } from './features/pythonApi';
import { registerCompletionProvider } from './features/settings/settingCompletions';
Expand All @@ -67,12 +71,7 @@ import { PythonStatusBarImpl } from './features/views/pythonStatusBar';
import { updateViewsAndStatus } from './features/views/revealHandler';
import { TemporaryStateManager } from './features/views/temporaryStateManager';
import { ProjectItem, PythonEnvTreeItem } from './features/views/treeViewItems';
import {
collectEnvironmentInfo,
getEnvManagerAndPackageManagerConfigLevels,
resolveDefaultInterpreter,
runPetInTerminalImpl,
} from './helpers';
import { collectEnvironmentInfo, getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl } from './helpers';
import { EnvironmentManagers, ProjectCreators, PythonProjectManager } from './internal.api';
import { registerSystemPythonFeatures } from './managers/builtin/main';
import { SysPythonManager } from './managers/builtin/sysPythonManager';
Expand Down Expand Up @@ -460,7 +459,12 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
shellStartupVarsMgr.initialize(),
]);

await resolveDefaultInterpreter(nativeFinder, envManagers, api);
await applyInitialEnvironmentSelection(envManagers, projectManager, nativeFinder, api);

// Register listener for interpreter settings changes for interpreter re-selection
context.subscriptions.push(
registerInterpreterSettingsChangeListener(envManagers, projectManager, nativeFinder, api),
);

sendTelemetryEvent(EventNames.EXTENSION_MANAGER_REGISTRATION_DURATION, start.elapsedTime);
await terminalManager.initialize(api);
Expand Down
2 changes: 1 addition & 1 deletion src/features/creators/newPackageProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class NewPackageProject implements PythonProjectCreator {
}

// 6. Set the Python environment for the package
this.envManagers.setEnvironment(createdPackage?.uri, createdEnv);
await this.envManagers.setEnvironment(createdPackage?.uri, createdEnv);

// 7. add custom github copilot instructions
if (createCopilotInstructions) {
Expand Down
Loading