Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: first pass at related code api #149286

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/vs/workbench/api/common/extHostTestItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as editorRange from 'vs/editor/common/core/range';
import { createPrivateApiFor, getPrivateApiFor, IExtHostTestItemApi } from 'vs/workbench/api/common/extHostTestingPrivateApi';
import { TestId, TestIdPathParts } from 'vs/workbench/contrib/testing/common/testId';
import { createTestItemChildren, ExtHostTestItemEvent, ITestChildrenLike, ITestItemApi, ITestItemChildren, TestItemCollection, TestItemEventOp } from 'vs/workbench/contrib/testing/common/testItemCollection';
import { denamespaceTestTag, ITestItem, ITestItemContext } from 'vs/workbench/contrib/testing/common/testTypes';
import { denamespaceTestTag, ITestItem, ITestItemContext, TestItemWritableProps } from 'vs/workbench/contrib/testing/common/testTypes';
import type * as vscode from 'vscode';
import * as Convert from 'vs/workbench/api/common/extHostTypeConverters';
import { URI } from 'vs/base/common/uri';
Expand Down Expand Up @@ -34,7 +34,7 @@ const testItemPropAccessor = <K extends keyof vscode.TestItem>(
};
};

type WritableProps = Pick<vscode.TestItem, 'range' | 'label' | 'description' | 'sortText' | 'canResolveChildren' | 'busy' | 'error' | 'tags'>;
type WritableProps = Pick<vscode.TestItem, (keyof TestItemWritableProps) | 'canResolveChildren'>;

const strictEqualComparator = <T>(a: T, b: T) => a === b;

Expand All @@ -44,6 +44,11 @@ const propComparators: { [K in keyof Required<WritableProps>]: (a: vscode.TestIt
if (!a || !b) { return false; }
return a.isEqual(b);
},
relatedCode: (a, b) => {
if (a === b) { return true; }
if (!a || !b) { return false; }
return a.length === b.length && a.every((r, i) => r.uri.toString() === b[i].uri.toString() && r.range.isEqual(b[i].range));
},
label: strictEqualComparator,
description: strictEqualComparator,
sortText: strictEqualComparator,
Expand All @@ -68,6 +73,9 @@ const evSetProps = <T>(fn: (newValue: T) => Partial<ITestItem>): (newValue: T) =

const makePropDescriptors = (api: IExtHostTestItemApi, label: string): { [K in keyof Required<WritableProps>]: PropertyDescriptor } => ({
range: testItemPropAccessor<'range'>(api, undefined, propComparators.range, evSetProps(r => ({ range: editorRange.Range.lift(Convert.Range.from(r)) }))),
relatedCode: testItemPropAccessor<'relatedCode'>(api, undefined, propComparators.relatedCode,
evSetProps(r => ({ relatedCode: r?.map(r2 => ({ uri: r2.uri, range: editorRange.Range.lift(Convert.Range.from(r2.range)) })) ?? null })),
),
label: testItemPropAccessor<'label'>(api, label, propComparators.label, evSetProps(label => ({ label }))),
description: testItemPropAccessor<'description'>(api, undefined, propComparators.description, evSetProps(description => ({ description }))),
sortText: testItemPropAccessor<'sortText'>(api, undefined, propComparators.sortText, evSetProps(sortText => ({ sortText }))),
Expand Down Expand Up @@ -111,6 +119,7 @@ export class TestItemImpl implements vscode.TestItem {
public readonly children!: ITestItemChildren<vscode.TestItem>;
public readonly parent!: TestItemImpl | undefined;

public relatedCode!: vscode.Location[] | undefined;
public range!: vscode.Range | undefined;
public description!: string | undefined;
public sortText!: string | undefined;
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/api/common/extHostTypeConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1765,6 +1765,7 @@ export namespace TestItem {
busy: false,
tags: item.tags.map(t => TestTag.namespace(ctrlId, t.id)),
range: editorRange.Range.lift(Range.from(item.range)),
relatedCode: item.relatedCode?.map(r => ({ uri: URI.revive(r.uri), range: editorRange.Range.lift(Range.from(r.range)) })) || null,
description: item.description || null,
sortText: item.sortText || null,
error: item.error ? (MarkdownString.fromStrict(item.error) || null) : null,
Expand Down
84 changes: 48 additions & 36 deletions src/vs/workbench/contrib/testing/browser/testingDecorations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ContentWidgetPositionPreference, ICodeEditor, IContentWidgetPosition, I
import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
import { EditorOption } from 'vs/editor/common/config/editorOptions';
import { editorCodeLensForeground, overviewRulerError, overviewRulerInfo } from 'vs/editor/common/core/editorColorRegistry';
import { IRange, Range } from 'vs/editor/common/core/range';
import { Range } from 'vs/editor/common/core/range';
import { IEditorContribution } from 'vs/editor/common/editorCommon';
import { IModelDeltaDecoration, ITextModel, OverviewRulerLane, TrackedRangeStickiness } from 'vs/editor/common/model';
import { IModelService } from 'vs/editor/common/services/model';
Expand Down Expand Up @@ -109,22 +109,33 @@ export class TestingDecorationService extends Disposable implements ITestingDeco
modelService.onModelRemoved(e => this.decorationCache.delete(e.uri));

const debounceInvalidate = this._register(new RunOnceScheduler(() => this.invalidate(), 100));
const invalidateDecorationsIn = (uri: URI) => {
const rec = this.decorationCache.get(uri);
if (rec) {
rec.testRangesUpdated = true;
}
};

// If ranges were updated in the document, mark that we should explicitly
// sync decorations to the published lines, since we assume that everything
// is up to date. This prevents issues, as in #138632, #138835, #138922.
this._register(this.testService.onWillProcessDiff(diff => {
for (const entry of diff) {
let uri: URI | undefined | null;
if (entry.op === TestDiffOpType.Add || entry.op === TestDiffOpType.Update) {
uri = entry.item.item?.uri;
} else if (entry.op === TestDiffOpType.Remove) {
uri = this.testService.collection.getNodeById(entry.itemId)?.item.uri;
}

const rec = uri && this.decorationCache.get(uri);
if (rec) {
rec.testRangesUpdated = true;
const item = entry.op === TestDiffOpType.Add || entry.op === TestDiffOpType.Update
? entry.item.item
: entry.op === TestDiffOpType.Remove
? this.testService.collection.getNodeById(entry.itemId)?.item
: undefined;

if (item) {
if (item.uri) {
invalidateDecorationsIn(item.uri);
}
if (item.relatedCode) {
for (const { uri } of item.relatedCode) {
invalidateDecorationsIn(uri);
}
}
}
}

Expand Down Expand Up @@ -201,13 +212,21 @@ export class TestingDecorationService extends Disposable implements ITestingDeco
model.changeDecorations(accessor => {
const runDecorations = new TestDecorations<{ line: number; id: ''; test: IncrementalTestCollectionItem; resultItem: TestResultItem | undefined }>();
for (const test of this.testService.collection.all) {
if (!test.item.range || test.item.uri?.toString() !== uriStr) {
continue;
if (test.item.range && test.item.uri?.toString() === uriStr) {
const stateLookup = this.results.getStateById(test.item.extId);
const line = test.item.range.startLineNumber;
runDecorations.push({ line, id: '', test, resultItem: stateLookup?.[1] });
}

const stateLookup = this.results.getStateById(test.item.extId);
const line = test.item.range.startLineNumber;
runDecorations.push({ line, id: '', test, resultItem: stateLookup?.[1] });
if (test.item.relatedCode) {
for (const { uri, range } of test.item.relatedCode) {
if (uri.toString() === uriStr) {
const stateLookup = this.results.getStateById(test.item.extId);
const line = range.startLineNumber;
runDecorations.push({ line, id: '', test, resultItem: stateLookup?.[1] });
}
}
}
}

for (const [line, tests] of runDecorations.lines()) {
Expand All @@ -226,8 +245,8 @@ export class TestingDecorationService extends Disposable implements ITestingDeco
newDecorations.push(existing);
} else {
newDecorations.push(multi
? this.instantiationService.createInstance(MultiRunTestDecoration, tests, gutterEnabled, model)
: this.instantiationService.createInstance(RunSingleTestDecoration, tests[0].test, tests[0].resultItem, model, gutterEnabled));
? this.instantiationService.createInstance(MultiRunTestDecoration, line, tests, gutterEnabled, model)
: this.instantiationService.createInstance(RunSingleTestDecoration, line, tests[0].test, tests[0].resultItem, model, gutterEnabled));
}
}

Expand Down Expand Up @@ -419,21 +438,16 @@ export class TestingDecorations extends Disposable implements IEditorContributio
}
}

const firstLineRange = (originalRange: IRange) => ({
startLineNumber: originalRange.startLineNumber,
endLineNumber: originalRange.startLineNumber,
const lineRange = (line: number) => ({
startLineNumber: line,
endLineNumber: line,
startColumn: 0,
endColumn: 1,
});

const createRunTestDecoration = (tests: readonly IncrementalTestCollectionItem[], states: readonly (TestResultItem | undefined)[], visible: boolean): IModelDeltaDecoration => {
const range = tests[0]?.item.range;
if (!range) {
throw new Error('Test decorations can only be created for tests with a range');
}

const createRunTestDecoration = (tests: readonly IncrementalTestCollectionItem[], states: readonly (TestResultItem | undefined)[], line: number, visible: boolean): IModelDeltaDecoration => {
if (!visible) {
return { range: firstLineRange(range), options: { isWholeLine: true, description: 'run-test-decoration' } };
return { range: lineRange(line), options: { isWholeLine: true, description: 'run-test-decoration' } };
}

let computedState = TestResultState.Unset;
Expand Down Expand Up @@ -462,7 +476,7 @@ const createRunTestDecoration = (tests: readonly IncrementalTestCollectionItem[]
let glyphMarginClassName = ThemeIcon.asClassName(icon) + ' testing-run-glyph';

return {
range: firstLineRange(range),
range: lineRange(line),
options: {
description: 'run-test-decoration',
isWholeLine: true,
Expand Down Expand Up @@ -596,13 +610,10 @@ abstract class RunTestDecoration {
/** @inheritdoc */
public id = '';

public get line() {
return this.editorDecoration.range.startLineNumber;
}

public editorDecoration: IModelDeltaDecoration;

constructor(
public readonly line: number,
protected tests: readonly {
test: IncrementalTestCollectionItem;
resultItem: TestResultItem | undefined;
Expand All @@ -618,7 +629,7 @@ abstract class RunTestDecoration {
@IContextKeyService protected readonly contextKeyService: IContextKeyService,
@IMenuService protected readonly menuService: IMenuService,
) {
this.editorDecoration = createRunTestDecoration(tests.map(t => t.test), tests.map(t => t.resultItem), visible);
this.editorDecoration = createRunTestDecoration(tests.map(t => t.test), tests.map(t => t.resultItem), line, visible);
this.editorDecoration.options.glyphMarginHoverMessage = new MarkdownString().appendText(this.getGutterLabel());
}

Expand Down Expand Up @@ -665,7 +676,7 @@ abstract class RunTestDecoration {

this.tests = newTests;
this.visible = visible;
this.editorDecoration.options = createRunTestDecoration(newTests.map(t => t.test), newTests.map(t => t.resultItem), visible).options;
this.editorDecoration.options = createRunTestDecoration(newTests.map(t => t.test), newTests.map(t => t.resultItem), this.line, visible).options;
return true;
}

Expand Down Expand Up @@ -827,6 +838,7 @@ class MultiRunTestDecoration extends RunTestDecoration implements ITestDecoratio

class RunSingleTestDecoration extends RunTestDecoration implements ITestDecoration {
constructor(
line: number,
test: IncrementalTestCollectionItem,
resultItem: TestResultItem | undefined,
model: ITextModel,
Expand All @@ -840,7 +852,7 @@ class RunSingleTestDecoration extends RunTestDecoration implements ITestDecorati
@IContextKeyService contextKeyService: IContextKeyService,
@IMenuService menuService: IMenuService,
) {
super([{ test, resultItem }], visible, model, codeEditorService, testService, contextMenuService, commandService, configurationService, testProfiles, contextKeyService, menuService);
super(line, [{ test, resultItem }], visible, model, codeEditorService, testService, contextMenuService, commandService, configurationService, testProfiles, contextKeyService, menuService);
}

protected override getContextMenuActions() {
Expand Down
10 changes: 8 additions & 2 deletions src/vs/workbench/contrib/testing/common/testItemCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Barrier, isThenable, RunOnceScheduler } from 'vs/base/common/async';
import { Emitter } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { assertNever } from 'vs/base/common/types';
import { applyTestItemUpdate, ITestItem, ITestTag, namespaceTestTag, TestDiffOpType, TestItemExpandState, TestsDiff, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testTypes';
import { applyTestItemUpdate, ITestItem, ITestTag, namespaceTestTag, TestDiffOpType, TestItemExpandState, TestItemWritableProps, TestsDiff, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testTypes';
import { TestId } from 'vs/workbench/contrib/testing/common/testId';

/**
Expand Down Expand Up @@ -96,12 +96,18 @@ export interface ITestItemCollectionOptions<T> {
}

const strictEqualComparator = <T>(a: T, b: T) => a === b;
const diffableProps: { [K in keyof ITestItem]?: (a: ITestItem[K], b: ITestItem[K]) => boolean } = {
const diffableProps: { [K in keyof TestItemWritableProps]: (a: ITestItem[K], b: ITestItem[K]) => boolean } = {
range: (a, b) => {
if (a === b) { return true; }
if (!a || !b) { return false; }
return a.equalsRange(b);
},
relatedCode: (a, b) => {
if (a === b) { return true; }
if (!a || !b) { return false; }
return a.length === b.length && a.every((r, i) => r.uri.toString() === b[i].uri.toString() && r.range.equalsRange(b[i].range));
},
sortText: strictEqualComparator,
busy: strictEqualComparator,
label: strictEqualComparator,
description: strictEqualComparator,
Expand Down
13 changes: 13 additions & 0 deletions src/vs/workbench/contrib/testing/common/testTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ export interface ITestTagDisplayInfo {

/**
* The TestItem from .d.ts, as a plain object without children.
*
* If you want to add a new property here (new API from vscode.TestItem), make
* sure to also add it to `TestItemWritableProps` and ITestItemUpdate's
* serialization, and follow the compile errors to add it everywhere else.
*/
export interface ITestItem {
/** ID of the test given by the test controller */
Expand All @@ -258,11 +262,15 @@ export interface ITestItem {
children?: never;
uri: URI | undefined;
range: Range | null;
relatedCode: { uri: URI; range: Range }[] | null;
description: string | null;
error: string | IMarkdownString | null;
sortText: string | null;
}

/** Subset of the ITestItem which is writable after the item's creation. */
export type TestItemWritableProps = Pick<ITestItem, 'range' | 'label' | 'description' | 'sortText' | 'busy' | 'error' | 'tags' | 'relatedCode'>;

export namespace ITestItem {
export interface Serialized {
extId: string;
Expand All @@ -272,6 +280,7 @@ export namespace ITestItem {
children?: never;
uri: UriComponents | undefined;
range: IRange | null;
relatedCode: { uri: UriComponents; range: IRange }[] | null;
description: string | null;
error: string | IMarkdownString | null;
sortText: string | null;
Expand All @@ -285,6 +294,7 @@ export namespace ITestItem {
children: undefined,
uri: item.uri?.toJSON(),
range: item.range?.toJSON() || null,
relatedCode: item.relatedCode || null,
description: item.description,
error: item.error,
sortText: item.sortText
Expand All @@ -298,6 +308,7 @@ export namespace ITestItem {
children: undefined,
uri: serialized.uri ? URI.revive(serialized.uri) : undefined,
range: serialized.range ? Range.lift(serialized.range) : null,
relatedCode: serialized.relatedCode?.map(r => ({ uri: URI.revive(r.uri), range: Range.lift(r.range) })) || null,
description: serialized.description,
error: serialized.error,
sortText: serialized.sortText
Expand Down Expand Up @@ -376,6 +387,7 @@ export namespace ITestItemUpdate {
if (u.item.description !== undefined) { item.description = u.item.description; }
if (u.item.error !== undefined) { item.error = u.item.error; }
if (u.item.sortText !== undefined) { item.sortText = u.item.sortText; }
if (u.item.relatedCode !== undefined) { item.relatedCode = u.item.relatedCode; }
}

return { extId: u.extId, expand: u.expand, item };
Expand All @@ -392,6 +404,7 @@ export namespace ITestItemUpdate {
if (u.item.description !== undefined) { item.description = u.item.description; }
if (u.item.error !== undefined) { item.error = u.item.error; }
if (u.item.sortText !== undefined) { item.sortText = u.item.sortText; }
if (u.item.relatedCode !== undefined) { item.relatedCode = u.item.relatedCode?.map(r => ({ uri: URI.revive(r.uri), range: Range.lift(r.range) })) || null; }
}

return { extId: u.extId, expand: u.expand, item };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ suite('Workbench - Testing Explorer Hierarchal by Location Projection', () => {
sortText: null,
tags: [],
uri: undefined,
relatedCode: null,
},
parent: 'id-root',
tasks: [],
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/testing/test/common/testStubs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class TestTestItem implements ITestItemLike {
label,
range: null,
sortText: null,
relatedCode: null,
tags: [],
uri,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const allApiProposals = Object.freeze({
terminalNameChangeEvent: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.terminalNameChangeEvent.d.ts',
testCoverage: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testCoverage.d.ts',
testObserver: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testObserver.d.ts',
testRelatedCode: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.testRelatedCode.d.ts',
textDocumentNotebook: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.textDocumentNotebook.d.ts',
textEditorDrop: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.textEditorDrop.d.ts',
textSearchProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.textSearchProvider.d.ts',
Expand Down
18 changes: 18 additions & 0 deletions src/vscode-dts/vscode.proposed.testRelatedCode.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

declare module 'vscode' {

// https://github.com/microsoft/vscode/issues/126932

export interface TestItem {
/**
* Ranges of implementation code related to this test. This is used to
* provide navigation between a test and its implementation, as well as
* commands to run tests related to the an implementation location.
*/
relatedCode?: readonly Location[];
}
}