Skip to content

Commit 089f5e5

Browse files
plemarquandplemarquand
committed
Weave cancellation through test discovery
1 parent 98ec9d5 commit 089f5e5

File tree

4 files changed

+85
-58
lines changed

4 files changed

+85
-58
lines changed

‎src/TestExplorer/TestExplorer.ts‎

Lines changed: 75 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class TestExplorer {
3737
private lspTestDiscovery: LSPTestDiscovery;
3838
private subscriptions: { dispose(): unknown }[];
3939
private testFileEdited = true;
40+
private tokenSource = new vscode.CancellationTokenSource();
4041

4142
// Emits after the `vscode.TestController` has been updated.
4243
private onTestItemsDidChangeEmitter = new vscode.EventEmitter<vscode.TestController>();
@@ -56,7 +57,7 @@ export class TestExplorer {
5657

5758
this.controller.resolveHandler = async item => {
5859
if (!item) {
59-
await this.discoverTestsInWorkspace();
60+
await this.discoverTestsInWorkspace(this.tokenSource.token);
6061
}
6162
};
6263

@@ -86,7 +87,7 @@ export class TestExplorer {
8687
this.testFileEdited = false;
8788
// only run discover tests if the library has tests
8889
if (this.folderContext.swiftPackage.getTargets(TargetType.test).length > 0) {
89-
this.discoverTestsInWorkspace();
90+
this.discoverTestsInWorkspace(this.tokenSource.token);
9091
}
9192
}
9293
});
@@ -99,6 +100,7 @@ export class TestExplorer {
99100
});
100101

101102
this.subscriptions = [
103+
this.tokenSource,
102104
fileWatcher,
103105
onDidEndTask,
104106
this.controller,
@@ -110,6 +112,9 @@ export class TestExplorer {
110112
}
111113

112114
dispose() {
115+
this.controller.refreshHandler = undefined;
116+
this.controller.resolveHandler = undefined;
117+
this.tokenSource.cancel();
113118
this.subscriptions.forEach(element => element.dispose());
114119
}
115120

@@ -120,47 +125,64 @@ export class TestExplorer {
120125
* @returns Observer disposable
121126
*/
122127
static observeFolders(workspaceContext: WorkspaceContext): vscode.Disposable {
123-
return workspaceContext.onDidChangeFolders(({ folder, operation, workspace }) => {
124-
switch (operation) {
125-
case FolderOperation.add:
126-
if (folder) {
127-
if (folder.swiftPackage.getTargets(TargetType.test).length > 0) {
128-
folder.addTestExplorer();
129-
// discover tests in workspace but only if disableAutoResolve is not on.
130-
// discover tests will kick off a resolve if required
131-
if (!configuration.folder(folder.workspaceFolder).disableAutoResolve) {
132-
folder.testExplorer?.discoverTestsInWorkspace();
128+
const tokenSource = new vscode.CancellationTokenSource();
129+
const disposable = workspaceContext.onDidChangeFolders(
130+
({ folder, operation, workspace }) => {
131+
switch (operation) {
132+
case FolderOperation.add:
133+
if (folder) {
134+
if (folder.swiftPackage.getTargets(TargetType.test).length > 0) {
135+
folder.addTestExplorer();
136+
// discover tests in workspace but only if disableAutoResolve is not on.
137+
// discover tests will kick off a resolve if required
138+
if (
139+
!configuration.folder(folder.workspaceFolder).disableAutoResolve
140+
) {
141+
folder.testExplorer?.discoverTestsInWorkspace(
142+
tokenSource.token
143+
);
144+
}
133145
}
134146
}
135-
}
136-
break;
137-
case FolderOperation.packageUpdated:
138-
if (folder) {
139-
const hasTestTargets =
140-
folder.swiftPackage.getTargets(TargetType.test).length > 0;
141-
if (hasTestTargets && !folder.hasTestExplorer()) {
142-
folder.addTestExplorer();
143-
// discover tests in workspace but only if disableAutoResolve is not on.
144-
// discover tests will kick off a resolve if required
145-
if (!configuration.folder(folder.workspaceFolder).disableAutoResolve) {
146-
folder.testExplorer?.discoverTestsInWorkspace();
147+
break;
148+
case FolderOperation.packageUpdated:
149+
if (folder) {
150+
const hasTestTargets =
151+
folder.swiftPackage.getTargets(TargetType.test).length > 0;
152+
if (hasTestTargets && !folder.hasTestExplorer()) {
153+
folder.addTestExplorer();
154+
// discover tests in workspace but only if disableAutoResolve is not on.
155+
// discover tests will kick off a resolve if required
156+
if (
157+
!configuration.folder(folder.workspaceFolder).disableAutoResolve
158+
) {
159+
folder.testExplorer?.discoverTestsInWorkspace(
160+
tokenSource.token
161+
);
162+
}
163+
} else if (!hasTestTargets && folder.hasTestExplorer()) {
164+
folder.removeTestExplorer();
165+
} else if (folder.hasTestExplorer()) {
166+
folder.refreshTestExplorer();
147167
}
148-
} else if (!hasTestTargets && folder.hasTestExplorer()) {
149-
folder.removeTestExplorer();
150-
} else if (folder.hasTestExplorer()) {
151-
folder.refreshTestExplorer();
152168
}
153-
}
154-
break;
155-
case FolderOperation.focus:
156-
if (folder) {
157-
workspace.languageClientManager.documentSymbolWatcher = (
158-
document,
159-
symbols
160-
) => TestExplorer.onDocumentSymbols(folder, document, symbols);
161-
}
169+
break;
170+
case FolderOperation.focus:
171+
if (folder) {
172+
workspace.languageClientManager.documentSymbolWatcher = (
173+
document,
174+
symbols
175+
) => TestExplorer.onDocumentSymbols(folder, document, symbols);
176+
}
177+
}
162178
}
163-
});
179+
);
180+
return {
181+
dispose: () => {
182+
tokenSource.dispose();
183+
disposable.dispose();
184+
},
185+
};
164186
}
165187

166188
/**
@@ -224,25 +246,25 @@ export class TestExplorer {
224246
/**
225247
* Discover tests
226248
*/
227-
async discoverTestsInWorkspace() {
249+
async discoverTestsInWorkspace(token: vscode.CancellationToken) {
228250
try {
229251
// If the LSP cannot produce a list of tests it throws and
230252
// we fall back to discovering tests with SPM.
231-
await this.discoverTestsInWorkspaceLSP();
253+
await this.discoverTestsInWorkspaceLSP(token);
232254
} catch {
233255
this.folderContext.workspaceContext.outputChannel.logDiagnostic(
234256
"workspace/tests LSP request not supported, falling back to SPM to discover tests.",
235257
"Test Discovery"
236258
);
237-
await this.discoverTestsInWorkspaceSPM();
259+
await this.discoverTestsInWorkspaceSPM(token);
238260
}
239261
}
240262

241263
/**
242264
* Discover tests
243265
* Uses `swift test --list-tests` to get the list of tests
244266
*/
245-
async discoverTestsInWorkspaceSPM() {
267+
async discoverTestsInWorkspaceSPM(token: vscode.CancellationToken) {
246268
async function runDiscover(explorer: TestExplorer, firstTry: boolean) {
247269
try {
248270
const toolchain = explorer.folderContext.workspaceContext.toolchain;
@@ -263,6 +285,11 @@ export class TestExplorer {
263285
return;
264286
}
265287
}
288+
289+
if (token.isCancellationRequested) {
290+
return;
291+
}
292+
266293
// get list of tests from `swift test --list-tests`
267294
let listTestArguments: string[];
268295
if (toolchain.swiftVersion.isGreaterThanOrEqual(new Version(5, 8, 0))) {
@@ -284,7 +311,7 @@ export class TestExplorer {
284311
explorer.updateTests(explorer.controller, tests);
285312
}
286313
);
287-
await explorer.folderContext.taskQueue.queueOperation(listTestsOperation);
314+
await explorer.folderContext.taskQueue.queueOperation(listTestsOperation, token);
288315
} catch (error) {
289316
// If a test list fails its possible the tests have not been built.
290317
// Build them and try again, and if we still fail then notify the user.
@@ -336,10 +363,14 @@ export class TestExplorer {
336363
/**
337364
* Discover tests
338365
*/
339-
async discoverTestsInWorkspaceLSP() {
366+
async discoverTestsInWorkspaceLSP(token: vscode.CancellationToken) {
340367
const tests = await this.lspTestDiscovery.getWorkspaceTests(
341368
this.folderContext.swiftPackage
342369
);
370+
if (token.isCancellationRequested) {
371+
return;
372+
}
373+
343374
TestDiscovery.updateTestsFromClasses(
344375
this.controller,
345376
this.folderContext.swiftPackage,

‎src/extension.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,10 @@ export interface Api {
5555
*/
5656
export async function activate(context: vscode.ExtensionContext): Promise<Api> {
5757
try {
58-
const outputChannel = new SwiftOutputChannel("Swift", !process.env["VSCODE_TEST"]);
58+
const outputChannel = new SwiftOutputChannel(
59+
`Swift ${process.env["VSCODE_TEST"]}`,
60+
!process.env["VSCODE_TEST"]
61+
);
5962
outputChannel.log("Activating Swift for Visual Studio Code...");
6063

6164
checkAndWarnAboutWindowsSymlinks(outputChannel);

‎src/tasks/TaskQueue.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ export class TaskOperation implements SwiftOperation {
8585
workspaceContext: WorkspaceContext,
8686
token?: vscode.CancellationToken
8787
): Promise<number | undefined> {
88+
if (token?.isCancellationRequested) {
89+
return Promise.resolve(undefined);
90+
}
8891
workspaceContext.outputChannel.log(`Exec Task: ${this.task.detail ?? this.task.name}`);
8992
return workspaceContext.tasks.executeTaskAndWait(this.task, token);
9093
}
@@ -230,6 +233,7 @@ export class TaskQueue {
230233
if (!this.activeOperation) {
231234
// get task from queue
232235
const operation = this.queue.shift();
236+
233237
if (operation) {
234238
//const task = operation.task;
235239
this.activeOperation = operation;
@@ -250,20 +254,14 @@ export class TaskQueue {
250254
.run(this.workspaceContext)
251255
.then(result => {
252256
// log result
253-
if (operation.log) {
257+
if (operation.log && !operation.token?.isCancellationRequested) {
254258
switch (result) {
255259
case 0:
256260
this.workspaceContext.outputChannel.log(
257261
`${operation.log}: ... done.`,
258262
this.folderContext.name
259263
);
260264
break;
261-
case undefined:
262-
this.workspaceContext.outputChannel.log(
263-
`${operation.log}: ... cancelled.`,
264-
this.folderContext.name
265-
);
266-
break;
267265
default:
268266
this.workspaceContext.outputChannel.log(
269267
`${operation.log}: ... failed.`,

‎test/integration-tests/ui/PackageDependencyProvider.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import {
2020
import { executeTaskAndWaitForResult, waitForNoRunningTasks } from "../../utilities";
2121
import { getBuildAllTask, SwiftTask } from "../../../src/tasks/SwiftTaskProvider";
2222
import { testAssetPath } from "../../fixtures";
23-
import { Version } from "../../../src/utilities/version";
2423
import {
2524
activateExtension,
2625
deactivateExtension,
@@ -33,10 +32,6 @@ suite("PackageDependencyProvider Test Suite", function () {
3332

3433
suiteSetup(async function () {
3534
const workspaceContext = await activateExtension();
36-
// workspace-state.json was not introduced until swift 5.7
37-
if (workspaceContext.toolchain.swiftVersion.isLessThan(new Version(5, 7, 0))) {
38-
this.skip();
39-
}
4035
await waitForNoRunningTasks();
4136
const folderContext = await folderInRootWorkspace("dependencies", workspaceContext);
4237
await executeTaskAndWaitForResult((await getBuildAllTask(folderContext)) as SwiftTask);
@@ -45,7 +40,7 @@ suite("PackageDependencyProvider Test Suite", function () {
4540
});
4641

4742
suiteTeardown(async () => {
48-
treeProvider?.dispose();
43+
treeProvider.dispose();
4944
await deactivateExtension();
5045
});
5146

0 commit comments

Comments
 (0)