From aeb4a695fc42e2f812b0f70dc98cfe67e33ebe71 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 15 Jul 2022 14:12:56 +0200 Subject: [PATCH] tests - speed up unit tests (#149712) (#155147) * tests - convert history tracker to in-memory (#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 --- src/vs/base/node/pfs.ts | 7 +- .../parts/storage/test/node/storage.test.ts | 69 ++++---- src/vs/base/test/node/pfs/pfs.test.ts | 13 +- .../common/inMemoryFilesystemProvider.ts | 4 + .../files/node/diskFileSystemProvider.ts | 11 +- .../files/test/node/diskFileService.test.ts | 14 +- .../test/browser/mainThreadEditors.test.ts | 4 +- .../snippets/browser/surroundWithSnippet.ts | 2 +- .../common/workingCopyHistoryTracker.ts | 3 +- .../test/browser/resourceWorkingCopy.test.ts | 25 +-- .../browser/storedFileWorkingCopy.test.ts | 153 +++++++++--------- .../workingCopyHistoryService.test.ts | 8 +- .../workingCopyHistoryTracker.test.ts | 68 ++++---- 13 files changed, 215 insertions(+), 166 deletions(-) diff --git a/src/vs/base/node/pfs.ts b/src/vs/base/node/pfs.ts index 2283c4a61d6..9b652eb627f 100644 --- a/src/vs/base/node/pfs.ts +++ b/src/vs/base/node/pfs.ts @@ -390,6 +390,9 @@ interface IEnsuredWriteFileOptions extends IWriteFileOptions { } let canFlush = true; +export function configureFlushOnWrite(enabled: boolean): void { + canFlush = enabled; +} // Calls fs.writeFile() followed by a fs.sync() call to flush the changes to disk // 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 // In that case we disable flushing and warn to the console if (syncError) { console.warn('[node.js fs] fdatasync is now disabled for this session because it failed: ', syncError); - canFlush = false; + configureFlushOnWrite(false); } return fs.close(fd, closeError => callback(closeError)); @@ -455,7 +458,7 @@ export function writeFileSync(path: string, data: string | Buffer, options?: IWr fs.fdatasyncSync(fd); // https://github.com/microsoft/vscode/issues/9589 } catch (syncError) { console.warn('[node.js fs] fdatasyncSync is now disabled for this session because it failed: ', syncError); - canFlush = false; + configureFlushOnWrite(false); } } finally { fs.closeSync(fd); diff --git a/src/vs/base/parts/storage/test/node/storage.test.ts b/src/vs/base/parts/storage/test/node/storage.test.ts index f53fdb2c387..70ff91977be 100644 --- a/src/vs/base/parts/storage/test/node/storage.test.ts +++ b/src/vs/base/parts/storage/test/node/storage.test.ts @@ -670,56 +670,57 @@ flakySuite('SQLite Storage Library', function () { }); test('multiple concurrent writes execute in sequence', async () => { - - class TestStorage extends Storage { - getStorage(): IStorageDatabase { - return this.database; + return runWithFakedTimers({}, async () => { + class TestStorage extends Storage { + getStorage(): IStorageDatabase { + return this.database; + } } - } - const storage = new TestStorage(new SQLiteStorageDatabase(join(testdir, 'storage.db'))); + const storage = new TestStorage(new SQLiteStorageDatabase(join(testdir, 'storage.db'))); - await storage.init(); + await storage.init(); - storage.set('foo', 'bar'); - storage.set('some/foo/path', 'some/bar/path'); + storage.set('foo', 'bar'); + storage.set('some/foo/path', 'some/bar/path'); - await timeout(2); + await timeout(2); - storage.set('foo1', 'bar'); - storage.set('some/foo1/path', 'some/bar/path'); + storage.set('foo1', 'bar'); + storage.set('some/foo1/path', 'some/bar/path'); - await timeout(2); + await timeout(2); - storage.set('foo2', 'bar'); - storage.set('some/foo2/path', 'some/bar/path'); + storage.set('foo2', 'bar'); + storage.set('some/foo2/path', 'some/bar/path'); - await timeout(2); + await timeout(2); - storage.delete('foo1'); - storage.delete('some/foo1/path'); + storage.delete('foo1'); + storage.delete('some/foo1/path'); - await timeout(2); + await timeout(2); - storage.delete('foo4'); - storage.delete('some/foo4/path'); + storage.delete('foo4'); + storage.delete('some/foo4/path'); - await timeout(5); + await timeout(5); - storage.set('foo3', 'bar'); - await storage.set('some/foo3/path', 'some/bar/path'); + storage.set('foo3', 'bar'); + await storage.set('some/foo3/path', 'some/bar/path'); - const items = await storage.getStorage().getItems(); - strictEqual(items.get('foo'), 'bar'); - strictEqual(items.get('some/foo/path'), 'some/bar/path'); - strictEqual(items.has('foo1'), false); - strictEqual(items.has('some/foo1/path'), false); - strictEqual(items.get('foo2'), 'bar'); - strictEqual(items.get('some/foo2/path'), 'some/bar/path'); - strictEqual(items.get('foo3'), 'bar'); - strictEqual(items.get('some/foo3/path'), 'some/bar/path'); + const items = await storage.getStorage().getItems(); + strictEqual(items.get('foo'), 'bar'); + strictEqual(items.get('some/foo/path'), 'some/bar/path'); + strictEqual(items.has('foo1'), false); + strictEqual(items.has('some/foo1/path'), false); + strictEqual(items.get('foo2'), 'bar'); + strictEqual(items.get('some/foo2/path'), 'some/bar/path'); + strictEqual(items.get('foo3'), 'bar'); + strictEqual(items.get('some/foo3/path'), 'some/bar/path'); - await storage.close(); + await storage.close(); + }); }); test('lots of INSERT & DELETE (below inline max)', async () => { diff --git a/src/vs/base/test/node/pfs/pfs.test.ts b/src/vs/base/test/node/pfs/pfs.test.ts index e45782e236f..4c15c3ce143 100644 --- a/src/vs/base/test/node/pfs/pfs.test.ts +++ b/src/vs/base/test/node/pfs/pfs.test.ts @@ -11,21 +11,28 @@ import { VSBuffer } from 'vs/base/common/buffer'; import { randomPath } from 'vs/base/common/extpath'; import { join, sep } from 'vs/base/common/path'; import { isWindows } from 'vs/base/common/platform'; -import { Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs'; +import { configureFlushOnWrite, Promises, RimRafMode, rimrafSync, SymlinkSupport, writeFileSync } from 'vs/base/node/pfs'; import { flakySuite, getPathFromAmdModule, getRandomTestPath } from 'vs/base/test/node/testUtils'; +configureFlushOnWrite(false); // speed up all unit tests by disabling flush on write + flakySuite('PFS', function () { let testDir: string; setup(() => { + configureFlushOnWrite(true); // but enable flushing for the purpose of these tests testDir = getRandomTestPath(tmpdir(), 'vsctests', 'pfs'); return Promises.mkdir(testDir, { recursive: true }); }); - teardown(() => { - return Promises.rm(testDir); + teardown(async () => { + try { + await Promises.rm(testDir); + } finally { + configureFlushOnWrite(false); + } }); test('writeFile', async () => { diff --git a/src/vs/platform/files/common/inMemoryFilesystemProvider.ts b/src/vs/platform/files/common/inMemoryFilesystemProvider.ts index f4d13e998ec..5ee8e5366b6 100644 --- a/src/vs/platform/files/common/inMemoryFilesystemProvider.ts +++ b/src/vs/platform/files/common/inMemoryFilesystemProvider.ts @@ -143,6 +143,10 @@ export class InMemoryFileSystemProvider extends Disposable implements IFileSyste } async mkdir(resource: URI): Promise { + if (this._lookup(resource, true)) { + throw new FileSystemProviderError('file exists already', FileSystemProviderErrorCode.FileExists); + } + const basename = resources.basename(resource); const dirname = resources.dirname(resource); const parent = this._lookupAsDirectory(dirname, false); diff --git a/src/vs/platform/files/node/diskFileSystemProvider.ts b/src/vs/platform/files/node/diskFileSystemProvider.ts index 08c155e011e..708221073ac 100644 --- a/src/vs/platform/files/node/diskFileSystemProvider.ts +++ b/src/vs/platform/files/node/diskFileSystemProvider.ts @@ -258,7 +258,12 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple private readonly mapHandleToLock = new Map(); private readonly writeHandles = new Map(); - private canFlush: boolean = true; + + private static canFlush: boolean = true; + + static configureFlushOnWrite(enabled: boolean): void { + DiskFileSystemProvider.canFlush = enabled; + } async open(resource: URI, opts: IFileOpenOptions): Promise { const filePath = this.toFilePath(resource); @@ -389,13 +394,13 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple // If a handle is closed that was used for writing, ensure // to flush the contents to disk if possible. - if (this.writeHandles.delete(fd) && this.canFlush) { + if (this.writeHandles.delete(fd) && DiskFileSystemProvider.canFlush) { try { await Promises.fdatasync(fd); // https://github.com/microsoft/vscode/issues/9589 } catch (error) { // In some exotic setups it is well possible that node fails to sync // In that case we disable flushing and log the error to our logger - this.canFlush = false; + DiskFileSystemProvider.configureFlushOnWrite(false); this.logService.error(error); } } diff --git a/src/vs/platform/files/test/node/diskFileService.test.ts b/src/vs/platform/files/test/node/diskFileService.test.ts index de1f4521122..c76b85fe292 100644 --- a/src/vs/platform/files/test/node/diskFileService.test.ts +++ b/src/vs/platform/files/test/node/diskFileService.test.ts @@ -127,6 +127,8 @@ export class TestDiskFileSystemProvider extends DiskFileSystemProvider { } } +DiskFileSystemProvider.configureFlushOnWrite(false); // speed up all unit tests by disabling flush on write + flakySuite('Disk File Service', function () { const testSchema = 'test'; @@ -140,6 +142,8 @@ flakySuite('Disk File Service', function () { const disposables = new DisposableStore(); setup(async () => { + DiskFileSystemProvider.configureFlushOnWrite(true); // but enable flushing for the purpose of these tests + const logService = new NullLogService(); service = new FileService(logService); @@ -160,10 +164,14 @@ flakySuite('Disk File Service', function () { await Promises.copy(sourceDir, testDir, { preserveSymlinks: false }); }); - teardown(() => { - disposables.clear(); + teardown(async () => { + try { + disposables.clear(); - return Promises.rm(testDir); + await Promises.rm(testDir); + } finally { + DiskFileSystemProvider.configureFlushOnWrite(false); + } }); test('createFolder', async () => { diff --git a/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts b/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts index 859c9c28ee8..6ee05e73b5a 100644 --- a/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts +++ b/src/vs/workbench/api/test/browser/mainThreadEditors.test.ts @@ -17,7 +17,7 @@ import { Range } from 'vs/editor/common/core/range'; import { Position } from 'vs/editor/common/core/position'; import { IModelService } from 'vs/editor/common/services/model'; import { EditOperation } from 'vs/editor/common/core/editOperation'; -import { TestFileService, TestEditorService, TestEditorGroupsService, TestEnvironmentService, TestLifecycleService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestFileService, TestEditorService, TestEditorGroupsService, TestEnvironmentService, TestLifecycleService, TestWorkingCopyService } from 'vs/workbench/test/browser/workbenchTestServices'; import { BulkEditService } from 'vs/workbench/contrib/bulkEdit/browser/bulkEditService'; import { NullLogService, ILogService } from 'vs/platform/log/common/log'; import { ITextModelService, IResolvedTextEditorModel } from 'vs/editor/common/services/resolverService'; @@ -56,6 +56,7 @@ import { LanguageService } from 'vs/editor/common/services/languageService'; import { LanguageFeatureDebounceService } from 'vs/editor/common/services/languageFeatureDebounce'; import { LanguageFeaturesService } from 'vs/editor/common/services/languageFeaturesService'; import { MainThreadBulkEdits } from 'vs/workbench/api/browser/mainThreadBulkEdits'; +import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService'; suite('MainThreadEditors', () => { @@ -114,6 +115,7 @@ suite('MainThreadEditors', () => { services.set(IFileService, new TestFileService()); services.set(IEditorService, new TestEditorService()); services.set(ILifecycleService, new TestLifecycleService()); + services.set(IWorkingCopyService, new TestWorkingCopyService()); services.set(IEditorGroupsService, new TestEditorGroupsService()); services.set(ITextFileService, new class extends mock() { override isDirty() { return false; } diff --git a/src/vs/workbench/contrib/snippets/browser/surroundWithSnippet.ts b/src/vs/workbench/contrib/snippets/browser/surroundWithSnippet.ts index 02805db79eb..e8a9551fe19 100644 --- a/src/vs/workbench/contrib/snippets/browser/surroundWithSnippet.ts +++ b/src/vs/workbench/contrib/snippets/browser/surroundWithSnippet.ts @@ -83,7 +83,7 @@ class SurroundWithSnippetEditorAction extends EditorAction2 { } SnippetController2.get(editor)?.insert(snippet.codeSnippet, { clipboardText }); - snippetService.updateUsageTimestamp(snippet); + snippetsService.updateUsageTimestamp(snippet); } } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyHistoryTracker.ts b/src/vs/workbench/services/workingCopy/common/workingCopyHistoryTracker.ts index ed05ae2c5ff..7e3d7a7c2b4 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyHistoryTracker.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyHistoryTracker.ts @@ -193,7 +193,8 @@ export class WorkingCopyHistoryTracker extends Disposable implements IWorkbenchC private shouldTrackHistory(resource: URI, stat: IFileStatWithMetadata): boolean { if ( resource.scheme !== this.pathService.defaultUriScheme && // track history for all workspace resources - resource.scheme !== Schemas.vscodeUserData // track history for all settings + resource.scheme !== Schemas.vscodeUserData && // track history for all settings + resource.scheme !== Schemas.inMemory // track history for tests that use in-memory ) { return false; // do not support unknown resources } diff --git a/src/vs/workbench/services/workingCopy/test/browser/resourceWorkingCopy.test.ts b/src/vs/workbench/services/workingCopy/test/browser/resourceWorkingCopy.test.ts index 758c611b6a5..b7f6501ab42 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/resourceWorkingCopy.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/resourceWorkingCopy.test.ts @@ -14,6 +14,7 @@ import { IRevertOptions, ISaveOptions } from 'vs/workbench/common/editor'; import { ResourceWorkingCopy } from 'vs/workbench/services/workingCopy/common/resourceWorkingCopy'; import { WorkingCopyCapabilities, IWorkingCopyBackup } from 'vs/workbench/services/workingCopy/common/workingCopy'; import { DisposableStore } from 'vs/base/common/lifecycle'; +import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; suite('ResourceWorkingCopy', function () { @@ -55,21 +56,23 @@ suite('ResourceWorkingCopy', function () { }); test('orphaned tracking', async () => { - assert.strictEqual(workingCopy.isOrphaned(), false); + runWithFakedTimers({}, async () => { + assert.strictEqual(workingCopy.isOrphaned(), false); - let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - accessor.fileService.notExistsSet.set(resource, true); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); + let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + accessor.fileService.notExistsSet.set(resource, true); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); - await onDidChangeOrphanedPromise; - assert.strictEqual(workingCopy.isOrphaned(), true); + await onDidChangeOrphanedPromise; + assert.strictEqual(workingCopy.isOrphaned(), true); - onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - accessor.fileService.notExistsSet.delete(resource); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false)); + onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + accessor.fileService.notExistsSet.delete(resource); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false)); - await onDidChangeOrphanedPromise; - assert.strictEqual(workingCopy.isOrphaned(), false); + await onDidChangeOrphanedPromise; + assert.strictEqual(workingCopy.isOrphaned(), false); + }); }); diff --git a/src/vs/workbench/services/workingCopy/test/browser/storedFileWorkingCopy.test.ts b/src/vs/workbench/services/workingCopy/test/browser/storedFileWorkingCopy.test.ts index 0fce73b2ce7..4dc6c6e598b 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/storedFileWorkingCopy.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/storedFileWorkingCopy.test.ts @@ -17,6 +17,7 @@ import { FileChangesEvent, FileChangeType, FileOperationError, FileOperationResu import { SaveReason, SaveSourceRegistry } from 'vs/workbench/common/editor'; import { Promises } from 'vs/base/common/async'; import { consumeReadable, consumeStream, isReadableStream } from 'vs/base/common/stream'; +import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler'; export class TestStoredFileWorkingCopyModel extends Disposable implements IStoredFileWorkingCopyModel { @@ -126,21 +127,23 @@ suite('StoredFileWorkingCopy', function () { }); test('orphaned tracking', async () => { - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); + runWithFakedTimers({}, async () => { + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); - let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - accessor.fileService.notExistsSet.set(resource, true); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); + let onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + accessor.fileService.notExistsSet.set(resource, true); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); - await onDidChangeOrphanedPromise; - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); + await onDidChangeOrphanedPromise; + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - accessor.fileService.notExistsSet.delete(resource); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false)); + onDidChangeOrphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + accessor.fileService.notExistsSet.delete(resource); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.ADDED }], false)); - await onDidChangeOrphanedPromise; - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); + await onDidChangeOrphanedPromise; + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); + }); }); test('dirty', async () => { @@ -294,56 +297,60 @@ suite('StoredFileWorkingCopy', function () { }); test('resolve (with backup, preserves metadata and orphaned state)', async () => { - await workingCopy.resolve({ contents: bufferToStream(VSBuffer.fromString('hello backup')) }); + runWithFakedTimers({}, async () => { + await workingCopy.resolve({ contents: bufferToStream(VSBuffer.fromString('hello backup')) }); - const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - accessor.fileService.notExistsSet.set(resource, true); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); + accessor.fileService.notExistsSet.set(resource, true); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); - await orphanedPromise; - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); + await orphanedPromise; + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - const backup = await workingCopy.backup(CancellationToken.None); - await accessor.workingCopyBackupService.backup(workingCopy, backup.content, undefined, backup.meta); + const backup = await workingCopy.backup(CancellationToken.None); + await accessor.workingCopyBackupService.backup(workingCopy, backup.content, undefined, backup.meta); - assert.strictEqual(accessor.workingCopyBackupService.hasBackupSync(workingCopy), true); + assert.strictEqual(accessor.workingCopyBackupService.hasBackupSync(workingCopy), true); - workingCopy.dispose(); + workingCopy.dispose(); - workingCopy = createWorkingCopy(); - await workingCopy.resolve(); + workingCopy = createWorkingCopy(); + await workingCopy.resolve(); - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - const backup2 = await workingCopy.backup(CancellationToken.None); - assert.deepStrictEqual(backup.meta, backup2.meta); + const backup2 = await workingCopy.backup(CancellationToken.None); + assert.deepStrictEqual(backup.meta, backup2.meta); + }); }); test('resolve (updates orphaned state accordingly)', async () => { - await workingCopy.resolve(); - - const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - - accessor.fileService.notExistsSet.set(resource, true); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); - - await orphanedPromise; - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - - // resolving clears orphaned state when successful - accessor.fileService.notExistsSet.delete(resource); - await workingCopy.resolve({ forceReadFromFile: true }); - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); - - // resolving adds orphaned state when fail to read - try { - accessor.fileService.readShouldThrowError = new FileOperationError('file not found', FileOperationResult.FILE_NOT_FOUND); + runWithFakedTimers({}, async () => { await workingCopy.resolve(); + + const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + + accessor.fileService.notExistsSet.set(resource, true); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); + + await orphanedPromise; assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - } finally { - accessor.fileService.readShouldThrowError = undefined; - } + + // resolving clears orphaned state when successful + accessor.fileService.notExistsSet.delete(resource); + await workingCopy.resolve({ forceReadFromFile: true }); + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); + + // resolving adds orphaned state when fail to read + try { + accessor.fileService.readShouldThrowError = new FileOperationError('file not found', FileOperationResult.FILE_NOT_FOUND); + await workingCopy.resolve(); + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); + } finally { + accessor.fileService.readShouldThrowError = undefined; + } + }); }); test('resolve (FILE_NOT_MODIFIED_SINCE can be handled for resolved working copies)', async () => { @@ -573,32 +580,34 @@ suite('StoredFileWorkingCopy', function () { }); test('save (no errors) - save clears orphaned', async () => { - let savedCounter = 0; - workingCopy.onDidSave(e => { - savedCounter++; + runWithFakedTimers({}, async () => { + let savedCounter = 0; + workingCopy.onDidSave(e => { + savedCounter++; + }); + + let saveErrorCounter = 0; + workingCopy.onDidSaveError(() => { + saveErrorCounter++; + }); + + await workingCopy.resolve(); + + // save clears orphaned + const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); + + accessor.fileService.notExistsSet.set(resource, true); + accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); + + await orphanedPromise; + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); + + await workingCopy.save({ force: true }); + assert.strictEqual(savedCounter, 1); + assert.strictEqual(saveErrorCounter, 0); + assert.strictEqual(workingCopy.isDirty(), false); + assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); }); - - let saveErrorCounter = 0; - workingCopy.onDidSaveError(() => { - saveErrorCounter++; - }); - - await workingCopy.resolve(); - - // save clears orphaned - const orphanedPromise = Event.toPromise(workingCopy.onDidChangeOrphaned); - - accessor.fileService.notExistsSet.set(resource, true); - accessor.fileService.fireFileChanges(new FileChangesEvent([{ resource, type: FileChangeType.DELETED }], false)); - - await orphanedPromise; - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), true); - - await workingCopy.save({ force: true }); - assert.strictEqual(savedCounter, 1); - assert.strictEqual(saveErrorCounter, 0); - assert.strictEqual(workingCopy.isDirty(), false); - assert.strictEqual(workingCopy.hasState(StoredFileWorkingCopyState.ORPHAN), false); }); test('save (errors)', async () => { diff --git a/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryService.test.ts b/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryService.test.ts index 6bf694104dd..169bbdfc8bf 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryService.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryService.test.ts @@ -30,12 +30,12 @@ import { firstOrDefault } from 'vs/base/common/arrays'; class TestWorkbenchEnvironmentService extends NativeWorkbenchEnvironmentService { - constructor(private readonly testDir: string) { - super({ ...TestNativeWindowConfiguration, 'user-data-dir': testDir }, TestProductService); + constructor(private readonly testDir: URI | string) { + super({ ...TestNativeWindowConfiguration, 'user-data-dir': URI.isUri(testDir) ? testDir.fsPath : testDir }, TestProductService); } override get localHistoryHome() { - return joinPath(URI.file(this.testDir), 'History'); + return joinPath(URI.isUri(this.testDir) ? this.testDir : URI.file(this.testDir), 'History'); } } @@ -45,7 +45,7 @@ export class TestWorkingCopyHistoryService extends NativeWorkingCopyHistoryServi readonly _configurationService: TestConfigurationService; readonly _lifecycleService: TestLifecycleService; - constructor(testDir: string) { + constructor(testDir: URI | string) { const environmentService = new TestWorkbenchEnvironmentService(testDir); const logService = new NullLogService(); const fileService = new FileService(logService); diff --git a/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryTracker.test.ts b/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryTracker.test.ts index 945f8e4b6ab..931ac6cbaef 100644 --- a/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryTracker.test.ts +++ b/src/vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryTracker.test.ts @@ -5,12 +5,10 @@ import * as assert from 'assert'; import { Event } from 'vs/base/common/event'; -import { flakySuite } from 'vs/base/test/common/testUtils'; import { TestContextService, TestWorkingCopy } from 'vs/workbench/test/common/workbenchTestServices'; -import { getRandomTestPath } from 'vs/base/test/node/testUtils'; +import { randomPath } from 'vs/base/common/extpath'; import { tmpdir } from 'os'; import { join } from 'vs/base/common/path'; -import { Promises } from 'vs/base/node/pfs'; import { URI } from 'vs/base/common/uri'; import { TestWorkingCopyHistoryService } from 'vs/workbench/services/workingCopy/test/electron-browser/workingCopyHistoryService.test'; import { WorkingCopyHistoryTracker } from 'vs/workbench/services/workingCopy/common/workingCopyHistoryTracker'; @@ -28,22 +26,26 @@ import { TestNotificationService } from 'vs/platform/notification/test/common/te import { CancellationToken } from 'vs/base/common/cancellation'; import { IWorkingCopyHistoryEntry, IWorkingCopyHistoryEntryDescriptor } from 'vs/workbench/services/workingCopy/common/workingCopyHistory'; import { assertIsDefined } from 'vs/base/common/types'; +import { VSBuffer } from 'vs/base/common/buffer'; +import { InMemoryFileSystemProvider } from 'vs/platform/files/common/inMemoryFilesystemProvider'; +import { IDisposable } from 'vs/base/common/lifecycle'; -flakySuite('WorkingCopyHistoryTracker', () => { +suite('WorkingCopyHistoryTracker', () => { - let testDir: string; - let historyHome: string; - let workHome: string; + let testDir: URI; + let historyHome: URI; + let workHome: URI; let workingCopyHistoryService: TestWorkingCopyHistoryService; let workingCopyService: WorkingCopyService; let fileService: IFileService; let configurationService: TestConfigurationService; + let inMemoryFileSystemDisposable: IDisposable; let tracker: WorkingCopyHistoryTracker; - let testFile1Path: string; - let testFile2Path: string; + let testFile1Path: URI; + let testFile2Path: URI; const testFile1PathContents = 'Hello Foo'; const testFile2PathContents = [ @@ -65,25 +67,27 @@ flakySuite('WorkingCopyHistoryTracker', () => { } setup(async () => { - testDir = getRandomTestPath(tmpdir(), 'vsctests', 'workingcopyhistorytracker'); - historyHome = join(testDir, 'User', 'History'); - workHome = join(testDir, 'work'); + testDir = URI.file(randomPath(join(tmpdir(), 'vsctests', 'workingcopyhistorytracker'))).with({ scheme: Schemas.inMemory }); + historyHome = joinPath(testDir, 'User', 'History'); + workHome = joinPath(testDir, 'work'); workingCopyHistoryService = new TestWorkingCopyHistoryService(testDir); workingCopyService = new WorkingCopyService(); fileService = workingCopyHistoryService._fileService; configurationService = workingCopyHistoryService._configurationService; + inMemoryFileSystemDisposable = fileService.registerProvider(Schemas.inMemory, new InMemoryFileSystemProvider()); + tracker = createTracker(); - await Promises.mkdir(historyHome, { recursive: true }); - await Promises.mkdir(workHome, { recursive: true }); + await fileService.createFolder(historyHome); + await fileService.createFolder(workHome); - testFile1Path = join(workHome, 'foo.txt'); - testFile2Path = join(workHome, 'bar.txt'); + testFile1Path = joinPath(workHome, 'foo.txt'); + testFile2Path = joinPath(workHome, 'bar.txt'); - await Promises.writeFile(testFile1Path, testFile1PathContents); - await Promises.writeFile(testFile2Path, testFile2PathContents); + await fileService.writeFile(testFile1Path, VSBuffer.fromString(testFile1PathContents)); + await fileService.writeFile(testFile2Path, VSBuffer.fromString(testFile2PathContents)); }); function createTracker() { @@ -99,17 +103,19 @@ flakySuite('WorkingCopyHistoryTracker', () => { ); } - teardown(() => { + teardown(async () => { workingCopyHistoryService.dispose(); workingCopyService.dispose(); tracker.dispose(); - return Promises.rm(testDir); + await fileService.del(testDir, { recursive: true }); + + inMemoryFileSystemDisposable.dispose(); }); test('history entry added on save', async () => { - const workingCopy1 = new TestWorkingCopy(URI.file(testFile1Path)); - const workingCopy2 = new TestWorkingCopy(URI.file(testFile2Path)); + const workingCopy1 = new TestWorkingCopy(testFile1Path); + const workingCopy2 = new TestWorkingCopy(testFile2Path); const stat1 = await fileService.resolve(workingCopy1.resource, { resolveMetadata: true }); const stat2 = await fileService.resolve(workingCopy2.resource, { resolveMetadata: true }); @@ -136,7 +142,7 @@ flakySuite('WorkingCopyHistoryTracker', () => { }); test('history entry skipped when setting disabled (globally)', async () => { - configurationService.setUserConfiguration('workbench.localHistory.enabled', false, URI.file(testFile1Path)); + configurationService.setUserConfiguration('workbench.localHistory.enabled', false, testFile1Path); return assertNoLocalHistoryEntryAddedWithSettingsConfigured(); }); @@ -152,14 +158,14 @@ flakySuite('WorkingCopyHistoryTracker', () => { }); test('history entry skipped when too large', async () => { - configurationService.setUserConfiguration('workbench.localHistory.maxFileSize', 0, URI.file(testFile1Path)); + configurationService.setUserConfiguration('workbench.localHistory.maxFileSize', 0, testFile1Path); return assertNoLocalHistoryEntryAddedWithSettingsConfigured(); }); async function assertNoLocalHistoryEntryAddedWithSettingsConfigured(): Promise { - const workingCopy1 = new TestWorkingCopy(URI.file(testFile1Path)); - const workingCopy2 = new TestWorkingCopy(URI.file(testFile2Path)); + const workingCopy1 = new TestWorkingCopy(testFile1Path); + const workingCopy2 = new TestWorkingCopy(testFile2Path); const stat1 = await fileService.resolve(workingCopy1.resource, { resolveMetadata: true }); const stat2 = await fileService.resolve(workingCopy2.resource, { resolveMetadata: true }); @@ -187,7 +193,7 @@ flakySuite('WorkingCopyHistoryTracker', () => { test('entries moved (file rename)', async () => { const entriesMoved = Event.toPromise(workingCopyHistoryService.onDidMoveEntries); - const workingCopy = new TestWorkingCopy(URI.file(testFile1Path)); + const workingCopy = new TestWorkingCopy(testFile1Path); const entry1 = await addEntry({ resource: workingCopy.resource, source: 'test-source' }, CancellationToken.None); const entry2 = await addEntry({ resource: workingCopy.resource, source: 'test-source' }, CancellationToken.None); @@ -233,8 +239,8 @@ flakySuite('WorkingCopyHistoryTracker', () => { test('entries moved (folder rename)', async () => { const entriesMoved = Event.toPromise(workingCopyHistoryService.onDidMoveEntries); - const workingCopy1 = new TestWorkingCopy(URI.file(testFile1Path)); - const workingCopy2 = new TestWorkingCopy(URI.file(testFile2Path)); + const workingCopy1 = new TestWorkingCopy(testFile1Path); + const workingCopy2 = new TestWorkingCopy(testFile2Path); const entry1A = await addEntry({ resource: workingCopy1.resource, source: 'test-source' }, CancellationToken.None); const entry2A = await addEntry({ resource: workingCopy1.resource, source: 'test-source' }, CancellationToken.None); @@ -250,8 +256,8 @@ flakySuite('WorkingCopyHistoryTracker', () => { entries = await workingCopyHistoryService.getEntries(workingCopy2.resource, CancellationToken.None); assert.strictEqual(entries.length, 3); - const renamedWorkHome = joinPath(dirname(URI.file(workHome)), 'renamed'); - await workingCopyHistoryService._fileService.move(URI.file(workHome), renamedWorkHome); + const renamedWorkHome = joinPath(dirname(testDir), 'renamed'); + await workingCopyHistoryService._fileService.move(workHome, renamedWorkHome); const renamedWorkingCopy1Resource = joinPath(renamedWorkHome, basename(workingCopy1.resource)); const renamedWorkingCopy2Resource = joinPath(renamedWorkHome, basename(workingCopy2.resource));