Merge pull request microsoft#209490 from microsoft/tyriar/test_leak

Clean up some tests, fix some leaks

Author: Tyriar

src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts

@@ -75,7 +75,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 	userHome: string | undefined;
 	environmentVariableInfo: IEnvironmentVariableInfo | undefined;
 	backend: ITerminalBackend | undefined;
-	readonly capabilities = new TerminalCapabilityStore();
+	readonly capabilities = this._register(new TerminalCapabilityStore());
 	readonly shellIntegrationNonce: string;
 
 	private _isDisposed: boolean = false;
@@ -154,8 +154,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 		this._cwdWorkspaceFolder = terminalEnvironment.getWorkspaceForTerminal(cwd, this._workspaceContextService, this._historyService);
 		this.ptyProcessReady = this._createPtyProcessReadyPromise();
 		this._ackDataBufferer = new AckDataBufferer(e => this._process?.acknowledgeDataEvent(e));
-		this._dataFilter = this._instantiationService.createInstance(SeamlessRelaunchDataFilter);
-		this._dataFilter.onProcessData(ev => {
+		this._dataFilter = this._register(this._instantiationService.createInstance(SeamlessRelaunchDataFilter));
+		this._register(this._dataFilter.onProcessData(ev => {
 			const data = (typeof ev === 'string' ? ev : ev.data);
 			const beforeProcessDataEvent: IBeforeProcessDataEvent = { data };
 			this._onBeforeProcessData.fire(beforeProcessDataEvent);
@@ -166,7 +166,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 				}
 				this._onProcessData.fire(typeof ev !== 'string' ? ev : { data: beforeProcessDataEvent.data, trackCommit: false });
 			}
-		});
+		}));
 
 		if (cwd && typeof cwd === 'object') {
 			this.remoteAuthority = getRemoteAuthority(cwd);
@@ -208,12 +208,14 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 	}
 
 	private _createPtyProcessReadyPromise(): Promise<void> {
+
 		return new Promise<void>(c => {
-			const listener = this.onProcessReady(() => {
+			const listener = Event.once(this.onProcessReady)(() => {
 				this._logService.debug(`Terminal process ready (shellProcessId: ${this.shellProcessId})`);
-				listener.dispose();
+				this._store.delete(listener);
 				c(undefined);
 			});
+			this._store.add(listener);
 		});
 	}
 

src/vs/workbench/contrib/terminal/test/browser/terminalInstanceService.test.ts

@@ -18,29 +18,17 @@ import { TestEnvironmentService } from 'vs/workbench/test/browser/workbenchTestS
 import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 
 suite('Workbench - TerminalInstanceService', () => {
-	let instantiationService: TestInstantiationService;
 	let terminalInstanceService: ITerminalInstanceService;
 
 	setup(async () => {
-		instantiationService = new TestInstantiationService();
-		// TODO: Should be able to create these services without this config set
-		instantiationService.stub(IConfigurationService, new TestConfigurationService({
-			terminal: {
-				integrated: {
-					fontWeight: 'normal'
-				}
-			}
-		}));
+		const instantiationService = new TestInstantiationService();
+		instantiationService.stub(IConfigurationService, new TestConfigurationService());
 		instantiationService.stub(IContextKeyService, instantiationService.createInstance(ContextKeyService));
 		instantiationService.stub(IWorkbenchEnvironmentService, TestEnvironmentService);
 
 		terminalInstanceService = instantiationService.createInstance(TerminalInstanceService);
 	});
 
-	teardown(() => {
-		instantiationService.dispose();
-	});
-
 	ensureNoDisposablesAreLeakedInTestSuite();
 
 	suite('convertProfileToShellLaunchConfig', () => {

src/vs/workbench/contrib/terminal/test/browser/terminalProcessManager.test.ts

@@ -4,24 +4,23 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { strictEqual } from 'assert';
+import { Event } from 'vs/base/common/event';
+import { Schemas } from 'vs/base/common/network';
+import { URI } from 'vs/base/common/uri';
+import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/browser/terminalProcessManager';
 import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
-import { ITestInstantiationService, TestTerminalProfileResolverService, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
+import { NullLogService } from 'vs/platform/log/common/log';
 import { IProductService } from 'vs/platform/product/common/productService';
+import { ITerminalChildProcess, ITerminalLogService } from 'vs/platform/terminal/common/terminal';
+import { ITerminalConfigurationService, ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal';
+import { TerminalConfigurationService } from 'vs/workbench/contrib/terminal/browser/terminalConfigurationService';
+import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/browser/terminalProcessManager';
 import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 import { EnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariableService';
-import { Schemas } from 'vs/base/common/network';
-import { URI } from 'vs/base/common/uri';
-import { ITerminalChildProcess, ITerminalLogService } from 'vs/platform/terminal/common/terminal';
 import { ITerminalProfileResolverService } from 'vs/workbench/contrib/terminal/common/terminal';
-import { ITerminalConfigurationService, ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal';
-import { DisposableStore } from 'vs/base/common/lifecycle';
-import { Event } from 'vs/base/common/event';
+import { TestTerminalProfileResolverService, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
 import { TestProductService } from 'vs/workbench/test/common/workbenchTestServices';
-import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
-import { NullLogService } from 'vs/platform/log/common/log';
-import { TerminalConfigurationService } from 'vs/workbench/contrib/terminal/browser/terminalConfigurationService';
 
 class TestTerminalChildProcess implements ITerminalChildProcess {
 	id: number = 0;
@@ -82,13 +81,12 @@ class TestTerminalInstanceService implements Partial<ITerminalInstanceService> {
 }
 
 suite('Workbench - TerminalProcessManager', () => {
-	let store: DisposableStore;
-	let instantiationService: ITestInstantiationService;
 	let manager: TerminalProcessManager;
 
+	const store = ensureNoDisposablesAreLeakedInTestSuite();
+
 	setup(async () => {
-		store = new DisposableStore();
-		instantiationService = workbenchInstantiationService(undefined, store);
+		const instantiationService = workbenchInstantiationService(undefined, store);
 		const configurationService = new TestConfigurationService();
 		await configurationService.setUserConfiguration('editor', { fontFamily: 'foo' });
 		await configurationService.setUserConfiguration('terminal', {
@@ -101,20 +99,16 @@ suite('Workbench - TerminalProcessManager', () => {
 			}
 		});
 		instantiationService.stub(IConfigurationService, configurationService);
-		instantiationService.stub(ITerminalConfigurationService, instantiationService.createInstance(TerminalConfigurationService));
+		instantiationService.stub(ITerminalConfigurationService, store.add(instantiationService.createInstance(TerminalConfigurationService)));
 		instantiationService.stub(IProductService, TestProductService);
 		instantiationService.stub(ITerminalLogService, new NullLogService());
-		instantiationService.stub(IEnvironmentVariableService, instantiationService.createInstance(EnvironmentVariableService));
+		instantiationService.stub(IEnvironmentVariableService, store.add(instantiationService.createInstance(EnvironmentVariableService)));
 		instantiationService.stub(ITerminalProfileResolverService, TestTerminalProfileResolverService);
 		instantiationService.stub(ITerminalInstanceService, new TestTerminalInstanceService());
 
 		manager = store.add(instantiationService.createInstance(TerminalProcessManager, 1, undefined, undefined, undefined));
 	});
 
-	teardown(() => store.dispose());
-
-	ensureNoDisposablesAreLeakedInTestSuite();
-
 	suite('process persistence', () => {
 		suite('local', () => {
 			test('regular terminal should persist', async () => {