diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 462c6f737fa1..04c9e8b212cc 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -7,11 +7,11 @@ import * as fileSystem from 'fs'; import * as fs from 'fs-extra'; import * as glob from 'glob'; import { inject, injectable } from 'inversify'; -import * as tmp from 'tmp'; import { promisify } from 'util'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; import { FileSystemPaths, FileSystemPathUtils } from './fs-paths'; +import { TemporaryFileSystem } from './fs-temp'; // prettier-ignore import { FileStat, FileType, @@ -85,6 +85,7 @@ export function convertStat(old: fs.Stats, filetype: FileType): FileStat { export class FileSystem implements IFileSystem { private readonly paths: IFileSystemPaths; private readonly pathUtils: FileSystemPathUtils; + private readonly tmp: TemporaryFileSystem; // prettier-ignore constructor( @inject(IPlatformService) platformService: IPlatformService @@ -94,6 +95,7 @@ export class FileSystem implements IFileSystem { platformService.isWindows ); this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); + this.tmp = TemporaryFileSystem.withDefaults(); } //================================= @@ -307,14 +309,7 @@ export class FileSystem implements IFileSystem { } public createTemporaryFile(extension: string): Promise { - return new Promise((resolve, reject) => { - tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => { - if (err) { - return reject(err); - } - resolve({ filePath: tmpFile, dispose: cleanupCallback }); - }); - }); + return this.tmp.createFile(extension); } public async isDirReadonly(dirname: string): Promise { diff --git a/src/client/common/platform/fs-temp.ts b/src/client/common/platform/fs-temp.ts new file mode 100644 index 000000000000..ed9b48e8214c --- /dev/null +++ b/src/client/common/platform/fs-temp.ts @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as tmp from 'tmp'; +import { ITempFileSystem, TemporaryFile } from './types'; + +interface IRawTempFS { + // tslint:disable-next-line:no-any + file(config: tmp.Options, callback?: (err: any, path: string, fd: number, cleanupCallback: () => void) => void): void; +} + +// Operations related to temporary files and directories. +export class TemporaryFileSystem implements ITempFileSystem { + // prettier-ignore + constructor( + private readonly raw: IRawTempFS + ) { } + public static withDefaults(): TemporaryFileSystem { + // prettier-ignore + return new TemporaryFileSystem( + tmp + ); + } + + // Create a new temp file with the given filename suffix. + public createFile(suffix: string): Promise { + const opts = { + postfix: suffix + }; + return new Promise((resolve, reject) => { + this.raw.file(opts, (err, filename, _fd, cleanUp) => { + if (err) { + return reject(err); + } + resolve({ + filePath: filename, + dispose: cleanUp + }); + }); + }); + } +} diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index c0541fb8298c..bd3cbda012cc 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -47,6 +47,10 @@ export interface IPlatformService { export type TemporaryFile = { filePath: string } & vscode.Disposable; export type TemporaryDirectory = { path: string } & vscode.Disposable; +export interface ITempFileSystem { + createFile(suffix: string): Promise; +} + //=========================== // FS paths diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index bb8a07a141c7..d964796562a6 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -9,7 +9,7 @@ import * as path from 'path'; import { convertStat, FileSystem } from '../../../client/common/platform/fileSystem'; import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; import { PlatformService } from '../../../client/common/platform/platformService'; -import { FileType, TemporaryFile } from '../../../client/common/platform/types'; +import { FileType } from '../../../client/common/platform/types'; import { sleep } from '../../../client/common/utils/async'; // prettier-ignore import { @@ -1127,63 +1127,15 @@ suite('FileSystem', () => { }); }); - suite('createTemporaryFile', () => { - async function createTemporaryFile(suffix: string): Promise { - const tempfile = await fileSystem.createTemporaryFile(suffix); - fix.addFSCleanup(tempfile.filePath, tempfile.dispose); - return tempfile; - } + suite('createFile', () => { + // tested fully in the TemporaryFileSystem tests. - test('TemporaryFile is created properly', async () => { + test('calls wrapped object', async () => { const tempfile = await fileSystem.createTemporaryFile('.tmp'); fix.addFSCleanup(tempfile.filePath, tempfile.dispose); await assertExists(tempfile.filePath); - expect(tempfile.filePath.endsWith('.tmp')).to.equal(true, `bad suffix on ${tempfile.filePath}`); - }); - - test('TemporaryFile is disposed properly', async () => { - const tempfile = await createTemporaryFile('.tmp'); - await assertExists(tempfile.filePath); - - tempfile.dispose(); - - await assertDoesNotExist(tempfile.filePath); - }); - - test('Ensure creating a temporary file results in a unique temp file path', async () => { - const tempFile = await createTemporaryFile('.tmp'); - const tempFile2 = await createTemporaryFile('.tmp'); - - const filename1 = tempFile.filePath; - const filename2 = tempFile2.filePath; - - expect(filename1).to.not.equal(filename2); - }); - - test('Ensure writing to a temp file is supported via file stream', async function() { - if (WINDOWS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - const tempfile = await createTemporaryFile('.tmp'); - const stream = fileSystem.createWriteStream(tempfile.filePath); - fix.addCleanup(() => stream.destroy()); - const data = '...'; - - stream.write(data, 'utf8'); - - const actual = await fs.readFile(tempfile.filePath, 'utf8'); - expect(actual).to.equal(data); - }); - - test('Ensure chmod works against a temporary file', async () => { - // Note that on Windows chmod is a noop. - const tempfile = await createTemporaryFile('.tmp'); - - const promise = fs.chmod(tempfile.filePath, '7777'); - - await expect(promise).to.not.eventually.be.rejected; + expect(tempfile.filePath.endsWith('.tmp')).to.equal(true); }); }); diff --git a/src/test/common/platform/fs-temp.functional.test.ts b/src/test/common/platform/fs-temp.functional.test.ts new file mode 100644 index 000000000000..c29a5461d2f4 --- /dev/null +++ b/src/test/common/platform/fs-temp.functional.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-func-body-length chai-vague-errors + +import { expect } from 'chai'; +import * as fs from 'fs-extra'; +import { TemporaryFileSystem } from '../../../client/common/platform/fs-temp'; +import { TemporaryFile } from '../../../client/common/platform/types'; +// prettier-ignore +import { + assertDoesNotExist, assertExists, FSFixture, WINDOWS +} from './utils'; + +suite('FileSystem - TemporaryFileSystem', () => { + let tmpfs: TemporaryFileSystem; + let fix: FSFixture; + setup(async () => { + tmpfs = TemporaryFileSystem.withDefaults(); + fix = new FSFixture(); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('createFile', () => { + async function createFile(suffix: string): Promise { + const tempfile = await tmpfs.createFile(suffix); + fix.addFSCleanup(tempfile.filePath, tempfile.dispose); + return tempfile; + } + + test('TemporaryFile is created properly', async () => { + const tempfile = await tmpfs.createFile('.tmp'); + fix.addFSCleanup(tempfile.filePath, tempfile.dispose); + await assertExists(tempfile.filePath); + + expect(tempfile.filePath.endsWith('.tmp')).to.equal(true, `bad suffix on ${tempfile.filePath}`); + }); + + test('TemporaryFile is disposed properly', async () => { + const tempfile = await createFile('.tmp'); + await assertExists(tempfile.filePath); + + tempfile.dispose(); + + await assertDoesNotExist(tempfile.filePath); + }); + + test('Ensure creating a temporary file results in a unique temp file path', async () => { + const tempFile = await createFile('.tmp'); + const tempFile2 = await createFile('.tmp'); + + const filename1 = tempFile.filePath; + const filename2 = tempFile2.filePath; + + expect(filename1).to.not.equal(filename2); + }); + + test('Ensure writing to a temp file is supported via file stream', async function() { + if (WINDOWS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const tempfile = await createFile('.tmp'); + const stream = fs.createWriteStream(tempfile.filePath); + fix.addCleanup(() => stream.destroy()); + const data = '...'; + + stream.write(data, 'utf8'); + + const actual = await fs.readFile(tempfile.filePath, 'utf8'); + expect(actual).to.equal(data); + }); + + test('Ensure chmod works against a temporary file', async () => { + // Note that on Windows chmod is a noop. + const tempfile = await createFile('.tmp'); + + const promise = fs.chmod(tempfile.filePath, '7777'); + + await expect(promise).to.not.eventually.be.rejected; + }); + }); +}); diff --git a/src/test/common/platform/fs-temp.unit.test.ts b/src/test/common/platform/fs-temp.unit.test.ts new file mode 100644 index 000000000000..e687bae90e0d --- /dev/null +++ b/src/test/common/platform/fs-temp.unit.test.ts @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-func-body-length + +import { expect } from 'chai'; +import * as TypeMoq from 'typemoq'; +import { TemporaryFileSystem } from '../../../client/common/platform/fs-temp'; + +interface IDeps { + // tmp module + // tslint:disable-next-line:no-any + file(config: { postfix?: string }, callback?: (err: any, path: string, fd: number, cleanupCallback: () => void) => void): void; +} + +suite('FileSystem - temp files', () => { + let deps: TypeMoq.IMock; + let temp: TemporaryFileSystem; + setup(() => { + deps = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + temp = new TemporaryFileSystem(deps.object); + }); + function verifyAll() { + deps.verifyAll(); + } + + suite('createFile', () => { + test(`fails if the raw call fails`, async () => { + const failure = new Error('oops'); + // prettier-ignore + deps.setup(d => d.file({ postfix: '.tmp' }, TypeMoq.It.isAny())) + .throws(failure); + + const promise = temp.createFile('.tmp'); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + + test(`fails if the raw call "returns" an error`, async () => { + const failure = new Error('oops'); + deps.setup(d => d.file({ postfix: '.tmp' }, TypeMoq.It.isAny())) + // tslint:disable-next-line:no-empty + .callback((_cfg, cb) => cb(failure, '...', -1, () => {})); + + const promise = temp.createFile('.tmp'); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); +});