Skip to content

Commit e553c6b

Browse files
authored
testing: fix resolveHandler not called on item replacement (microsoft#209121)
Fixes microsoft#207574
1 parent 9d98da3 commit e553c6b

File tree

2 files changed

+53
-5
lines changed

2 files changed

+53
-5
lines changed

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

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import * as assert from 'assert';
77
import * as sinon from 'sinon';
8+
import { timeout } from 'vs/base/common/async';
89
import { VSBuffer } from 'vs/base/common/buffer';
910
import { CancellationTokenSource } from 'vs/base/common/cancellation';
1011
import { Event } from 'vs/base/common/event';
@@ -85,11 +86,14 @@ suite('ExtHost Testing', () => {
8586
const ds = ensureNoDisposablesAreLeakedInTestSuite();
8687

8788
let single: TestExtHostTestItemCollection;
89+
let resolveCalls: (string | undefined)[] = [];
8890
setup(() => {
91+
resolveCalls = [];
8992
single = ds.add(new TestExtHostTestItemCollection('ctrlId', 'root', {
9093
getDocument: () => undefined,
9194
} as Partial<ExtHostDocumentsAndEditors> as ExtHostDocumentsAndEditors));
9295
single.resolveHandler = item => {
96+
resolveCalls.push(item?.id);
9397
if (item === undefined) {
9498
const a = new TestItemImpl('ctrlId', 'id-a', 'a', URI.file('/'));
9599
a.canResolveChildren = true;
@@ -305,7 +309,7 @@ suite('ExtHost Testing', () => {
305309
assert.deepStrictEqual(single.collectDiff(), [
306310
{
307311
op: TestDiffOpType.Update,
308-
item: { extId: new TestId(['ctrlId', 'id-a']).toString(), expand: TestItemExpandState.Expanded, item: { label: 'Hello world' } },
312+
item: { extId: new TestId(['ctrlId', 'id-a']).toString(), item: { label: 'Hello world' } },
309313
},
310314
{
311315
op: TestDiffOpType.DocumentSynced,
@@ -326,6 +330,40 @@ suite('ExtHost Testing', () => {
326330
assert.deepStrictEqual(single.collectDiff(), []);
327331
});
328332

333+
suite('expandibility restoration', () => {
334+
const doReplace = async (canResolveChildren = true) => {
335+
const uri = single.root.children.get('id-a')!.uri;
336+
const newA = new TestItemImpl('ctrlId', 'id-a', 'Hello world', uri);
337+
newA.canResolveChildren = canResolveChildren;
338+
single.root.children.replace([
339+
newA,
340+
new TestItemImpl('ctrlId', 'id-b', single.root.children.get('id-b')!.label, uri),
341+
]);
342+
await timeout(0); // drain microtasks
343+
};
344+
345+
test('does not restore an unexpanded state', async () => {
346+
await single.expand(single.root.id, 0);
347+
assert.deepStrictEqual(resolveCalls, [undefined]);
348+
await doReplace();
349+
assert.deepStrictEqual(resolveCalls, [undefined]);
350+
});
351+
352+
test('restores resolve state on replacement', async () => {
353+
await single.expand(single.root.id, Infinity);
354+
assert.deepStrictEqual(resolveCalls, [undefined, 'id-a']);
355+
await doReplace();
356+
assert.deepStrictEqual(resolveCalls, [undefined, 'id-a', 'id-a']);
357+
});
358+
359+
test('does not expand if new child is not expandable', async () => {
360+
await single.expand(single.root.id, Infinity);
361+
assert.deepStrictEqual(resolveCalls, [undefined, 'id-a']);
362+
await doReplace(false);
363+
assert.deepStrictEqual(resolveCalls, [undefined, 'id-a']);
364+
});
365+
});
366+
329367
test('treats in-place replacement as mutation deeply', () => {
330368
single.expand(single.root.id, Infinity);
331369
single.collectDiff();
@@ -340,10 +378,6 @@ suite('ExtHost Testing', () => {
340378
single.root.children.replace([newA, single.root.children.get('id-b')!]);
341379

342380
assert.deepStrictEqual(single.collectDiff(), [
343-
{
344-
op: TestDiffOpType.Update,
345-
item: { extId: new TestId(['ctrlId', 'id-a']).toString(), expand: TestItemExpandState.Expanded },
346-
},
347381
{
348382
op: TestDiffOpType.Update,
349383
item: { extId: TestId.fromExtHostTestItem(oldAB, 'ctrlId').toString(), item: { label: 'Hello world' } },

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ export class TestItemCollection<T extends ITestItemLike> extends Disposable {
396396
this.options.getApiFor(oldActual).listener = undefined;
397397

398398
internal.actual = actual;
399+
internal.resolveBarrier = undefined;
399400
internal.expand = TestItemExpandState.NotExpandable; // updated by `connectItemAndChildren`
400401

401402
if (update) {
@@ -416,6 +417,19 @@ export class TestItemCollection<T extends ITestItemLike> extends Disposable {
416417
}
417418
}
418419

420+
// Re-expand the element if it was previous expanded (#207574)
421+
const expandLevels = internal.expandLevels;
422+
if (expandLevels !== undefined) {
423+
// Wait until a microtask to allow the extension to finish setting up
424+
// properties of the element and children before we ask it to expand.
425+
queueMicrotask(() => {
426+
if (internal.expand === TestItemExpandState.Expandable) {
427+
internal.expandLevels = undefined;
428+
this.expand(fullId.toString(), expandLevels);
429+
}
430+
});
431+
}
432+
419433
// Mark ranges in the document as synced (#161320)
420434
this.documentSynced(internal.actual.uri);
421435
}

0 commit comments

Comments
 (0)