Skip to content

Commit aeb4a69

Browse files
bpaserobpasero
authored
tests - speed up unit tests (microsoft#149712) (microsoft#155147)
* tests - convert history tracker to in-memory (microsoft#149712) * fix warnings from missing service * sqlite slowness * disable flush on write in tests unless disk tests * more runWithFakedTimers * disable flush also in pfs * fix compile
1 parent 070f299 commit aeb4a69

File tree

13 files changed

+208
-159
lines changed

13 files changed

+208
-159
lines changed

‎src/vs/base/node/pfs.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,9 @@ interface IEnsuredWriteFileOptions extends IWriteFileOptions {
390390
}
391391

392392
let canFlush = true;
393+
export function configureFlushOnWrite(enabled: boolean): void {
394+
canFlush = enabled;
395+
}
393396

394397
// Calls fs.writeFile() followed by a fs.sync() call to flush the changes to disk
395398
// We do this in cases where we want to make sure the data is really on disk and
@@ -421,7 +424,7 @@ function doWriteFileAndFlush(path: string, data: string | Buffer | Uint8Array, o
421424
// In that case we disable flushing and warn to the console
422425
if (syncError) {
423426
console.warn('[node.js fs] fdatasync is now disabled for this session because it failed: ', syncError);
424-
canFlush = false;
427+
configureFlushOnWrite(false);
425428
}
426429

427430
return fs.close(fd, closeError => callback(closeError));
@@ -455,7 +458,7 @@ export function writeFileSync(path: string, data: string | Buffer, options?: IWr
455458
fs.fdatasyncSync(fd); // https://github.com/microsoft/vscode/issues/9589
456459
} catch (syncError) {
457460
console.warn('[node.js fs] fdatasyncSync is now disabled for this session because it failed: ', syncError);
458-
canFlush = false;
461+
configureFlushOnWrite(false);
459462
}
460463
} finally {
461464
fs.closeSync(fd);

‎src/vs/base/parts/storage/test/node/storage.test.ts

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -670,56 +670,57 @@ flakySuite('SQLite Storage Library', function () {
670670
});
671671

672672
test('multiple concurrent writes execute in sequence', async () => {
673-
674-
class TestStorage extends Storage {
675-
getStorage(): IStorageDatabase {
676-
return this.database;
673+
return runWithFakedTimers({}, async () => {
674+
class TestStorage extends Storage {
675+
getStorage(): IStorageDatabase {
676+
return this.database;
677+
}
677678
}
678-
}
679679

680-
const storage = new TestStorage(new SQLiteStorageDatabase(join(testdir, 'storage.db')));
680+
const storage = new TestStorage(new SQLiteStorageDatabase(join(testdir, 'storage.db')));
681681

682-
await storage.init();
682+
await storage.init();
683683

684-
storage.set('foo', 'bar');
685-
storage.set('some/foo/path', 'some/bar/path');
684+
storage.set('foo', 'bar');
685+
storage.set('some/foo/path', 'some/bar/path');
686686

687-
await timeout(2);
687+
await timeout(2);
688688

689-
storage.set('foo1', 'bar');
690-
storage.set('some/foo1/path', 'some/bar/path');
689+
storage.set('foo1', 'bar');
690+
storage.set('some/foo1/path', 'some/bar/path');
691691

692-
await timeout(2);
692+
await timeout(2);
693693

694-
storage.set('foo2', 'bar');
695-
storage.set('some/foo2/path', 'some/bar/path');
694+
storage.set('foo2', 'bar');
695+
storage.set('some/foo2/path', 'some/bar/path');
696696

697-
await timeout(2);
697+
await timeout(2);
698698

699-
storage.delete('foo1');
700-
storage.delete('some/foo1/path');
699+
storage.delete('foo1');
700+
storage.delete('some/foo1/path');
701701

702-
await timeout(2);
702+
await timeout(2);
703703

704-
storage.delete('foo4');
705-
storage.delete('some/foo4/path');
704+
storage.delete('foo4');
705+
storage.delete('some/foo4/path');
706706

707-
await timeout(5);
707+
await timeout(5);
708708

709-
storage.set('foo3', 'bar');
710-
await storage.set('some/foo3/path', 'some/bar/path');
709+
storage.set('foo3', 'bar');
710+
await storage.set('some/foo3/path', 'some/bar/path');
711711

712-
const items = await storage.getStorage().getItems();
713-
strictEqual(items.get('foo'), 'bar');
714-
strictEqual(items.get('some/foo/path'), 'some/bar/path');
715-
strictEqual(items.has('foo1'), false);
716-
strictEqual(items.has('some/foo1/path'), false);
717-
strictEqual(items.get('foo2'), 'bar');
718-
strictEqual(items.get('some/foo2/path'), 'some/bar/path');
719-
strictEqual(items.get('foo3'), 'bar');
720-
strictEqual(items.get('some/foo3/path'), 'some/bar/path');
712+
const items = await storage.getStorage().getItems();
713+
strictEqual(items.get('foo'), 'bar');
714+
strictEqual(items.get('some/foo/path'), 'some/bar/path');
715+
strictEqual(items.has('foo1'), false);
716+
strictEqual(items.has('some/foo1/path'), false);
717+
strictEqual(items.get('foo2'), 'bar');
718+
strictEqual(items.get('some/foo2/path'), 'some/bar/path');
719+
strictEqual(items.get('foo3'), 'bar');
720+
strictEqual(items.get('some/foo3/path'), 'some/bar/path');
721721

722-
await storage.close();
722+
await storage.close();
723+
});
723724
});
724725

725726
test('lots of INSERT & DELETE (below inline max)', async () => {

‎src/vs/base/test/node/pfs/pfs.test.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,28 @@ import { VSBuffer } from 'vs/base/common/buffer';
1111
import { randomPath } from 'vs/base/common/extpath';
1212
import { join, sep } from 'vs/base/common/path';
1313
import { isWindows } from 'vs/base/common/platform';
14-
import { Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs';
14+
import { configureFlushOnWrite, Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs';
1515
import { flakySuite, getPathFromAmdModule, getRandomTestPath } from 'vs/base/test/node/testUtils';
1616

17+
configureFlushOnWrite(false); // speed up all unit tests by disabling flush on write
18+
1719
flakySuite('PFS', function () {
1820

1921
let testDir: string;
2022

2123
setup(() => {
24+
configureFlushOnWrite(true); // but enable flushing for the purpose of these tests
2225
testDir = getRandomTestPath(tmpdir(), 'vsctests', 'pfs');
2326

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

27-
teardown(() => {
28-
return Promises.rm(testDir);
30+
teardown(async () => {
31+
try {
32+
await Promises.rm(testDir);
33+
} finally {
34+
configureFlushOnWrite(false);
35+
}
2936
});
3037

3138
test('writeFile', async () => {

‎src/vs/platform/files/common/inMemoryFilesystemProvider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ export class InMemoryFileSystemProvider extends Disposable implements IFileSyste
143143
}
144144

145145
async mkdir(resource: URI): Promise<void> {
146+
if (this._lookup(resource, true)) {
147+
throw new FileSystemProviderError('file exists already', FileSystemProviderErrorCode.FileExists);
148+
}
149+
146150
const basename = resources.basename(resource);
147151
const dirname = resources.dirname(resource);
148152
const parent = this._lookupAsDirectory(dirname, false);

‎src/vs/platform/files/node/diskFileSystemProvider.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,12 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
258258
private readonly mapHandleToLock = new Map<number, IDisposable>();
259259

260260
private readonly writeHandles = new Map<number, URI>();
261-
private canFlush: boolean = true;
261+
262+
private static canFlush: boolean = true;
263+
264+
static configureFlushOnWrite(enabled: boolean): void {
265+
DiskFileSystemProvider.canFlush = enabled;
266+
}
262267

263268
async open(resource: URI, opts: IFileOpenOptions): Promise<number> {
264269
const filePath = this.toFilePath(resource);
@@ -389,13 +394,13 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
389394

390395
// If a handle is closed that was used for writing, ensure
391396
// to flush the contents to disk if possible.
392-
if (this.writeHandles.delete(fd) && this.canFlush) {
397+
if (this.writeHandles.delete(fd) && DiskFileSystemProvider.canFlush) {
393398
try {
394399
await Promises.fdatasync(fd); // https://github.com/microsoft/vscode/issues/9589
395400
} catch (error) {
396401
// In some exotic setups it is well possible that node fails to sync
397402
// In that case we disable flushing and log the error to our logger
398-
this.canFlush = false;
403+
DiskFileSystemProvider.configureFlushOnWrite(false);
399404
this.logService.error(error);
400405
}
401406
}

‎src/vs/platform/files/test/node/diskFileService.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ export class TestDiskFileSystemProvider extends DiskFileSystemProvider {
127127
}
128128
}
129129

130+
DiskFileSystemProvider.configureFlushOnWrite(false); // speed up all unit tests by disabling flush on write
131+
130132
flakySuite('Disk File Service', function () {
131133

132134
const testSchema = 'test';
@@ -140,6 +142,8 @@ flakySuite('Disk File Service', function () {
140142
const disposables = new DisposableStore();
141143

142144
setup(async () => {
145+
DiskFileSystemProvider.configureFlushOnWrite(true); // but enable flushing for the purpose of these tests
146+
143147
const logService = new NullLogService();
144148

145149
service = new FileService(logService);
@@ -160,10 +164,14 @@ flakySuite('Disk File Service', function () {
160164
await Promises.copy(sourceDir, testDir, { preserveSymlinks: false });
161165
});
162166

163-
teardown(() => {
164-
disposables.clear();
167+
teardown(async () => {
168+
try {
169+
disposables.clear();
165170

166-
return Promises.rm(testDir);
171+
await Promises.rm(testDir);
172+
} finally {
173+
DiskFileSystemProvider.configureFlushOnWrite(false);
174+
}
167175
});
168176

169177
test('createFolder', async () => {

‎src/vs/workbench/api/test/browser/mainThreadEditors.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { Range } from 'vs/editor/common/core/range';
1717
import { Position } from 'vs/editor/common/core/position';
1818
import { IModelService } from 'vs/editor/common/services/model';
1919
import { EditOperation } from 'vs/editor/common/core/editOperation';
20-
import { TestFileService, TestEditorService, TestEditorGroupsService, TestEnvironmentService, TestLifecycleService } from 'vs/workbench/test/browser/workbenchTestServices';
20+
import { TestFileService, TestEditorService, TestEditorGroupsService, TestEnvironmentService, TestLifecycleService, TestWorkingCopyService } from 'vs/workbench/test/browser/workbenchTestServices';
2121
import { BulkEditService } from 'vs/workbench/contrib/bulkEdit/browser/bulkEditService';
2222
import { NullLogService, ILogService } from 'vs/platform/log/common/log';
2323
import { ITextModelService, IResolvedTextEditorModel } from 'vs/editor/common/services/resolverService';
@@ -56,6 +56,7 @@ import { LanguageService } from 'vs/editor/common/services/languageService';
5656
import { LanguageFeatureDebounceService } from 'vs/editor/common/services/languageFeatureDebounce';
5757
import { LanguageFeaturesService } from 'vs/editor/common/services/languageFeaturesService';
5858
import { MainThreadBulkEdits } from 'vs/workbench/api/browser/mainThreadBulkEdits';
59+
import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
5960

6061
suite('MainThreadEditors', () => {
6162

@@ -114,6 +115,7 @@ suite('MainThreadEditors', () => {
114115
services.set(IFileService, new TestFileService());
115116
services.set(IEditorService, new TestEditorService());
116117
services.set(ILifecycleService, new TestLifecycleService());
118+
services.set(IWorkingCopyService, new TestWorkingCopyService());
117119
services.set(IEditorGroupsService, new TestEditorGroupsService());
118120
services.set(ITextFileService, new class extends mock<ITextFileService>() {
119121
override isDirty() { return false; }

‎src/vs/workbench/contrib/snippets/browser/surroundWithSnippet.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class SurroundWithSnippetEditorAction extends EditorAction2 {
8383
}
8484

8585
SnippetController2.get(editor)?.insert(snippet.codeSnippet, { clipboardText });
86-
snippetService.updateUsageTimestamp(snippet);
86+
snippetsService.updateUsageTimestamp(snippet);
8787
}
8888
}
8989

‎src/vs/workbench/services/workingCopy/common/workingCopyHistoryTracker.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ export class WorkingCopyHistoryTracker extends Disposable implements IWorkbenchC
193193
private shouldTrackHistory(resource: URI, stat: IFileStatWithMetadata): boolean {
194194
if (
195195
resource.scheme !== this.pathService.defaultUriScheme && // track history for all workspace resources
196-
resource.scheme !== Schemas.vscodeUserData // track history for all settings
196+
resource.scheme !== Schemas.vscodeUserData && // track history for all settings
197+
resource.scheme !== Schemas.inMemory // track history for tests that use in-memory
197198
) {
198199
return false; // do not support unknown resources
199200
}

‎src/vs/workbench/services/workingCopy/test/browser/resourceWorkingCopy.test.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { IRevertOptions, ISaveOptions } from 'vs/workbench/common/editor';
1414
import { ResourceWorkingCopy } from 'vs/workbench/services/workingCopy/common/resourceWorkingCopy';
1515
import { WorkingCopyCapabilities, IWorkingCopyBackup } from 'vs/workbench/services/workingCopy/common/workingCopy';
1616
import { DisposableStore } from 'vs/base/common/lifecycle';
17+
import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler';
1718

1819
suite('ResourceWorkingCopy', function () {
1920

@@ -55,21 +56,23 @@ suite('ResourceWorkingCopy', function () {
5556
});
5657

5758
test('orphaned tracking', async () => {
58-
assert.strictEqual(workingCopy.isOrphaned(), false);
59+
runWithFakedTimers({}, async () => {
60+
assert.strictEqual(workingCopy.isOrphaned(), false);
5961

60-
let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned);
61-
accessor.fileService.notExistsSet.set(resource, true);
62-
accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false));
62+
let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned);
63+
accessor.fileService.notExistsSet.set(resource, true);
64+
accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false));
6365

64-
await onDidChangeOrphanedPromise;
65-
assert.strictEqual(workingCopy.isOrphaned(), true);
66+
await onDidChangeOrphanedPromise;
67+
assert.strictEqual(workingCopy.isOrphaned(), true);
6668

67-
onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned);
68-
accessor.fileService.notExistsSet.delete(resource);
69-
accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false));
69+
onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned);
70+
accessor.fileService.notExistsSet.delete(resource);
71+
accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false));
7072

71-
await onDidChangeOrphanedPromise;
72-
assert.strictEqual(workingCopy.isOrphaned(), false);
73+
await onDidChangeOrphanedPromise;
74+
assert.strictEqual(workingCopy.isOrphaned(), false);
75+
});
7376
});
7477

7578

0 commit comments

Comments
 (0)