testing: update test item async resolving

See https://github.com/microsoft/vscode/issues/126987#issuecomment-867031454
This commit makes the following changes:

- Keep the `resolveChildrenHandler`, and remove the CancellationToken parameter
- Remove `TestITem.status` and instead have `TestItem.canResolveChildren?: boolean`
- Add `TestItem.busy?: boolean`. Note that the UI would implicitly show
  the item as busy while `resolve` is being called. (This is a new feature)

Upgrading to account for these changes should take around 10 to 20 minutes:

1. Where you previously set `item.status = vscode.TestItemStatus.Pending`,
   instead set `item.canResolveChildren = true`.
2. If you used the cancellation token in resolveChildrenHandler, you no
   longer need to do so. What you do here instead is up to you:

	 - If you set up global workspace watchers, add those to `context.subscriptions`
	 - You _probably_ don't need to set up watchers for "file" test items,
	   since you will receive updates via `vscode.workspace.onDidChangeTextDocument`
		 and/or any FileWatcher you have set up.

Example of an update: https://github.com/microsoft/vscode-selfhost-test-provider/commit/7287c64bf72f4ed037a3cb57f05220bbe1b38d84

src/vs/vscode.proposed.d.ts

@@ -1907,28 +1907,20 @@ declare module 'vscode' {
 
 		/**
 		 * A function provided by the extension that the editor may call to request
-		 * children of a test item, if the {@link TestItem.status} is `Pending`.
+		 * children of a test item, if the {@link TestItem.canExpand} is `true`.
+		 * When called, the item should discover children and call
+		 * {@link TestController.createTestItem} as children are discovered.
 		 *
-		 * When called, the item should discover tests and call {@link TestItem.addChild}.
-		 * The items should set its {@link TestItem.status} to `Resolved` when
-		 * discovery is finished.
+		 * The item in the explorer will automatically be marked as "busy" until
+		 * the function returns or the returned thenable resolves.
 		 *
-		 * The item should continue watching for changes to the children and
-		 * firing updates until the `token` is cancelled. The process of watching
-		 * the tests may involve creating a file watcher, for example. After the
-		 * token is cancelled and watching stops, the TestItem should set its
-		 * {@link TestItem.status} back to `Pending`.
-		 *
-		 * The editor will only call this method when it's interested in refreshing
-		 * the children of the item, and will not call it again while there's an
-		 * existing, uncancelled discovery for an item.
+		 * The controller may wish to set up listeners or watchers to update the
+		 * children as files and documents change.
 		 *
 		 * @param item An unresolved test item for which
 		 * children are being requested
-		 * @param token Cancellation for the request. Cancellation will be
-		 * requested if the test changes before the previous call completes.
 		 */
-		resolveChildrenHandler?: (item: TestItem<T>, token: CancellationToken) => void;
+		resolveChildrenHandler?: (item: TestItem<T>) => Thenable<void> | void;
 
 		/**
 		 * Starts a test run. When called, the controller should call
@@ -2055,21 +2047,6 @@ declare module 'vscode' {
 		end(): void;
 	}
 
-	/**
-	 * Indicates the the activity state of the {@link TestItem}.
-	 */
-	export enum TestItemStatus {
-		/**
-		 * All children of the test item, if any, have been discovered.
-		 */
-		Resolved = 1,
-
-		/**
-		 * The test item may have children who have not been discovered yet.
-		 */
-		Pending = 0,
-	}
-
 	/**
 	 * A test item is an item shown in the "test explorer" view. It encompasses
 	 * both a suite and a test, since they have almost or identical capabilities.
@@ -2099,17 +2076,21 @@ declare module 'vscode' {
 		readonly parent?: TestItem<any>;
 
 		/**
-		 * Indicates the state of the test item's children. The editor will show
-		 * TestItems in the `Pending` state and with a `resolveHandler` as being
-		 * expandable, and will call the `resolveHandler` to request items.
+		 * Indicates whether this test item may have children discovered by resolving.
+		 * If so, it will be shown as expandable in the Test Explorer  view, and
+		 * expanding the item will cause {@link TestController.resolveChildrenHandler}
+		 * to be invoked with the item.
 		 *
-		 * A TestItem in the `Resolved` state is assumed to have discovered and be
-		 * watching for changes in its children if applicable. TestItems are in the
-		 * `Resolved` state when initially created; if the editor should call
-		 * the `resolveHandler` to discover children, set the state to `Pending`
-		 * after creating the item.
+		 * Default to false.
+		 */
+		canResolveChildren: boolean;
+
+		/**
+		 * Controls whether the item is shown as "busy" in the Test Explorer view.
+		 * This is useful for showing status while discovering children. Defaults
+		 * to false.
 		 */
-		status: TestItemStatus;
+		busy: boolean;
 
 		/**
 		 * Display name describing the test case.
@@ -2163,8 +2144,7 @@ declare module 'vscode' {
 		invalidate(): void;
 
 		/**
-		 * Removes the test and its children from the tree. Any tokens passed to
-		 * child `resolveHandler` methods will be cancelled.
+		 * Removes the test and its children from the tree.
 		 */
 		dispose(): void;
 	}

src/vs/workbench/api/common/extHost.api.impl.ts

@@ -1273,7 +1273,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 			NotebookControllerAffinity: extHostTypes.NotebookControllerAffinity,
 			PortAttributes: extHostTypes.PortAttributes,
 			LinkedEditingRanges: extHostTypes.LinkedEditingRanges,
-			TestItemStatus: extHostTypes.TestItemStatus,
 			TestResultState: extHostTypes.TestResultState,
 			TestRunRequest: extHostTypes.TestRunRequest,
 			TestMessage: extHostTypes.TestMessage,

src/vs/workbench/api/common/extHostTypeConverters.ts

@@ -1669,7 +1669,8 @@ export namespace TestItem {
 			uri: URI.revive(item.uri),
 			range: Range.to(item.range || undefined),
 			dispose: () => undefined,
-			status: types.TestItemStatus.Pending,
+			canExpand: false,
+			busy: false,
 			data: undefined as never,
 			debuggable: item.debuggable,
 			description: item.description || undefined,

src/vs/workbench/api/common/extHostTypes.ts

@@ -3297,11 +3297,6 @@ export enum TestMessageSeverity {
 	Hint = 3
 }
 
-export enum TestItemStatus {
-	Pending = 0,
-	Resolved = 1,
-}
-
 const testItemPropAccessor = <K extends keyof vscode.TestItem<never>>(
 	api: IExtHostTestItemApi,
 	key: K,
@@ -3351,7 +3346,8 @@ export class TestItemImpl<T = any> implements vscode.TestItem<T> {
 	public debuggable!: boolean;
 	public label!: string;
 	public error!: string | vscode.MarkdownString;
-	public status!: vscode.TestItemStatus;
+	public busy!: boolean;
+	public canResolveChildren!: boolean;
 
 	constructor(id: string, label: string, uri: vscode.Uri | undefined, public data: T, parent: vscode.TestItem | undefined) {
 		const api = getPrivateApiFor(this);
@@ -3382,7 +3378,8 @@ export class TestItemImpl<T = any> implements vscode.TestItem<T> {
 			description: testItemPropAccessor(api, 'description', undefined, strictEqualComparator),
 			runnable: testItemPropAccessor(api, 'runnable', true, strictEqualComparator),
 			debuggable: testItemPropAccessor(api, 'debuggable', false, strictEqualComparator),
-			status: testItemPropAccessor(api, 'status', TestItemStatus.Resolved, strictEqualComparator),
+			canResolveChildren: testItemPropAccessor(api, 'canResolveChildren', false, strictEqualComparator),
+			busy: testItemPropAccessor(api, 'busy', false, strictEqualComparator),
 			error: testItemPropAccessor(api, 'error', undefined, strictEqualComparator),
 		});
 

src/vs/workbench/contrib/testing/browser/testingExplorerView.ts

@@ -987,7 +987,10 @@ class TestItemRenderer extends ActionableItemTemplateData<TestItemTreeElement> {
 		const testHidden = this.testService.excludeTests.value.has(node.element.test.item.extId);
 		data.wrapper.classList.toggle('test-is-hidden', testHidden);
 
-		const icon = testingStatesToIcons.get(node.element.test.expand === TestItemExpandState.BusyExpanding ? TestResultState.Running : node.element.state);
+		const icon = testingStatesToIcons.get(
+			node.element.test.expand === TestItemExpandState.BusyExpanding || node.element.test.item.busy
+				? TestResultState.Running
+				: node.element.state);
 
 		data.icon.className = 'computed-state ' + (icon ? ThemeIcon.asClassName(icon) : '');
 		if (node.element.retired) {

src/vs/workbench/contrib/testing/common/ownedTestCollection.ts

@@ -3,15 +3,15 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { DeferredPromise, isThenable, RunOnceScheduler } from 'vs/base/common/async';
-import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
+import { Barrier, isThenable, RunOnceScheduler } from 'vs/base/common/async';
+import { CancellationToken } from 'vs/base/common/cancellation';
 import { Emitter } from 'vs/base/common/event';
 import { Iterable } from 'vs/base/common/iterator';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { assertNever } from 'vs/base/common/types';
 import { ExtHostTestItemEvent, ExtHostTestItemEventType, getPrivateApiFor } from 'vs/workbench/api/common/extHostTestingPrivateApi';
 import * as Convert from 'vs/workbench/api/common/extHostTypeConverters';
-import { TestItemImpl, TestItemStatus } from 'vs/workbench/api/common/extHostTypes';
+import { TestItemImpl } from 'vs/workbench/api/common/extHostTypes';
 import { applyTestItemUpdate, InternalTestItem, TestDiffOpType, TestItemExpandState, TestsDiff, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testCollection';
 
 type TestItemRaw = Convert.TestItem.Raw;
@@ -29,8 +29,7 @@ export interface OwnedCollectionTestItem extends InternalTestItem {
 	 * Number of levels of items below this one that are expanded. May be infinite.
 	 */
 	expandLevels?: number;
-	initialExpand?: DeferredPromise<void>;
-	discoverCts?: CancellationTokenSource;
+	resolveBarrier?: Barrier;
 }
 
 /**
@@ -145,8 +144,6 @@ export class TestTree<T extends InternalTestItem> {
 	}
 }
 
-type ResolveHandler = (item: TestItemRaw, token: CancellationToken) => void;
-
 /**
  * Maintains tests created and registered for a single set of hierarchies
  * for a workspace or document.
@@ -156,7 +153,7 @@ export class SingleUseTestCollection extends Disposable {
 	protected readonly testItemToInternal = new Map<TestItemRaw, OwnedCollectionTestItem>();
 	private readonly debounceSendDiff = this._register(new RunOnceScheduler(() => this.flushDiff(), 200));
 	private readonly diffOpEmitter = this._register(new Emitter<TestsDiff>());
-	private _resolveHandler?: ResolveHandler;
+	private _resolveHandler?: (item: TestItemRaw) => Promise<void> | void;
 
 	public readonly root = new TestItemImpl(`${this.controllerId}Root`, this.controllerId, undefined, undefined, undefined);
 	public readonly tree = new TestTree<OwnedCollectionTestItem>();
@@ -172,7 +169,7 @@ export class SingleUseTestCollection extends Disposable {
 	/**
 	 * Handler used for expanding test items.
 	 */
-	public set resolveHandler(handler: undefined | ((item: TestItemRaw, token: CancellationToken) => void)) {
+	public set resolveHandler(handler: undefined | ((item: TestItemRaw) => void)) {
 		this._resolveHandler = handler;
 		for (const test of this.testItemToInternal.values()) {
 			this.updateExpandability(test);
@@ -248,20 +245,19 @@ export class SingleUseTestCollection extends Disposable {
 		// try to avoid awaiting things if the provider returns synchronously in
 		// order to keep everything in a single diff and DOM update.
 		if (internal.expand === TestItemExpandState.Expandable) {
-			const r = this.refreshChildren(internal);
-			return !r.isSettled
-				? r.p.then(() => this.expandChildren(internal, levels - 1))
+			const r = this.resolveChildren(internal);
+			return !r.isOpen()
+				? r.wait().then(() => this.expandChildren(internal, levels - 1))
 				: this.expandChildren(internal, levels - 1);
 		} else if (internal.expand === TestItemExpandState.Expanded) {
-			return internal.initialExpand?.isSettled === false
-				? internal.initialExpand.p.then(() => this.expandChildren(internal, levels - 1))
+			return internal.resolveBarrier?.isOpen() === false
+				? internal.resolveBarrier.wait().then(() => this.expandChildren(internal, levels - 1))
 				: this.expandChildren(internal, levels - 1);
 		}
 	}
 
 	public override dispose() {
 		for (const item of this.testItemToInternal.values()) {
-			item.discoverCts?.dispose(true);
 			getPrivateApiFor(item.actual).bus.dispose();
 		}
 
@@ -288,7 +284,7 @@ export class SingleUseTestCollection extends Disposable {
 			case ExtHostTestItemEventType.SetProp:
 				const [_, key, value] = evt;
 				switch (key) {
-					case 'status':
+					case 'canResolveChildren':
 						this.updateExpandability(internal);
 						break;
 					case 'range':
@@ -363,14 +359,13 @@ export class SingleUseTestCollection extends Disposable {
 		let newState: TestItemExpandState;
 		if (!this._resolveHandler) {
 			newState = TestItemExpandState.NotExpandable;
-		} else if (internal.actual.status === TestItemStatus.Pending) {
-			newState = internal.discoverCts
-				? TestItemExpandState.BusyExpanding
-				: TestItemExpandState.Expandable;
-		} else {
-			internal.initialExpand?.complete();
-			newState = internal.actual.children.size > 0
+		} else if (internal.resolveBarrier) {
+			newState = internal.resolveBarrier.isOpen()
 				? TestItemExpandState.Expanded
+				: TestItemExpandState.BusyExpanding;
+		} else {
+			newState = internal.actual.canResolveChildren
+				? TestItemExpandState.Expandable
 				: TestItemExpandState.NotExpandable;
 		}
 
@@ -382,7 +377,7 @@ export class SingleUseTestCollection extends Disposable {
 		this.pushDiff([TestDiffOpType.Update, { extId: internal.actual.id, expand: newState }]);
 
 		if (newState === TestItemExpandState.Expandable && internal.expandLevels !== undefined) {
-			this.refreshChildren(internal);
+			this.resolveChildren(internal);
 		}
 	}
 
@@ -408,25 +403,40 @@ export class SingleUseTestCollection extends Disposable {
 	/**
 	 * Calls `discoverChildren` on the item, refreshing all its tests.
 	 */
-	private refreshChildren(internal: OwnedCollectionTestItem) {
-		if (internal.discoverCts) {
-			internal.discoverCts.dispose(true);
+	private resolveChildren(internal: OwnedCollectionTestItem) {
+		if (internal.resolveBarrier) {
+			return internal.resolveBarrier;
 		}
 
 		if (!this._resolveHandler) {
-			const p = new DeferredPromise<void>();
-			p.complete();
-			return p;
+			const b = new Barrier();
+			b.open();
+			return b;
 		}
 
 		internal.expand = TestItemExpandState.BusyExpanding;
-		internal.discoverCts = new CancellationTokenSource();
 		this.pushExpandStateUpdate(internal);
 
-		internal.initialExpand = new DeferredPromise<void>();
-		this._resolveHandler(internal.actual, internal.discoverCts.token);
+		const barrier = internal.resolveBarrier = new Barrier();
+
+		let r: Thenable<void> | void;
+		try {
+			r = this._resolveHandler(internal.actual);
+		} catch (err) {
+			internal.actual.error = err.stack || err.message;
+		}
+
+		if (isThenable(r)) {
+			r.catch(err => internal.actual.error = err.stack || err.message).then(() => {
+				barrier.open();
+				this.updateExpandability(internal);
+			});
+		} else {
+			barrier.open();
+			this.updateExpandability(internal);
+		}
 
-		return internal.initialExpand;
+		return internal.resolveBarrier;
 	}
 
 	private pushExpandStateUpdate(internal: OwnedCollectionTestItem) {
@@ -443,7 +453,6 @@ export class SingleUseTestCollection extends Disposable {
 				continue;
 			}
 
-			item.discoverCts?.dispose(true);
 			this.tree.delete(item.item.extId);
 			this.testItemToInternal.delete(item.actual);
 			for (const child of item.actual.children.values()) {

src/vs/workbench/contrib/testing/common/testCollection.ts

@@ -90,6 +90,7 @@ export interface ITestItem {
 	/** ID of the test given by the test controller */
 	extId: string;
 	label: string;
+	busy?: boolean;
 	children?: never;
 	uri?: URI;
 	range: IRange | null;

src/vs/workbench/contrib/testing/common/testStubs.ts

@@ -4,15 +4,13 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { URI } from 'vs/base/common/uri';
-import { TestItemImpl, TestItemStatus, TestResultState } from 'vs/workbench/api/common/extHostTypes';
+import { TestItemImpl, TestResultState } from 'vs/workbench/api/common/extHostTypes';
 import { MainThreadTestCollection } from 'vs/workbench/contrib/testing/common/mainThreadTestCollection';
 import { TestSingleUseCollection } from 'vs/workbench/contrib/testing/test/common/ownedTestCollection';
 
 export * as Convert from 'vs/workbench/api/common/extHostTypeConverters';
 export { TestItemImpl, TestResultState } from 'vs/workbench/api/common/extHostTypes';
 
-
-
 /**
  * Gets a main thread test collection initialized with the given set of
  * roots/stubs.
@@ -28,18 +26,16 @@ export const testStubs = {
 	nested: (idPrefix = 'id-') => {
 		const collection = new TestSingleUseCollection('ctrlId');
 		collection.root.label = 'root';
-		collection.root.status = TestItemStatus.Pending;
+		collection.root.canResolveChildren = true;
 		collection.resolveHandler = item => {
 			if (item === collection.root) {
 				const a = new TestItemImpl(idPrefix + 'a', 'a', URI.file('/'), undefined, collection.root);
-				a.status = TestItemStatus.Pending;
+				a.canResolveChildren = true;
 				new TestItemImpl(idPrefix + 'b', 'b', URI.file('/'), undefined, collection.root);
 			} else if (item.id === idPrefix + 'a') {
 				new TestItemImpl(idPrefix + 'aa', 'aa', URI.file('/'), undefined, item);
 				new TestItemImpl(idPrefix + 'ab', 'ab', URI.file('/'), undefined, item);
 			}
-
-			item.status = TestItemStatus.Resolved;
 		};
 
 		return collection;