diff --git a/src/vs/workbench/api/common/extHostTestItem.ts b/src/vs/workbench/api/common/extHostTestItem.ts index d406c5bc870..25fa2780a45 100644 --- a/src/vs/workbench/api/common/extHostTestItem.ts +++ b/src/vs/workbench/api/common/extHostTestItem.ts @@ -68,7 +68,24 @@ const evSetProps = (fn: (newValue: T) => Partial): (newValue: T) = v => ({ op: TestItemEventOp.SetProp, update: fn(v) }); const makePropDescriptors = (api: IExtHostTestItemApi, label: string): { [K in keyof Required]: PropertyDescriptor } => ({ - range: testItemPropAccessor<'range'>(api, undefined, propComparators.range, evSetProps(r => ({ range: editorRange.Range.lift(Convert.Range.from(r)) }))), + range: (() => { + let value: vscode.Range | undefined; + const updateProps = evSetProps(r => ({ range: editorRange.Range.lift(Convert.Range.from(r)) })); + return { + enumerable: true, + configurable: false, + get() { + return value; + }, + set(newValue: vscode.Range | undefined) { + api.listener?.({ op: TestItemEventOp.DocumentSynced }); + if (!propComparators.range(value, newValue)) { + value = newValue; + api.listener?.(updateProps(newValue)); + } + }, + }; + })(), 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 }))), diff --git a/src/vs/workbench/api/test/browser/extHostTesting.test.ts b/src/vs/workbench/api/test/browser/extHostTesting.test.ts index 2cdcad84cb5..dad5ae26397 100644 --- a/src/vs/workbench/api/test/browser/extHostTesting.test.ts +++ b/src/vs/workbench/api/test/browser/extHostTesting.test.ts @@ -18,6 +18,7 @@ import { TestDiffOpType, TestItemExpandState, TestMessageType, TestsDiff } from import { TestId } from 'vs/workbench/contrib/testing/common/testId'; import type { TestItem, TestRunRequest } from 'vscode'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; +import * as editorRange from 'vs/editor/common/core/range'; const simplify = (item: TestItem) => ({ id: item.id, @@ -153,7 +154,7 @@ suite('ExtHost Testing', () => { assert.deepStrictEqual(single.collectDiff(), [ { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a']).toString(), docv: undefined, item: { description: 'Hello world' } }, + item: { extId: new TestId(['ctrlId', 'id-a']).toString(), item: { description: 'Hello world' } }, } ]); }); @@ -254,7 +255,7 @@ suite('ExtHost Testing', () => { assert.deepStrictEqual(single.collectDiff(), [ { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a']).toString(), expand: TestItemExpandState.Expanded, docv: undefined, item: { label: 'Hello world' } }, + item: { extId: new TestId(['ctrlId', 'id-a']).toString(), expand: TestItemExpandState.Expanded, item: { label: 'Hello world' } }, }, ]); @@ -262,7 +263,7 @@ suite('ExtHost Testing', () => { assert.deepStrictEqual(single.collectDiff(), [ { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a']).toString(), docv: undefined, item: { label: 'still connected' } } + item: { extId: new TestId(['ctrlId', 'id-a']).toString(), item: { label: 'still connected' } } }, ]); @@ -289,7 +290,7 @@ suite('ExtHost Testing', () => { }, { op: TestDiffOpType.Update, - item: { extId: TestId.fromExtHostTestItem(oldAB, 'ctrlId').toString(), docv: undefined, item: { label: 'Hello world' } }, + item: { extId: TestId.fromExtHostTestItem(oldAB, 'ctrlId').toString(), item: { label: 'Hello world' } }, }, ]); @@ -299,11 +300,11 @@ suite('ExtHost Testing', () => { assert.deepStrictEqual(single.collectDiff(), [ { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a', 'id-aa']).toString(), docv: undefined, item: { label: 'still connected1' } } + item: { extId: new TestId(['ctrlId', 'id-a', 'id-aa']).toString(), item: { label: 'still connected1' } } }, { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a', 'id-ab']).toString(), docv: undefined, item: { label: 'still connected2' } } + item: { extId: new TestId(['ctrlId', 'id-a', 'id-ab']).toString(), item: { label: 'still connected2' } } }, ]); @@ -333,13 +334,65 @@ suite('ExtHost Testing', () => { assert.deepStrictEqual(single.collectDiff(), [ { op: TestDiffOpType.Update, - item: { extId: new TestId(['ctrlId', 'id-a', 'id-b']).toString(), docv: undefined, item: { label: 'still connected' } } + item: { extId: new TestId(['ctrlId', 'id-a', 'id-b']).toString(), item: { label: 'still connected' } } }, ]); assert.deepStrictEqual([...single.root.children].map(([_, item]) => item), [single.root.children.get('id-a')]); assert.deepStrictEqual(b.parent, a); }); + + test('sends document sync events', async () => { + await single.expand(single.root.id, 0); + single.collectDiff(); + + const a = single.root.children.get('id-a') as TestItemImpl; + a.range = new Range(new Position(0, 0), new Position(1, 0)); + + assert.deepStrictEqual(single.collectDiff(), [ + { + op: TestDiffOpType.DocumentSynced, + docv: undefined, + uri: URI.file('/') + }, + { + op: TestDiffOpType.Update, + item: { + extId: new TestId(['ctrlId', 'id-a']).toString(), + item: { + range: editorRange.Range.lift({ + endColumn: 1, + endLineNumber: 2, + startColumn: 1, + startLineNumber: 1 + }) + } + }, + }, + ]); + + // sends on replace even if it's a no-op + a.range = a.range; + assert.deepStrictEqual(single.collectDiff(), [ + { + op: TestDiffOpType.DocumentSynced, + docv: undefined, + uri: URI.file('/') + }, + ]); + + // sends on a child replacement + const a2 = new TestItemImpl('ctrlId', 'id-a', 'a', URI.file('/')); + a2.range = a.range; + single.root.children.replace([a2, single.root.children.get('id-b')!]); + assert.deepStrictEqual(single.collectDiff(), [ + { + op: TestDiffOpType.DocumentSynced, + docv: undefined, + uri: URI.file('/') + }, + ]); + }); }); diff --git a/src/vs/workbench/contrib/testing/browser/testingDecorations.ts b/src/vs/workbench/contrib/testing/browser/testingDecorations.ts index 5c50f064381..7287bcccafa 100644 --- a/src/vs/workbench/contrib/testing/browser/testingDecorations.ts +++ b/src/vs/workbench/contrib/testing/browser/testingDecorations.ts @@ -115,14 +115,13 @@ export class TestingDecorationService extends Disposable implements ITestingDeco // is up to date. This prevents issues, as in #138632, #138835, #138922. this._register(this.testService.onWillProcessDiff(diff => { for (const entry of diff) { - if (entry.op !== TestDiffOpType.Update || entry.item.docv === undefined || entry.item.item?.range === undefined) { + if (entry.op !== TestDiffOpType.DocumentSynced) { continue; } - const uri = this.testService.collection.getNodeById(entry.item.extId)?.item.uri; - const rec = uri && this.decorationCache.get(uri); + const rec = this.decorationCache.get(entry.uri); if (rec) { - rec.rangeUpdateVersionId = entry.item.docv; + rec.rangeUpdateVersionId = entry.docv; } } @@ -193,7 +192,7 @@ export class TestingDecorationService extends Disposable implements ITestingDeco const uriStr = model.uri.toString(); const cached = this.decorationCache.get(model.uri); const testRangesUpdated = cached?.rangeUpdateVersionId === model.getVersionId(); - const lastDecorations = cached?.value ?? new TestDecorations(); + const lastDecorations = cached?.value ?? new TestDecorations(); const newDecorations = new TestDecorations(); model.changeDecorations(accessor => { @@ -210,7 +209,7 @@ export class TestingDecorationService extends Disposable implements ITestingDeco for (const [line, tests] of runDecorations.lines()) { const multi = tests.length > 1; - let existing = lastDecorations.findOnLine(line, d => multi ? d instanceof MultiRunTestDecoration : d instanceof RunSingleTestDecoration) as RunTestDecoration | undefined; + let existing = lastDecorations.value.find(d => d instanceof RunTestDecoration && d.exactlyContainsTests(tests)) as RunTestDecoration | undefined; // see comment in the constructor for what's going on here if (existing && testRangesUpdated && model.getDecorationRange(existing.id)?.startLineNumber !== line) { @@ -652,6 +651,27 @@ abstract class RunTestDecoration { return true; } + public exactlyContainsTests(tests: readonly { test: IncrementalTestCollectionItem }[]): boolean { + if (tests.length !== this.tests.length) { + return false; + } + if (tests.length === 1) { + return tests[0].test.item.extId === this.tests[0].test.item.extId; + } + + const ownTests = new Set(); + for (const t of this.tests) { + ownTests.add(t.test.item.extId); + } + for (const t of tests) { + if (!ownTests.delete(t.test.item.extId)) { + return false; + } + } + + return true; + } + /** * Updates the decoration to match the new set of tests. * @returns true if options were changed, false otherwise diff --git a/src/vs/workbench/contrib/testing/common/testItemCollection.ts b/src/vs/workbench/contrib/testing/common/testItemCollection.ts index 4d55e567807..34ef310d046 100644 --- a/src/vs/workbench/contrib/testing/common/testItemCollection.ts +++ b/src/vs/workbench/contrib/testing/common/testItemCollection.ts @@ -33,6 +33,7 @@ export const enum TestItemEventOp { RemoveChild, SetProp, Bulk, + DocumentSynced, } export interface ITestItemUpsertChild { @@ -65,13 +66,18 @@ export interface ITestItemBulkReplace { ops: (ITestItemUpsertChild | ITestItemRemoveChild)[]; } +export interface ITestItemDocumentSynced { + op: TestItemEventOp.DocumentSynced; +} + export type ExtHostTestItemEvent = | ITestItemSetTags | ITestItemUpsertChild | ITestItemRemoveChild | ITestItemUpdateCanResolveChildren | ITestItemSetProp - | ITestItemBulkReplace; + | ITestItemBulkReplace + | ITestItemDocumentSynced; export interface ITestItemApi { controllerId: string; @@ -201,17 +207,32 @@ export class TestItemCollection extends Disposable { * Pushes a new diff entry onto the collected diff list. */ public pushDiff(diff: TestsDiffOp) { - // Try to merge updates, since they're invoked per-property - const last = this.diff[this.diff.length - 1]; - if (last && diff.op === TestDiffOpType.Update) { - if (last.op === TestDiffOpType.Update && last.item.extId === diff.item.extId) { - applyTestItemUpdate(last.item, diff.item); - return; - } + switch (diff.op) { + case TestDiffOpType.DocumentSynced: { + for (const existing of this.diff) { + if (existing.op === TestDiffOpType.DocumentSynced && existing.uri === diff.uri) { + existing.docv = diff.docv; + return; + } + } - if (last.op === TestDiffOpType.Add && last.item.item.extId === diff.item.extId) { - applyTestItemUpdate(last.item, diff.item); - return; + break; + } + case TestDiffOpType.Update: { + // Try to merge updates, since they're invoked per-property + const last = this.diff[this.diff.length - 1]; + if (last) { + if (last.op === TestDiffOpType.Update && last.item.extId === diff.item.extId) { + applyTestItemUpdate(last.item, diff.item); + return; + } + + if (last.op === TestDiffOpType.Add && last.item.item.extId === diff.item.extId) { + applyTestItemUpdate(last.item, diff.item); + return; + } + } + break; } } @@ -291,15 +312,29 @@ export class TestItemCollection extends Disposable { item: { extId: internal.fullId.toString(), item: evt.update, - docv: this.options.getDocumentVersion(internal.actual.uri), } }); break; + + case TestItemEventOp.DocumentSynced: + this.documentSynced(internal.actual.uri); + break; + default: assertNever(evt); } } + private documentSynced(uri: URI | undefined) { + if (uri) { + this.pushDiff({ + op: TestDiffOpType.DocumentSynced, + uri, + docv: this.options.getDocumentVersion(uri) + }); + } + } + private upsertItem(actual: T, parent: CollectionItem | undefined) { const fullId = TestId.fromExtHostTestItem(actual, this.root.id, parent?.actual); @@ -370,6 +405,9 @@ export class TestItemCollection extends Disposable { this.removeItem(TestId.joinToString(fullId, child.id)); } } + + // Mark ranges in the document as synced (#161320) + this.documentSynced(internal.actual.uri); } private diffTagRefs(newTags: readonly ITestTag[], oldTags: readonly ITestTag[], extId: string) { diff --git a/src/vs/workbench/contrib/testing/common/testTypes.ts b/src/vs/workbench/contrib/testing/common/testTypes.ts index 7edb36fb6e3..bf2ac6d4ba6 100644 --- a/src/vs/workbench/contrib/testing/common/testTypes.ts +++ b/src/vs/workbench/contrib/testing/common/testTypes.ts @@ -369,12 +369,6 @@ export interface ITestItemUpdate { extId: string; expand?: TestItemExpandState; item?: Partial; - - /** - * The document version at the time the operation was made, if the test has - * a URI and the document was open in the extension host. - */ - docv?: number; } export namespace ITestItemUpdate { @@ -382,7 +376,6 @@ export namespace ITestItemUpdate { extId: string; expand?: TestItemExpandState; item?: Partial; - docv?: number; } export const serialize = (u: ITestItemUpdate): Serialized => { @@ -399,7 +392,7 @@ export namespace ITestItemUpdate { if (u.item.sortText !== undefined) { item.sortText = u.item.sortText; } } - return { extId: u.extId, expand: u.expand, item, docv: u.docv }; + return { extId: u.extId, expand: u.expand, item }; }; export const deserialize = (u: Serialized): ITestItemUpdate => { @@ -415,7 +408,7 @@ export namespace ITestItemUpdate { if (u.item.sortText !== undefined) { item.sortText = u.item.sortText; } } - return { extId: u.extId, expand: u.expand, item, docv: u.docv }; + return { extId: u.extId, expand: u.expand, item }; }; } @@ -533,6 +526,8 @@ export const enum TestDiffOpType { Add, /** Shallow-updates an existing test */ Update, + /** Ranges of some tests in a document were synced, so it should be considered up-to-date */ + DocumentSynced, /** Removes a test (and all its children) */ Remove, /** Changes the number of controllers who are yet to publish their collection roots. */ @@ -552,7 +547,8 @@ export type TestsDiffOp = | { op: TestDiffOpType.Retire; itemId: string } | { op: TestDiffOpType.IncrementPendingExtHosts; amount: number } | { op: TestDiffOpType.AddTag; tag: ITestTagDisplayInfo } - | { op: TestDiffOpType.RemoveTag; id: string }; + | { op: TestDiffOpType.RemoveTag; id: string } + | { op: TestDiffOpType.DocumentSynced; uri: URI; docv?: number }; export namespace TestsDiffOp { export type Serialized = @@ -562,13 +558,16 @@ export namespace TestsDiffOp { | { op: TestDiffOpType.Retire; itemId: string } | { op: TestDiffOpType.IncrementPendingExtHosts; amount: number } | { op: TestDiffOpType.AddTag; tag: ITestTagDisplayInfo } - | { op: TestDiffOpType.RemoveTag; id: string }; + | { op: TestDiffOpType.RemoveTag; id: string } + | { op: TestDiffOpType.DocumentSynced; uri: UriComponents; docv?: number }; export const deserialize = (u: Serialized): TestsDiffOp => { if (u.op === TestDiffOpType.Add) { return { op: u.op, item: InternalTestItem.deserialize(u.item) }; } else if (u.op === TestDiffOpType.Update) { return { op: u.op, item: ITestItemUpdate.deserialize(u.item) }; + } else if (u.op === TestDiffOpType.DocumentSynced) { + return { op: u.op, uri: URI.revive(u.uri), docv: u.docv }; } else { return u; }