Skip to content

feat: switch to event-driven approach for environment retrieval in integration tests #1311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eleanorjboyd merged 7 commits into from
Mar 25, 2026
Merged

feat: switch to event-driven approach for environment retrieval in integration tests #1311

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bf34940
feat: switch to event-driven approach for environment retrieval in in…
eleanorjboyd Mar 3, 2026
d8117e6
Merge origin/main into confused-mule (resolve conflict in integration…
eleanorjboyd Mar 19, 2026
f7fe643
fix listening
eleanorjboyd Mar 25, 2026
0047da9
Merge branch 'main' into confused-mule
eleanorjboyd Mar 25, 2026
0bb04cb
event-driven + "pick a different env" pattern
eleanorjboyd Mar 25, 2026
e03f10b
updates
eleanorjboyd Mar 25, 2026
9b78d87
updates to tests based on copilot feedback
eleanorjboyd Mar 25, 2026
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
181 changes: 100 additions & 81 deletions src/test/integration/pythonProjects.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,54 +162,53 @@ suite('Integration: Python Projects', function () {

const environments = await api.getEnvironments('all');

if (environments.length === 0) {
if (environments.length < 2) {
this.skip();
return;
}

const project = projects[0];
const env = environments[0];

// Diagnostic logging for CI debugging
console.log(`[TEST DEBUG] Project URI: ${project.uri.fsPath}`);
console.log(`[TEST DEBUG] Setting environment with envId: ${env.envId.id}`);
console.log(`[TEST DEBUG] Environment path: ${env.environmentPath?.fsPath}`);
console.log(`[TEST DEBUG] Total environments available: ${environments.length}`);
environments.forEach((e, i) => {
console.log(`[TEST DEBUG] env[${i}]: ${e.envId.id} (${e.displayName})`);
});

// Set environment for project
await api.setEnvironment(project.uri, env);

// Track what getEnvironment returns during polling for diagnostics
let pollCount = 0;
let lastRetrievedId: string | undefined;
let lastRetrievedManagerId: string | undefined;

// Wait for the environment to be retrievable with the correct ID
// This handles async persistence across platforms
// Use 15s timeout - CI runners (especially macos) can be slow with settings persistence
await waitForCondition(
async () => {
const retrieved = await api.getEnvironment(project.uri);
pollCount++;
const retrievedId = retrieved?.envId?.id;
lastRetrievedManagerId = retrieved?.envId?.managerId;
if (retrievedId !== lastRetrievedId) {
console.log(
`[TEST DEBUG] Poll #${pollCount}: getEnvironment returned envId=${retrievedId ?? 'undefined'}, managerId=${lastRetrievedManagerId ?? 'undefined'}`,
);
lastRetrievedId = retrievedId;
// Pick an environment different from the current one so setEnvironment
// actually triggers a change event. If we pick the already-active env,
// the event never fires and the test times out.
const currentEnv = await api.getEnvironment(project.uri);
let env = environments[0];
if (currentEnv && currentEnv.envId.id === env.envId.id) {
env = environments[1];
}
Comment thread
eleanorjboyd marked this conversation as resolved.
Outdated

// Wait for the change event, then verify getEnvironment.
// Using an event-driven approach instead of polling avoids a race condition where
// setEnvironment's async settings write hasn't landed by the time getEnvironment
// reads back the manager from settings.
await new Promise<void>((resolve, reject) => {
const timeout = setTimeout(() => {
subscription.dispose();
reject(
new Error(
`onDidChangeEnvironment did not fire for project within 15s. Expected envId: ${env.envId.id}`,
),
);
}, 15_000);

const subscription = api.onDidChangeEnvironment((e) => {
Comment thread
eleanorjboyd marked this conversation as resolved.
Outdated
if (e.uri?.toString() === project.uri.toString() && e.new?.envId.id === env.envId.id) {
clearTimeout(timeout);
subscription.dispose();
resolve();
}
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
},
15_000,
() =>
`Environment was not set correctly. Expected envId: ${env.envId.id} (manager: ${env.envId.managerId}), last retrieved: ${lastRetrievedId ?? 'undefined'} (manager: ${lastRetrievedManagerId ?? 'undefined'}) after ${pollCount} polls`,
);

// Final verification
});

// Set environment after subscribing so we don't miss the event
api.setEnvironment(project.uri, env).catch((err) => {
clearTimeout(timeout);
subscription.dispose();
reject(err);
});
});

// Verify getEnvironment returns the correct value now that setEnvironment has fully completed
const retrievedEnv = await api.getEnvironment(project.uri);
assert.ok(retrievedEnv, 'Should get environment after setting');
assert.strictEqual(retrievedEnv.envId.id, env.envId.id, 'Retrieved environment should match set environment');
Expand Down Expand Up @@ -275,30 +274,41 @@ suite('Integration: Python Projects', function () {
const projects = api.getPythonProjects();
const environments = await api.getEnvironments('all');

if (projects.length === 0 || environments.length === 0) {
if (projects.length === 0 || environments.length < 2) {
this.skip();
return;
}

const project = projects[0];
const env = environments[0];

// Set environment first
await api.setEnvironment(project.uri, env);

// Wait for it to be set
// Use 15s timeout - CI runners can be slow with settings persistence
let clearTestLastId: string | undefined;
await waitForCondition(
async () => {
const retrieved = await api.getEnvironment(project.uri);
clearTestLastId = retrieved?.envId?.id;
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
},
15_000,
() =>
`Environment was not set before clearing. Expected: ${env.envId.id} (manager: ${env.envId.managerId}), got: ${clearTestLastId ?? 'undefined'}`,
);

// Pick an environment different from the current one to guarantee a change event
const currentEnv = await api.getEnvironment(project.uri);
let env = environments[0];
if (currentEnv && currentEnv.envId.id === env.envId.id) {
env = environments[1];
}

// Set environment first, using event-driven wait
await new Promise<void>((resolve, reject) => {
const timeout = setTimeout(() => {
subscription.dispose();
reject(new Error(`onDidChangeEnvironment did not fire within 15s. Expected envId: ${env.envId.id}`));
}, 15_000);

const subscription = api.onDidChangeEnvironment((e) => {
if (e.uri?.toString() === project.uri.toString() && e.new?.envId.id === env.envId.id) {
clearTimeout(timeout);
subscription.dispose();
resolve();
}
});

api.setEnvironment(project.uri, env).catch((err) => {
clearTimeout(timeout);
subscription.dispose();
reject(err);
});
});
Comment thread
eleanorjboyd marked this conversation as resolved.
Outdated

// Verify it was set
const beforeClear = await api.getEnvironment(project.uri);
Expand Down Expand Up @@ -337,32 +347,41 @@ suite('Integration: Python Projects', function () {
const projects = api.getPythonProjects();
const environments = await api.getEnvironments('all');

if (projects.length === 0 || environments.length === 0) {
if (projects.length === 0 || environments.length < 2) {
this.skip();
return;
}

const project = projects[0];
const env = environments[0];

// Set environment for project
await api.setEnvironment(project.uri, env);

// Wait for it to be set
// Use 15s timeout - CI runners can be slow with settings persistence
let fileTestLastId: string | undefined;
let fileTestLastManagerId: string | undefined;
await waitForCondition(
async () => {
const retrieved = await api.getEnvironment(project.uri);
fileTestLastId = retrieved?.envId?.id;
fileTestLastManagerId = retrieved?.envId?.managerId;
return retrieved !== undefined && retrieved.envId.id === env.envId.id;
},
15_000,
() =>
`Environment was not set for project. Expected: ${env.envId.id} (manager: ${env.envId.managerId}), got: ${fileTestLastId ?? 'undefined'} (manager: ${fileTestLastManagerId ?? 'undefined'})`,
);

// Pick an environment different from the current one to guarantee a change event
const currentEnv = await api.getEnvironment(project.uri);
let env = environments[0];
if (currentEnv && currentEnv.envId.id === env.envId.id) {
env = environments[1];
}

// Set environment for project, using event-driven wait
await new Promise<void>((resolve, reject) => {
const timeout = setTimeout(() => {
subscription.dispose();
reject(new Error(`onDidChangeEnvironment did not fire within 15s. Expected envId: ${env.envId.id}`));
}, 15_000);

const subscription = api.onDidChangeEnvironment((e) => {
if (e.uri?.toString() === project.uri.toString() && e.new?.envId.id === env.envId.id) {
clearTimeout(timeout);
subscription.dispose();
resolve();
}
});

api.setEnvironment(project.uri, env).catch((err) => {
clearTimeout(timeout);
subscription.dispose();
reject(err);
});
});

// Create a hypothetical file path inside the project
const fileUri = vscode.Uri.joinPath(project.uri, 'some_script.py');
Expand Down
Loading