Eric Snow (ericsnowcurrently) · Jan 21, 2020 · Dec 31, 2019 · Dec 31, 2019 · Dec 31, 2019 · Jan 9, 2020
diff --git 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<TemporaryFile> {
-        return new Promise<TemporaryFile>((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<boolean> {

diff --git a/src/client/common/platform/fs-temp.ts 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<TemporaryFile> {
+        const opts = {
+            postfix: suffix
+        };
+        return new Promise<TemporaryFile>((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
@@ -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<TemporaryFile>;
+}
+
 //===========================
 // FS paths
 

diff --git 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<TemporaryFile> {
-                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
@@ -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<TemporaryFile> {
+            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
@@ -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<IDeps>;
+    let temp: TemporaryFileSystem;
+    setup(() => {
+        deps = TypeMoq.Mock.ofType<IDeps>(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();
+        });
+    });
+});