Skip to content

Commit 733f005

Browse files
committed
test fixes and learning more
1 parent 171a389 commit 733f005

3 files changed

Lines changed: 40 additions & 18 deletions

File tree

.github/instructions/generic.instructions.md

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,35 @@ Provide project context and coding guidelines that AI should follow when generat
66

77
## Localization
88

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

1212
## Logging
1313

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

1717
## Settings Precedence
1818

19-
- Always consider VS Code settings precedence:
19+
- Always consider VS Code settings precedence:
2020
1. Workspace folder
2121
2. Workspace
2222
3. User/global
23-
- Remove or update settings from the highest precedence scope first.
23+
- Remove or update settings from the highest precedence scope first.
2424

2525
## Error Handling & User Notifications
2626

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

3030
## Documentation
3131

32-
- Add clear docstrings to public functions, describing their purpose, parameters, and behavior.
32+
- Add clear docstrings to public functions, describing their purpose, parameters, and behavior.
33+
34+
## Cross-Platform Path Handling
35+
36+
**CRITICAL**: This extension runs on Windows, macOS, and Linux. NEVER hardcode POSIX-style paths.
3337

3438
## Learnings
3539

36-
- 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)
40+
- 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)

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,23 @@ This guide covers the full testing lifecycle:
3333
- Test failures or compilation errors
3434
- Coverage reports or test output analysis
3535

36+
## 🚨 CRITICAL: Common Mistakes (Read First!)
37+
38+
These mistakes have occurred REPEATEDLY. Check this list BEFORE writing any test code:
39+
40+
| Mistake | Fix |
41+
| ---------------------------------------------- | ------------------------------------------------------------------ |
42+
| Hardcoded POSIX paths like `'/test/workspace'` | Use `'.'` for relative paths, `Uri.file(x).fsPath` for comparisons |
43+
| Stubbing `workspace.getConfiguration` directly | Stub the wrapper `workspaceApis.getConfiguration` instead |
44+
| Stubbing `workspace.workspaceFolders` property | Stub wrapper function `workspaceApis.getWorkspaceFolders()` |
45+
| Comparing `fsPath` to raw string | Compare `fsPath` to `Uri.file(expected).fsPath` |
46+
47+
**Pre-flight checklist before completing test work:**
48+
49+
- [ ] All paths use `Uri.file().fsPath` (no hardcoded `/path/to/x`)
50+
- [ ] All VS Code API stubs use wrapper modules, not `vscode.*` directly
51+
- [ ] Tests pass on both Windows and POSIX
52+
3653
## Test Types
3754

3855
When implementing tests as an AI agent, choose between two main types:
@@ -579,4 +596,5 @@ envConfig.inspect
579596
- Create shared mock helpers (e.g., `createMockLogOutputChannel()`) instead of duplicating mock setup across multiple test files (1)
580597
- 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)
581598
- 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)
582-
- 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 (1)
599+
- **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)
600+
- **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)

src/test/features/interpreterSelection.unit.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ suite('Interpreter Selection - Priority Chain', () => {
122122
sandbox.stub(workspaceApis, 'getConfiguration').returns(
123123
createMockConfig([
124124
{
125-
path: '/test/workspace',
125+
path: '.',
126126
envManager: 'ms-python.python:venv',
127127
packageManager: 'ms-python.python:pip',
128128
},
@@ -301,7 +301,7 @@ suite('Interpreter Selection - Priority Chain', () => {
301301
);
302302
assert.strictEqual(
303303
result.environment.environmentPath?.fsPath,
304-
userPyenvPath,
304+
Uri.file(userPyenvPath).fsPath,
305305
'environmentPath should use user configured path',
306306
);
307307
});
@@ -1117,10 +1117,11 @@ suite('Interpreter Selection - Settings over Cache Priority', () => {
11171117

11181118
test('should use pythonProjects manager even when defaultEnvManager is set', async () => {
11191119
// This test verifies pythonProjects[] has highest priority (Priority 1 over Priority 2)
1120+
// Use '.' as relative path - path.resolve(workspaceFolder, '.') equals workspaceFolder
11201121
sandbox.stub(workspaceApis, 'getConfiguration').returns(
11211122
createMockConfig([
11221123
{
1123-
path: '/test/workspace',
1124+
path: '.',
11241125
envManager: 'ms-python.python:venv', // Project says venv
11251126
packageManager: 'ms-python.python:pip',
11261127
},
@@ -1357,16 +1358,15 @@ suite('Interpreter Selection - Multi-Root Workspace', () => {
13571358
]);
13581359

13591360
// Different pythonProjects settings for each folder
1361+
// Use '.' as relative path - path.resolve(workspaceFolder, '.') equals workspaceFolder
13601362
sandbox.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => {
13611363
const scopeUri = scope as Uri | undefined;
13621364
if (scopeUri?.fsPath === folder1Uri.fsPath) {
1363-
return createMockConfig([
1364-
{ path: '/workspace/folder1', envManager: 'ms-python.python:venv' },
1365-
]) as WorkspaceConfiguration;
1365+
return createMockConfig([{ path: '.', envManager: 'ms-python.python:venv' }]) as WorkspaceConfiguration;
13661366
}
13671367
if (scopeUri?.fsPath === folder2Uri.fsPath) {
13681368
return createMockConfig([
1369-
{ path: '/workspace/folder2', envManager: 'ms-python.python:system' },
1369+
{ path: '.', envManager: 'ms-python.python:system' },
13701370
]) as WorkspaceConfiguration;
13711371
}
13721372
return createMockConfig([]) as WorkspaceConfiguration;

0 commit comments

Comments
 (0)