Skip to content

Commit db97ada

Browse files
eleanorjboydeleanorjboyd
committed
fix test
1 parent 3d7d0c6 commit db97ada

File tree

3 files changed

+91
-22
lines changed

3 files changed

+91
-22
lines changed

‎src/client/testing/testController/common/discoveryHelpers.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,13 @@ export function cleanupOnCancellation(
9696
): void {
9797
traceInfo(`Test discovery cancelled, killing ${testProvider} subprocess for workspace ${uri.fsPath}`);
9898
if (proc) {
99+
traceVerbose(`Killing ${testProvider} subprocess for workspace ${uri.fsPath}`);
99100
proc.kill();
101+
} else {
102+
traceVerbose(`No ${testProvider} subprocess to kill for workspace ${uri.fsPath} (proc is undefined)`);
100103
}
104+
traceVerbose(`Resolving process completion deferred for ${testProvider} discovery in workspace ${uri.fsPath}`);
101105
processCompletion.resolve();
106+
traceVerbose(`Cancelling discovery pipe for ${testProvider} discovery in workspace ${uri.fsPath}`);
102107
pipeCancellation.cancel();
103108
}

‎src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
9696
uri,
9797
);
9898

99+
// Setup process handlers deferred (used by both execution paths)
100+
const deferredTillExecClose: Deferred<void> = createTestingDeferred();
101+
99102
try {
100103
// Build pytest command and arguments
101104
const settings = this.configSettings.getSettings(uri);
@@ -111,7 +114,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
111114
const mutableEnv = await configureDiscoveryEnv(this.envVarsService, uri, discoveryPipeName);
112115

113116
// Setup process handlers (shared by both execution paths)
114-
const deferredTillExecClose: Deferred<void> = createTestingDeferred();
115117
const handlers = createProcessHandlers('pytest', uri, cwd, this.resultResolver, deferredTillExecClose, [5]);
116118

117119
// Execute using environment extension if available
@@ -122,6 +124,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
122124
traceError(
123125
`Python environment not found for workspace ${uri.fsPath}. Cannot proceed with test discovery.`,
124126
);
127+
deferredTillExecClose.resolve();
125128
return;
126129
}
127130
traceVerbose(`Using Python environment: ${JSON.stringify(pythonEnv)}`);
@@ -161,11 +164,19 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
161164
traceError(
162165
`Failed to create execution service for workspace ${uri.fsPath}. Cannot proceed with test discovery.`,
163166
);
167+
deferredTillExecClose.resolve();
164168
return;
165169
}
166170
const execInfo = await execService.getExecutablePath();
167171
traceVerbose(`Using Python executable: ${execInfo} for workspace ${uri.fsPath}`);
168172

173+
// Check for cancellation before spawning process
174+
if (token?.isCancellationRequested) {
175+
traceInfo(`Pytest discovery cancelled before spawning process for workspace ${uri.fsPath}`);
176+
deferredTillExecClose.resolve();
177+
return;
178+
}
179+
169180
const spawnOptions: SpawnOptions = {
170181
cwd,
171182
throwOnStdErr: true,
@@ -174,30 +185,51 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
174185
};
175186

176187
let resultProc: ChildProcess | undefined;
177-
const result = execService.execObservable(commandArgs, spawnOptions);
178-
resultProc = result?.proc;
179188

180-
if (!resultProc) {
181-
traceError(`Failed to spawn pytest discovery subprocess for workspace ${uri.fsPath}`);
182-
return;
183-
}
184-
traceInfo(`Started pytest discovery subprocess (execution factory) for workspace ${uri.fsPath}`);
185-
186-
// Wire up cancellation and process events
187-
token?.onCancellationRequested(() => {
189+
// Set up cancellation handler before execObservable to catch early cancellations
190+
const cancellationHandler = token?.onCancellationRequested(() => {
191+
traceInfo(`Cancellation requested during pytest discovery for workspace ${uri.fsPath}`);
188192
cleanupOnCancellation('pytest', resultProc, deferredTillExecClose, discoveryPipeCancellation, uri);
189193
});
194+
195+
try {
196+
const result = execService.execObservable(commandArgs, spawnOptions);
197+
resultProc = result?.proc;
198+
199+
if (!resultProc) {
200+
traceError(`Failed to spawn pytest discovery subprocess for workspace ${uri.fsPath}`);
201+
deferredTillExecClose.resolve();
202+
return;
203+
}
204+
traceInfo(`Started pytest discovery subprocess (execution factory) for workspace ${uri.fsPath}`);
205+
} catch (error) {
206+
traceError(`Error spawning pytest discovery subprocess for workspace ${uri.fsPath}: ${error}`);
207+
deferredTillExecClose.resolve();
208+
cancellationHandler?.dispose();
209+
throw error;
210+
}
190211
resultProc.stdout?.on('data', handlers.onStdout);
191212
resultProc.stderr?.on('data', handlers.onStderr);
192213
resultProc.on('exit', handlers.onExit);
193214
resultProc.on('close', handlers.onClose);
194215

216+
cancellationHandler?.dispose();
217+
218+
// Check for early cancellation before awaitingre awaiting
219+
if (token?.isCancellationRequested) {
220+
traceInfo(`Pytest discovery was cancelled before process completion for workspace ${uri.fsPath}`);
221+
deferredTillExecClose.resolve();
222+
}
223+
224+
traceVerbose(`Waiting for pytest discovery subprocess to complete for workspace ${uri.fsPath}`);
195225
await deferredTillExecClose.promise;
196226
traceInfo(`Pytest discovery completed for workspace ${uri.fsPath}`);
197227
} catch (error) {
198228
traceError(`Error during pytest discovery for workspace ${uri.fsPath}: ${error}`);
229+
deferredTillExecClose.resolve();
199230
throw error;
200231
} finally {
232+
traceVerbose(`Cleaning up pytest discovery resources for workspace ${uri.fsPath}`);
201233
discoveryPipeCancellation.dispose();
202234
}
203235
}

‎src/client/testing/testController/unittest/testDiscoveryAdapter.ts‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
9595
uri,
9696
);
9797

98+
// Setup process handlers deferred (used by both execution paths)
99+
const deferredTillExecClose = createTestingDeferred();
100+
98101
try {
99102
// Build unittest command and arguments
100103
const settings = this.configSettings.getSettings(uri);
@@ -107,7 +110,6 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
107110
const mutableEnv = await configureDiscoveryEnv(this.envVarsService, uri, discoveryPipeName);
108111

109112
// Setup process handlers (shared by both execution paths)
110-
const deferredTillExecClose = createTestingDeferred();
111113
const handlers = createProcessHandlers('unittest', uri, cwd, this.resultResolver, deferredTillExecClose);
112114

113115
// Execute using environment extension if available
@@ -118,6 +120,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
118120
traceError(
119121
`Python environment not found for workspace ${uri.fsPath}. Cannot proceed with test discovery.`,
120122
);
123+
deferredTillExecClose.resolve();
121124
return;
122125
}
123126
traceVerbose(`Using Python environment: ${JSON.stringify(pythonEnv)}`);
@@ -157,11 +160,19 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
157160
traceError(
158161
`Failed to create execution service for workspace ${uri.fsPath}. Cannot proceed with test discovery.`,
159162
);
163+
deferredTillExecClose.resolve();
160164
return;
161165
}
162166
const execInfo = await execService.getExecutablePath();
163167
traceVerbose(`Using Python executable: ${execInfo} for workspace ${uri.fsPath}`);
164168

169+
// Check for cancellation before spawning process
170+
if (token?.isCancellationRequested) {
171+
traceInfo(`Unittest discovery cancelled before spawning process for workspace ${uri.fsPath}`);
172+
deferredTillExecClose.resolve();
173+
return;
174+
}
175+
165176
const spawnOptions: SpawnOptions = {
166177
cwd,
167178
throwOnStdErr: true,
@@ -170,30 +181,51 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
170181
};
171182

172183
let resultProc: ChildProcess | undefined;
173-
const result = execService.execObservable(execArgs, spawnOptions);
174-
resultProc = result?.proc;
175184

176-
if (!resultProc) {
177-
traceError(`Failed to spawn unittest discovery subprocess for workspace ${uri.fsPath}`);
178-
return;
179-
}
180-
traceInfo(`Started unittest discovery subprocess (execution factory) for workspace ${uri.fsPath}`);
181-
182-
// Wire up cancellation and process events
183-
token?.onCancellationRequested(() => {
185+
// Set up cancellation handler before execObservable to catch early cancellations
186+
const cancellationHandler = token?.onCancellationRequested(() => {
187+
traceInfo(`Cancellation requested during unittest discovery for workspace ${uri.fsPath}`);
184188
cleanupOnCancellation('unittest', resultProc, deferredTillExecClose, discoveryPipeCancellation, uri);
185189
});
190+
191+
try {
192+
const result = execService.execObservable(execArgs, spawnOptions);
193+
resultProc = result?.proc;
194+
195+
if (!resultProc) {
196+
traceError(`Failed to spawn unittest discovery subprocess for workspace ${uri.fsPath}`);
197+
deferredTillExecClose.resolve();
198+
return;
199+
}
200+
traceInfo(`Started unittest discovery subprocess (execution factory) for workspace ${uri.fsPath}`);
201+
} catch (error) {
202+
traceError(`Error spawning unittest discovery subprocess for workspace ${uri.fsPath}: ${error}`);
203+
deferredTillExecClose.resolve();
204+
cancellationHandler?.dispose();
205+
throw error;
206+
}
186207
resultProc.stdout?.on('data', handlers.onStdout);
187208
resultProc.stderr?.on('data', handlers.onStderr);
188209
resultProc.on('exit', handlers.onExit);
189210
resultProc.on('close', handlers.onClose);
190211

212+
cancellationHandler?.dispose();
213+
214+
// Check for early cancellation before awaitingre awaiting
215+
if (token?.isCancellationRequested) {
216+
traceInfo(`Unittest discovery was cancelled before process completion for workspace ${uri.fsPath}`);
217+
deferredTillExecClose.resolve();
218+
}
219+
220+
traceVerbose(`Waiting for unittest discovery subprocess to complete for workspace ${uri.fsPath}`);
191221
await deferredTillExecClose.promise;
192222
traceInfo(`Unittest discovery completed for workspace ${uri.fsPath}`);
193223
} catch (error) {
194224
traceError(`Error during unittest discovery for workspace ${uri.fsPath}: ${error}`);
225+
deferredTillExecClose.resolve();
195226
throw error;
196227
} finally {
228+
traceVerbose(`Cleaning up unittest discovery resources for workspace ${uri.fsPath}`);
197229
discoveryPipeCancellation.dispose();
198230
}
199231
}

0 commit comments

Comments
 (0)