Skip to content

Commit

Permalink
Merge pull request #114129 from microsoft/alex/configuration-editing-…
Browse files Browse the repository at this point in the history 
…tests-improvements

Dispose all Disposables from tests
  • Loading branch information
@alexdima
alexdima authored Jan 11, 2021
2 parents c173fb7 + ea75690 commit bd5c204
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 74 deletions.
6 changes: 3 additions & 3 deletions src/vs/platform/files/common/fileService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -901,19 +901,19 @@ export class FileService extends Disposable implements IFileService {

watch(resource: URI, options: IWatchOptions = { recursive: false, excludes: [] }): IDisposable {
let watchDisposed = false;
let watchDisposable = toDisposable(() => watchDisposed = true);
let disposeWatch = () => { watchDisposed = true; };

// Watch and wire in disposable which is async but
// check if we got disposed meanwhile and forward
this.doWatch(resource, options).then(disposable => {
if (watchDisposed) {
dispose(disposable);
} else {
watchDisposable = disposable;
disposeWatch = () => dispose(disposable);
}
}, error => this.logService.error(error));

return toDisposable(() => dispose(watchDisposable));
return toDisposable(() => disposeWatch());
}

async doWatch(resource: URI, options: IWatchOptions): Promise<IDisposable> {
Expand Down
58 changes: 16 additions & 42 deletions src/vs/workbench/services/configuration/browser/configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { URI } from 'vs/base/common/uri';
import { Event, Emitter } from 'vs/base/common/event';
import * as errors from 'vs/base/common/errors';
import { Disposable, IDisposable, dispose, toDisposable, MutableDisposable, combinedDisposable } from 'vs/base/common/lifecycle';
import { Disposable, IDisposable, dispose, toDisposable, MutableDisposable, combinedDisposable, DisposableStore } from 'vs/base/common/lifecycle';
import { RunOnceScheduler } from 'vs/base/common/async';
import { FileChangeType, FileChangesEvent, IFileService, whenProviderRegistered, FileOperationError, FileOperationResult } from 'vs/platform/files/common/files';
import { ConfigurationModel, ConfigurationModelParser, UserSettings } from 'vs/platform/configuration/common/configurationModels';
Expand Down Expand Up @@ -397,8 +397,8 @@ export class WorkspaceConfiguration extends Disposable {

private readonly _fileService: IFileService;
private readonly _cachedConfiguration: CachedWorkspaceConfiguration;
private _workspaceConfiguration: IWorkspaceConfiguration;
private _workspaceConfigurationChangeDisposable: IDisposable = Disposable.None;
private _workspaceConfiguration: CachedWorkspaceConfiguration | FileServiceBasedWorkspaceConfiguration;
private _workspaceConfigurationDisposables = this._register(new DisposableStore());
private _workspaceIdentifier: IWorkspaceIdentifier | null = null;

private readonly _onDidUpdateConfiguration: Emitter<void> = this._register(new Emitter<void>());
Expand Down Expand Up @@ -466,10 +466,9 @@ export class WorkspaceConfiguration extends Disposable {
}

private doInitialize(fileServiceBasedWorkspaceConfiguration: FileServiceBasedWorkspaceConfiguration): void {
this._workspaceConfiguration.dispose();
this._workspaceConfigurationChangeDisposable.dispose();
this._workspaceConfiguration = this._register(fileServiceBasedWorkspaceConfiguration);
this._workspaceConfigurationChangeDisposable = this._register(this._workspaceConfiguration.onDidChange(e => this.onDidWorkspaceConfigurationChange(true)));
this._workspaceConfigurationDisposables.clear();
this._workspaceConfiguration = this._workspaceConfigurationDisposables.add(fileServiceBasedWorkspaceConfiguration);
this._workspaceConfigurationDisposables.add(this._workspaceConfiguration.onDidChange(e => this.onDidWorkspaceConfigurationChange(true)));
this._initialized = true;
}

Expand All @@ -490,19 +489,7 @@ export class WorkspaceConfiguration extends Disposable {
}
}

interface IWorkspaceConfiguration extends IDisposable {
readonly onDidChange: Event<void>;
workspaceConfigurationModelParser: WorkspaceConfigurationModelParser;
workspaceSettings: ConfigurationModel;
workspaceIdentifier: IWorkspaceIdentifier | null;
load(workspaceIdentifier: IWorkspaceIdentifier): Promise<void>;
getConfigurationModel(): ConfigurationModel;
getFolders(): IStoredWorkspaceFolder[];
getWorkspaceSettings(): ConfigurationModel;
reprocessWorkspaceSettings(): ConfigurationModel;
}

class FileServiceBasedWorkspaceConfiguration extends Disposable implements IWorkspaceConfiguration {
class FileServiceBasedWorkspaceConfiguration extends Disposable {

workspaceConfigurationModelParser: WorkspaceConfigurationModelParser;
workspaceSettings: ConfigurationModel;
Expand Down Expand Up @@ -586,16 +573,14 @@ class FileServiceBasedWorkspaceConfiguration extends Disposable implements IWork
}
}

class CachedWorkspaceConfiguration extends Disposable implements IWorkspaceConfiguration {
class CachedWorkspaceConfiguration {

private readonly _onDidChange: Emitter<void> = this._register(new Emitter<void>());
readonly onDidChange: Event<void> = this._onDidChange.event;
readonly onDidChange: Event<void> = Event.None;

workspaceConfigurationModelParser: WorkspaceConfigurationModelParser;
workspaceSettings: ConfigurationModel;

constructor(private readonly configurationCache: IConfigurationCache) {
super();
this.workspaceConfigurationModelParser = new WorkspaceConfigurationModelParser('');
this.workspaceSettings = new ConfigurationModel();
}
Expand Down Expand Up @@ -651,16 +636,9 @@ class CachedWorkspaceConfiguration extends Disposable implements IWorkspaceConfi
}
}

export interface IFolderConfiguration extends IDisposable {
readonly onDidChange: Event<void>;
loadConfiguration(): Promise<ConfigurationModel>;
reprocess(): ConfigurationModel;
}

class CachedFolderConfiguration extends Disposable implements IFolderConfiguration {
class CachedFolderConfiguration {

private readonly _onDidChange: Emitter<void> = this._register(new Emitter<void>());
readonly onDidChange: Event<void> = this._onDidChange.event;
readonly onDidChange = Event.None;

private configurationModel: ConfigurationModel;
private readonly key: ConfigurationKey;
Expand All @@ -670,7 +648,6 @@ class CachedFolderConfiguration extends Disposable implements IFolderConfigurati
configFolderRelativePath: string,
private readonly configurationCache: IConfigurationCache
) {
super();
this.key = { type: 'folder', key: hash(join(folder.path, configFolderRelativePath)).toString(16) };
this.configurationModel = new ConfigurationModel();
}
Expand Down Expand Up @@ -702,13 +679,12 @@ class CachedFolderConfiguration extends Disposable implements IFolderConfigurati
}
}

export class FolderConfiguration extends Disposable implements IFolderConfiguration {
export class FolderConfiguration extends Disposable {

protected readonly _onDidChange: Emitter<void> = this._register(new Emitter<void>());
readonly onDidChange: Event<void> = this._onDidChange.event;

private folderConfiguration: IFolderConfiguration;
private folderConfigurationDisposable: IDisposable = Disposable.None;
private folderConfiguration: CachedFolderConfiguration | FileServiceBasedConfiguration;
private readonly configurationFolder: URI;
private cachedFolderConfiguration: CachedFolderConfiguration;

Expand All @@ -728,15 +704,13 @@ export class FolderConfiguration extends Disposable implements IFolderConfigurat
this.folderConfiguration = this.cachedFolderConfiguration;
whenProviderRegistered(workspaceFolder.uri, fileService)
.then(() => {
this.folderConfiguration.dispose();
this.folderConfigurationDisposable.dispose();
this.folderConfiguration = this.createFileServiceBasedConfiguration(fileService, uriIdentityService);
this.folderConfiguration = this._register(this.createFileServiceBasedConfiguration(fileService, uriIdentityService));
this._register(this.folderConfiguration.onDidChange(e => this.onDidFolderConfigurationChange()));
this.onDidFolderConfigurationChange();
});
} else {
this.folderConfiguration = this.createFileServiceBasedConfiguration(fileService, uriIdentityService);
this.folderConfigurationDisposable = this._register(this.folderConfiguration.onDidChange(e => this.onDidFolderConfigurationChange()));
this.folderConfiguration = this._register(this.createFileServiceBasedConfiguration(fileService, uriIdentityService));
this._register(this.folderConfiguration.onDidChange(e => this.onDidFolderConfigurationChange()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/configuration/browser/configurationService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat

if (!this.localUserConfiguration.hasTasksLoaded) {
// Reload local user configuration again to load user tasks
runWhenIdle(() => this.reloadLocalUserConfiguration(), 5000);
this._register(runWhenIdle(() => this.reloadLocalUserConfiguration(), 5000));
}
});
}
Expand Down
14 changes: 7 additions & 7 deletions ...orkbench/services/configuration/test/electron-browser/configurationEditingService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,23 +104,23 @@ suite('ConfigurationEditingService', () => {
}

async function setUpServices(noWorkspace: boolean = false): Promise<void> {
instantiationService = <TestInstantiationService>workbenchInstantiationService();
instantiationService = <TestInstantiationService>workbenchInstantiationService(undefined, disposables);
const environmentService = new TestWorkbenchEnvironmentService(URI.file(workspaceDir));
instantiationService.stub(IEnvironmentService, environmentService);
const remoteAgentService = instantiationService.createInstance(RemoteAgentService);
const remoteAgentService = disposables.add(instantiationService.createInstance(RemoteAgentService));
const fileService = disposables.add(new FileService(new NullLogService()));
const diskFileSystemProvider = disposables.add(new DiskFileSystemProvider(new NullLogService()));
fileService.registerProvider(Schemas.file, diskFileSystemProvider);
fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(Schemas.file, diskFileSystemProvider, Schemas.userData, new NullLogService())));
disposables.add(fileService.registerProvider(Schemas.file, diskFileSystemProvider));
disposables.add(fileService.registerProvider(Schemas.userData, disposables.add(new FileUserDataProvider(Schemas.file, diskFileSystemProvider, Schemas.userData, new NullLogService()))));
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IRemoteAgentService, remoteAgentService);
const workspaceService = disposables.add(new WorkspaceService({ configurationCache: new ConfigurationCache(environmentService) }, environmentService, fileService, remoteAgentService, new UriIdentityService(fileService), new NullLogService()));
instantiationService.stub(IWorkspaceContextService, workspaceService);
await workspaceService.initialize(noWorkspace ? { id: '' } : { folder: URI.file(workspaceDir), id: createHash('md5').update(URI.file(workspaceDir).toString()).digest('hex') });
instantiationService.stub(IConfigurationService, workspaceService);
instantiationService.stub(IKeybindingEditingService, instantiationService.createInstance(KeybindingsEditingService));
instantiationService.stub(ITextFileService, instantiationService.createInstance(TestTextFileService));
instantiationService.stub(ITextModelService, <ITextModelService>instantiationService.createInstance(TextModelResolverService));
instantiationService.stub(IKeybindingEditingService, disposables.add(instantiationService.createInstance(KeybindingsEditingService)));
instantiationService.stub(ITextFileService, disposables.add(instantiationService.createInstance(TestTextFileService)));
instantiationService.stub(ITextModelService, <ITextModelService>disposables.add(instantiationService.createInstance(TextModelResolverService)));
instantiationService.stub(ICommandService, CommandService);
testObject = instantiationService.createInstance(ConfigurationEditingService);
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi
super();

// register a default working copy provider that uses the working copy service
this.registerWorkingCopyProvider(resource => {
this._register(this.registerWorkingCopyProvider(resource => {
return this.workingCopyService.workingCopies.filter(workingCopy => {
if (this.fileService.canHandleResource(resource)) {
// only check for parents if the resource can be handled
Expand All @@ -265,7 +265,7 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi

return this.uriIdentityService.extUri.isEqual(workingCopy.resource, resource);
});
});
}));
}


Expand Down
41 changes: 22 additions & 19 deletions src/vs/workbench/test/browser/workbenchTestServices.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,18 @@ export interface ITestInstantiationService extends IInstantiationService {
stub<T>(service: ServiceIdentifier<T>, ctor: any): T;
}

export function workbenchInstantiationService(overrides?: {
textFileService?: (instantiationService: IInstantiationService) => ITextFileService
pathService?: (instantiationService: IInstantiationService) => IPathService,
editorService?: (instantiationService: IInstantiationService) => IEditorService,
contextKeyService?: (instantiationService: IInstantiationService) => IContextKeyService,
}): ITestInstantiationService {
export function workbenchInstantiationService(
overrides?: {
textFileService?: (instantiationService: IInstantiationService) => ITextFileService
pathService?: (instantiationService: IInstantiationService) => IPathService,
editorService?: (instantiationService: IInstantiationService) => IEditorService,
contextKeyService?: (instantiationService: IInstantiationService) => IContextKeyService,
},
disposables: DisposableStore = new DisposableStore()
): ITestInstantiationService {
const instantiationService = new TestInstantiationService(new ServiceCollection([ILifecycleService, new TestLifecycleService()]));

instantiationService.stub(IWorkingCopyService, new TestWorkingCopyService());
instantiationService.stub(IWorkingCopyService, disposables.add(new TestWorkingCopyService()));
instantiationService.stub(IEnvironmentService, TestEnvironmentService);
const contextKeyService = overrides?.contextKeyService ? overrides.contextKeyService(instantiationService) : instantiationService.createInstance(MockContextKeyService);
instantiationService.stub(IContextKeyService, contextKeyService);
Expand All @@ -140,50 +143,50 @@ export function workbenchInstantiationService(overrides?: {
instantiationService.stub(IWorkspaceContextService, workspaceContextService);
const configService = new TestConfigurationService();
instantiationService.stub(IConfigurationService, configService);
instantiationService.stub(IFilesConfigurationService, new TestFilesConfigurationService(contextKeyService, configService));
instantiationService.stub(IFilesConfigurationService, disposables.add(new TestFilesConfigurationService(contextKeyService, configService)));
instantiationService.stub(ITextResourceConfigurationService, new TestTextResourceConfigurationService(configService));
instantiationService.stub(IUntitledTextEditorService, instantiationService.createInstance(UntitledTextEditorService));
instantiationService.stub(IStorageService, new TestStorageService());
instantiationService.stub(IUntitledTextEditorService, disposables.add(instantiationService.createInstance(UntitledTextEditorService)));
instantiationService.stub(IStorageService, disposables.add(new TestStorageService()));
instantiationService.stub(IPathService, overrides?.pathService ? overrides.pathService(instantiationService) : new TestPathService());
const layoutService = new TestLayoutService();
instantiationService.stub(IWorkbenchLayoutService, layoutService);
instantiationService.stub(IDialogService, new TestDialogService());
const accessibilityService = new TestAccessibilityService();
instantiationService.stub(IAccessibilityService, accessibilityService);
instantiationService.stub(IFileDialogService, instantiationService.createInstance(TestFileDialogService));
instantiationService.stub(IModeService, instantiationService.createInstance(ModeServiceImpl));
instantiationService.stub(IModeService, disposables.add(instantiationService.createInstance(ModeServiceImpl)));
instantiationService.stub(IHistoryService, new TestHistoryService());
instantiationService.stub(ITextResourcePropertiesService, new TestTextResourcePropertiesService(configService));
instantiationService.stub(IUndoRedoService, instantiationService.createInstance(UndoRedoService));
const themeService = new TestThemeService();
instantiationService.stub(IThemeService, themeService);
instantiationService.stub(IModelService, instantiationService.createInstance(ModelServiceImpl));
instantiationService.stub(IModelService, disposables.add(instantiationService.createInstance(ModelServiceImpl)));
const fileService = new TestFileService();
instantiationService.stub(IFileService, fileService);
instantiationService.stub(IUriIdentityService, new UriIdentityService(fileService));
instantiationService.stub(IBackupFileService, new TestBackupFileService());
instantiationService.stub(ITelemetryService, NullTelemetryService);
instantiationService.stub(INotificationService, new TestNotificationService());
instantiationService.stub(IUntitledTextEditorService, instantiationService.createInstance(UntitledTextEditorService));
instantiationService.stub(IUntitledTextEditorService, disposables.add(instantiationService.createInstance(UntitledTextEditorService)));
instantiationService.stub(IMenuService, new TestMenuService());
const keybindingService = new MockKeybindingService();
instantiationService.stub(IKeybindingService, keybindingService);
instantiationService.stub(IDecorationsService, new TestDecorationsService());
instantiationService.stub(IExtensionService, new TestExtensionService());
instantiationService.stub(IWorkingCopyFileService, instantiationService.createInstance(WorkingCopyFileService));
instantiationService.stub(ITextFileService, overrides?.textFileService ? overrides.textFileService(instantiationService) : <ITextFileService>instantiationService.createInstance(TestTextFileService));
instantiationService.stub(IWorkingCopyFileService, disposables.add(instantiationService.createInstance(WorkingCopyFileService)));
instantiationService.stub(ITextFileService, overrides?.textFileService ? overrides.textFileService(instantiationService) : disposables.add(<ITextFileService>instantiationService.createInstance(TestTextFileService)));
instantiationService.stub(IHostService, <IHostService>instantiationService.createInstance(TestHostService));
instantiationService.stub(ITextModelService, <ITextModelService>instantiationService.createInstance(TextModelResolverService));
instantiationService.stub(ITextModelService, <ITextModelService>disposables.add(instantiationService.createInstance(TextModelResolverService)));
instantiationService.stub(ILogService, new NullLogService());
const editorGroupService = new TestEditorGroupsService([new TestEditorGroupView(0)]);
instantiationService.stub(IEditorGroupsService, editorGroupService);
instantiationService.stub(ILabelService, <ILabelService>instantiationService.createInstance(LabelService));
instantiationService.stub(ILabelService, <ILabelService>disposables.add(instantiationService.createInstance(LabelService)));
const editorService = overrides?.editorService ? overrides.editorService(instantiationService) : new TestEditorService(editorGroupService);
instantiationService.stub(IEditorService, editorService);
instantiationService.stub(ICodeEditorService, new CodeEditorService(editorService, themeService, configService));
instantiationService.stub(ICodeEditorService, disposables.add(new CodeEditorService(editorService, themeService, configService)));
instantiationService.stub(IViewletService, new TestViewletService());
instantiationService.stub(IListService, new TestListService());
instantiationService.stub(IQuickInputService, new QuickInputService(configService, instantiationService, keybindingService, contextKeyService, themeService, accessibilityService, layoutService));
instantiationService.stub(IQuickInputService, disposables.add(new QuickInputService(configService, instantiationService, keybindingService, contextKeyService, themeService, accessibilityService, layoutService)));

return instantiationService;
}
Expand Down

0 comments on commit bd5c204

Please sign in to comment.