Skip to content

Commit af93b59

Browse files
committed
update tests and instructions
1 parent d1b84fc commit af93b59

File tree

2 files changed

+70
-55
lines changed

2 files changed

+70
-55
lines changed

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

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

540-
## 🧠 Agent Learning Patterns
541-
542-
### Key Implementation Insights
540+
## 🧠 Agent Learnings
543541

544542
- 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)
545543
- Use `runTests` tool for programmatic test execution rather than terminal commands for better integration and result parsing (1)
546-
- 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)
547545
- Start compilation with `npm run watch-tests` before test execution to ensure TypeScript files are built (1)
548-
- 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)
549547
- Fix test issues iteratively - run tests, analyze failures, apply fixes, repeat until passing (1)
550548
- When fixing mock environment creation, use `null` to truly omit properties rather than `undefined` (1)
551549
- 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)
552550
- 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)
553551
- 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)
552+
- 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)
Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,44 @@
11
import assert from 'assert';
2+
import * as path from 'path';
23
import * as sinon from 'sinon';
3-
import { Uri } from 'vscode';
4-
import { getProjectInstallable } from '../../../managers/builtin/pipUtils';
5-
import * as wapi from '../../../common/workspace.apis';
4+
import { CancellationToken, Progress, ProgressOptions, Uri } from 'vscode';
5+
import { PythonEnvironmentApi, PythonProject } from '../../../api';
66
import * as winapi from '../../../common/window.apis';
7+
import * as wapi from '../../../common/workspace.apis';
8+
import { getProjectInstallable } from '../../../managers/builtin/pipUtils';
79

810
suite('Pip Utils - getProjectInstallable', () => {
911
let findFilesStub: sinon.SinonStub;
1012
let withProgressStub: sinon.SinonStub;
11-
let mockApi: any;
13+
// Minimal mock that only implements the methods we need for this test
14+
// Using type assertion to satisfy TypeScript since we only need getPythonProject
15+
let mockApi: { getPythonProject: (uri: Uri) => PythonProject | undefined };
1216

1317
setup(() => {
1418
findFilesStub = sinon.stub(wapi, 'findFiles');
1519
// Stub withProgress to immediately execute the callback
1620
withProgressStub = sinon.stub(winapi, 'withProgress');
17-
withProgressStub.callsFake(async (_options: any, callback: any) => {
18-
return await callback(undefined, { isCancellationRequested: false });
19-
});
20-
21+
withProgressStub.callsFake(
22+
async (
23+
_options: ProgressOptions,
24+
callback: (
25+
progress: Progress<{ message?: string; increment?: number }>,
26+
token: CancellationToken,
27+
) => Thenable<unknown>,
28+
) => {
29+
return await callback(
30+
{} as Progress<{ message?: string; increment?: number }>,
31+
{ isCancellationRequested: false } as CancellationToken,
32+
);
33+
},
34+
);
35+
36+
const workspacePath = path.resolve('/workspace');
2137
mockApi = {
2238
getPythonProject: (uri: Uri) => {
23-
// Return a project for any URI in /workspace
24-
if (uri.fsPath.startsWith('/workspace')) {
25-
return { uri: Uri.file('/workspace') };
39+
// Return a project for any URI in workspace
40+
if (uri.fsPath.startsWith(workspacePath)) {
41+
return { name: 'workspace', uri: Uri.file(workspacePath) };
2642
}
2743
return undefined;
2844
},
@@ -41,10 +57,11 @@ suite('Pip Utils - getProjectInstallable', () => {
4157
return Promise.resolve([]);
4258
} else if (pattern === '*requirements*.txt') {
4359
// This pattern should match root-level files
60+
const workspacePath = path.resolve('/workspace');
4461
return Promise.resolve([
45-
Uri.file('/workspace/requirements.txt'),
46-
Uri.file('/workspace/dev-requirements.txt'),
47-
Uri.file('/workspace/test-requirements.txt'),
62+
Uri.file(path.join(workspacePath, 'requirements.txt')),
63+
Uri.file(path.join(workspacePath, 'dev-requirements.txt')),
64+
Uri.file(path.join(workspacePath, 'test-requirements.txt')),
4865
]);
4966
} else if (pattern === '**/requirements/*.txt') {
5067
return Promise.resolve([]);
@@ -55,12 +72,13 @@ suite('Pip Utils - getProjectInstallable', () => {
5572
});
5673

5774
// Act: Call getProjectInstallable
58-
const projects = [{ name: 'workspace', uri: Uri.file('/workspace') }];
59-
const result = await getProjectInstallable(mockApi, projects);
75+
const workspacePath = path.resolve('/workspace');
76+
const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
77+
const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
6078

6179
// Assert: Should find all three requirements files
6280
assert.strictEqual(result.length, 3, 'Should find three requirements files');
63-
81+
6482
const names = result.map((r) => r.name).sort();
6583
assert.deepStrictEqual(
6684
names,
@@ -82,13 +100,13 @@ suite('Pip Utils - getProjectInstallable', () => {
82100
// Arrange: Mock both patterns to return the same file
83101
findFilesStub.callsFake((pattern: string) => {
84102
if (pattern === '**/*requirements*.txt') {
85-
return Promise.resolve([
86-
Uri.file('/workspace/dev-requirements.txt'),
87-
]);
103+
const workspacePath = path.resolve('/workspace');
104+
return Promise.resolve([Uri.file(path.join(workspacePath, 'dev-requirements.txt'))]);
88105
} else if (pattern === '*requirements*.txt') {
106+
const workspacePath = path.resolve('/workspace');
89107
return Promise.resolve([
90-
Uri.file('/workspace/dev-requirements.txt'),
91-
Uri.file('/workspace/requirements.txt'),
108+
Uri.file(path.join(workspacePath, 'dev-requirements.txt')),
109+
Uri.file(path.join(workspacePath, 'requirements.txt')),
92110
]);
93111
} else if (pattern === '**/requirements/*.txt') {
94112
return Promise.resolve([]);
@@ -99,48 +117,43 @@ suite('Pip Utils - getProjectInstallable', () => {
99117
});
100118

101119
// Act: Call getProjectInstallable
102-
const projects = [{ name: 'workspace', uri: Uri.file('/workspace') }];
103-
const result = await getProjectInstallable(mockApi, projects);
120+
const workspacePath = path.resolve('/workspace');
121+
const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
122+
const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
104123

105124
// Assert: Should deduplicate and only have 2 unique files
106125
assert.strictEqual(result.length, 2, 'Should deduplicate and have 2 unique files');
107-
126+
108127
const names = result.map((r) => r.name).sort();
109-
assert.deepStrictEqual(
110-
names,
111-
['dev-requirements.txt', 'requirements.txt'],
112-
'Should have deduplicated results',
113-
);
128+
assert.deepStrictEqual(names, ['dev-requirements.txt', 'requirements.txt'], 'Should have deduplicated results');
114129
});
115130

116131
test('should find requirements files in subdirectories', async () => {
117132
// Arrange: Mock findFiles to return files in subdirectories
118133
findFilesStub.callsFake((pattern: string) => {
119134
if (pattern === '**/*requirements*.txt') {
120-
return Promise.resolve([
121-
Uri.file('/workspace/subdir/dev-requirements.txt'),
122-
]);
135+
const workspacePath = path.resolve('/workspace');
136+
return Promise.resolve([Uri.file(path.join(workspacePath, 'subdir', 'dev-requirements.txt'))]);
123137
} else if (pattern === '*requirements*.txt') {
124-
return Promise.resolve([
125-
Uri.file('/workspace/requirements.txt'),
126-
]);
138+
const workspacePath = path.resolve('/workspace');
139+
return Promise.resolve([Uri.file(path.join(workspacePath, 'requirements.txt'))]);
127140
} else if (pattern === '**/requirements/*.txt') {
128-
return Promise.resolve([
129-
Uri.file('/workspace/requirements/test.txt'),
130-
]);
141+
const workspacePath = path.resolve('/workspace');
142+
return Promise.resolve([Uri.file(path.join(workspacePath, 'requirements', 'test.txt'))]);
131143
} else if (pattern === '**/pyproject.toml') {
132144
return Promise.resolve([]);
133145
}
134146
return Promise.resolve([]);
135147
});
136148

137149
// Act: Call getProjectInstallable
138-
const projects = [{ name: 'workspace', uri: Uri.file('/workspace') }];
139-
const result = await getProjectInstallable(mockApi, projects);
150+
const workspacePath = path.resolve('/workspace');
151+
const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
152+
const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
140153

141154
// Assert: Should find all files
142155
assert.strictEqual(result.length, 3, 'Should find three files');
143-
156+
144157
const names = result.map((r) => r.name).sort();
145158
assert.deepStrictEqual(
146159
names,
@@ -151,7 +164,7 @@ suite('Pip Utils - getProjectInstallable', () => {
151164

152165
test('should return empty array when no projects provided', async () => {
153166
// Act: Call with no projects
154-
const result = await getProjectInstallable(mockApi, undefined);
167+
const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, undefined);
155168

156169
// Assert: Should return empty array
157170
assert.strictEqual(result.length, 0, 'Should return empty array');
@@ -162,25 +175,28 @@ suite('Pip Utils - getProjectInstallable', () => {
162175
// Arrange: Mock findFiles to return files from multiple directories
163176
findFilesStub.callsFake((pattern: string) => {
164177
if (pattern === '*requirements*.txt') {
178+
const workspacePath = path.resolve('/workspace');
179+
const otherPath = path.resolve('/other-dir');
165180
return Promise.resolve([
166-
Uri.file('/workspace/requirements.txt'),
167-
Uri.file('/other-dir/requirements.txt'), // Should be filtered out
181+
Uri.file(path.join(workspacePath, 'requirements.txt')),
182+
Uri.file(path.join(otherPath, 'requirements.txt')), // Should be filtered out
168183
]);
169184
} else {
170185
return Promise.resolve([]);
171186
}
172187
});
173188

174-
// Act: Call with only /workspace project
175-
const projects = [{ name: 'workspace', uri: Uri.file('/workspace') }];
176-
const result = await getProjectInstallable(mockApi, projects);
189+
// Act: Call with only workspace project
190+
const workspacePath = path.resolve('/workspace');
191+
const projects = [{ name: 'workspace', uri: Uri.file(workspacePath) }];
192+
const result = await getProjectInstallable(mockApi as PythonEnvironmentApi, projects);
177193

178-
// Assert: Should only include files from /workspace
194+
// Assert: Should only include files from workspace
179195
assert.strictEqual(result.length, 1, 'Should only include files from project directory');
180196
const firstResult = result[0];
181197
assert.ok(firstResult, 'Should have at least one result');
182198
assert.strictEqual(firstResult.name, 'requirements.txt');
183199
assert.ok(firstResult.uri, 'Should have a URI');
184-
assert.ok(firstResult.uri.fsPath.startsWith('/workspace'), 'Should be in workspace directory');
200+
assert.ok(firstResult.uri.fsPath.startsWith(workspacePath), 'Should be in workspace directory');
185201
});
186202
});

0 commit comments

Comments
 (0)