Skip to content

Commit 3e7759c

Browse files
edvilmeCopilot
andcommitted
fix: remove drain handlers that fail on idempotent sets
setEnvironment only fires onDidChangeActiveEnvironment when the new env differs from the _activeSelection cache. When teardown restores the original env, the next test's setEnvironment is idempotent — no event fires — and drain handlers wait forever. Fix: persistence tests (setEnvironment persists, project independent, idempotent) don't need event waits at all — getEnvironment calls manager.get() directly, returning correct values immediately. The event test uses a setImmediate-based drain that tolerates silence when the first set is idempotent — just enough to let any pending setImmediate callbacks fire without blocking on a 15s timeout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent faa34f6 commit 3e7759c

1 file changed

Lines changed: 14 additions & 36 deletions

File tree

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

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,11 @@ suite('Integration: Interpreter Selection Priority', function () {
9696

9797
const envToSet = environments[0];
9898

99-
// Subscribe before setting so we can wait for the event to propagate.
100-
const drainHandler = new TestEventHandler<DidChangeEnvironmentEventArgs>(
101-
api.onDidChangeEnvironment,
102-
'drain',
103-
);
104-
10599
// Set environment globally
106100
await api.setEnvironment(undefined, envToSet);
107-
await drainHandler.assertFired(15_000);
108-
drainHandler.dispose();
109101

110-
// Get and verify
102+
// getEnvironment calls manager.get() directly (no cache),
103+
// so it returns the correct value immediately after set.
111104
const retrieved = await api.getEnvironment(undefined);
112105

113106
assert.ok(retrieved, 'Should have environment after setting');
@@ -137,26 +130,14 @@ suite('Integration: Interpreter Selection Priority', function () {
137130
const projectEnv = environments[1];
138131
const project = projects[0];
139132

140-
// Subscribe before setting so we can wait for events to propagate.
141-
const globalDrain = new TestEventHandler<DidChangeEnvironmentEventArgs>(
142-
api.onDidChangeEnvironment,
143-
'globalDrain',
144-
);
145-
146133
// Set global environment
147134
await api.setEnvironment(undefined, globalEnv);
148-
await globalDrain.assertFired(15_000);
149-
globalDrain.dispose();
150-
151-
const projectDrain = new TestEventHandler<DidChangeEnvironmentEventArgs>(
152-
api.onDidChangeEnvironment,
153-
'projectDrain',
154-
);
155135

156136
// Set different environment for project
157137
await api.setEnvironment(project.uri, projectEnv);
158-
await projectDrain.assertFired(15_000);
159-
projectDrain.dispose();
138+
139+
// getEnvironment calls manager.get() directly (no cache),
140+
// so it returns the correct value immediately after set.
160141

161142
// Verify global is unchanged
162143
const globalRetrieved = await api.getEnvironment(undefined);
@@ -194,16 +175,20 @@ suite('Integration: Interpreter Selection Priority', function () {
194175
const oldEnv = environments[0];
195176
const newEnv = environments[1];
196177

197-
// Subscribe a temporary handler BEFORE the first setEnvironment so we
198-
// can wait for its event to drain. This avoids the real handler
199-
// capturing a stale event from this initial call.
178+
// Set initial environment. Subscribe a temporary handler to drain
179+
// any async event from this call before we create the real handler.
180+
// If the env is already set (e.g. restored by teardown), no event
181+
// fires — that's fine, nothing to drain.
200182
const drainHandler = new TestEventHandler<DidChangeEnvironmentEventArgs>(
201183
api.onDidChangeEnvironment,
202184
'drain',
203185
);
204186

205187
await api.setEnvironment(undefined, oldEnv);
206-
await drainHandler.assertFired(15_000);
188+
189+
// Short wait: if an event fires, it arrives within one setImmediate tick.
190+
// If none fires (idempotent set), we don't block.
191+
await new Promise((resolve) => setImmediate(resolve));
207192
drainHandler.dispose();
208193

209194
const handler = new TestEventHandler<DidChangeEnvironmentEventArgs>(
@@ -311,15 +296,8 @@ suite('Integration: Interpreter Selection Priority', function () {
311296

312297
const env = environments[0];
313298

314-
// Set environment first time and wait for the event to propagate.
315-
const drainHandler = new TestEventHandler<DidChangeEnvironmentEventArgs>(
316-
api.onDidChangeEnvironment,
317-
'drain',
318-
);
319-
299+
// Set environment first time
320300
await api.setEnvironment(undefined, env);
321-
await drainHandler.assertFired(15_000);
322-
drainHandler.dispose();
323301

324302
// Set same environment again
325303
await api.setEnvironment(undefined, env);

0 commit comments

Comments
 (0)