Skip to content

Commit d83c814

Browse files
committed
replace items with new uris instead of diffing
1 parent 7ae2857 commit d83c814

File tree

2 files changed

+37
-7
lines changed

2 files changed

+37
-7
lines changed

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

+27
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,33 @@ suite('ExtHost Testing', () => {
251251
]);
252252
});
253253

254+
test('replaces on uri change', () => {
255+
single.expand(single.root.id, Infinity);
256+
single.collectDiff();
257+
258+
const oldA = single.root.children.get('id-a') as TestItemImpl;
259+
const uri = single.root.children.get('id-a')!.uri?.with({ path: '/different' });
260+
const newA = new TestItemImpl('ctrlId', 'id-a', 'Hello world', uri);
261+
newA.children.replace([...oldA.children].map(([_, item]) => item));
262+
single.root.children.replace([...single.root.children].map(([id, i]) => id === 'id-a' ? newA : i));
263+
264+
assert.deepStrictEqual(single.collectDiff(), [
265+
{ op: TestDiffOpType.Remove, itemId: new TestId(['ctrlId', 'id-a']).toString() },
266+
{
267+
op: TestDiffOpType.Add,
268+
item: { controllerId: 'ctrlId', expand: TestItemExpandState.NotExpandable, item: { ...convert.TestItem.from(newA) } }
269+
},
270+
{
271+
op: TestDiffOpType.Add,
272+
item: { controllerId: 'ctrlId', expand: TestItemExpandState.NotExpandable, item: convert.TestItem.from(newA.children.get('id-aa') as TestItemImpl) }
273+
},
274+
{
275+
op: TestDiffOpType.Add,
276+
item: { controllerId: 'ctrlId', expand: TestItemExpandState.NotExpandable, item: convert.TestItem.from(newA.children.get('id-ab') as TestItemImpl) }
277+
},
278+
]);
279+
});
280+
254281
test('treats in-place replacement as mutation', () => {
255282
single.expand(single.root.id, Infinity);
256283
single.collectDiff();

src/vs/workbench/contrib/testing/common/testItemCollection.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,6 @@ const diffableProps: { [K in keyof ITestItem]?: (a: ITestItem[K], b: ITestItem[K
116116
description: strictEqualComparator,
117117
error: strictEqualComparator,
118118
sortText: strictEqualComparator,
119-
uri: (a, b) => {
120-
if (a === b) { return true; }
121-
if (!a || !b) { return false; }
122-
return a.toString() === b.toString();
123-
},
124119
tags: (a, b) => {
125120
if (a.length !== b.length) {
126121
return false;
@@ -134,9 +129,11 @@ const diffableProps: { [K in keyof ITestItem]?: (a: ITestItem[K], b: ITestItem[K
134129
},
135130
};
136131

132+
const diffableEntries = Object.entries(diffableProps) as readonly [keyof ITestItem, (a: any, b: any) => boolean][];
133+
137134
const diffTestItems = (a: ITestItem, b: ITestItem) => {
138135
let output: Record<string, unknown> | undefined;
139-
for (const [key, cmp] of Object.entries(diffableProps) as [keyof ITestItem, (a: any, b: any) => boolean][]) {
136+
for (const [key, cmp] of diffableEntries) {
140137
if (!cmp(a[key], b[key])) {
141138
if (output) {
142139
output[key] = b[key];
@@ -340,7 +337,7 @@ export class TestItemCollection<T extends ITestItemLike> extends Disposable {
340337
}
341338
}
342339

343-
private upsertItem(actual: T, parent: CollectionItem<T> | undefined) {
340+
private upsertItem(actual: T, parent: CollectionItem<T> | undefined): void {
344341
const fullId = TestId.fromExtHostTestItem(actual, this.root.id, parent?.actual);
345342

346343
// If this test item exists elsewhere in the tree already (exists at an
@@ -383,6 +380,12 @@ export class TestItemCollection<T extends ITestItemLike> extends Disposable {
383380
}
384381

385382
// Case 3: upsert of an existing item by ID, with a new instance
383+
if (internal.actual.uri?.toString() !== actual.uri?.toString()) {
384+
// If the item has a new URI, re-insert it; we don't support updating
385+
// URIs on existing test items.
386+
this.removeItem(fullId.toString());
387+
return this.upsertItem(actual, parent);
388+
}
386389
const oldChildren = this.options.getChildren(internal.actual);
387390
const oldActual = internal.actual;
388391
const update = diffTestItems(this.options.toITestItem(oldActual), this.options.toITestItem(actual));

0 commit comments

Comments
 (0)