Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

debt - dispose things in tests #192392

Merged
merged 8 commits into from Sep 7, 2023
Merged

debt - dispose things in tests #192392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4bd49aa
debt - ensure more dispose from tests
bpasero Sep 7, 2023
65f34a7
more
bpasero Sep 7, 2023
f924ef5
input
bpasero Sep 7, 2023
18c4148
.
bpasero Sep 7, 2023
0b4d86a
more
bpasero Sep 7, 2023
35ecf16
input
bpasero Sep 7, 2023
0068f34
inp
bpasero Sep 7, 2023
5711d76
fux
bpasero Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/vs/base/common/async.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { CancellationError } from 'vs/base/common/errors';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, IDisposable, MutableDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { Disposable, DisposableMap, IDisposable, MutableDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { extUri as defaultExtUri, IExtUri } from 'vs/base/common/resources';
import { URI } from 'vs/base/common/uri';
import { setTimeout0 } from 'vs/base/common/platform';
Expand Down Expand Up @@ -717,6 +717,9 @@ export class ResourceQueue implements IDisposable {

private readonly drainers = new Set<DeferredPromise<void>>();

private drainListeners: DisposableMap<number> | undefined = undefined;
private drainListenerCount = 0;

async whenDrained(): Promise<void> {
if (this.isDrained()) {
return;
Expand Down Expand Up @@ -744,12 +747,20 @@ export class ResourceQueue implements IDisposable {
let queue = this.queues.get(key);
if (!queue) {
queue = new Queue<void>();
Event.once(queue.onDrained)(() => {
const drainListenerId = this.drainListenerCount++;
const drainListener = Event.once(queue.onDrained)(() => {
queue?.dispose();
this.queues.delete(key);
this.onDidQueueDrain();

this.drainListeners?.deleteAndDispose(drainListenerId);
});

if (!this.drainListeners) {
this.drainListeners = new DisposableMap();
}
this.drainListeners.set(drainListenerId, drainListener);

this.queues.set(key, queue);
}

Expand Down Expand Up @@ -786,6 +797,8 @@ export class ResourceQueue implements IDisposable {
// promises when the resource queue is being
// disposed.
this.releaseDrainers();

this.drainListeners?.dispose();
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/vs/platform/state/node/stateService.ts
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@

import { ThrottledDelayer } from 'vs/base/common/async';
import { VSBuffer } from 'vs/base/common/buffer';
import { Disposable } from 'vs/base/common/lifecycle';
import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
Expand All @@ -19,7 +20,7 @@ export const enum SaveStrategy {
DELAYED
}

export class FileStorage {
export class FileStorage extends Disposable {

private storage: StorageDatabase = Object.create(null);
private lastSavedStorageContents = '';
Expand All @@ -35,7 +36,9 @@ export class FileStorage {
private readonly logService: ILogService,
private readonly fileService: IFileService,
) {
this.flushDelayer = saveStrategy === SaveStrategy.IMMEDIATE ? undefined : new ThrottledDelayer<void>(100 /* buffer saves over a short time */);
super();

this.flushDelayer = saveStrategy === SaveStrategy.IMMEDIATE ? undefined : this._register(new ThrottledDelayer<void>(100 /* buffer saves over a short time */));
}

init(): Promise<void> {
Expand Down Expand Up @@ -157,7 +160,7 @@ export class FileStorage {
}
}

export class StateReadonlyService implements IStateReadService {
export class StateReadonlyService extends Disposable implements IStateReadService {

declare readonly _serviceBrand: undefined;

Expand All @@ -169,7 +172,9 @@ export class StateReadonlyService implements IStateReadService {
@ILogService logService: ILogService,
@IFileService fileService: IFileService
) {
this.fileStorage = new FileStorage(environmentService.stateResource, saveStrategy, logService, fileService);
super();

this.fileStorage = this._register(new FileStorage(environmentService.stateResource, saveStrategy, logService, fileService));
}

async init(): Promise<void> {
Expand Down
39 changes: 23 additions & 16 deletions src/vs/platform/state/test/node/state.test.ts
View file
Open in desktop
Expand Up @@ -6,10 +6,12 @@
import * as assert from 'assert';
import { readFileSync } from 'fs';
import { tmpdir } from 'os';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { Schemas } from 'vs/base/common/network';
import { join } from 'vs/base/common/path';
import { URI } from 'vs/base/common/uri';
import { Promises, writeFileSync } from 'vs/base/node/pfs';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils';
import { IFileService } from 'vs/platform/files/common/files';
import { FileService } from 'vs/platform/files/common/fileService';
Expand All @@ -24,21 +26,22 @@ flakySuite('StateService', () => {
let logService: ILogService;
let diskFileSystemProvider: DiskFileSystemProvider;

const disposables = new DisposableStore();

setup(() => {
testDir = getRandomTestPath(tmpdir(), 'vsctests', 'statemainservice');

logService = new NullLogService();

fileService = new FileService(logService);
diskFileSystemProvider = new DiskFileSystemProvider(logService);
fileService.registerProvider(Schemas.file, diskFileSystemProvider);
fileService = disposables.add(new FileService(logService));
diskFileSystemProvider = disposables.add(new DiskFileSystemProvider(logService));
disposables.add(fileService.registerProvider(Schemas.file, diskFileSystemProvider));

return Promises.mkdir(testDir, { recursive: true });
});

teardown(() => {
fileService.dispose();
diskFileSystemProvider.dispose();
disposables.clear();

return Promises.rm(testDir);
});
Expand All @@ -47,7 +50,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

let service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
let service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));
await service.init();

service.setItem('some.key', 'some.value');
Expand All @@ -62,7 +65,7 @@ flakySuite('StateService', () => {

await service.close();

service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));
await service.init();

assert.strictEqual(service.getItem('some.other.key'), 'some.other.value');
Expand Down Expand Up @@ -103,13 +106,15 @@ flakySuite('StateService', () => {
assert.strictEqual(service.getItem('some.setItems.key3'), undefined);
assert.strictEqual(service.getItem('some.setItems.key4'), undefined);
assert.strictEqual(service.getItem('some.setItems.key5'), undefined);

return service.close();
});

test('Basics (immediate strategy)', async function () {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

let service = new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService);
let service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService));
await service.init();

service.setItem('some.key', 'some.value');
Expand All @@ -124,7 +129,7 @@ flakySuite('StateService', () => {

await service.close();

service = new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService);
service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService));
await service.init();

assert.strictEqual(service.getItem('some.other.key'), 'some.other.value');
Expand Down Expand Up @@ -171,7 +176,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

let service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
let service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));
await service.init();

service.setItem('some.key1', 'some.value1');
Expand All @@ -187,7 +192,7 @@ flakySuite('StateService', () => {

await service.close();

service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));
await service.init();

assert.strictEqual(service.getItem('some.key1'), 'some.value1');
Expand All @@ -200,7 +205,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

let service = new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService);
let service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService));
await service.init();

service.setItem('some.key1', 'some.value1');
Expand All @@ -216,7 +221,7 @@ flakySuite('StateService', () => {

await service.close();

service = new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService);
service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.IMMEDIATE, logService, fileService));
await service.init();

assert.strictEqual(service.getItem('some.key1'), 'some.value1');
Expand All @@ -229,7 +234,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

const service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
const service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));

service.setItem('some.key1', 'some.value1');
service.setItem('some.key2', 'some.value2');
Expand All @@ -254,7 +259,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

const service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
const service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));

await service.init();

Expand All @@ -278,7 +283,7 @@ flakySuite('StateService', () => {
const storageFile = join(testDir, 'storage.json');
writeFileSync(storageFile, '');

const service = new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService);
const service = disposables.add(new FileStorage(URI.file(storageFile), SaveStrategy.DELAYED, logService, fileService));

service.setItem('some.key1', 'some.value1');
service.setItem('some.key2', 'some.value2');
Expand All @@ -290,4 +295,6 @@ flakySuite('StateService', () => {
const contents = readFileSync(storageFile).toString();
assert.strictEqual(contents.length, 0);
});

ensureNoDisposablesAreLeakedInTestSuite();
});
12 changes: 11 additions & 1 deletion src/vs/platform/storage/test/common/storageService.test.ts
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@

import { deepStrictEqual, ok, strictEqual } from 'assert';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { InMemoryStorageService, IStorageService, IStorageTargetChangeEvent, IStorageValueChangeEvent, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';

export function createSuite<T extends IStorageService>(params: { setup: () => Promise<T>; teardown: (service: T) => Promise<void> }): void {
Expand Down Expand Up @@ -284,8 +285,17 @@ export function createSuite<T extends IStorageService>(params: { setup: () => Pr
}

suite('StorageService (in-memory)', function () {

const disposables = new DisposableStore();

teardown(() => {
disposables.clear();
});

createSuite<InMemoryStorageService>({
setup: async () => new InMemoryStorageService(),
setup: async () => disposables.add(new InMemoryStorageService()),
teardown: async () => { }
});

ensureNoDisposablesAreLeakedInTestSuite();
});
2 changes: 1 addition & 1 deletion src/vs/platform/storage/test/electron-main/storageMainService.test.ts
View file
Open in desktop
Expand Up @@ -115,7 +115,7 @@ suite('StorageMainService', function () {
const environmentService = new NativeEnvironmentService(parseArgs(process.argv, OPTIONS), productService);
const fileService = disposables.add(new FileService(new NullLogService()));
const uriIdentityService = disposables.add(new UriIdentityService(fileService));
const testStorageService = disposables.add(new TestStorageMainService(new NullLogService(), environmentService, disposables.add(new UserDataProfilesMainService(new StateService(SaveStrategy.DELAYED, environmentService, new NullLogService(), fileService), disposables.add(uriIdentityService), environmentService, fileService, new NullLogService())), lifecycleMainService, fileService, uriIdentityService));
const testStorageService = disposables.add(new TestStorageMainService(new NullLogService(), environmentService, disposables.add(new UserDataProfilesMainService(disposables.add(new StateService(SaveStrategy.DELAYED, environmentService, new NullLogService(), fileService)), disposables.add(uriIdentityService), environmentService, fileService, new NullLogService())), lifecycleMainService, fileService, uriIdentityService));

disposables.add(testStorageService.applicationStorage);

Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/browser/parts/editor/titleControl.ts
View file
Open in desktop
Expand Up @@ -438,6 +438,7 @@ export abstract class TitleControl extends Themable {
}

updateOptions(oldOptions: IEditorPartOptions, newOptions: IEditorPartOptions): void {

// Update title height
if (oldOptions.tabHeight !== newOptions.tabHeight) {
this.updateTitleHeight();
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/browser/parts/statusbar/statusbarModel.ts
View file
Open in desktop
Expand Up @@ -41,7 +41,6 @@ export class StatusbarViewModel extends Disposable {

this.restoreState();
this.registerListeners();

}

private restoreState(): void {
Expand Down
21 changes: 11 additions & 10 deletions src/vs/workbench/contrib/codeEditor/test/browser/saveParticipant.test.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
import { FinalNewLineParticipant, TrimFinalNewLinesParticipant, TrimWhitespaceParticipant } from 'vs/workbench/contrib/codeEditor/browser/saveParticipants';
import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService';
import { workbenchInstantiationService, TestServiceAccessor } from 'vs/workbench/test/browser/workbenchTestServices';
import { toResource } from 'vs/base/test/common/utils';
import { ensureNoDisposablesAreLeakedInTestSuite, toResource } from 'vs/base/test/common/utils';
import { Range } from 'vs/editor/common/core/range';
import { Selection } from 'vs/editor/common/core/selection';
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
Expand All @@ -19,23 +19,22 @@ import { DisposableStore } from 'vs/base/common/lifecycle';

suite('Save Participants', function () {

let disposables: DisposableStore;
const disposables = new DisposableStore();
let instantiationService: IInstantiationService;
let accessor: TestServiceAccessor;

setup(() => {
disposables = new DisposableStore();
instantiationService = workbenchInstantiationService(undefined, disposables);
accessor = instantiationService.createInstance(TestServiceAccessor);
disposables.add(<TextFileEditorModelManager>accessor.textFileService.files);
});

teardown(() => {
(<TextFileEditorModelManager>accessor.textFileService.files).dispose();
disposables.dispose();
disposables.clear();
});

test('insert final new line', async function () {
const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel;
const model: IResolvedTextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel);

await model.resolve();
const configService = new TestConfigurationService();
Expand Down Expand Up @@ -68,7 +67,7 @@ suite('Save Participants', function () {
});

test('trim final new lines', async function () {
const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel;
const model: IResolvedTextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel);

await model.resolve();
const configService = new TestConfigurationService();
Expand Down Expand Up @@ -103,7 +102,7 @@ suite('Save Participants', function () {
});

test('trim final new lines bug#39750', async function () {
const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel;
const model: IResolvedTextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel);

await model.resolve();
const configService = new TestConfigurationService();
Expand All @@ -130,7 +129,7 @@ suite('Save Participants', function () {
});

test('trim final new lines bug#46075', async function () {
const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel;
const model: IResolvedTextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel);

await model.resolve();
const configService = new TestConfigurationService();
Expand All @@ -157,7 +156,7 @@ suite('Save Participants', function () {
});

test('trim whitespace', async function () {
const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel;
const model: IResolvedTextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/trim_final_new_line.txt'), 'utf8', undefined) as IResolvedTextFileEditorModel);

await model.resolve();
const configService = new TestConfigurationService();
Expand All @@ -175,4 +174,6 @@ suite('Save Participants', function () {
// confirm trimming
assert.strictEqual(snapshotToString(model.createSnapshot()!), `${textContent}`);
});

ensureNoDisposablesAreLeakedInTestSuite();
});