未验证 提交 6a76c622 编写于 作者: C Connor Peet

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:

https://github.com/microsoft/vscode-extension-samples/commit/8fdf822985243c42aa7ed922a9a6c1fc5540c478#diff-2fe3ad6ad19447c57c5db14c5a6ccb5544944494db6b909540d70ea499784b49R9
上级 44445888
......@@ -1796,14 +1796,14 @@ declare module 'vscode' {
*
* @param id Identifier for the controller, must be globally unique.
*/
export function createTestController<T>(id: string): TestController<T>;
export function createTestController(id: string): TestController;
/**
* Requests that tests be run by their controller.
* @param run Run options to use
* @param token Cancellation token for the test run
*/
export function runTests<T>(run: TestRunRequest<T>, token?: CancellationToken): Thenable<void>;
export function runTests(run: TestRunRequest, token?: CancellationToken): Thenable<void>;
/**
* Returns an observer that watches and can request tests.
......@@ -1832,7 +1832,7 @@ declare module 'vscode' {
/**
* List of tests returned by test provider for files in the workspace.
*/
readonly tests: ReadonlyArray<TestItem<never>>;
readonly tests: ReadonlyArray<TestItem>;
/**
* An event that fires when an existing test in the collection changes, or
......@@ -1855,25 +1855,27 @@ declare module 'vscode' {
/**
* List of all tests that are newly added.
*/
readonly added: ReadonlyArray<TestItem<never>>;
readonly added: ReadonlyArray<TestItem>;
/**
* List of existing tests that have updated.
*/
readonly updated: ReadonlyArray<TestItem<never>>;
readonly updated: ReadonlyArray<TestItem>;
/**
* List of existing tests that have been removed.
*/
readonly removed: ReadonlyArray<TestItem<never>>;
readonly removed: ReadonlyArray<TestItem>;
}
/**
* Interface to discover and execute tests.
*/
export interface TestController<T = any> {
//todo@API expose `readonly id: string` that createTestController asked me for
export interface TestController {
/**
* The ID of the controller, passed in {@link vscode.test.createTestController}
*/
readonly id: string;
/**
* Root test item. Tests in the workspace should be added as children of
......@@ -1901,13 +1903,12 @@ declare module 'vscode' {
* @param uri URI this TestItem is associated with. May be a file or directory.
* @param data Custom data to be stored in {@link TestItem.data}
*/
createTestItem<TChild = T>(
createTestItem(
id: string,
label: string,
parent: TestItem,
uri?: Uri,
data?: TChild,
): TestItem<TChild>;
): TestItem;
/**
......@@ -1925,7 +1926,7 @@ declare module 'vscode' {
* @param item An unresolved test item for which
* children are being requested
*/
resolveChildrenHandler?: (item: TestItem<T>) => Thenable<void> | void;
resolveChildrenHandler?: (item: TestItem) => Thenable<void> | void;
/**
* Starts a test run. When called, the controller should call
......@@ -1939,7 +1940,7 @@ declare module 'vscode' {
* instances associated with the request will be
* automatically cancelled as well.
*/
runHandler?: (request: TestRunRequest<T>, token: CancellationToken) => Thenable<void> | void;
runHandler?: (request: TestRunRequest, token: CancellationToken) => Thenable<void> | void;
/**
* Creates a {@link TestRun<T>}. This should be called by the
* {@link TestRunner} when a request is made to execute tests, and may also
......@@ -1956,7 +1957,7 @@ declare module 'vscode' {
* persisted in the editor. This may be false if the results are coming from
* a file already saved externally, such as a coverage information file.
*/
createTestRun<T>(request: TestRunRequest<T>, name?: string, persist?: boolean): TestRun<T>;
createTestRun(request: TestRunRequest, name?: string, persist?: boolean): TestRun;
/**
* Unregisters the test controller, disposing of its associated tests
......@@ -1968,20 +1969,20 @@ declare module 'vscode' {
/**
* Options given to {@link test.runTests}.
*/
export class TestRunRequest<T> {
export class TestRunRequest {
/**
* Array of specific tests to run. The controllers should run all of the
* given tests and all children of the given tests, excluding any tests
* that appear in {@link TestRunRequest.exclude}.
*/
tests: TestItem<T>[];
tests: TestItem[];
/**
* An array of tests the user has marked as excluded in the editor. May be
* omitted if no exclusions were requested. Test controllers should not run
* excluded tests or any children of excluded tests.
*/
exclude?: TestItem<T>[];
exclude?: TestItem[];
/**
* Whether tests in this run should be debugged.
......@@ -1993,13 +1994,13 @@ declare module 'vscode' {
* @param exclude Tests to exclude from the run
* @param debug Whether tests in this run should be debugged.
*/
constructor(tests: readonly TestItem<T>[], exclude?: readonly TestItem<T>[], debug?: boolean);
constructor(tests: readonly TestItem[], exclude?: readonly TestItem[], debug?: boolean);
}
/**
* Options given to {@link TestController.runTests}
*/
export interface TestRun<T = void> {
export interface TestRun {
/**
* The human-readable name of the run. This can be used to
* disambiguate multiple sets of results in a test run. It is useful if
......@@ -2023,7 +2024,7 @@ declare module 'vscode' {
* @param duration Optionally sets how long the test took to run, in milliseconds
*/
//todo@API is this "update" state or set final state? should this be called setTestResult?
setState(test: TestItem<T>, state: TestResultState, duration?: number): void;
setState(test: TestItem, state: TestResultState, duration?: number): void;
/**
* Appends a message, such as an assertion error, to the test item.
......@@ -2034,7 +2035,7 @@ declare module 'vscode' {
* @param test The test to update
* @param message The message to add
*/
appendMessage(test: TestItem<T>, message: TestMessage): void;
appendMessage(test: TestItem, message: TestMessage): void;
/**
* Appends raw output from the test runner. On the user's request, the
......@@ -2057,7 +2058,7 @@ declare module 'vscode' {
* 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.
*/
export interface TestItem<T = any> {
export interface TestItem {
/**
* Unique identifier for the TestItem. This is used to correlate
* test results and tests in the document with those in the workspace
......@@ -2077,12 +2078,10 @@ declare module 'vscode' {
readonly children: ReadonlyMap<string, TestItem>;
/**
* The parent of this item, if any. Assigned automatically when calling
* {@link TestItem.addChild}.
* The parent of this item, given in {@link TestController.createTestItem}.
* This is undefined only for the {@link TestController.root}.
*/
//todo@API jsdoc outdated (likely in many places)
//todo@API is this needed? with TestController#root this is never undefined but `root` is questionable (see above)
readonly parent?: TestItem<any>;
readonly parent?: TestItem;
/**
* Indicates whether this test item may have children discovered by resolving.
......@@ -2136,14 +2135,6 @@ declare module 'vscode' {
*/
debuggable: boolean;
/**
* Custom extension data on the item. This data will never be serialized
* or shared outside the extenion who created the item.
*/
// todo@API remove? this brings in a ton of generics, into every single type... extension own test items, they create and dispose them,
// and therefore can have a WeakMap<TestItem, T> or Map<id, T> to the side.
data: T;
/**
* Marks the test as outdated. This can happen as a result of file changes,
* for example. In "auto run" mode, tests that are outdated will be
......@@ -2152,8 +2143,7 @@ declare module 'vscode' {
*
* Extensions should generally not override this method.
*/
// todo@API boolean property instead? stale: boolean
invalidate(): void;
invalidateResults(): void;
/**
* Removes the test and its children from the tree.
......
......@@ -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);
}
......
......@@ -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>;
......
......@@ -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);
}
......
......@@ -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]);
}
......
......@@ -17,7 +17,7 @@ import { TestItemImpl, TestResultState, testStubs } from 'vs/workbench/contrib/t
import { TestSingleUseCollection } from 'vs/workbench/contrib/testing/test/common/ownedTestCollection';
import type { TestItem, TestRunRequest } from 'vscode';
const simplify = (item: TestItem<unknown>) => ({
const simplify = (item: TestItem) => ({
id: item.id,
label: item.label,
uri: item.uri,
......@@ -26,7 +26,7 @@ const simplify = (item: TestItem<unknown>) => ({
debuggable: item.debuggable,
});
const assertTreesEqual = (a: TestItem<unknown> | undefined, b: TestItem<unknown> | undefined) => {
const assertTreesEqual = (a: TestItem | undefined, b: TestItem | undefined) => {
if (!a) {
throw new assert.AssertionError({ message: 'Expected a to be defined', actual: a });
}
......@@ -303,7 +303,7 @@ suite('ExtHost Testing', () => {
let c: TestRunCoordinator;
let cts: CancellationTokenSource;
let req: TestRunRequest<unknown>;
let req: TestRunRequest;
let dto: TestRunDto;
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册