testing: improve RequiredTestItem api

Fixes #117384

src/vs/vscode.proposed.d.ts

@@ -2200,7 +2200,7 @@ declare module 'vscode' {
 		/**
 		 * List of tests returned by test provider for files in the workspace.
 		 */
-		readonly tests: ReadonlyArray<RequiredTestItem>;
+		readonly tests: ReadonlyArray<ObservedTestItem>;
 
 		/**
 		 * An event that fires when an existing test in the collection changes, or
@@ -2230,23 +2230,17 @@ declare module 'vscode' {
 		/**
 		 * List of all tests that are newly added.
 		 */
-		readonly added: ReadonlyArray<RequiredTestItem>;
+		readonly added: ReadonlyArray<ObservedTestItem>;
 
 		/**
 		 * List of existing tests that have updated.
 		 */
-		readonly updated: ReadonlyArray<RequiredTestItem>;
+		readonly updated: ReadonlyArray<ObservedTestItem>;
 
 		/**
 		 * List of existing tests that have been removed.
 		 */
-		readonly removed: ReadonlyArray<RequiredTestItem>;
-
-		/**
-		 * Highest node in the test tree under which changes were made. This can
-		 * be easily plugged into events like the TreeDataProvider update event.
-		 */
-		readonly commonChangeAncestor: RequiredTestItem | null;
+		readonly removed: ReadonlyArray<ObservedTestItem>;
 	}
 
 	/**
@@ -2432,13 +2426,45 @@ declare module 'vscode' {
 	}
 
 	/**
-	 * A {@link TestItem} with its defaults filled in.
+	 * A readonly {@link TestItem}-like interface with its defaults filled in,
+	 * exposed in the {@link TestObserver} interface.
 	 */
-	export type RequiredTestItem = {
-		[K in keyof Required<TestItem>]: K extends 'children'
-		? RequiredTestItem[]
-		: (K extends 'description' | 'location' ? TestItem[K] : Required<TestItem>[K])
-	};
+	export interface ObservedTestItem {
+		/**
+		 * See {@link TestItem.id}
+		 */
+		readonly id: string;
+
+		/**
+		 * See {@link TestItem.label}
+		 */
+		readonly label: string;
+
+		/**
+		 * Optional description that appears next to the label.
+		 */
+		readonly description?: string;
+
+		/**
+		 * See {@link TestItem.runnable}
+		 */
+		readonly runnable: boolean;
+
+		/**
+		 * See {@link TestItem.debuggable}
+		 */
+		readonly debuggable: boolean;
+
+		/**
+		 * See {@link TestItem.location}
+		 */
+		readonly location?: Location;
+
+		/**
+		 * See {@link TestItem.children}
+		 */
+		readonly children: ReadonlyArray<ObservedTestItem>;
+	}
 
 	/**
 	 * Possible states of tests in a test run.

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

@@ -501,7 +501,7 @@ export class TestItemFilteredWrapper implements vscode.TestItem {
 interface MirroredCollectionTestItem extends IncrementalTestCollectionItem {
 	revived: vscode.TestItem;
 	depth: number;
-	wrapped?: vscode.RequiredTestItem;
+	wrapped?: vscode.ObservedTestItem;
 }
 
 class MirroredChangeCollector extends IncrementalChangeCollector<MirroredCollectionTestItem> {
@@ -564,74 +564,6 @@ class MirroredChangeCollector extends IncrementalChangeCollector<MirroredCollect
 			get added() { return [...added].map(collection.getPublicTestItem, collection); },
 			get updated() { return [...updated].map(collection.getPublicTestItem, collection); },
 			get removed() { return [...removed].map(collection.getPublicTestItem, collection); },
-			get commonChangeAncestor() {
-				let ancestorPath: MirroredCollectionTestItem[] | undefined;
-				const buildAncestorPath = (node: MirroredCollectionTestItem | undefined) => {
-					if (!node) {
-						return undefined;
-					}
-
-					// add the node and all its parents to the list of ancestors. If
-					// the node is detached, do not return a path (its parent will
-					// also have been passed to remove() and be present)
-					const path: MirroredCollectionTestItem[] = new Array(node.depth + 1);
-					for (let i = node.depth; i >= 0; i--) {
-						if (!node) {
-							return undefined; // detached child
-						}
-
-						path[node.depth] = node;
-						node = node.parent ? collection.getMirroredTestDataById(node.parent) : undefined;
-					}
-
-					return path;
-				};
-
-				const addAncestorPath = (node: MirroredCollectionTestItem) => {
-					// fast path: if the common ancestor is already the root, no more work to do
-					if (ancestorPath && ancestorPath.length === 0) {
-						return;
-					}
-
-					const thisPath = buildAncestorPath(node);
-					if (!thisPath) {
-						return;
-					}
-
-					if (!ancestorPath) {
-						ancestorPath = thisPath;
-						return;
-					}
-
-					// removes node from the path to the ancestor that don't match
-					// the corresponding node in *this* path.
-					for (let i = ancestorPath.length - 1; i >= 0; i--) {
-						if (ancestorPath[i] !== thisPath[i]) {
-							ancestorPath.pop();
-						}
-					}
-				};
-
-				const addParentAncestor = (node: MirroredCollectionTestItem) => {
-					if (ancestorPath && ancestorPath.length === 0) {
-						// no-op
-					} else if (node.parent === null) {
-						ancestorPath = [];
-					} else {
-						const parent = collection.getMirroredTestDataById(node.parent);
-						if (parent) {
-							addAncestorPath(parent);
-						}
-					}
-				};
-
-				for (const node of added) { addParentAncestor(node); }
-				for (const node of updated) { addAncestorPath(node); }
-				for (const node of removed) { addParentAncestor(node); }
-
-				const ancestor = ancestorPath && ancestorPath[ancestorPath.length - 1];
-				return ancestor ? collection.getPublicTestItem(ancestor) : null;
-			},
 		};
 	}
 
@@ -664,8 +596,8 @@ export class MirroredTestCollection extends AbstractIncrementalTestCollection<Mi
 	/**
 	 * Translates the item IDs to TestItems for exposure to extensions.
 	 */
-	public getAllAsTestItem(itemIds: Iterable<string>): vscode.RequiredTestItem[] {
-		let output: vscode.RequiredTestItem[] = [];
+	public getAllAsTestItem(itemIds: Iterable<string>) {
+		let output: vscode.ObservedTestItem[] = [];
 		for (const itemId of itemIds) {
 			const item = this.items.get(itemId);
 			if (item) {
@@ -708,7 +640,7 @@ export class MirroredTestCollection extends AbstractIncrementalTestCollection<Mi
 	/**
 	 * Gets the public test item instance for the given mirrored record.
 	 */
-	public getPublicTestItem(item: MirroredCollectionTestItem): vscode.RequiredTestItem {
+	public getPublicTestItem(item: MirroredCollectionTestItem): vscode.ObservedTestItem {
 		if (!item.wrapped) {
 			item.wrapped = new TestItemFromMirror(item, this);
 		}
@@ -717,7 +649,7 @@ export class MirroredTestCollection extends AbstractIncrementalTestCollection<Mi
 	}
 }
 
-class TestItemFromMirror implements vscode.RequiredTestItem {
+class TestItemFromMirror implements vscode.ObservedTestItem {
 	readonly #internal: MirroredCollectionTestItem;
 	readonly #collection: MirroredTestCollection;
 
@@ -737,7 +669,7 @@ class TestItemFromMirror implements vscode.RequiredTestItem {
 	}
 
 	public toJSON() {
-		const serialized: vscode.RequiredTestItem & TestIdWithProvider = {
+		const serialized: vscode.ObservedTestItem & TestIdWithProvider = {
 			id: this.id,
 			label: this.label,
 			description: this.description,

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

@@ -1637,7 +1637,7 @@ export namespace TestItem {
 		};
 	}
 
-	export function toShallow(item: ITestItem): Omit<vscode.RequiredTestItem, 'children'> {
+	export function toShallow(item: ITestItem): Omit<types.RequiredTestItem, 'children'> {
 		return {
 			id: item.extId,
 			label: item.label,

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

@@ -3294,7 +3294,11 @@ export enum TestMessageSeverity {
 	Hint = 3
 }
 
-export type RequiredTestItem = vscode.RequiredTestItem;
+export type RequiredTestItem = {
+	[K in keyof Required<TestItem>]: K extends 'children'
+	? RequiredTestItem[]
+	: (K extends 'description' | 'location' ? TestItem[K] : Required<TestItem>[K])
+};
 
 export type TestItem = vscode.TestItem;
 

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

@@ -9,12 +9,12 @@ import * as convert from 'vs/workbench/api/common/extHostTypeConverters';
 import { TestDiffOpType } from 'vs/workbench/contrib/testing/common/testCollection';
 import { stubTest, testStubs } from 'vs/workbench/contrib/testing/common/testStubs';
 import { TestOwnedTestCollection, TestSingleUseCollection } from 'vs/workbench/contrib/testing/test/common/ownedTestCollection';
-import { TestChangeEvent, TestItem, TextDocument } from 'vscode';
+import { ObservedTestItem, TestChangeEvent, TestItem, TextDocument } from 'vscode';
 import { URI } from 'vs/base/common/uri';
 import { Location } from 'vs/editor/common/modes';
 import { Range } from 'vs/editor/common/core/range';
 
-const simplify = (item: TestItem) => {
+const simplify = (item: TestItem | ObservedTestItem) => {
 	if ('toJSON' in item) {
 		item = (item as any).toJSON();
 		delete (item as any).providerId;
@@ -24,16 +24,16 @@ const simplify = (item: TestItem) => {
 	return { ...item, children: undefined };
 };
 
-const assertTreesEqual = (a: Readonly<TestItem>, b: Readonly<TestItem>) => {
+const assertTreesEqual = (a: TestItem | ObservedTestItem, b: TestItem | ObservedTestItem) => {
 	assert.deepStrictEqual(simplify(a), simplify(b));
 
-	const aChildren = (a.children ?? []).sort();
-	const bChildren = (b.children ?? []).sort();
+	const aChildren = (a.children ?? []).slice().sort();
+	const bChildren = (b.children ?? []).slice().sort();
 	assert.strictEqual(aChildren.length, bChildren.length, `expected ${a.label}.children.length == ${b.label}.children.length`);
 	aChildren.forEach((_, i) => assertTreesEqual(aChildren[i], bChildren[i]));
 };
 
-const assertTreeListEqual = (a: ReadonlyArray<Readonly<TestItem>>, b: ReadonlyArray<Readonly<TestItem>>) => {
+const assertTreeListEqual = (a: ReadonlyArray<TestItem | ObservedTestItem>, b: ReadonlyArray<TestItem | ObservedTestItem>) => {
 	assert.strictEqual(a.length, b.length, `expected a.length == n.length`);
 	a.forEach((_, i) => assertTreesEqual(a[i], b[i]));
 };
@@ -187,7 +187,6 @@ suite('ExtHost Testing', () => {
 			});
 
 			test('creates change for root', () => {
-				assert.deepStrictEqual(m.changeEvent.commonChangeAncestor, null);
 				assertTreeListEqual(m.changeEvent.added, [
 					tests,
 					tests.children[0],
@@ -204,7 +203,6 @@ suite('ExtHost Testing', () => {
 				single.onItemChange(tests, 'pid');
 				m.apply(single.collectDiff());
 
-				assertTreesEqual(m.changeEvent.commonChangeAncestor!, tests);
 				assertTreeListEqual(m.changeEvent.added, []);
 				assertTreeListEqual(m.changeEvent.removed, [
 					{ ...rm, children: [] },
@@ -219,7 +217,6 @@ suite('ExtHost Testing', () => {
 				single.onItemChange(tests, 'pid');
 				m.apply(single.collectDiff());
 
-				assert.deepStrictEqual(m.changeEvent.commonChangeAncestor?.label, 'updated!');
 				assertTreeListEqual(m.changeEvent.added, []);
 				assertTreeListEqual(m.changeEvent.removed, []);
 				assertTreeListEqual(m.changeEvent.updated, [tests.children[0]]);
@@ -244,7 +241,6 @@ suite('ExtHost Testing', () => {
 				single.onItemChange(tests, 'pid');
 				m.apply(single.collectDiff());
 
-				assert.strictEqual(m.changeEvent.commonChangeAncestor?.label, 'root');
 				assertTreeListEqual(m.changeEvent.added, [child]);
 				assertTreeListEqual(m.changeEvent.removed, []);
 				assertTreeListEqual(m.changeEvent.updated, []);
@@ -256,7 +252,6 @@ suite('ExtHost Testing', () => {
 				single.onItemChange(tests, 'pid');
 				m.apply(single.collectDiff());
 
-				assert.strictEqual(m.changeEvent.commonChangeAncestor?.label, 'a');
 			});
 
 			test('gets the common ancestor (2)', () => {
@@ -264,8 +259,6 @@ suite('ExtHost Testing', () => {
 				tests.children![1].label = 'ab';
 				single.onItemChange(tests, 'pid');
 				m.apply(single.collectDiff());
-
-				assert.strictEqual(m.changeEvent.commonChangeAncestor?.label, 'root');
 			});
 		});