From ec65867a75eff83788d810c57b9bd60452333e9f Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Mon, 14 Jun 2021 16:45:52 -0700 Subject: [PATCH] testing: improve cancellation, and allow test runs to be individually cancelled Fixes #125712 --- src/vs/base/test/common/mock.ts | 22 ++ src/vs/vscode.proposed.d.ts | 15 +- .../api/browser/mainThreadTesting.ts | 4 + .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 1 + src/vs/workbench/api/common/extHostTesting.ts | 281 ++++++++++-------- src/vs/workbench/api/common/extHostTypes.ts | 1 + .../testing/browser/testExplorerActions.ts | 7 +- .../testing/common/ownedTestCollection.ts | 1 - .../contrib/testing/common/testService.ts | 10 +- .../contrib/testing/common/testServiceImpl.ts | 35 ++- .../test/browser/api/extHostTesting.test.ts | 133 +++++++-- 12 files changed, 354 insertions(+), 158 deletions(-) diff --git a/src/vs/base/test/common/mock.ts b/src/vs/base/test/common/mock.ts index cb13ec7d523..3b31f0fb3d5 100644 --- a/src/vs/base/test/common/mock.ts +++ b/src/vs/base/test/common/mock.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { SinonStub, stub } from 'sinon'; + export interface Ctor { new(): T; } @@ -10,3 +12,23 @@ export interface Ctor { export function mock(): Ctor { return function () { } as any; } + +export type MockObject = { [K in keyof T]: K extends keyof TP ? TP[K] : SinonStub }; + +// Creates an object object that returns sinon mocks for every property. Optionally +// takes base properties. +export function mockObject>(properties?: TP): MockObject { + return new Proxy({ ...properties } as any, { + get(target, key) { + if (!target.hasOwnProperty(key)) { + target[key] = stub(); + } + + return target[key]; + }, + set(target, key, value) { + target[key] = value; + return true; + }, + }); +} diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 9ec3ab217e2..7da4d2b93ab 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1967,10 +1967,13 @@ declare module 'vscode' { * run should be created before the function returns or the reutrned * promise is resolved. * - * @param options Options for this test run - * @param cancellationToken Token that signals the used asked to abort the test run. + * @param request Request information for the test run + * @param cancellationToken Token that signals the used asked to abort the + * test run. If cancellation is requested on this token, all {@link TestRun} + * instances associated with the request will be + * automatically cancelled as well. */ - runTests(options: TestRunRequest, token: CancellationToken): Thenable | void; + runTests(request: TestRunRequest, token: CancellationToken): Thenable | void; } /** @@ -2008,6 +2011,12 @@ declare module 'vscode' { */ readonly name?: string; + /** + * A cancellation token which will be triggered when the test run is + * canceled from the UI. + */ + readonly token: CancellationToken; + /** * Updates the state of the test in the run. Calling with method with nodes * outside the {@link TestRunRequest.tests} or in the diff --git a/src/vs/workbench/api/browser/mainThreadTesting.ts b/src/vs/workbench/api/browser/mainThreadTesting.ts index 64f6a4a307c..a595d628d9a 100644 --- a/src/vs/workbench/api/browser/mainThreadTesting.ts +++ b/src/vs/workbench/api/browser/mainThreadTesting.ts @@ -52,6 +52,10 @@ export class MainThreadTesting extends Disposable implements MainThreadTestingSh this.proxy.$publishTestResults(prevResults); } + this._register(this.testService.onCancelTestRun(({ runId }) => { + this.proxy.$cancelExtensionTestRun(runId); + })); + this._register(resultService.onResultsChanged(evt => { const results = 'completed' in evt ? evt.completed : ('inserted' in evt ? evt.inserted : undefined); const serialized = results?.toJSON(); diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index f24fc12689f..3dad3e7b70e 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -379,7 +379,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I }, createTestRun(request, name, persist) { checkProposedApiEnabled(extension); - return extHostTesting.createTestRun(extension.identifier.value, request, name, persist); + return extHostTesting.createTestRun(request, name, persist); }, get onDidChangeTestResults() { checkProposedApiEnabled(extension); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index a07ea4f7a60..74fe7b5b4c0 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2063,6 +2063,7 @@ export const enum ExtHostTestingResource { export interface ExtHostTestingShape { $runTestsForProvider(req: RunTestForProviderRequest, token: CancellationToken): Promise; + $cancelExtensionTestRun(runId: string | undefined): void; $subscribeToTests(resource: ExtHostTestingResource, uri: UriComponents): void; $unsubscribeFromTests(resource: ExtHostTestingResource, uri: UriComponents): void; $lookupTest(test: TestIdWithSrc): Promise; diff --git a/src/vs/workbench/api/common/extHostTesting.ts b/src/vs/workbench/api/common/extHostTesting.ts index 213ffe7495f..507f122dbea 100644 --- a/src/vs/workbench/api/common/extHostTesting.ts +++ b/src/vs/workbench/api/common/extHostTesting.ts @@ -4,13 +4,13 @@ *--------------------------------------------------------------------------------------------*/ import { mapFind } from 'vs/base/common/arrays'; -import { Barrier, DeferredPromise, disposableTimeout, isThenable } from 'vs/base/common/async'; +import { disposableTimeout } from 'vs/base/common/async'; import { VSBuffer } from 'vs/base/common/buffer'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; -import { Emitter } from 'vs/base/common/event'; +import { Emitter, Event } from 'vs/base/common/event'; import { once } from 'vs/base/common/functional'; import { Iterable } from 'vs/base/common/iterator'; -import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { deepFreeze } from 'vs/base/common/objects'; import { isDefined } from 'vs/base/common/types'; import { URI, UriComponents } from 'vs/base/common/uri'; @@ -21,7 +21,7 @@ import { IExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocu import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService'; import { ExtHostTestItemEventType, getPrivateApiFor } from 'vs/workbench/api/common/extHostTestingPrivateApi'; import * as Convert from 'vs/workbench/api/common/extHostTypeConverters'; -import { Disposable, TestItemImpl } from 'vs/workbench/api/common/extHostTypes'; +import { TestItemImpl } from 'vs/workbench/api/common/extHostTypes'; import { IExtHostWorkspace } from 'vs/workbench/api/common/extHostWorkspace'; import { OwnedTestCollection, SingleUseTestCollection, TestPosition } from 'vs/workbench/contrib/testing/common/ownedTestCollection'; import { AbstractIncrementalTestCollection, IncrementalChangeCollector, IncrementalTestCollectionItem, InternalTestItem, ISerializedTestResults, ITestItem, RunTestForProviderRequest, TestDiffOpType, TestIdWithSrc, TestsDiff } from 'vs/workbench/contrib/testing/common/testCollection'; @@ -37,7 +37,7 @@ export class ExtHostTesting implements ExtHostTestingShape { }>(); private readonly proxy: MainThreadTestingShape; private readonly ownedTests = new OwnedTestCollection(); - private readonly runQueue: TestRunQueue; + private readonly runTracker: TestRunCoordinator; private readonly testControllers = new Map { + return toDisposable(() => { this.controllers.delete(controllerId); this.proxy.$unregisterTestController(controllerId); }); @@ -113,8 +113,8 @@ export class ExtHostTesting implements ExtHostTestingShape { /** * Implements vscode.test.createTestRun */ - public createTestRun(extensionId: string, request: vscode.TestRunRequest, name: string | undefined, persist = true): vscode.TestRun { - return this.runQueue.createTestRun(extensionId, request, name, persist); + public createTestRun(request: vscode.TestRunRequest, name: string | undefined, persist = true): vscode.TestRun { + return this.runTracker.createTestRun(request, name, persist); } /** @@ -274,13 +274,28 @@ export class ExtHostTesting implements ExtHostTestingShape { debug: req.debug, }; - await this.runQueue.enqueueRun({ - dto: TestRunDto.fromInternal(req), - token, - extensionId: controller.extensionId, - req: publicReq, - doRun: () => controller!.instance.runTests(publicReq, token) - }); + const tracker = this.runTracker.prepareForMainThreadTestRun(publicReq, TestRunDto.fromInternal(req), token); + + try { + await controller.instance.runTests(publicReq, token); + } finally { + if (tracker.isRunning && !token.isCancellationRequested) { + await Event.toPromise(tracker.onEnd); + } + + this.runTracker.cancelRunById(req.runId); + } + } + + /** + * Cancels an ongoing test run. + */ + public $cancelExtensionTestRun(runId: string | undefined) { + if (runId === undefined) { + this.runTracker.cancelAllRuns(); + } else { + this.runTracker.cancelRunById(runId); + } } public $lookupTest(req: TestIdWithSrc): Promise { @@ -316,121 +331,142 @@ export class ExtHostTesting implements ExtHostTestingShape { } } +class TestRunTracker extends Disposable { + private readonly task = new Set>(); + private readonly sharedTestIds = new Set(); + private readonly cts: CancellationTokenSource; + private readonly endEmitter = this._register(new Emitter()); + private disposed = false; + + /** + * Fires when a test ends, and no more tests are left running. + */ + public readonly onEnd = this.endEmitter.event; + + /** + * Gets whether there are any tests running. + */ + public get isRunning() { + return this.task.size > 0; + } + + /** + * Gets the run ID. + */ + public get id() { + return this.dto.id; + } + + constructor(private readonly dto: TestRunDto, private readonly proxy: MainThreadTestingShape, parentToken?: CancellationToken) { + super(); + this.cts = this._register(new CancellationTokenSource(parentToken)); + this._register(this.cts.token.onCancellationRequested(() => { + for (const task of this.task) { + task.end(); + } + })); + } + + public createRun(name: string | undefined) { + const run = new TestRunImpl(name, this.cts.token, this.dto, this.sharedTestIds, this.proxy, () => { + this.task.delete(run); + if (!this.isRunning) { + this.dispose(); + } + }); + + this.task.add(run); + return run; + } + + public override dispose() { + if (!this.disposed) { + this.disposed = true; + this.endEmitter.fire(); + this.cts.cancel(); + super.dispose(); + } + } +} + /** * Queues runs for a single extension and provides the currently-executing * run so that `createTestRun` can be properly correlated. */ -class TestRunQueue { - private readonly state = new Map, - factory: (name: string | undefined) => TestRunTask, - }, - queue: (() => (Promise | void))[]; - }>(); +export class TestRunCoordinator { + private tracked = new Map, TestRunTracker>(); + + public get trackers() { + return this.tracked.values(); + } constructor(private readonly proxy: MainThreadTestingShape) { } /** - * Registers and enqueues a test run. `doRun` will be called when an - * invokation to {@link TestController.runTests} should be called. + * Registers a request as being invoked by the main thread, so + * `$startedExtensionTestRun` is not invoked. The run must eventually + * be cancelled manually. */ - public enqueueRun(opts: { - extensionId: string, - req: vscode.TestRunRequest, - dto: TestRunDto, - token: CancellationToken, - doRun: () => Thenable | void, - }, - ) { - let record = this.state.get(opts.extensionId); - if (!record) { - record = { queue: [], current: undefined as any }; - this.state.set(opts.extensionId, record); - } - - const deferred = new DeferredPromise(); - const runner = () => { - const tasks: TestRunTask[] = []; - const shared = new Set(); - record!.current = { - publicReq: opts.req, - factory: name => { - const task = new TestRunTask(name, opts.dto, shared, this.proxy); - tasks.push(task); - opts.token.onCancellationRequested(() => task.end()); - return task; - }, - }; - - this.invokeRunner(opts.extensionId, opts.dto.id, opts.doRun, tasks).finally(() => deferred.complete()); - }; - - record.queue.push(runner); - if (record.queue.length === 1) { - runner(); - } - - return deferred.p; + public prepareForMainThreadTestRun(req: vscode.TestRunRequest, dto: TestRunDto, token: CancellationToken) { + return this.getTracker(req, dto, token); } + /** + * Cancels an existing test run via its cancellation token. + */ + public cancelRunById(runId: string) { + for (const tracker of this.tracked.values()) { + if (tracker.id === runId) { + tracker.dispose(); + return; + } + } + } + + /** + * Cancels an existing test run via its cancellation token. + */ + public cancelAllRuns() { + for (const tracker of this.tracked.values()) { + tracker.dispose(); + } + } + + /** * Implements the public `createTestRun` API. */ - public createTestRun(extensionId: string, request: vscode.TestRunRequest, name: string | undefined, persist: boolean): vscode.TestRun { - const state = this.state.get(extensionId); - // If the request is for the currently-executing `runTests`, then correlate - // it to that existing run. Otherwise return a new, detached run. - if (state?.current.publicReq === request) { - return state.current.factory(name); + public createTestRun(request: vscode.TestRunRequest, name: string | undefined, persist: boolean): vscode.TestRun { + const existing = this.tracked.get(request); + if (existing) { + return existing.createRun(name); } + // If there is not an existing tracked extension for the request, start + // a new, detached session. const dto = TestRunDto.fromPublic(request); this.proxy.$startedExtensionTestRun({ debug: request.debug, exclude: request.exclude?.map(t => t.id) ?? [], id: dto.id, tests: request.tests.map(t => t.id), - persist: persist + persist }); - const task = new TestRunTask(name, dto, new Set(), this.proxy); - task.onEnd.wait().then(() => this.proxy.$finishedExtensionTestRun(dto.id)); - return task; + + const tracker = this.getTracker(request, dto); + tracker.onEnd(() => this.proxy.$finishedExtensionTestRun(dto.id)); + return tracker.createRun(name); } - private invokeRunner(extensionId: string, runId: string, fn: () => Thenable | void, tasks: TestRunTask[]): Promise { - try { - const res = fn(); - if (isThenable(res)) { - return res - .then(() => this.handleInvokeResult(extensionId, runId, tasks, undefined)) - .catch(err => this.handleInvokeResult(extensionId, runId, tasks, err)); - } else { - return this.handleInvokeResult(extensionId, runId, tasks, undefined); - } - } catch (e) { - return this.handleInvokeResult(extensionId, runId, tasks, e); - } - } - - private async handleInvokeResult(extensionId: string, runId: string, tasks: TestRunTask[], error?: Error) { - const record = this.state.get(extensionId); - if (!record) { - return; - } - - record.queue.shift(); - if (record.queue.length > 0) { - record.queue[0](); - } else { - this.state.delete(extensionId); - } - - await Promise.all(tasks.map(t => t.onEnd.wait())); + private getTracker(req: vscode.TestRunRequest, dto: TestRunDto, token?: CancellationToken) { + const tracker = new TestRunTracker(dto, this.proxy, token); + this.tracked.set(req, tracker); + tracker.onEnd(() => this.tracked.delete(req)); + return tracker; } } -class TestRunDto { +export class TestRunDto { public static fromPublic(request: vscode.TestRunRequest) { return new TestRunDto( generateUuid(), @@ -466,46 +502,55 @@ class TestRunDto { } } -class TestRunTask implements vscode.TestRun { +class TestRunImpl implements vscode.TestRun { readonly #proxy: MainThreadTestingShape; readonly #req: TestRunDto; - readonly #taskId = generateUuid(); readonly #sharedIds: Set; - public readonly onEnd = new Barrier(); + readonly #onEnd: () => void; + #ended = false; + public readonly taskId = generateUuid(); constructor( public readonly name: string | undefined, + public readonly token: CancellationToken, dto: TestRunDto, sharedTestIds: Set, proxy: MainThreadTestingShape, + onEnd: () => void, ) { + this.#onEnd = onEnd; this.#proxy = proxy; this.#req = dto; this.#sharedIds = sharedTestIds; - proxy.$startedTestRunTask(dto.id, { id: this.#taskId, name, running: true }); + proxy.$startedTestRunTask(dto.id, { id: this.taskId, name, running: true }); } setState(test: vscode.TestItem, state: vscode.TestResultState, duration?: number): void { - if (this.#req.isIncluded(test)) { + if (!this.#ended && this.#req.isIncluded(test)) { this.ensureTestIsKnown(test); - this.#proxy.$updateTestStateInRun(this.#req.id, this.#taskId, test.id, state, duration); + this.#proxy.$updateTestStateInRun(this.#req.id, this.taskId, test.id, state, duration); } } appendMessage(test: vscode.TestItem, message: vscode.TestMessage): void { - if (this.#req.isIncluded(test)) { + if (!this.#ended && this.#req.isIncluded(test)) { this.ensureTestIsKnown(test); - this.#proxy.$appendTestMessageInRun(this.#req.id, this.#taskId, test.id, Convert.TestMessage.from(message)); + this.#proxy.$appendTestMessageInRun(this.#req.id, this.taskId, test.id, Convert.TestMessage.from(message)); } } appendOutput(output: string): void { - this.#proxy.$appendOutputToRun(this.#req.id, this.#taskId, VSBuffer.fromString(output)); + if (!this.#ended) { + this.#proxy.$appendOutputToRun(this.#req.id, this.taskId, VSBuffer.fromString(output)); + } } end(): void { - this.#proxy.$finishedTestRunTask(this.#req.id, this.#taskId); - this.onEnd.open(); + if (!this.#ended) { + this.#ended = true; + this.#proxy.$finishedTestRunTask(this.#req.id, this.taskId); + this.#onEnd(); + } } private ensureTestIsKnown(test: vscode.TestItem) { @@ -891,7 +936,7 @@ abstract class AbstractTestObserverFactory { /** * Starts listening to test information for the given resource. */ - protected abstract listen(resourceUri: URI, onDiff: (diff: TestsDiff) => void): Disposable; + protected abstract listen(resourceUri: URI, onDiff: (diff: TestsDiff) => void): IDisposable; private createObserverData(resourceUri: URI): IObserverData { const tests = new MirroredTestCollection(); @@ -939,7 +984,7 @@ class WorkspaceFolderTestObserverFactory extends AbstractTestObserverFactory { const uriString = resourceUri.toString(); this.diffListeners.set(uriString, onDiff); - return new Disposable(() => { + return toDisposable(() => { this.proxy.$unsubscribeFromDiffs(ExtHostTestingResource.Workspace, resourceUri); this.diffListeners.delete(uriString); }); @@ -966,14 +1011,14 @@ class TextDocumentTestObserverFactory extends AbstractTestObserverFactory { public listen(resourceUri: URI, onDiff: (diff: TestsDiff) => void) { const document = this.documents.getDocument(resourceUri); if (!document) { - return new Disposable(() => undefined); + return toDisposable(() => undefined); } const uriString = resourceUri.toString(); this.diffListeners.set(uriString, onDiff); this.proxy.$subscribeToDiffs(ExtHostTestingResource.TextDocument, resourceUri); - return new Disposable(() => { + return toDisposable(() => { this.proxy.$unsubscribeFromDiffs(ExtHostTestingResource.TextDocument, resourceUri); this.diffListeners.delete(uriString); }); diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 0e7a67b314e..f60700f0d0b 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3382,6 +3382,7 @@ export class TestItemImpl implements vscode.TestItem { } api.children.set(child.id, child); + getPrivateApiFor(child).parent = this; api.bus.fire([ExtHostTestItemEventType.NewChild, child]); } } diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index 6e86032e748..494345e6712 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -380,9 +380,12 @@ export class CancelTestRunAction extends Action2 { * @override */ public async run(accessor: ServicesAccessor) { + const resultService = accessor.get(ITestResultService); const testService = accessor.get(ITestService); - for (const run of testService.testRuns) { - testService.cancelTestRun(run); + for (const run of resultService.results) { + if (!run.completedAt) { + testService.cancelTestRun(run.id); + } } } } diff --git a/src/vs/workbench/contrib/testing/common/ownedTestCollection.ts b/src/vs/workbench/contrib/testing/common/ownedTestCollection.ts index 85aa126f556..db884efb473 100644 --- a/src/vs/workbench/contrib/testing/common/ownedTestCollection.ts +++ b/src/vs/workbench/contrib/testing/common/ownedTestCollection.ts @@ -370,7 +370,6 @@ export class SingleUseTestCollection implements IDisposable { this.pushDiff([TestDiffOpType.Add, { parent: parentId, src, expand, item: internal.item }]); const api = getPrivateApiFor(actual); - api.parent = parent?.actual; api.bus.event(this.onTestItemEvent.bind(this, internal)); // important that this comes after binding the event bus otherwise we diff --git a/src/vs/workbench/contrib/testing/common/testService.ts b/src/vs/workbench/contrib/testing/common/testService.ts index b6dad0dfac7..4c626df8fa1 100644 --- a/src/vs/workbench/contrib/testing/common/testService.ts +++ b/src/vs/workbench/contrib/testing/common/testService.ts @@ -161,9 +161,13 @@ export interface ITestService { readonly onShouldSubscribe: Event<{ resource: ExtHostTestingResource, uri: URI; }>; readonly onShouldUnsubscribe: Event<{ resource: ExtHostTestingResource, uri: URI; }>; readonly onDidChangeProviders: Event<{ delta: number; }>; + /** + * Fires when the user requests to cancel a test run -- or all runs, if no + * runId is given. + */ + readonly onCancelTestRun: Event<{ runId: string | undefined; }>; readonly providers: number; readonly subscriptions: ReadonlyArray<{ resource: ExtHostTestingResource, uri: URI; }>; - readonly testRuns: Iterable; /** * Set of test IDs the user asked to exclude. @@ -199,9 +203,9 @@ export interface ITestService { runTests(req: RunTestsRequest, token?: CancellationToken): Promise; /** - * Cancels an ongoign test run request. + * Cancels an ongoing test run by its ID, or all runs if no ID is given. */ - cancelTestRun(req: RunTestsRequest): void; + cancelTestRun(runId?: string): void; publishDiff(resource: ExtHostTestingResource, uri: URI, diff: TestsDiff): void; subscribeToDiffs(resource: ExtHostTestingResource, uri: URI, acceptDiff?: TestDiffListener): IReference; diff --git a/src/vs/workbench/contrib/testing/common/testServiceImpl.ts b/src/vs/workbench/contrib/testing/common/testServiceImpl.ts index db236368561..07a7103a567 100644 --- a/src/vs/workbench/contrib/testing/common/testServiceImpl.ts +++ b/src/vs/workbench/contrib/testing/common/testServiceImpl.ts @@ -44,10 +44,15 @@ export class TestService extends Disposable implements ITestService { private readonly unsubscribeEmitter = new Emitter(); private readonly busyStateChangeEmitter = new Emitter(); private readonly changeProvidersEmitter = new Emitter<{ delta: number }>(); + private readonly cancelExtensionTestRunEmitter = new Emitter<{ runId: string | undefined }>(); private readonly providerCount: IContextKey; private readonly hasRunnable: IContextKey; private readonly hasDebuggable: IContextKey; - private readonly runningTests = new Map(); + /** + * Cancellation for runs requested by the user being managed by the UI. + * Test runs initiated by extensions are not included here. + */ + private readonly uiRunningTests = new Map(); private readonly rootProviders = new Set(); public readonly excludeTests = MutableObservableValue.stored(new StoredValue>({ @@ -103,13 +108,6 @@ export class TestService extends Disposable implements ITestService { } } - /** - * Gets currently running tests. - */ - public get testRuns() { - return this.runningTests.keys(); - } - /** * Gets the current provider count. */ @@ -137,6 +135,11 @@ export class TestService extends Disposable implements ITestService { */ public readonly onBusyStateChange = this.busyStateChangeEmitter.event; + /** + * @inheritdoc + */ + public readonly onCancelTestRun = this.cancelExtensionTestRunEmitter.event; + /** * @inheritdoc */ @@ -147,8 +150,16 @@ export class TestService extends Disposable implements ITestService { /** * @inheritdoc */ - public cancelTestRun(req: RunTestsRequest) { - this.runningTests.get(req)?.cancel(); + public cancelTestRun(runId?: string) { + this.cancelExtensionTestRunEmitter.fire({ runId }); + + if (runId === undefined) { + for (const runCts of this.uiRunningTests.values()) { + runCts.cancel(); + } + } else { + this.uiRunningTests.get(runId)?.cancel(); + } } /** @@ -222,7 +233,7 @@ export class TestService extends Disposable implements ITestService { try { const tests = groupBy(testsWithIds, (a, b) => a.src.controller === b.src.controller ? 0 : 1); const cancelSource = new CancellationTokenSource(token); - this.runningTests.set(req, cancelSource); + this.uiRunningTests.set(result.id, cancelSource); const requests = tests.map( group => this.testControllers.get(group[0].src.controller)?.runTests( @@ -241,7 +252,7 @@ export class TestService extends Disposable implements ITestService { await Promise.all(requests); return result; } finally { - this.runningTests.delete(req); + this.uiRunningTests.delete(result.id); result.markComplete(); } } diff --git a/src/vs/workbench/test/browser/api/extHostTesting.test.ts b/src/vs/workbench/test/browser/api/extHostTesting.test.ts index 62340f1787f..4cb37deebb5 100644 --- a/src/vs/workbench/test/browser/api/extHostTesting.test.ts +++ b/src/vs/workbench/test/browser/api/extHostTesting.test.ts @@ -4,14 +4,19 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { CancellationToken } from 'vs/base/common/cancellation'; +import { VSBuffer } from 'vs/base/common/buffer'; +import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; +import { Iterable } from 'vs/base/common/iterator'; import { URI } from 'vs/base/common/uri'; -import { createDefaultDocumentTestRoot, TestItemFilteredWrapper } from 'vs/workbench/api/common/extHostTesting'; +import { mockObject, MockObject } from 'vs/base/test/common/mock'; +import { MainThreadTestingShape } from 'vs/workbench/api/common/extHost.protocol'; +import { createDefaultDocumentTestRoot, TestItemFilteredWrapper, TestRunCoordinator, TestRunDto } from 'vs/workbench/api/common/extHostTesting'; import * as convert from 'vs/workbench/api/common/extHostTypeConverters'; +import { TestMessage } from 'vs/workbench/api/common/extHostTypes'; import { TestDiffOpType, TestItemExpandState } from 'vs/workbench/contrib/testing/common/testCollection'; -import { stubTest, TestItemImpl, testStubs } from 'vs/workbench/contrib/testing/common/testStubs'; +import { stubTest, TestItemImpl, TestResultState, testStubs, testStubsChain } from 'vs/workbench/contrib/testing/common/testStubs'; import { TestOwnedTestCollection, TestSingleUseCollection } from 'vs/workbench/contrib/testing/test/common/ownedTestCollection'; -import { TestItem, TextDocument } from 'vscode'; +import type { TestItem, TestRunRequest, TextDocument } from 'vscode'; const simplify = (item: TestItem) => ({ id: item.id, @@ -403,24 +408,116 @@ suite('ExtHost Testing', () => { assert.strictEqual(invisibleWrapper.children.size, 1); assert.strictEqual(wrapper.children.get('id-b'), invisibleWrapper); }); + }); + }); - // test('can reset cached value of hasNodeMatchingFilter of parents up to visible parent', () => { - // const rootWrapper = TestItemFilteredWrapper.getWrapperForTestItem(testsWithLocation, textDocumentFilter); + suite('TestRunTracker', () => { + let proxy: MockObject; + let c: TestRunCoordinator; + let cts: CancellationTokenSource; - // const invisibleParent = testsWithLocation.children.get('id-b')!; - // const invisibleParentWrapper = TestItemFilteredWrapper.getWrapperForTestItem(invisibleParent, textDocumentFilter); - // const invisible = invisibleParent.children.get('id-bb')!; - // const invisibleWrapper = TestItemFilteredWrapper.getWrapperForTestItem(invisible, textDocumentFilter); + const req: TestRunRequest = { tests: [], debug: false }; + const dto = TestRunDto.fromInternal({ + debug: false, + excludeExtIds: [], + runId: 'run-id', + tests: [], + }); - // assert.strictEqual(invisibleParentWrapper.hasNodeMatchingFilter, false); - // invisible.location = location1 as any; - // assert.strictEqual(invisibleParentWrapper.hasNodeMatchingFilter, false); - // invisibleWrapper.reset(); - // assert.strictEqual(invisibleParentWrapper.hasNodeMatchingFilter, true); + setup(() => { + proxy = mockObject(); + cts = new CancellationTokenSource(); + c = new TestRunCoordinator(proxy); + }); - // // the root should be undefined due to the reset. - // assert.strictEqual((rootWrapper as any).matchesFilter, undefined); - // }); + test('tracks a run started from a main thread request', () => { + const tracker = c.prepareForMainThreadTestRun(req, dto, cts.token); + assert.strictEqual(tracker.isRunning, false); + + const task1 = c.createTestRun(req, 'run1', true); + const task2 = c.createTestRun(req, 'run2', true); + assert.strictEqual(proxy.$startedExtensionTestRun.called, false); + assert.strictEqual(tracker.isRunning, true); + + task1.appendOutput('hello'); + assert.deepStrictEqual([['run-id', (task1 as any).taskId, VSBuffer.fromString('hello')]], proxy.$appendOutputToRun.args); + task1.end(); + + assert.strictEqual(proxy.$finishedExtensionTestRun.called, false); + assert.strictEqual(tracker.isRunning, true); + + task2.end(); + + assert.strictEqual(proxy.$finishedExtensionTestRun.called, false); + assert.strictEqual(tracker.isRunning, false); + }); + + test('tracks a run started from an extension request', () => { + const task1 = c.createTestRun(req, 'hello world', false); + + const tracker = Iterable.first(c.trackers)!; + assert.strictEqual(tracker.isRunning, true); + assert.deepStrictEqual(proxy.$startedExtensionTestRun.args, [ + [{ + id: tracker.id, + tests: [], + exclude: [], + debug: false, + persist: false, + }] + ]); + + const task2 = c.createTestRun(req, 'run2', true); + const task3Detached = c.createTestRun({ ...req }, 'task3Detached', true); + + task1.end(); + assert.strictEqual(proxy.$finishedExtensionTestRun.called, false); + assert.strictEqual(tracker.isRunning, true); + + task2.end(); + assert.deepStrictEqual(proxy.$finishedExtensionTestRun.args, [[tracker.id]]); + assert.strictEqual(tracker.isRunning, false); + + task3Detached.end(); + }); + + test('adds tests to run smartly', () => { + const task1 = c.createTestRun(req, 'hello world', false); + const tracker = Iterable.first(c.trackers)!; + const tests = testStubs.nested(); + const expectedArgs: unknown[][] = []; + assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs); + + task1.setState(testStubsChain(tests, ['id-a', 'id-aa']).pop()!, TestResultState.Passed); + expectedArgs.push([ + tracker.id, + testStubsChain(tests, ['id-a', 'id-aa']).map(convert.TestItem.from) + ]); + assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs); + + + task1.setState(testStubsChain(tests, ['id-a', 'id-ab']).pop()!, TestResultState.Queued); + expectedArgs.push([ + tracker.id, + testStubsChain(tests, ['id-a', 'id-ab']).slice(1).map(convert.TestItem.from) + ]); + assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs); + + task1.setState(testStubsChain(tests, ['id-a', 'id-ab']).pop()!, TestResultState.Passed); + assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs); + }); + + test('guards calls after runs are ended', () => { + const task = c.createTestRun(req, 'hello world', false); + task.end(); + + task.setState(testStubs.nested(), TestResultState.Passed); + task.appendMessage(testStubs.nested(), new TestMessage('some message')); + task.appendOutput('output'); + + assert.strictEqual(proxy.$addTestsToRun.called, false); + assert.strictEqual(proxy.$appendOutputToRun.called, false); + assert.strictEqual(proxy.$appendTestMessageInRun.called, false); }); }); });