Skip to content

Commit 4de3a2c

Browse files
authored
debt: adopt ensureNoDisposablesAreLeakedInTestSuite (#192226)
Finishes adoption in test I own
1 parent e1b3512 commit 4de3a2c

File tree

11 files changed

+125
-85
lines changed

11 files changed

+125
-85
lines changed

src/vs/workbench/api/common/extHostTesting.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,11 @@ class TestRunTracker extends Disposable {
606606

607607
this.proxy.$addTestsToRun(this.dto.controllerId, this.dto.id, chain);
608608
}
609+
610+
public override dispose(): void {
611+
this.markEnded();
612+
super.dispose();
613+
}
609614
}
610615

611616
/**
@@ -676,7 +681,7 @@ export class TestRunCoordinator {
676681
});
677682

678683
const tracker = this.getTracker(request, dto, extension);
679-
tracker.onEnd(() => {
684+
Event.once(tracker.onEnd)(() => {
680685
this.proxy.$finishedExtensionTestRun(dto.id);
681686
tracker.dispose();
682687
});
@@ -687,7 +692,7 @@ export class TestRunCoordinator {
687692
private getTracker(req: vscode.TestRunRequest, dto: TestRunDto, extension: IRelaxedExtensionDescription, token?: CancellationToken) {
688693
const tracker = new TestRunTracker(dto, this.proxy, extension, token);
689694
this.tracked.set(req, tracker);
690-
tracker.onEnd(() => this.tracked.delete(req));
695+
Event.once(tracker.onEnd)(() => this.tracked.delete(req));
691696
return tracker;
692697
}
693698
}

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

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { CancellationTokenSource } from 'vs/base/common/cancellation';
1010
import { Iterable } from 'vs/base/common/iterator';
1111
import { URI } from 'vs/base/common/uri';
1212
import { mockObject, MockObject } from 'vs/base/test/common/mock';
13+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
1314
import * as editorRange from 'vs/editor/common/core/range';
1415
import { IRelaxedExtensionDescription } from 'vs/platform/extensions/common/extensions';
1516
import { MainThreadTestingShape } from 'vs/workbench/api/common/extHost.protocol';
@@ -71,11 +72,17 @@ suite('ExtHost Testing', () => {
7172
}
7273
}
7374

75+
teardown(() => {
76+
sinon.restore();
77+
});
78+
79+
const ds = ensureNoDisposablesAreLeakedInTestSuite();
80+
7481
let single: TestExtHostTestItemCollection;
7582
setup(() => {
76-
single = new TestExtHostTestItemCollection('ctrlId', 'root', {
83+
single = ds.add(new TestExtHostTestItemCollection('ctrlId', 'root', {
7784
getDocument: () => undefined,
78-
} as Partial<ExtHostDocumentsAndEditors> as ExtHostDocumentsAndEditors);
85+
} as Partial<ExtHostDocumentsAndEditors> as ExtHostDocumentsAndEditors));
7986
single.resolveHandler = item => {
8087
if (item === undefined) {
8188
const a = new TestItemImpl('ctrlId', 'id-a', 'a', URI.file('/'));
@@ -89,12 +96,7 @@ suite('ExtHost Testing', () => {
8996
}
9097
};
9198

92-
single.onDidGenerateDiff(d => single.setDiff(d /* don't clear during testing */));
93-
});
94-
95-
teardown(() => {
96-
single.dispose();
97-
sinon.restore();
99+
ds.add(single.onDidGenerateDiff(d => single.setDiff(d /* don't clear during testing */)));
98100
});
99101

100102
suite('OwnedTestCollection', () => {
@@ -623,7 +625,7 @@ suite('ExtHost Testing', () => {
623625
});
624626

625627
test('tracks a run started from a main thread request', () => {
626-
const tracker = c.prepareForMainThreadTestRun(req, dto, ext, cts.token);
628+
const tracker = ds.add(c.prepareForMainThreadTestRun(req, dto, ext, cts.token));
627629
assert.strictEqual(tracker.hasRunningTasks, false);
628630

629631
const task1 = c.createTestRun(ext, 'ctrl', single, req, 'run1', true);
@@ -648,10 +650,10 @@ suite('ExtHost Testing', () => {
648650
test('run cancel force ends after a timeout', () => {
649651
const clock = sinon.useFakeTimers();
650652
try {
651-
const tracker = c.prepareForMainThreadTestRun(req, dto, ext, cts.token);
653+
const tracker = ds.add(c.prepareForMainThreadTestRun(req, dto, ext, cts.token));
652654
const task = c.createTestRun(ext, 'ctrl', single, req, 'run1', true);
653655
const onEnded = sinon.stub();
654-
tracker.onEnd(onEnded);
656+
ds.add(tracker.onEnd(onEnded));
655657

656658
assert.strictEqual(task.token.isCancellationRequested, false);
657659
assert.strictEqual(tracker.hasRunningTasks, true);
@@ -673,10 +675,10 @@ suite('ExtHost Testing', () => {
673675
});
674676

675677
test('run cancel force ends on second cancellation request', () => {
676-
const tracker = c.prepareForMainThreadTestRun(req, dto, ext, cts.token);
678+
const tracker = ds.add(c.prepareForMainThreadTestRun(req, dto, ext, cts.token));
677679
const task = c.createTestRun(ext, 'ctrl', single, req, 'run1', true);
678680
const onEnded = sinon.stub();
679-
tracker.onEnd(onEnded);
681+
ds.add(tracker.onEnd(onEnded));
680682

681683
assert.strictEqual(task.token.isCancellationRequested, false);
682684
assert.strictEqual(tracker.hasRunningTasks, true);
@@ -753,6 +755,8 @@ suite('ExtHost Testing', () => {
753755

754756
task1.passed(single.root.children.get('id-a')!.children.get('id-ab')!);
755757
assert.deepStrictEqual(proxy.$addTestsToRun.args, expectedArgs);
758+
759+
task1.end();
756760
});
757761

758762
test('adds test messages to run', () => {
@@ -796,6 +800,8 @@ suite('ExtHost Testing', () => {
796800
location: convert.location.from({ uri: test2.uri!, range: test2.range! }),
797801
}]
798802
]);
803+
804+
task.end();
799805
});
800806

801807
test('guards calls after runs are ended', () => {
@@ -829,6 +835,7 @@ suite('ExtHost Testing', () => {
829835
TestResultState.Passed,
830836
undefined,
831837
]]);
838+
task.end();
832839
});
833840

834841
test('sets state of test with identical local IDs (#131827)', () => {
@@ -856,6 +863,8 @@ suite('ExtHost Testing', () => {
856863
[single.root, testB, childB].map(t => convert.TestItem.from(t as TestItemImpl)),
857864
],
858865
]);
866+
867+
task1.end();
859868
});
860869
});
861870
});

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,15 @@ import { Emitter } from 'vs/base/common/event';
1010
import { DisposableStore } from 'vs/base/common/lifecycle';
1111
import { SocketCloseEvent } from 'vs/base/parts/ipc/common/ipc.net';
1212
import { mock } from 'vs/base/test/common/mock';
13+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
1314
import { RemoteSocketHalf } from 'vs/platform/remote/common/managedSocket';
1415
import { MainThreadManagedSocket } from 'vs/workbench/api/browser/mainThreadManagedSockets';
1516
import { ExtHostManagedSocketsShape } from 'vs/workbench/api/common/extHost.protocol';
1617

1718
suite('MainThreadManagedSockets', () => {
1819

20+
const ds = ensureNoDisposablesAreLeakedInTestSuite();
21+
1922
suite('ManagedSocket', () => {
2023
let extHost: ExtHostMock;
2124
let half: RemoteSocketHalf;
@@ -72,7 +75,7 @@ suite('MainThreadManagedSockets', () => {
7275
const socket = MainThreadManagedSocket.connect(1, extHost, '/hello', 'world=true', '', half);
7376
await extHost.expectEvent(evt => evt.data && evt.data.startsWith('GET ws://localhost/hello?world=true&skipWebSocketFrames=true HTTP/1.1\r\nConnection: Upgrade\r\nUpgrade: websocket\r\nSec-WebSocket-Key:'), 'websocket open event');
7477
half.onData.fire(VSBuffer.fromString('Opened successfully ;)\r\n\r\n'));
75-
return await socket;
78+
return ds.add(await socket);
7679
}
7780

7881
test('connects', async () => {
@@ -83,18 +86,18 @@ suite('MainThreadManagedSockets', () => {
8386
const socketProm = MainThreadManagedSocket.connect(1, extHost, '/hello', 'world=true', '', half);
8487
await extHost.expectEvent(evt => evt.data && evt.data.includes('GET ws://localhost'), 'websocket open event');
8588
half.onData.fire(VSBuffer.fromString('Opened successfully ;)\r\n\r\nSome trailing data'));
86-
const socket = await socketProm;
89+
const socket = ds.add(await socketProm);
8790

8891
const data: string[] = [];
89-
socket.onData(d => data.push(d.toString()));
92+
ds.add(socket.onData(d => data.push(d.toString())));
9093
await timeout(1); // allow microtasks to flush
9194
assert.deepStrictEqual(data, ['Some trailing data']);
9295
});
9396

9497
test('round trips data', async () => {
9598
const socket = await doConnect();
9699
const data: string[] = [];
97-
socket.onData(d => data.push(d.toString()));
100+
ds.add(socket.onData(d => data.push(d.toString())));
98101

99102
socket.write(VSBuffer.fromString('ping'));
100103
await extHost.expectEvent(evt => evt.data === 'ping', 'expected ping');

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as assert from 'assert';
7-
import { DisposableStore } from 'vs/base/common/lifecycle';
87
import { mock } from 'vs/base/test/common/mock';
8+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
99
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
1010
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
1111
import { NullLogService } from 'vs/platform/log/common/log';
@@ -45,25 +45,29 @@ suite('MainThreadHostTreeView', function () {
4545
let container: ViewContainer;
4646
let mainThreadTreeViews: MainThreadTreeViews;
4747
let extHostTreeViewsShape: MockExtHostTreeViewsShape;
48-
let disposables: DisposableStore;
48+
49+
teardown(() => {
50+
ViewsRegistry.deregisterViews(ViewsRegistry.getViews(container), container);
51+
});
52+
53+
const disposables = ensureNoDisposablesAreLeakedInTestSuite();
4954

5055
setup(async () => {
51-
disposables = new DisposableStore();
5256
const instantiationService: TestInstantiationService = <TestInstantiationService>workbenchInstantiationService(undefined, disposables);
53-
const viewDescriptorService = instantiationService.createInstance(ViewDescriptorService);
57+
const viewDescriptorService = disposables.add(instantiationService.createInstance(ViewDescriptorService));
5458
instantiationService.stub(IViewDescriptorService, viewDescriptorService);
5559
container = Registry.as<IViewContainersRegistry>(Extensions.ViewContainersRegistry).registerViewContainer({ id: 'testContainer', title: { value: 'test', original: 'test' }, ctorDescriptor: new SyncDescriptor(<any>{}) }, ViewContainerLocation.Sidebar);
5660
const viewDescriptor: ITreeViewDescriptor = {
5761
id: testTreeViewId,
5862
ctorDescriptor: null!,
5963
name: 'Test View 1',
60-
treeView: instantiationService.createInstance(CustomTreeView, 'testTree', 'Test Title', 'extension.id'),
64+
treeView: disposables.add(instantiationService.createInstance(CustomTreeView, 'testTree', 'Test Title', 'extension.id')),
6165
};
6266
ViewsRegistry.registerViews([viewDescriptor], container);
6367

6468
const testExtensionService = new TestExtensionService();
6569
extHostTreeViewsShape = new MockExtHostTreeViewsShape();
66-
mainThreadTreeViews = new MainThreadTreeViews(
70+
mainThreadTreeViews = disposables.add(new MainThreadTreeViews(
6771
new class implements IExtHostContext {
6872
remoteAuthority = '';
6973
extensionHostKind = ExtensionHostKind.LocalProcess;
@@ -74,16 +78,11 @@ suite('MainThreadHostTreeView', function () {
7478
return extHostTreeViewsShape;
7579
}
7680
drain(): any { return null; }
77-
}, new TestViewsService(), new TestNotificationService(), testExtensionService, new NullLogService());
81+
}, new TestViewsService(), new TestNotificationService(), testExtensionService, new NullLogService()));
7882
mainThreadTreeViews.$registerTreeViewDataProvider(testTreeViewId, { showCollapseAll: false, canSelectMany: false, dropMimeTypes: [], dragMimeTypes: [], hasHandleDrag: false, hasHandleDrop: false, manuallyManageCheckboxes: false });
7983
await testExtensionService.whenInstalledExtensionsRegistered();
8084
});
8185

82-
teardown(() => {
83-
ViewsRegistry.deregisterViews(ViewsRegistry.getViews(container), container);
84-
disposables.dispose();
85-
});
86-
8786
test('getChildren keeps custom properties', async () => {
8887
const treeView: ITreeView = (<ITreeViewDescriptor>ViewsRegistry.getView(testTreeViewId)).treeView;
8988
const children = await treeView.dataProvider?.getChildren({ handle: 'root', collapsibleState: TreeItemCollapsibleState.Expanded });

src/vs/workbench/contrib/notebook/test/browser/notebookRendererMessagingService.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,20 @@ import { stub } from 'sinon';
88
import { NotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/browser/services/notebookRendererMessagingServiceImpl';
99
import * as assert from 'assert';
1010
import { timeout } from 'vs/base/common/async';
11+
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
1112

1213
suite('NotebookRendererMessaging', () => {
1314
let extService: NullExtensionService;
1415
let m: NotebookRendererMessagingService;
1516
let sent: unknown[] = [];
1617

18+
const ds = ensureNoDisposablesAreLeakedInTestSuite();
19+
1720
setup(() => {
1821
sent = [];
1922
extService = new NullExtensionService();
20-
m = new NotebookRendererMessagingService(extService);
21-
m.onShouldPostMessage(e => sent.push(e));
23+
m = ds.add(new NotebookRendererMessagingService(extService));
24+
ds.add(m.onShouldPostMessage(e => sent.push(e)));
2225
});
2326

2427
test('activates on prepare', () => {

0 commit comments

Comments
 (0)