testing: api feedback

src/vs/vscode.proposed.d.ts

@@ -2067,13 +2067,13 @@ declare module 'vscode' {
 		/**
 		 * Available test items. Tests in the workspace should be added in this
 		 * collection. The extension controls when to add these, although the
-		 * editor may request children using the {@link resolveChildrenHandler},
+		 * editor may request children using the {@link resolveHandler},
 		 * and the extension should add tests for a file when
 		 * {@link vscode.workspace.onDidOpenTextDocument} fires in order for
 		 * decorations for tests within the file to be visible.
 		 *
 		 * Tests in this collection should be watched and updated by the extension
-		 * as files change. See {@link resolveChildrenHandler} for details around
+		 * as files change. See {@link resolveHandler} for details around
 		 * for the lifecycle of watches.
 		 */
 		readonly items: TestItemCollection;
@@ -2103,8 +2103,7 @@ declare module 'vscode' {
 		 * @param item An unresolved test item for which
 		 * children are being requested
 		 */
-		// todo@API resolveHandler
-		resolveChildrenHandler?: (item: TestItem | undefined) => Thenable<void> | void;
+		resolveHandler?: (item: TestItem | undefined) => Thenable<void> | void;
 
 		/**
 		 * Creates a {@link TestRun<T>}. This should be called by the
@@ -2208,38 +2207,38 @@ declare module 'vscode' {
 		readonly isPersisted: boolean;
 
 		/**
-		 * Updates the state of the test in the run. Calling with method with nodes
-		 * outside the {@link TestRunRequest.tests} or in the {@link TestRunRequest.exclude}
-		 * array will no-op. This will usually be called multiple times for a test
-		 * as it is queued, enters the running state, and then passes or fails.
-		 *
-		 * @param test The test to update
-		 * @param state The state to assign to the test
+		 * Indicates a test in the run is queued for later execution.
+		 * @param test Test item to update
 		 */
-		setState(test: TestItem, state: TestResultState): void;
+		enqueued(test: TestItem): void;
 
 		/**
-		 * Updates the state of the test in the run. Calling with method with nodes
-		 * outside the {@link TestRunRequest.tests} or in the {@link TestRunRequest.exclude}
-		 * array will no-op. This override moves the test into a terminal state and
-		 * indicates how long it ran for.
-		 *
-		 * @param test The terminal test state
-		 * @param state The state to assign to the test
-		 * @param duration Optionally sets how long the test took to run, in milliseconds
+		 * Indicates a test in the run has started running.
+		 * @param test Test item to update
 		 */
-		setState(test: TestItem, state: TestResultState.Passed | TestResultState.Failed | TestResultState.Errored, duration: number): void;
+		started(test: TestItem): void;
 
 		/**
-		 * Appends a message, such as an assertion error, to the test item.
-		 *
-		 * Calling with method with nodes outside the {@link TestRunRequest.tests}
-		 * or in the {@link TestRunRequest.exclude} array will no-op.
-		 *
-		 * @param test The test to update
-		 * @param message The message to add
+		 * Indicates a test in the run has been skipped.
+		 * @param test Test item to update
+		 */
+		skipped(test: TestItem): void;
+
+		/**
+		 * Indicates a test in the run has failed. You should pass one or more
+		 * {@link TestMessage | TestMessages} to describe the failure.
+		 * @param test Test item to update
+		 * @param messages Messages associated with the test failure
+		 * @param duration How long the test took to execute, in milliseconds
 		 */
-		appendMessage(test: TestItem, message: TestMessage): void;
+		failed(test: TestItem, message: TestMessage | readonly TestMessage[], duration?: number): void;
+
+		/**
+		 * Indicates a test in the run has passed.
+		 * @param test Test item to update
+		 * @param duration How long the test took to execute, in milliseconds
+		 */
+		passed(test: TestItem, duration?: number): void;
 
 		/**
 		 * Appends raw output from the test runner. On the user's request, the
@@ -2262,6 +2261,11 @@ declare module 'vscode' {
 	 * {@link TestController.items}.
 	 */
 	export interface TestItemCollection {
+		/**
+		 * Gets the number of items in the collection.
+		 */
+		readonly size: number;
+
 		/**
 		 * Replaces the items stored by the collection.
 		 * @param items Items to store, can be an array or other iterable.
@@ -2329,12 +2333,11 @@ declare module 'vscode' {
 		/**
 		 * 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}
+		 * expanding the item will cause {@link TestController.resolveHandler}
 		 * to be invoked with the item.
 		 *
 		 * Default to false.
 		 */
-		// todo@API better names: isLeaf, isLeaf{Type|Node}, canHaveChildren
 		canResolveChildren: boolean;
 
 		/**

src/vs/workbench/api/browser/mainThreadTesting.ts

@@ -171,15 +171,17 @@ export class MainThreadTesting extends Disposable implements MainThreadTestingSh
 	/**
 	 * @inheritdoc
 	 */
-	public $appendTestMessageInRun(runId: string, taskId: string, testId: string, message: ITestMessage): void {
+	public $appendTestMessagesInRun(runId: string, taskId: string, testId: string, messages: ITestMessage[]): void {
 		const r = this.resultService.getResult(runId);
 		if (r && r instanceof LiveTestResult) {
-			if (message.location) {
-				message.location.uri = URI.revive(message.location.uri);
-				message.location.range = Range.lift(message.location.range);
-			}
+			for (const message of messages) {
+				if (message.location) {
+					message.location.uri = URI.revive(message.location.uri);
+					message.location.range = Range.lift(message.location.range);
+				}
 
-			r.appendMessage(testId, taskId, message);
+				r.appendMessage(testId, taskId, message);
+			}
 		}
 	}
 

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

@@ -2147,7 +2147,7 @@ export interface MainThreadTestingShape {
 	/** Updates the state of a test run in the given run. */
 	$updateTestStateInRun(runId: string, taskId: string, testId: string, state: TestResultState, duration?: number): void;
 	/** Appends a message to a test in the run. */
-	$appendTestMessageInRun(runId: string, taskId: string, testId: string, message: ITestMessage): void;
+	$appendTestMessagesInRun(runId: string, taskId: string, testId: string, messages: ITestMessage[]): void;
 	/** Appends raw output to the test run.. */
 	$appendOutputToRun(runId: string, taskId: string, output: VSBuffer): void;
 	/** Triggered when coverage is added to test results. */

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

@@ -92,10 +92,10 @@ export class ExtHostTesting implements ExtHostTestingShape {
 			createTestRun: (request, name, persist = true) => {
 				return this.runTracker.createTestRun(controllerId, collection, request, name, persist);
 			},
-			set resolveChildrenHandler(fn) {
+			set resolveHandler(fn) {
 				collection.resolveHandler = fn;
 			},
-			get resolveChildrenHandler() {
+			get resolveHandler() {
 				return collection.resolveHandler;
 			},
 			dispose: () => {
@@ -274,7 +274,7 @@ export class ExtHostTesting implements ExtHostTestingShape {
 }
 
 class TestRunTracker extends Disposable {
-	private readonly tasks = new Map</* task ID */string, { run: TestRunImpl, coverage: TestRunCoverageBearer }>();
+	private readonly tasks = new Map</* task ID */string, { run: vscode.TestRun, coverage: TestRunCoverageBearer }>();
 	private readonly sharedTestIds = new Set<string>();
 	private readonly cts: CancellationTokenSource;
 	private readonly endEmitter = this._register(new Emitter<void>());
@@ -314,16 +314,78 @@ class TestRunTracker extends Disposable {
 	}
 
 	public createRun(name: string | undefined) {
+		const runId = this.dto.id;
+		const ctrlId = this.dto.controllerId;
 		const taskId = generateUuid();
-		const coverage = new TestRunCoverageBearer(this.proxy, this.dto.id, taskId);
-		const run = new TestRunImpl(name, this.cts.token, taskId, coverage, this.dto, this.sharedTestIds, this.proxy, () => {
-			this.tasks.delete(run.taskId);
-			if (!this.isRunning) {
-				this.dispose();
+		const coverage = new TestRunCoverageBearer(this.proxy, runId, taskId);
+
+		const guardTestMutation = <Args extends unknown[]>(fn: (test: vscode.TestItem, ...args: Args) => void) =>
+			(test: vscode.TestItem, ...args: Args) => {
+				if (ended) {
+					console.warn(`Setting the state of test "${test.id}" is a no-op after the run ends.`);
+					return;
+				}
+
+				if (!this.dto.isIncluded(test)) {
+					return;
+				}
+
+				this.ensureTestIsKnown(test);
+				fn(test, ...args);
+			};
+
+		let ended = false;
+		const run: vscode.TestRun = {
+			isPersisted: this.dto.isPersisted,
+			token: this.cts.token,
+			name,
+			get coverageProvider() {
+				return coverage.coverageProvider;
+			},
+			set coverageProvider(provider) {
+				coverage.coverageProvider = provider;
+			},
+			//#region state mutation
+			enqueued: guardTestMutation(test => {
+				this.proxy.$updateTestStateInRun(runId, taskId, TestId.fromExtHostTestItem(test, ctrlId).toString(), TestResultState.Queued);
+			}),
+			skipped: guardTestMutation(test => {
+				this.proxy.$updateTestStateInRun(runId, taskId, TestId.fromExtHostTestItem(test, ctrlId).toString(), TestResultState.Skipped);
+			}),
+			started: guardTestMutation(test => {
+				this.proxy.$updateTestStateInRun(runId, taskId, TestId.fromExtHostTestItem(test, ctrlId).toString(), TestResultState.Running);
+			}),
+			failed: guardTestMutation((test, messages, duration) => {
+				this.proxy.$appendTestMessagesInRun(runId, taskId, TestId.fromExtHostTestItem(test, ctrlId).toString(),
+					messages instanceof Array ? messages.map(Convert.TestMessage.from) : [Convert.TestMessage.from(messages)]);
+				this.proxy.$updateTestStateInRun(runId, taskId, TestId.fromExtHostTestItem(test, ctrlId).toString(), TestResultState.Failed, duration);
+			}),
+			passed: guardTestMutation((test, duration) => {
+				this.proxy.$updateTestStateInRun(runId, taskId, TestId.fromExtHostTestItem(test, this.dto.controllerId).toString(), TestResultState.Passed, duration);
+			}),
+			//#endregion
+			appendOutput: output => {
+				if (!ended) {
+					this.proxy.$appendOutputToRun(runId, taskId, VSBuffer.fromString(output));
+				}
+			},
+			end: () => {
+				if (ended) {
+					return;
+				}
+
+				ended = true;
+				this.proxy.$finishedTestRunTask(runId, taskId);
+				this.tasks.delete(taskId);
+				if (!this.isRunning) {
+					this.dispose();
+				}
 			}
-		});
+		};
+
+		this.tasks.set(taskId, { run, coverage });
+		this.proxy.$startedTestRunTask(runId, { id: taskId, name, running: true });
 
-		this.tasks.set(run.taskId, { run, coverage });
 		return run;
 	}
 
@@ -335,6 +397,41 @@ class TestRunTracker extends Disposable {
 			super.dispose();
 		}
 	}
+
+
+	private ensureTestIsKnown(test: vscode.TestItem) {
+		if (!(test instanceof TestItemImpl)) {
+			throw new InvalidTestItemError(test.id);
+		}
+
+		if (this.sharedTestIds.has(test.id)) {
+			return;
+		}
+
+		const chain: ITestItem[] = [];
+		while (true) {
+			chain.unshift(Convert.TestItem.from(test as TestItemImpl));
+
+			if (this.sharedTestIds.has(test.id)) {
+				break;
+			}
+
+			this.sharedTestIds.add(test.id);
+			if (!test.parent) {
+				break;
+			}
+
+			test = test.parent;
+		}
+
+		const root = this.dto.colllection.root;
+		if (!this.sharedTestIds.has(root.id)) {
+			this.sharedTestIds.add(root.id);
+			chain.unshift(Convert.TestItem.from(root));
+		}
+
+		this.proxy.$addTestsToRun(this.dto.controllerId, this.dto.id, chain);
+	}
 }
 
 /**
@@ -545,110 +642,6 @@ class TestRunCoverageBearer {
 	}
 }
 
-class TestRunImpl implements vscode.TestRun {
-	readonly #proxy: MainThreadTestingShape;
-	readonly #req: TestRunDto;
-	readonly #sharedIds: Set<string>;
-	readonly #onEnd: () => void;
-	readonly #coverage: TestRunCoverageBearer;
-	#ended = false;
-
-	public set coverageProvider(provider: vscode.TestCoverageProvider | undefined) {
-		this.#coverage.coverageProvider = provider;
-	}
-
-	public get coverageProvider() {
-		return this.#coverage.coverageProvider;
-	}
-
-	public get isPersisted() {
-		return this.#req.isPersisted;
-	}
-
-	constructor(
-		public readonly name: string | undefined,
-		public readonly token: CancellationToken,
-		public readonly taskId: string,
-		coverage: TestRunCoverageBearer,
-		dto: TestRunDto,
-		sharedTestIds: Set<string>,
-		proxy: MainThreadTestingShape,
-		onEnd: () => void,
-	) {
-		this.#onEnd = onEnd;
-		this.#proxy = proxy;
-		this.#req = dto;
-		this.#coverage = coverage;
-		this.#sharedIds = sharedTestIds;
-		proxy.$startedTestRunTask(dto.id, { id: this.taskId, name, running: true });
-	}
-
-	setState(test: vscode.TestItem, state: vscode.TestResultState, duration?: number): void {
-		const req = this.#req;
-		if (!this.#ended && req.isIncluded(test)) {
-			this.ensureTestIsKnown(test);
-			this.#proxy.$updateTestStateInRun(req.id, this.taskId, TestId.fromExtHostTestItem(test, req.controllerId).toString(), state as number as TestResultState, duration);
-		}
-	}
-
-	appendMessage(test: vscode.TestItem, message: vscode.TestMessage): void {
-		const req = this.#req;
-		if (!this.#ended && req.isIncluded(test)) {
-			this.ensureTestIsKnown(test);
-			this.#proxy.$appendTestMessageInRun(req.id, this.taskId, TestId.fromExtHostTestItem(test, req.controllerId).toString(), Convert.TestMessage.from(message));
-		}
-	}
-
-	appendOutput(output: string): void {
-		if (!this.#ended) {
-			this.#proxy.$appendOutputToRun(this.#req.id, this.taskId, VSBuffer.fromString(output));
-		}
-	}
-
-	end(): void {
-		if (!this.#ended) {
-			this.#ended = true;
-			this.#proxy.$finishedTestRunTask(this.#req.id, this.taskId);
-			this.#onEnd();
-		}
-	}
-
-	private ensureTestIsKnown(test: vscode.TestItem) {
-		if (!(test instanceof TestItemImpl)) {
-			throw new InvalidTestItemError(test.id);
-		}
-
-		const sent = this.#sharedIds;
-		if (sent.has(test.id)) {
-			return;
-		}
-
-		const chain: ITestItem[] = [];
-		while (true) {
-			chain.unshift(Convert.TestItem.from(test as TestItemImpl));
-
-			if (sent.has(test.id)) {
-				break;
-			}
-
-			sent.add(test.id);
-			if (!test.parent) {
-				break;
-			}
-
-			test = test.parent;
-		}
-
-		const root = this.#req.colllection.root;
-		if (!sent.has(root.id)) {
-			sent.add(root.id);
-			chain.unshift(Convert.TestItem.from(root));
-		}
-
-		this.#proxy.$addTestsToRun(this.#req.controllerId, this.#req.id, chain);
-	}
-}
-
 /**
  * @private
  */

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

@@ -158,6 +158,11 @@ const createTestItemCollection = (owningItem: TestItemImpl): TestItemCollectionI
 	let mapped = new Map<string, TestItemImpl>();
 
 	return {
+		/** @inheritdoc */
+		get size() {
+			return mapped.size;
+		},
+
 		/** @inheritdoc */
 		forEach(callback: (item: vscode.TestItem, collection: vscode.TestItemCollection) => unknown, thisArg?: unknown) {
 			for (const item of mapped.values()) {

src/vs/workbench/test/browser/api/extHostTesting.test.ts

@@ -456,7 +456,8 @@ suite('ExtHost Testing', () => {
 			assert.strictEqual(tracker.isRunning, true);
 
 			task1.appendOutput('hello');
-			assert.deepStrictEqual([['run-id', (task1 as any).taskId, VSBuffer.fromString('hello')]], proxy.$appendOutputToRun.args);
+			const taskId = proxy.$appendOutputToRun.args[0]?.[1];
+			assert.deepStrictEqual([['run-id', taskId, VSBuffer.fromString('hello')]], proxy.$appendOutputToRun.args);
 			task1.end();
 
 			assert.strictEqual(proxy.$finishedExtensionTestRun.called, false);
@@ -504,7 +505,7 @@ suite('ExtHost Testing', () => {
 			const expectedArgs: unknown[][] = [];
 			assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs);
 
-			task1.setState(single.root.children.get('id-a')!.children.get('id-aa')!, TestResultState.Passed);
+			task1.passed(single.root.children.get('id-a')!.children.get('id-aa')!);
 			expectedArgs.push([
 				'ctrl',
 				tracker.id,
@@ -517,7 +518,7 @@ suite('ExtHost Testing', () => {
 			assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs);
 
 
-			task1.setState(single.root.children.get('id-a')!.children.get('id-ab')!, TestResultState.Queued);
+			task1.enqueued(single.root.children.get('id-a')!.children.get('id-ab')!);
 			expectedArgs.push([
 				'ctrl',
 				tracker.id,
@@ -528,7 +529,7 @@ suite('ExtHost Testing', () => {
 			]);
 			assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs);
 
-			task1.setState(single.root.children.get('id-a')!.children.get('id-ab')!, TestResultState.Passed);
+			task1.passed(single.root.children.get('id-a')!.children.get('id-ab')!);
 			assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs);
 		});
 
@@ -536,13 +537,12 @@ suite('ExtHost Testing', () => {
 			const task = c.createTestRun('ctrl', single, req, 'hello world', false);
 			task.end();
 
-			task.setState(single.root, TestResultState.Passed);
-			task.appendMessage(single.root, new TestMessage('some message'));
+			task.failed(single.root, 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);
+			assert.strictEqual(proxy.$appendTestMessagesInRun.called, false);
 		});
 
 		test('excludes tests outside tree or explicitly excluded', () => {
@@ -552,8 +552,8 @@ suite('ExtHost Testing', () => {
 				exclude: [single.root.children.get('id-a')!.children.get('id-aa')!],
 			}, 'hello world', false);
 
-			task.setState(single.root.children.get('id-a')!.children.get('id-aa')!, TestResultState.Passed);
-			task.setState(single.root.children.get('id-a')!.children.get('id-ab')!, TestResultState.Passed);
+			task.passed(single.root.children.get('id-a')!.children.get('id-aa')!);
+			task.passed(single.root.children.get('id-a')!.children.get('id-ab')!);
 
 			assert.deepStrictEqual(proxy.$updateTestStateInRun.args.length, 1);
 			const args = proxy.$updateTestStateInRun.args[0];