Skip to content

Commit 0ada7cc

Browse files
eleanorjboydeleanorjboyd
committed
tests improved
1 parent 4dfc6a0 commit 0ada7cc

6 files changed

+127
-90
lines changed

‎src/test/integration/envCreation.integration.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ suite('Integration: Environment Creation', function () {
102102
try {
103103
await api.removeEnvironment(createdEnv);
104104
} catch (e) {
105-
console.log('Cleanup failed (may already be removed):', e);
105+
console.warn('Cleanup warning - failed to remove environment:', e);
106106
}
107107
}
108108
}
@@ -158,8 +158,8 @@ suite('Integration: Environment Creation', function () {
158158
if (createdEnv) {
159159
try {
160160
await api.removeEnvironment(createdEnv);
161-
} catch {
162-
// Ignore cleanup errors
161+
} catch (e) {
162+
console.warn('Cleanup warning - failed to remove environment:', e);
163163
}
164164
}
165165
}
@@ -248,8 +248,8 @@ suite('Integration: Environment Creation', function () {
248248
if (createdEnv) {
249249
try {
250250
await api.removeEnvironment(createdEnv);
251-
} catch {
252-
// Ignore cleanup errors
251+
} catch (e) {
252+
console.warn('Cleanup warning - failed to remove environment:', e);
253253
}
254254
}
255255
}

‎src/test/integration/envDiscovery.integration.test.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ suite('Integration: Environment Discovery', function () {
220220
* Test: Multiple refreshes are idempotent
221221
*
222222
* Calling refresh multiple times should not cause duplicate
223-
* environments or errors.
223+
* environments or errors. Counts should be strictly equal.
224224
*/
225225
test('Multiple refreshes do not create duplicates', async function () {
226226
// First refresh
@@ -235,10 +235,16 @@ suite('Integration: Environment Discovery', function () {
235235
await api.refreshEnvironments(undefined);
236236
const thirdCount = (await api.getEnvironments('all')).length;
237237

238-
// Counts should be consistent (may vary slightly due to system changes)
239-
assert.ok(
240-
Math.abs(firstCount - secondCount) <= 1 && Math.abs(secondCount - thirdCount) <= 1,
241-
`Environment counts should be stable: ${firstCount}, ${secondCount}, ${thirdCount}`,
238+
// Counts should be strictly equal for idempotent refresh
239+
assert.strictEqual(
240+
firstCount,
241+
secondCount,
242+
`Refresh should be idempotent: first=${firstCount}, second=${secondCount}`,
243+
);
244+
assert.strictEqual(
245+
secondCount,
246+
thirdCount,
247+
`Refresh should be idempotent: second=${secondCount}, third=${thirdCount}`,
242248
);
243249
});
244250
});

‎src/test/integration/interpreterSelection.integration.test.ts‎

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ suite('Integration: Interpreter Selection Priority', function () {
240240
* Test: Setting same environment doesn't fire extra events
241241
*
242242
* Setting the same environment twice should not fire change event
243-
* on the second call.
243+
* on the second call. This ensures idempotent behavior.
244244
*/
245245
test('Setting same environment is idempotent', async function () {
246246
const environments = await api.getEnvironments('all');
@@ -272,28 +272,11 @@ suite('Integration: Interpreter Selection Priority', function () {
272272
// Set same environment again
273273
await api.setEnvironment(undefined, env);
274274

275-
// Wait a bit and check - should not fire (or fire with same old/new)
275+
// Wait for any potential events to fire
276276
await new Promise((resolve) => setTimeout(resolve, 1000));
277277

278-
// Either no event fired, or event fired (with valid structure)
279-
// The exact behavior depends on implementation details
280-
if (handler.fired) {
281-
const event = handler.last;
282-
assert.ok(event, 'Event should have value if fired');
283-
assert.ok(event.new, 'Event should have new environment');
284-
285-
// If event fired, old and new should be the same (idempotent)
286-
if (event.old) {
287-
assert.strictEqual(
288-
event.old.envId.id,
289-
event.new.envId.id,
290-
'Idempotent operation should have same old and new environment',
291-
);
292-
}
293-
} else {
294-
// No event fired - this is the ideal idempotent behavior
295-
assert.ok(!handler.fired, 'No event should fire when setting same environment');
296-
}
278+
// Idempotent behavior: no event should fire when setting same environment
279+
assert.strictEqual(handler.fired, false, 'No event should fire when setting the same environment');
297280
} finally {
298281
handler.dispose();
299282
}
@@ -303,9 +286,10 @@ suite('Integration: Interpreter Selection Priority', function () {
303286
* Test: Clearing selection falls back to auto-discovery
304287
*
305288
* After clearing an explicit selection, auto-discovery should
306-
* provide a fallback environment if available.
289+
* provide a fallback environment. The system should find an
290+
* auto-discovered environment (e.g., .venv in workspace).
307291
*/
308-
test('Clearing selection allows auto-discovery fallback', async function () {
292+
test('Clearing selection falls back to auto-discovery', async function () {
309293
const environments = await api.getEnvironments('all');
310294

311295
if (environments.length === 0) {
@@ -321,19 +305,13 @@ suite('Integration: Interpreter Selection Priority', function () {
321305
// Clear the selection
322306
await api.setEnvironment(undefined, undefined);
323307

324-
// Get environment - may return auto-discovered env or undefined
308+
// Get environment - should return auto-discovered environment
325309
const autoEnv = await api.getEnvironment(undefined);
326310

327-
// Key assertion: clearing should have an effect
328-
// Either returns undefined OR returns a different environment (auto-discovered)
329-
if (autoEnv) {
330-
assert.ok(autoEnv.envId, 'Auto-discovered env must have envId');
331-
assert.ok(autoEnv.envId.id, 'Auto-discovered env must have envId.id');
332-
// Note: autoEnv might equal beforeClear if auto-discovery picks the same one,
333-
// but the explicit selection should be cleared
334-
} else {
335-
// Undefined is a valid result - no auto-discovery available
336-
assert.strictEqual(autoEnv, undefined, 'Cleared selection with no auto-discovery should return undefined');
337-
}
311+
// Auto-discovery should provide a fallback environment
312+
assert.ok(autoEnv, 'Auto-discovery should provide a fallback environment after clearing');
313+
assert.ok(autoEnv.envId, 'Auto-discovered env must have envId');
314+
assert.ok(autoEnv.envId.id, 'Auto-discovered env must have envId.id');
315+
assert.ok(autoEnv.displayName, 'Auto-discovered env must have displayName');
338316
});
339317
});

‎src/test/integration/packageManagement.integration.test.ts‎

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ suite('Integration: Package Management', function () {
126126
/**
127127
* Test: refreshPackages updates package list
128128
*
129-
* After refreshing, the package list should be up to date.
129+
* After refreshing, the package list should be consistent.
130+
* Multiple calls should return the same packages (idempotent).
130131
*/
131132
test('refreshPackages updates list', async function () {
132133
const environments = await api.getEnvironments('all');
@@ -146,6 +147,8 @@ suite('Integration: Package Management', function () {
146147
return;
147148
}
148149

150+
const initialCount = initial.length;
151+
149152
// Refresh
150153
await api.refreshPackages(env);
151154

@@ -154,16 +157,20 @@ suite('Integration: Package Management', function () {
154157

155158
assert.ok(Array.isArray(after), 'Should return array after refresh');
156159

157-
// Package counts should be similar (no external changes during test)
158-
// Allow some variance for cache effects
160+
// Package counts should be identical (no external changes during test)
161+
assert.strictEqual(
162+
after.length,
163+
initialCount,
164+
`Package count should be stable after refresh: expected ${initialCount}, got ${after.length}`,
165+
);
159166
});
160167

161168
/**
162-
* Test: Standard library packages typically present
169+
* Test: getPackages returns non-empty array for environments with packages
163170
*
164-
* Most Python environments should have pip installed.
171+
* For virtual environments, at minimum pip should typically be present.
165172
*/
166-
test('Common packages are discoverable', async function () {
173+
test('getPackages returns packages for virtual environment', async function () {
167174
const environments = await api.getEnvironments('all');
168175

169176
if (environments.length === 0) {
@@ -172,35 +179,32 @@ suite('Integration: Package Management', function () {
172179
}
173180

174181
// Find a virtual environment (more likely to have pip)
175-
let targetEnv = environments.find(
182+
const targetEnv = environments.find(
176183
(env) =>
177184
env.displayName.includes('venv') ||
178185
env.displayName.includes('.venv') ||
179186
env.envId.managerId.includes('venv'),
180187
);
181188

182189
if (!targetEnv) {
183-
// Fall back to first environment
184-
targetEnv = environments[0];
190+
console.log('No virtual environment found, skipping');
191+
this.skip();
192+
return;
185193
}
186194

187195
const packages = await api.getPackages(targetEnv);
188196

189-
if (!packages || packages.length === 0) {
190-
console.log('No packages found in:', targetEnv.displayName);
197+
if (packages === undefined) {
198+
console.log('Package manager not available for:', targetEnv.displayName);
191199
this.skip();
192200
return;
193201
}
194202

195-
// Look for common packages
203+
// Virtual environments should have at least pip installed
196204
const pipInstalled = packages.some((p) => p.name.toLowerCase() === 'pip');
197-
const setuptoolsInstalled = packages.some((p) => p.name.toLowerCase() === 'setuptools');
205+
assert.ok(pipInstalled, `Virtual environment ${targetEnv.displayName} should have pip installed`);
198206

199-
// Virtual environment should have pip or at least some packages
200-
assert.ok(pipInstalled || packages.length > 0, 'Virtual environment should have pip or at least some packages');
201-
console.log(
202-
`pip installed: ${pipInstalled}, setuptools installed: ${setuptoolsInstalled}, total: ${packages.length}`,
203-
);
207+
console.log(`Found ${packages.length} packages in ${targetEnv.displayName}`);
204208
});
205209

206210
/**
@@ -353,25 +357,40 @@ suite('Integration: Package Management', function () {
353357
});
354358

355359
/**
356-
* Test: Invalid environment returns undefined packages
360+
* Test: getPackages returns array or undefined, never throws
357361
*
358-
* For an environment without a package manager, should return undefined.
362+
* For any environment, getPackages should return either a valid
363+
* array of packages or undefined (if no package manager), never throw.
359364
*/
360-
test('Missing package manager returns undefined', async function () {
365+
test('getPackages returns array or undefined for all environments', async function () {
361366
const environments = await api.getEnvironments('all');
362367

363368
if (environments.length === 0) {
364369
this.skip();
365370
return;
366371
}
367372

368-
// System Python or unusual environments may not have package managers
373+
let arrayCount = 0;
374+
let undefinedCount = 0;
375+
376+
// Verify each environment returns valid result
369377
for (const env of environments) {
370378
const packages = await api.getPackages(env);
371-
// Result should be either array or undefined, never throw
372379
if (packages !== undefined) {
373-
assert.ok(Array.isArray(packages), 'Should be array if defined');
380+
assert.ok(Array.isArray(packages), `getPackages should return array for ${env.displayName}`);
381+
arrayCount++;
382+
} else {
383+
undefinedCount++;
374384
}
375385
}
386+
387+
// Log results for visibility
388+
console.log(`getPackages results: ${arrayCount} returned arrays, ${undefinedCount} returned undefined`);
389+
390+
// At least some should return arrays (unless all envs lack package managers)
391+
assert.ok(
392+
arrayCount > 0 || undefinedCount === environments.length,
393+
'At least one environment should have a package manager, or all should return undefined consistently',
394+
);
376395
});
377396
});

‎src/test/integration/pythonProjects.integration.test.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,19 @@ suite('Integration: Python Projects', function () {
173173
// Set environment for project
174174
await api.setEnvironment(project.uri, env);
175175

176-
// Get environment and verify
176+
// Wait for the environment to be retrievable with the correct ID
177+
// This handles async persistence across platforms
178+
await waitForCondition(
179+
async () => {
180+
const retrieved = await api.getEnvironment(project.uri);
181+
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
182+
},
183+
5000,
184+
`Environment was not set correctly. Expected envId: ${env.envId.id}`,
185+
);
186+
187+
// Final verification
177188
const retrievedEnv = await api.getEnvironment(project.uri);
178-
179189
assert.ok(retrievedEnv, 'Should get environment after setting');
180190
assert.strictEqual(retrievedEnv.envId.id, env.envId.id, 'Retrieved environment should match set environment');
181191
});
@@ -189,38 +199,41 @@ suite('Integration: Python Projects', function () {
189199
const projects = api.getPythonProjects();
190200
const environments = await api.getEnvironments('all');
191201

192-
if (projects.length === 0 || environments.length === 0) {
202+
if (projects.length === 0 || environments.length < 2) {
203+
// Need at least 2 environments to guarantee a change
193204
this.skip();
194205
return;
195206
}
196207

197208
const project = projects[0];
198209

199-
// Get current environment to ensure we make an actual change
210+
// Get current environment to pick a different one
200211
const currentEnv = await api.getEnvironment(project.uri);
201212

202-
// Pick an environment different from current, or clear and re-set
213+
// Pick an environment different from current
203214
let targetEnv = environments[0];
204-
if (currentEnv && currentEnv.envId.id === targetEnv.envId.id && environments.length > 1) {
215+
if (currentEnv && currentEnv.envId.id === targetEnv.envId.id) {
205216
targetEnv = environments[1];
206217
}
207218

208-
// Clear environment first to ensure change event fires
209-
// (in case targetEnv is already set)
210-
await api.setEnvironment(project.uri, undefined);
211-
219+
// Register handler BEFORE making the change
212220
const handler = new TestEventHandler(api.onDidChangeEnvironment, 'onDidChangeEnvironment');
213221

214222
try {
215223
// Set environment - this should fire the event
216224
await api.setEnvironment(project.uri, targetEnv);
217225

218-
// Event should fire
219-
await handler.assertFired(5000);
226+
// Wait for an event where event.new is defined (the actual change event)
227+
await waitForCondition(
228+
() => handler.all.some((e) => e.new !== undefined),
229+
5000,
230+
'onDidChangeEnvironment with new environment was not fired',
231+
);
220232

221-
const event = handler.last;
222-
assert.ok(event, 'Event should have value');
223-
assert.ok(event.new, 'Event should have new environment');
233+
// Find the event with the new environment
234+
const changeEvent = handler.all.find((e) => e.new !== undefined);
235+
assert.ok(changeEvent, 'Should have change event with new environment');
236+
assert.ok(changeEvent.new, 'Event should have new environment');
224237
} finally {
225238
handler.dispose();
226239
}
@@ -247,6 +260,16 @@ suite('Integration: Python Projects', function () {
247260
// Set environment first
248261
await api.setEnvironment(project.uri, env);
249262

263+
// Wait for it to be set
264+
await waitForCondition(
265+
async () => {
266+
const retrieved = await api.getEnvironment(project.uri);
267+
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
268+
},
269+
5000,
270+
'Environment was not set before clearing',
271+
);
272+
250273
// Verify it was set
251274
const beforeClear = await api.getEnvironment(project.uri);
252275
assert.ok(beforeClear, 'Environment should be set before clearing');
@@ -295,6 +318,16 @@ suite('Integration: Python Projects', function () {
295318
// Set environment for project
296319
await api.setEnvironment(project.uri, env);
297320

321+
// Wait for it to be set
322+
await waitForCondition(
323+
async () => {
324+
const retrieved = await api.getEnvironment(project.uri);
325+
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
326+
},
327+
5000,
328+
'Environment was not set for project',
329+
);
330+
298331
// Create a hypothetical file path inside the project
299332
const fileUri = vscode.Uri.joinPath(project.uri, 'some_script.py');
300333

0 commit comments

Comments
 (0)