From 8627ee4504c5922277a82656bb0704a97a315832 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Fri, 8 Sep 2023 19:16:20 +0200 Subject: [PATCH] preps for ensureNoDisposablesAreLeakedInTestSuite (#192603) #190503 preps for ensureNoDisposablesAreLeakedInTestSuite --- ...ensionRecommendationNotificationService.ts | 33 +++++---- .../extensionRecommendationsService.ts | 11 ++- .../extensionRecommendationsService.test.ts | 71 +++++++++---------- 3 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/vs/workbench/contrib/extensions/browser/extensionRecommendationNotificationService.ts b/src/vs/workbench/contrib/extensions/browser/extensionRecommendationNotificationService.ts index 4b63da8f0c2..7aa8ac10873 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionRecommendationNotificationService.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionRecommendationNotificationService.ts @@ -9,7 +9,7 @@ import { CancelablePromise, createCancelablePromise, Promises, raceCancellablePr import { CancellationToken } from 'vs/base/common/cancellation'; import { isCancellationError } from 'vs/base/common/errors'; import { Emitter, Event } from 'vs/base/common/event'; -import { DisposableStore, isDisposable, MutableDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, isDisposable, MutableDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { areSameExtensions } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; @@ -50,12 +50,12 @@ type RecommendationsNotificationActions = { onDidNeverShowRecommendedExtensionsAgain(extensions: IExtension[]): void; }; -class RecommendationsNotification { +class RecommendationsNotification extends Disposable { - private _onDidClose = new Emitter(); + private _onDidClose = this._register(new Emitter()); readonly onDidClose = this._onDidClose.event; - private _onDidChangeVisibility = new Emitter(); + private _onDidChangeVisibility = this._register(new Emitter()); readonly onDidChangeVisibility = this._onDidChangeVisibility.event; private notificationHandle: INotificationHandle | undefined; @@ -66,7 +66,9 @@ class RecommendationsNotification { private readonly message: string, private readonly choices: IPromptChoice[], private readonly notificationService: INotificationService - ) { } + ) { + super(); + } show(): void { if (!this.notificationHandle) { @@ -87,8 +89,8 @@ class RecommendationsNotification { return this.cancelled; } - private onDidCloseDisposable = new MutableDisposable(); - private onDidChangeVisibilityDisposable = new MutableDisposable(); + private onDidCloseDisposable = this._register(new MutableDisposable()); + private onDidChangeVisibilityDisposable = this._register(new MutableDisposable()); private updateNotificationHandle(notificationHandle: INotificationHandle) { this.onDidCloseDisposable.clear(); this.onDidChangeVisibilityDisposable.clear(); @@ -110,7 +112,7 @@ class RecommendationsNotification { type PendingRecommendationsNotification = { recommendationsNotification: RecommendationsNotification; source: RecommendationSource; token: CancellationToken }; type VisibleRecommendationsNotification = { recommendationsNotification: RecommendationsNotification; source: RecommendationSource; from: number }; -export class ExtensionRecommendationNotificationService implements IExtensionRecommendationNotificationService { +export class ExtensionRecommendationNotificationService extends Disposable implements IExtensionRecommendationNotificationService { declare readonly _serviceBrand: undefined; @@ -138,7 +140,9 @@ export class ExtensionRecommendationNotificationService implements IExtensionRec @IExtensionIgnoredRecommendationsService private readonly extensionIgnoredRecommendationsService: IExtensionIgnoredRecommendationsService, @IUserDataSyncEnablementService private readonly userDataSyncEnablementService: IUserDataSyncEnablementService, @IWorkbenchEnvironmentService private readonly workbenchEnvironmentService: IWorkbenchEnvironmentService, - ) { } + ) { + super(); + } hasToIgnoreRecommendationNotifications(): boolean { const config = this.configurationService.getValue<{ ignoreRecommendations: boolean; showRecommendationsOnlyOnDemand?: boolean }>('extensions'); @@ -255,8 +259,8 @@ export class ExtensionRecommendationNotificationService implements IExtensionRec } return raceCancellablePromises([ - this.showRecommendationsNotification(extensions, message, searchValue, source, recommendationsNotificationActions), - this.waitUntilRecommendationsAreInstalled(extensions) + this._registerP(this.showRecommendationsNotification(extensions, message, searchValue, source, recommendationsNotificationActions)), + this._registerP(this.waitUntilRecommendationsAreInstalled(extensions)) ]); } @@ -344,7 +348,7 @@ export class ExtensionRecommendationNotificationService implements IExtensionRec private async doShowRecommendationsNotification(severity: Severity, message: string, choices: IPromptChoice[], source: RecommendationSource, token: CancellationToken): Promise { const disposables = new DisposableStore(); try { - const recommendationsNotification = new RecommendationsNotification(severity, message, choices, this.notificationService); + const recommendationsNotification = disposables.add(new RecommendationsNotification(severity, message, choices, this.notificationService)); disposables.add(Event.once(Event.filter(recommendationsNotification.onDidChangeVisibility, e => !e))(() => this.showNextNotification())); if (this.visibleNotification) { const index = this.pendingNotificaitons.length; @@ -442,4 +446,9 @@ export class ExtensionRecommendationNotificationService implements IExtensionRec private setIgnoreRecommendationsConfig(configVal: boolean) { this.configurationService.updateValue('extensions.ignoreRecommendations', configVal); } + + private _registerP(o: CancelablePromise): CancelablePromise { + this._register(toDisposable(() => o.cancel())); + return o; + } } diff --git a/src/vs/workbench/contrib/extensions/browser/extensionRecommendationsService.ts b/src/vs/workbench/contrib/extensions/browser/extensionRecommendationsService.ts index b13f1894ee4..a6806b20159 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionRecommendationsService.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionRecommendationsService.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, toDisposable } from 'vs/base/common/lifecycle'; import { IExtensionManagementService, IExtensionGalleryService, InstallOperation, InstallExtensionResult } from 'vs/platform/extensionManagement/common/extensionManagement'; import { IExtensionRecommendationsService, ExtensionRecommendationReason, IExtensionIgnoredRecommendationsService } from 'vs/workbench/services/extensionRecommendations/common/extensionRecommendations'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -20,7 +20,7 @@ import { LanguageRecommendations } from 'vs/workbench/contrib/extensions/browser import { ExtensionRecommendation } from 'vs/workbench/contrib/extensions/browser/extensionRecommendations'; import { ConfigBasedRecommendations } from 'vs/workbench/contrib/extensions/browser/configBasedRecommendations'; import { IExtensionRecommendationNotificationService } from 'vs/platform/extensionRecommendations/common/extensionRecommendations'; -import { timeout } from 'vs/base/common/async'; +import { CancelablePromise, timeout } from 'vs/base/common/async'; import { URI } from 'vs/base/common/uri'; import { WebRecommendations } from 'vs/workbench/contrib/extensions/browser/webRecommendations'; import { IExtensionsWorkbenchService } from 'vs/workbench/contrib/extensions/common/extensions'; @@ -276,8 +276,13 @@ export class ExtensionRecommendationsService extends Disposable implements IExte .filter(extensionId => this.isExtensionAllowedToBeRecommended(extensionId)); if (allowedRecommendations.length) { - await timeout(5000); + await this._registerP(timeout(5000)); await this.extensionRecommendationNotificationService.promptWorkspaceRecommendations(allowedRecommendations); } } + + private _registerP(o: CancelablePromise): CancelablePromise { + this._register(toDisposable(() => o.cancel())); + return o; + } } diff --git a/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionRecommendationsService.test.ts b/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionRecommendationsService.test.ts index d19b0fedbdd..82b0f4d4298 100644 --- a/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionRecommendationsService.test.ts +++ b/src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionRecommendationsService.test.ts @@ -61,6 +61,7 @@ import { VSBuffer } from 'vs/base/common/buffer'; import { platform } from 'vs/base/common/platform'; import { arch } from 'vs/base/common/process'; import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; +import { DisposableStore } from 'vs/base/common/lifecycle'; const mockExtensionGallery: IGalleryExtension[] = [ aGalleryExtension('MockExtension1', { @@ -180,6 +181,7 @@ function aGalleryExtension(name: string, properties: any = {}, galleryExtensionP } suite('ExtensionRecommendationsService Test', () => { + const disposableStore = new DisposableStore(); let workspaceService: IWorkspaceContextService; let instantiationService: TestInstantiationService; let testConfigurationService: TestConfigurationService; @@ -189,18 +191,19 @@ suite('ExtensionRecommendationsService Test', () => { uninstallEvent: Emitter, didUninstallEvent: Emitter; let prompted: boolean; - const promptedEmitter = new Emitter(); + let promptedEmitter: Emitter; let onModelAddedEvent: Emitter; - suiteSetup(() => { - instantiationService = new TestInstantiationService(); - installEvent = new Emitter(); - didInstallEvent = new Emitter(); - uninstallEvent = new Emitter(); - didUninstallEvent = new Emitter(); + setup(() => { + instantiationService = disposableStore.add(new TestInstantiationService()); + promptedEmitter = disposableStore.add(new Emitter()); + installEvent = disposableStore.add(new Emitter()); + didInstallEvent = disposableStore.add(new Emitter()); + uninstallEvent = disposableStore.add(new Emitter()); + didUninstallEvent = disposableStore.add(new Emitter()); instantiationService.stub(IExtensionGalleryService, ExtensionGalleryService); instantiationService.stub(ISharedProcessService, TestSharedProcessService); - instantiationService.stub(ILifecycleService, new TestLifecycleService()); + instantiationService.stub(ILifecycleService, disposableStore.add(new TestLifecycleService())); testConfigurationService = new TestConfigurationService(); instantiationService.stub(IConfigurationService, testConfigurationService); instantiationService.stub(INotificationService, new TestNotificationService()); @@ -222,11 +225,11 @@ suite('ExtensionRecommendationsService Test', () => { extensions: [], async whenInstalledExtensionsRegistered() { return true; } }); - instantiationService.stub(IWorkbenchExtensionEnablementService, new TestExtensionEnablementService(instantiationService)); + instantiationService.stub(IWorkbenchExtensionEnablementService, disposableStore.add(new TestExtensionEnablementService(instantiationService))); instantiationService.stub(ITelemetryService, NullTelemetryService); instantiationService.stub(IURLService, NativeURLService); instantiationService.stub(IWorkspaceTagsService, new NoOpWorkspaceTagsService()); - instantiationService.stub(IStorageService, new TestStorageService()); + instantiationService.stub(IStorageService, disposableStore.add(new TestStorageService())); instantiationService.stub(ILogService, new NullLogService()); instantiationService.stub(IProductService, >{ extensionTips: { @@ -275,13 +278,11 @@ suite('ExtensionRecommendationsService Test', () => { }, }); - instantiationService.set(IExtensionsWorkbenchService, instantiationService.createInstance(ExtensionsWorkbenchService)); - instantiationService.stub(IExtensionTipsService, instantiationService.createInstance(TestExtensionTipsService)); + instantiationService.set(IExtensionsWorkbenchService, disposableStore.add(instantiationService.createInstance(ExtensionsWorkbenchService))); + instantiationService.stub(IExtensionTipsService, disposableStore.add(instantiationService.createInstance(TestExtensionTipsService))); onModelAddedEvent = new Emitter(); - }); - setup(() => { instantiationService.stub(IEnvironmentService, >{}); instantiationService.stubPromise(IExtensionManagementService, 'getInstalled', []); instantiationService.stub(IExtensionGalleryService, 'isEnabled', true); @@ -307,11 +308,7 @@ suite('ExtensionRecommendationsService Test', () => { }); }); - teardown(() => (testObject).dispose()); - - suiteTeardown(() => { - instantiationService.dispose(); - }); + teardown(() => disposableStore.clear()); function setUpFolderWorkspace(folderName: string, recommendedExtensions: string[], ignoredRecommendations: string[] = []): Promise { return setUpFolder(folderName, recommendedExtensions, ignoredRecommendations); @@ -320,9 +317,9 @@ suite('ExtensionRecommendationsService Test', () => { async function setUpFolder(folderName: string, recommendedExtensions: string[], ignoredRecommendations: string[] = []): Promise { const ROOT = URI.file('tests').with({ scheme: 'vscode-tests' }); const logService = new NullLogService(); - const fileService = new FileService(logService); - const fileSystemProvider = new InMemoryFileSystemProvider(); - fileService.registerProvider(ROOT.scheme, fileSystemProvider); + const fileService = disposableStore.add(new FileService(logService)); + const fileSystemProvider = disposableStore.add(new InMemoryFileSystemProvider()); + disposableStore.add(fileService.registerProvider(ROOT.scheme, fileSystemProvider)); const folderDir = joinPath(ROOT, folderName); const workspaceSettingsDir = joinPath(folderDir, '.vscode'); @@ -338,14 +335,14 @@ suite('ExtensionRecommendationsService Test', () => { instantiationService.stub(IFileService, fileService); workspaceService = new TestContextService(myWorkspace); instantiationService.stub(IWorkspaceContextService, workspaceService); - instantiationService.stub(IWorkspaceExtensionsConfigService, instantiationService.createInstance(WorkspaceExtensionsConfigService)); - instantiationService.stub(IExtensionIgnoredRecommendationsService, instantiationService.createInstance(ExtensionIgnoredRecommendationsService)); - instantiationService.stub(IExtensionRecommendationNotificationService, instantiationService.createInstance(ExtensionRecommendationNotificationService)); + instantiationService.stub(IWorkspaceExtensionsConfigService, disposableStore.add(instantiationService.createInstance(WorkspaceExtensionsConfigService))); + instantiationService.stub(IExtensionIgnoredRecommendationsService, disposableStore.add(instantiationService.createInstance(ExtensionIgnoredRecommendationsService))); + instantiationService.stub(IExtensionRecommendationNotificationService, disposableStore.add(instantiationService.createInstance(ExtensionRecommendationNotificationService))); } function testNoPromptForValidRecommendations(recommendations: string[]) { return setUpFolderWorkspace('myFolder', recommendations).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); return testObject.activationPromise.then(() => { assert.strictEqual(Object.keys(testObject.getAllRecommendationsWithReason()).length, recommendations.length); assert.ok(!prompted); @@ -355,7 +352,7 @@ suite('ExtensionRecommendationsService Test', () => { function testNoPromptOrRecommendationsForValidRecommendations(recommendations: string[]) { return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); assert.ok(!prompted); return testObject.getWorkspaceRecommendations().then(() => { @@ -384,7 +381,7 @@ suite('ExtensionRecommendationsService Test', () => { test('ExtensionRecommendationsService: Prompt for valid workspace recommendations', () => runWithFakedTimers({ useFakeTimers: true }, async () => { await setUpFolderWorkspace('myFolder', mockTestData.recommendedExtensions); - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); await Event.toPromise(promptedEmitter.event); const recommendations = Object.keys(testObject.getAllRecommendationsWithReason()); @@ -413,7 +410,7 @@ suite('ExtensionRecommendationsService Test', () => { test('ExtensionRecommendationsService: No Prompt for valid workspace recommendations if showRecommendationsOnlyOnDemand is set', () => runWithFakedTimers({ useFakeTimers: true }, async () => { testConfigurationService.setUserConfiguration(ConfigurationKey, { showRecommendationsOnlyOnDemand: true }); return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); return testObject.activationPromise.then(() => { assert.ok(!prompted); }); @@ -431,7 +428,7 @@ suite('ExtensionRecommendationsService Test', () => { instantiationService.get(IStorageService).store('extensionsAssistant/ignored_recommendations', '["ms-dotnettools.csharp", "mockpublisher2.mockextension2"]', StorageScope.PROFILE, StorageTarget.MACHINE); return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); return testObject.activationPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(!recommendations['ms-dotnettools.csharp']); // stored recommendation that has been globally ignored @@ -449,7 +446,7 @@ suite('ExtensionRecommendationsService Test', () => { instantiationService.get(IStorageService).store('extensionsAssistant/recommendations', storedRecommendations, StorageScope.PROFILE, StorageTarget.MACHINE); return setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions, ignoredRecommendations).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); return testObject.activationPromise.then(() => { const recommendations = testObject.getAllRecommendationsWithReason(); assert.ok(!recommendations['ms-dotnettools.csharp']); // stored recommendation that has been workspace ignored @@ -471,7 +468,7 @@ suite('ExtensionRecommendationsService Test', () => { storageService.store('extensionsAssistant/ignored_recommendations', globallyIgnoredRecommendations, StorageScope.PROFILE, StorageTarget.MACHINE); await setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions, workspaceIgnoredRecommendations); - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); await testObject.activationPromise; const recommendations = testObject.getAllRecommendationsWithReason(); @@ -489,7 +486,7 @@ suite('ExtensionRecommendationsService Test', () => { await setUpFolderWorkspace('myFolder', mockTestData.validRecommendedExtensions); const extensionIgnoredRecommendationsService = instantiationService.get(IExtensionIgnoredRecommendationsService); - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); await testObject.activationPromise; let recommendations = testObject.getAllRecommendationsWithReason(); @@ -521,9 +518,9 @@ suite('ExtensionRecommendationsService Test', () => { storageService.store('extensionsAssistant/ignored_recommendations', '["ms-vscode.vscode"]', StorageScope.PROFILE, StorageTarget.MACHINE); await setUpFolderWorkspace('myFolder', []); - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); const extensionIgnoredRecommendationsService = instantiationService.get(IExtensionIgnoredRecommendationsService); - extensionIgnoredRecommendationsService.onDidChangeGlobalIgnoredRecommendation(changeHandlerTarget); + disposableStore.add(extensionIgnoredRecommendationsService.onDidChangeGlobalIgnoredRecommendation(changeHandlerTarget)); extensionIgnoredRecommendationsService.toggleGlobalIgnoredRecommendation(ignoredExtensionId, true); await testObject.activationPromise; @@ -536,7 +533,7 @@ suite('ExtensionRecommendationsService Test', () => { instantiationService.get(IStorageService).store('extensionsAssistant/recommendations', storedRecommendations, StorageScope.PROFILE, StorageTarget.MACHINE); return setUpFolderWorkspace('myFolder', []).then(() => { - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); return testObject.activationPromise.then(() => { const recommendations = testObject.getFileBasedRecommendations(); assert.strictEqual(recommendations.length, 2); @@ -555,7 +552,7 @@ suite('ExtensionRecommendationsService Test', () => { instantiationService.get(IStorageService).store('extensionsAssistant/recommendations', storedRecommendations, StorageScope.PROFILE, StorageTarget.MACHINE); await setUpFolderWorkspace('myFolder', []); - testObject = instantiationService.createInstance(ExtensionRecommendationsService); + testObject = disposableStore.add(instantiationService.createInstance(ExtensionRecommendationsService)); await testObject.activationPromise; const recommendations = testObject.getFileBasedRecommendations();