testing: remove data from TestItems

Data was viral and spread generics all over the place. It was also
hard to type correctly and consistently, which ended up in the type
being `any` unless great care was taken.

This removes the `data`. Instead, consumers can use a `WeakMap<TestItem, T>`
to keep data associated with the test item. This is a similar pattern
to what is used to store data about documents and debug sessions, for
example. Here's an example of a migration:

8fdf822985 (diff-2fe3ad6ad19447c57c5db14c5a6ccb5544944494db6b909540d70ea499784b49R9)
This commit is contained in:
Connor Peet
2021-06-24 11:54:07 -07:00
parent 44445888fa
commit 6a76c62232
6 changed files with 87 additions and 86 deletions

View File

@@ -27,7 +27,7 @@ import type * as vscode from 'vscode';
export class ExtHostTesting implements ExtHostTestingShape {
private readonly resultsChangedEmitter = new Emitter<void>();
private readonly controllers = new Map</* controller ID */ string, {
controller: vscode.TestController<any>,
controller: vscode.TestController,
collection: SingleUseTestCollection,
}>();
private readonly proxy: MainThreadTestingShape;
@@ -51,22 +51,25 @@ export class ExtHostTesting implements ExtHostTestingShape {
/**
* Implements vscode.test.registerTestProvider
*/
public createTestController<T>(controllerId: string): vscode.TestController<T> {
public createTestController(controllerId: string): vscode.TestController {
const disposable = new DisposableStore();
const collection = disposable.add(new SingleUseTestCollection(controllerId));
const initialExpand = disposable.add(new RunOnceScheduler(() => collection.expand(collection.root.id, 0), 0));
const controller: vscode.TestController<T> = {
const controller: vscode.TestController = {
root: collection.root,
get id() {
return controllerId;
},
createTestRun: (request, name, persist = true) => {
return this.runTracker.createTestRun(controllerId, request, name, persist);
},
createTestItem<TChild>(id: string, label: string, parent: vscode.TestItem, uri: vscode.Uri, data?: TChild) {
createTestItem(id: string, label: string, parent: vscode.TestItem, uri: vscode.Uri, data?: unknown) {
if (!(parent instanceof TestItemImpl)) {
throw new Error(`The "parent" passed in for TestItem ${id} is invalid`);
}
return new TestItemImpl<TChild>(id, label, uri, data as TChild, parent);
return new TestItemImpl(id, label, uri, data, parent);
},
set resolveChildrenHandler(fn) {
collection.resolveHandler = fn;
@@ -104,8 +107,8 @@ export class ExtHostTesting implements ExtHostTestingShape {
/**
* Implements vscode.test.runTests
*/
public async runTests(req: vscode.TestRunRequest<unknown>, token = CancellationToken.None) {
const testListToProviders = (tests: ReadonlyArray<vscode.TestItem<unknown>>) =>
public async runTests(req: vscode.TestRunRequest, token = CancellationToken.None) {
const testListToProviders = (tests: ReadonlyArray<vscode.TestItem>) =>
tests
.map(this.getInternalTestForReference, this)
.filter(isDefined)
@@ -181,7 +184,7 @@ export class ExtHostTesting implements ExtHostTestingShape {
return;
}
const publicReq: vscode.TestRunRequest<unknown> = {
const publicReq: vscode.TestRunRequest = {
tests: includeTests.map(t => t.actual),
exclude: excludeTests.map(t => t.actual),
debug: req.debug,
@@ -214,14 +217,14 @@ export class ExtHostTesting implements ExtHostTestingShape {
/**
* Gets the internal test item associated with the reference from the extension.
*/
private getInternalTestForReference(test: vscode.TestItem<unknown>) {
private getInternalTestForReference(test: vscode.TestItem) {
return mapFind(this.controllers.values(), ({ collection }) => collection.getTestByReference(test))
?? this.observer.getMirroredTestDataByReference(test);
}
}
class TestRunTracker<T> extends Disposable {
private readonly task = new Set<TestRunImpl<T>>();
class TestRunTracker extends Disposable {
private readonly task = new Set<TestRunImpl>();
private readonly sharedTestIds = new Set<string>();
private readonly cts: CancellationTokenSource;
private readonly endEmitter = this._register(new Emitter<void>());
@@ -283,7 +286,7 @@ class TestRunTracker<T> extends Disposable {
* run so that `createTestRun` can be properly correlated.
*/
export class TestRunCoordinator {
private tracked = new Map<vscode.TestRunRequest<unknown>, TestRunTracker<unknown>>();
private tracked = new Map<vscode.TestRunRequest, TestRunTracker>();
public get trackers() {
return this.tracked.values();
@@ -296,7 +299,7 @@ export class TestRunCoordinator {
* `$startedExtensionTestRun` is not invoked. The run must eventually
* be cancelled manually.
*/
public prepareForMainThreadTestRun(req: vscode.TestRunRequest<unknown>, dto: TestRunDto, token: CancellationToken) {
public prepareForMainThreadTestRun(req: vscode.TestRunRequest, dto: TestRunDto, token: CancellationToken) {
return this.getTracker(req, dto, token);
}
@@ -325,7 +328,7 @@ export class TestRunCoordinator {
/**
* Implements the public `createTestRun` API.
*/
public createTestRun<T>(controllerId: string, request: vscode.TestRunRequest<T>, name: string | undefined, persist: boolean): vscode.TestRun<T> {
public createTestRun(controllerId: string, request: vscode.TestRunRequest, name: string | undefined, persist: boolean): vscode.TestRun {
const existing = this.tracked.get(request);
if (existing) {
return existing.createRun(name);
@@ -347,7 +350,7 @@ export class TestRunCoordinator {
return tracker.createRun(name);
}
private getTracker(req: vscode.TestRunRequest<unknown>, dto: TestRunDto, token?: CancellationToken) {
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));
@@ -356,7 +359,7 @@ export class TestRunCoordinator {
}
export class TestRunDto {
public static fromPublic(controllerId: string, request: vscode.TestRunRequest<unknown>) {
public static fromPublic(controllerId: string, request: vscode.TestRunRequest) {
return new TestRunDto(
controllerId,
generateUuid(),
@@ -381,8 +384,8 @@ export class TestRunDto {
private readonly exclude: ReadonlySet<string>,
) { }
public isIncluded(test: vscode.TestItem<unknown>) {
for (let t: vscode.TestItem<unknown> | undefined = test; t; t = t.parent) {
public isIncluded(test: vscode.TestItem) {
for (let t: vscode.TestItem | undefined = test; t; t = t.parent) {
if (this.include.has(t.id)) {
return true;
} else if (this.exclude.has(t.id)) {
@@ -394,7 +397,7 @@ export class TestRunDto {
}
}
class TestRunImpl<T> implements vscode.TestRun<T> {
class TestRunImpl implements vscode.TestRun {
readonly #proxy: MainThreadTestingShape;
readonly #req: TestRunDto;
readonly #sharedIds: Set<string>;
@@ -417,14 +420,14 @@ class TestRunImpl<T> implements vscode.TestRun<T> {
proxy.$startedTestRunTask(dto.id, { id: this.taskId, name, running: true });
}
setState(test: vscode.TestItem<T>, state: vscode.TestResultState, duration?: number): void {
setState(test: vscode.TestItem, state: vscode.TestResultState, duration?: number): void {
if (!this.#ended && this.#req.isIncluded(test)) {
this.ensureTestIsKnown(test);
this.#proxy.$updateTestStateInRun(this.#req.id, this.taskId, test.id, state, duration);
}
}
appendMessage(test: vscode.TestItem<T>, message: vscode.TestMessage): void {
appendMessage(test: vscode.TestItem, message: vscode.TestMessage): void {
if (!this.#ended && this.#req.isIncluded(test)) {
this.ensureTestIsKnown(test);
this.#proxy.$appendTestMessageInRun(this.#req.id, this.taskId, test.id, Convert.TestMessage.from(message));
@@ -445,7 +448,7 @@ class TestRunImpl<T> implements vscode.TestRun<T> {
}
}
private ensureTestIsKnown(test: vscode.TestItem<T>) {
private ensureTestIsKnown(test: vscode.TestItem) {
const sent = this.#sharedIds;
if (sent.has(test.id)) {
return;
@@ -475,7 +478,7 @@ class TestRunImpl<T> implements vscode.TestRun<T> {
* @private
*/
interface MirroredCollectionTestItem extends IncrementalTestCollectionItem {
revived: vscode.TestItem<never>;
revived: vscode.TestItem;
depth: number;
}
@@ -579,7 +582,7 @@ export class MirroredTestCollection extends AbstractIncrementalTestCollection<Mi
/**
* If the test item is a mirrored test item, returns its underlying ID.
*/
public getMirroredTestDataByReference(item: vscode.TestItem<unknown>) {
public getMirroredTestDataByReference(item: vscode.TestItem) {
return this.items.get(item.id);
}
@@ -590,7 +593,7 @@ export class MirroredTestCollection extends AbstractIncrementalTestCollection<Mi
return {
...item,
// todo@connor4312: make this work well again with children
revived: Convert.TestItem.toPlain(item.item) as vscode.TestItem<never>,
revived: Convert.TestItem.toPlain(item.item) as vscode.TestItem,
depth: parent ? parent.depth + 1 : 0,
children: new Set(),
};
@@ -636,7 +639,7 @@ class TestObservers {
/**
* Gets the internal test data by its reference.
*/
public getMirroredTestDataByReference(ref: vscode.TestItem<unknown>) {
public getMirroredTestDataByReference(ref: vscode.TestItem) {
return this.current?.tests.getMirroredTestDataByReference(ref);
}

View File

@@ -18,7 +18,7 @@ export type ExtHostTestItemEvent =
| [evt: ExtHostTestItemEventType.NewChild, item: TestItemImpl]
| [evt: ExtHostTestItemEventType.Disposed]
| [evt: ExtHostTestItemEventType.Invalidated]
| [evt: ExtHostTestItemEventType.SetProp, key: keyof vscode.TestItem<never>, value: any];
| [evt: ExtHostTestItemEventType.SetProp, key: keyof vscode.TestItem, value: any];
export interface IExtHostTestItemApi {
children: Map<string, TestItemImpl>;

View File

@@ -1634,9 +1634,9 @@ export namespace TestMessage {
}
export namespace TestItem {
export type Raw<T = unknown> = vscode.TestItem<T>;
export type Raw<T = unknown> = vscode.TestItem;
export function from(item: vscode.TestItem<unknown>): ITestItem {
export function from(item: vscode.TestItem): ITestItem {
return {
extId: item.id,
label: item.label,
@@ -1662,23 +1662,23 @@ export namespace TestItem {
};
}
export function toPlain(item: ITestItem): Omit<vscode.TestItem<never>, 'children' | 'invalidate' | 'discoverChildren'> {
export function toPlain(item: ITestItem): Omit<vscode.TestItem, 'children' | 'invalidate' | 'discoverChildren'> {
return {
id: item.extId,
label: item.label,
uri: URI.revive(item.uri),
range: Range.to(item.range || undefined),
dispose: () => undefined,
invalidateResults: () => undefined,
canResolveChildren: false,
busy: false,
data: undefined as never,
debuggable: item.debuggable,
description: item.description || undefined,
runnable: item.runnable,
};
}
export function to(item: ITestItem, parent?: vscode.TestItem<void>): types.TestItemImpl<void> {
export function to(item: ITestItem, parent?: vscode.TestItem): types.TestItemImpl {
const testItem = new types.TestItemImpl(item.extId, item.label, URI.revive(item.uri), undefined, parent);
testItem.range = Range.to(item.range || undefined);
testItem.debuggable = item.debuggable;
@@ -1687,8 +1687,8 @@ export namespace TestItem {
return testItem;
}
export function toItemFromContext(context: ITestItemContext): types.TestItemImpl<void> {
let node: types.TestItemImpl<void> | undefined;
export function toItemFromContext(context: ITestItemContext): types.TestItemImpl {
let node: types.TestItemImpl | undefined;
for (const test of context.tests) {
node = to(test.item, node);
}

View File

@@ -3297,11 +3297,11 @@ export enum TestMessageSeverity {
Hint = 3
}
const testItemPropAccessor = <K extends keyof vscode.TestItem<never>>(
const testItemPropAccessor = <K extends keyof vscode.TestItem>(
api: IExtHostTestItemApi,
key: K,
defaultValue: vscode.TestItem<never>[K],
equals: (a: vscode.TestItem<never>[K], b: vscode.TestItem<never>[K]) => boolean
defaultValue: vscode.TestItem[K],
equals: (a: vscode.TestItem[K], b: vscode.TestItem[K]) => boolean
) => {
let value = defaultValue;
return {
@@ -3310,7 +3310,7 @@ const testItemPropAccessor = <K extends keyof vscode.TestItem<never>>(
get() {
return value;
},
set(newValue: vscode.TestItem<never>[K]) {
set(newValue: vscode.TestItem[K]) {
if (!equals(value, newValue)) {
value = newValue;
api.bus.fire([ExtHostTestItemEventType.SetProp, key, newValue]);
@@ -3326,19 +3326,19 @@ const rangeComparator = (a: vscode.Range | undefined, b: vscode.Range | undefine
return a.isEqual(b);
};
export class TestRunRequest<T> implements vscode.TestRunRequest<T> {
export class TestRunRequest implements vscode.TestRunRequest {
constructor(
public readonly tests: vscode.TestItem<T>[],
public readonly exclude?: vscode.TestItem<T>[] | undefined,
public readonly tests: vscode.TestItem[],
public readonly exclude?: vscode.TestItem[] | undefined,
public readonly debug = false,
) { }
}
export class TestItemImpl<T = any> implements vscode.TestItem<T> {
export class TestItemImpl implements vscode.TestItem {
public readonly id!: string;
public readonly uri!: vscode.Uri | undefined;
public readonly children!: ReadonlyMap<string, TestItemImpl<T>>;
public readonly parent!: TestItemImpl<T> | undefined;
public readonly children!: ReadonlyMap<string, TestItemImpl>;
public readonly parent!: TestItemImpl | undefined;
public range!: vscode.Range | undefined;
public description!: string | undefined;
@@ -3349,7 +3349,10 @@ export class TestItemImpl<T = any> implements vscode.TestItem<T> {
public busy!: boolean;
public canResolveChildren!: boolean;
constructor(id: string, label: string, uri: vscode.Uri | undefined, public data: T, parent: vscode.TestItem | undefined) {
/**
* Note that data is deprecated and here for back-compat only
*/
constructor(id: string, label: string, uri: vscode.Uri | undefined, public data: any, parent: vscode.TestItem | undefined) {
const api = getPrivateApiFor(this);
Object.defineProperties(this, {
@@ -3398,7 +3401,12 @@ export class TestItemImpl<T = any> implements vscode.TestItem<T> {
}
}
/** @deprecated back compat */
public invalidate() {
return this.invalidateResults();
}
public invalidateResults() {
getPrivateApiFor(this).bus.fire([ExtHostTestItemEventType.Invalidated]);
}