From 0eec1f372310ed5faee57a3938501adab644b7e3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 21 Nov 2019 16:30:24 -0700 Subject: [PATCH 01/58] Get some errant system tests working with the system Python on Ubuntu. --- .../workspaceVirtualEnvService.test.ts | 144 ++++++++++++++---- 1 file changed, 111 insertions(+), 33 deletions(-) diff --git a/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts b/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts index f5f0f9acd36b..76d8af54a136 100644 --- a/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts +++ b/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts @@ -5,10 +5,13 @@ // tslint:disable:no-any max-classes-per-file max-func-body-length no-invalid-this import { expect } from 'chai'; -import { exec } from 'child_process'; +import { exec, ExecOptions } from 'child_process'; import * as path from 'path'; +import { promisify } from 'util'; import { Uri } from 'vscode'; import '../../../client/common/extensions'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { PlatformService } from '../../../client/common/platform/platformService'; import { createDeferredFromPromise, Deferred } from '../../../client/common/utils/async'; import { StopWatch } from '../../../client/common/utils/stopWatch'; import { @@ -18,21 +21,116 @@ import { } from '../../../client/interpreter/contracts'; import { WorkspaceVirtualEnvWatcherService } from '../../../client/interpreter/locators/services/workspaceVirtualEnvWatcherService'; import { IServiceContainer } from '../../../client/ioc/types'; +import { IS_CI_SERVER } from '../../ciConstants'; import { deleteFiles, getOSType, isPythonVersionInProcess, OSType, PYTHON_PATH, rootWorkspaceUri, waitForCondition } from '../../common'; import { IS_MULTI_ROOT_TEST } from '../../constants'; import { sleep } from '../../core'; import { initialize, multirootPath } from '../../initialize'; -const timeoutMs = 60_000; +const execAsync = promisify(exec); + +class Venvs { + private readonly prefix = '.venv-'; + private readonly python = PYTHON_PATH.fileToCommandArgument(); + private readonly fullPrefix: string; + private readonly procEnv: ExecOptions; + private pipInstaller?: string; + constructor ( + private readonly topDir: string + ) { + this.fullPrefix = path.join(this.topDir, this.prefix); + this.procEnv = { + cwd: this.topDir + }; + } + + public async create(id: string): Promise { + // Ensure env is unique to avoid conflicts in tests (corrupting + // test data). + const timestamp = new Date().getTime().toString(); + const root = `${this.fullPrefix}${id}-${timestamp}`; + let argv = [ + this.python, '-m', 'venv', + root + ]; + + try { + try { + await this.run(argv); + } catch (err) { + if (!`${err}`.includes('ensurepip') && getOSType() !== OSType.Linux) { + throw err; // re-throw + } + if (IS_CI_SERVER) { + throw err; // re-throw + } + argv = [ + this.python, '-m', 'venv', + '--system-site-packages', + '--without-pip', + root + ]; + await this.run(argv); + await this.installPip(root); + } + } catch (err2) { + throw Error(`command failed ${root}, ${this.python}, Error: ${err2}`); + } + + return root; + } + + public async cleanUp() { + await deleteFiles(`${this.fullPrefix}*`); + if (this.pipInstaller) { + await deleteFiles(this.pipInstaller); + this.pipInstaller = undefined; + } + } + + private async installPip(root: string) { + const script = this.pipInstaller + ? this.pipInstaller + : path.join(this.topDir, 'get-pip.py'); + if (!this.pipInstaller) { + const fs = new FileSystem(new PlatformService()); + if (!await fs.fileExists(script)) { + await this.run([ + 'curl', + 'https://bootstrap.pypa.io/get-pip.py', + '-o', script + ]); + } + this.pipInstaller = script; + } + await this.run([ + path.join(root, 'bin', 'python'), + script + ]); + } + + private async run(argv: string[]) { + const cmdline = argv.join(' '); + const { stderr } = await execAsync(cmdline, this.procEnv); + if (stderr && stderr.length > 0) { + throw Error(stderr); + } + } +} + +const timeoutMs = IS_CI_SERVER ? 60_000 : 15_000; suite('Interpreters - Workspace VirtualEnv Service', function() { this.timeout(timeoutMs); this.retries(0); - let locator: IInterpreterLocatorService; - const workspaceUri = IS_MULTI_ROOT_TEST ? Uri.file(path.join(multirootPath, 'workspace3')) : rootWorkspaceUri!; + const workspaceUri = IS_MULTI_ROOT_TEST + ? Uri.file(path.join(multirootPath, 'workspace3')) + : rootWorkspaceUri!; + const venvs = new Venvs(workspaceUri.fsPath); const workspace4 = Uri.file(path.join(multirootPath, 'workspace4')); - const venvPrefix = '.venv'; + let serviceContainer: IServiceContainer; + let locator: IInterpreterLocatorService; async function manuallyTriggerFSWatcher(deferred: Deferred) { // Monitoring files on virtualized environments can be finicky... @@ -64,26 +162,6 @@ suite('Interpreters - Workspace VirtualEnv Service', function() { manuallyTriggerFSWatcher(deferred).ignoreErrors(); await deferred.promise; } - async function createVirtualEnvironment(envSuffix: string) { - // Ensure env is random to avoid conflicts in tests (currupting test data). - const envName = `${venvPrefix}${envSuffix}${new Date().getTime().toString()}`; - return new Promise((resolve, reject) => { - exec( - `${PYTHON_PATH.fileToCommandArgument()} -m venv ${envName}`, - { cwd: workspaceUri.fsPath }, - (ex, _, stderr) => { - if (ex) { - return reject(ex); - } - if (stderr && stderr.length > 0) { - reject(new Error(`Failed to create Env ${envName}, ${PYTHON_PATH}, Error: ${stderr}`)); - } else { - resolve(envName); - } - } - ); - }); - } suiteSetup(async function() { // skip for Python < 3, no venv support @@ -98,24 +176,24 @@ suite('Interpreters - Workspace VirtualEnv Service', function() { ); // This test is required, we need to wait for interpreter listing completes, // before proceeding with other tests. - await deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`)); + await venvs.cleanUp(); await locator.getInterpreters(workspaceUri); }); - suiteTeardown(async () => deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`))); - teardown(async () => deleteFiles(path.join(workspaceUri.fsPath, `${venvPrefix}*`))); + suiteTeardown(venvs.cleanUp); + teardown(venvs.cleanUp); test('Detect Virtual Environment', async () => { - const envName = await createVirtualEnvironment('one'); + const envName = await venvs.create('one'); await waitForInterpreterToBeDetected(envName); }); test('Detect a new Virtual Environment', async () => { - const env1 = await createVirtualEnvironment('first'); + const env1 = await venvs.create('first'); await waitForInterpreterToBeDetected(env1); // Ensure second environment in our workspace folder is detected when created. - const env2 = await createVirtualEnvironment('second'); + const env2 = await venvs.create('second'); await waitForInterpreterToBeDetected(env2); }); @@ -128,8 +206,8 @@ suite('Interpreters - Workspace VirtualEnv Service', function() { expect(items4).to.be.lengthOf(0); const [env1, env2] = await Promise.all([ - createVirtualEnvironment('first3'), - createVirtualEnvironment('second3') + venvs.create('first3'), + venvs.create('second3') ]); await Promise.all([waitForInterpreterToBeDetected(env1), waitForInterpreterToBeDetected(env2)]); From 72b225496dbea325a6797ff3da8084fa915e5728 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 22 Oct 2019 15:47:30 -0600 Subject: [PATCH 02/58] Do not depend on IFileSystem in the old debug adapter. --- src/client/debugger/debugAdapter/main.ts | 10 ++++++---- src/client/debugger/debugAdapter/serviceRegistry.ts | 4 +--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/client/debugger/debugAdapter/main.ts b/src/client/debugger/debugAdapter/main.ts index 0403dc5c7905..c078ee92bb8e 100644 --- a/src/client/debugger/debugAdapter/main.ts +++ b/src/client/debugger/debugAdapter/main.ts @@ -9,6 +9,7 @@ if ((Reflect as any).metadata === undefined) { require('reflect-metadata'); } +import * as fsextra from 'fs-extra'; import { Socket } from 'net'; import { EOL } from 'os'; import * as path from 'path'; @@ -20,7 +21,6 @@ import { DebugProtocol } from 'vscode-debugprotocol'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import '../../common/extensions'; import { isNotInstalledError } from '../../common/helpers'; -import { IFileSystem } from '../../common/platform/types'; import { ICurrentProcess, IDisposable, IDisposableRegistry } from '../../common/types'; import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import { noop } from '../../common/utils/misc'; @@ -47,7 +47,10 @@ export class PythonDebugger extends DebugSession { public debugServer?: BaseDebugServer; public client = createDeferred(); private supportsRunInTerminalRequest: boolean = false; - constructor(private readonly serviceContainer: IServiceContainer) { + constructor( + private readonly serviceContainer: IServiceContainer, + private readonly fileExistsSync = fsextra.existsSync + ) { super(false); } public shutdown(): void { @@ -106,8 +109,7 @@ export class PythonDebugger extends DebugSession { } protected launchRequest(response: DebugProtocol.LaunchResponse, args: LaunchRequestArguments): void { - const fs = this.serviceContainer.get(IFileSystem); - if ((typeof args.module !== 'string' || args.module.length === 0) && args.program && !fs.fileExistsSync(args.program)) { + if ((typeof args.module !== 'string' || args.module.length === 0) && args.program && !this.fileExistsSync(args.program)) { return this.sendErrorResponse(response, { format: `File does not exist. "${args.program}"`, id: 1 }, undefined, undefined, ErrorDestination.User); } diff --git a/src/client/debugger/debugAdapter/serviceRegistry.ts b/src/client/debugger/debugAdapter/serviceRegistry.ts index 2c015eb99297..24a93b9f8632 100644 --- a/src/client/debugger/debugAdapter/serviceRegistry.ts +++ b/src/client/debugger/debugAdapter/serviceRegistry.ts @@ -5,9 +5,8 @@ import { Container } from 'inversify'; import { SocketServer } from '../../common/net/socket/socketServer'; -import { FileSystem } from '../../common/platform/fileSystem'; import { PlatformService } from '../../common/platform/platformService'; -import { IFileSystem, IPlatformService } from '../../common/platform/types'; +import { IPlatformService } from '../../common/platform/types'; import { CurrentProcess } from '../../common/process/currentProcess'; import { BufferDecoder } from '../../common/process/decoder'; import { IBufferDecoder, IProcessServiceFactory } from '../../common/process/types'; @@ -37,7 +36,6 @@ function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDebugStreamProvider, DebugStreamProvider); serviceManager.addSingleton(IProtocolLogger, ProtocolLogger); serviceManager.add(IProtocolParser, ProtocolParser); - serviceManager.addSingleton(IFileSystem, FileSystem); serviceManager.addSingleton(IPlatformService, PlatformService); serviceManager.addSingleton(ISocketServer, SocketServer); serviceManager.addSingleton(IProtocolMessageWriter, ProtocolMessageWriter); From 8e3af6fb7c01f9fe9a971905025b69d33b5eb6e6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:20:31 -0600 Subject: [PATCH 03/58] Excise IPlatformService from the old debug adapter. --- src/client/debugger/debugAdapter/serviceRegistry.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/debugger/debugAdapter/serviceRegistry.ts b/src/client/debugger/debugAdapter/serviceRegistry.ts index 24a93b9f8632..0feccf1baa85 100644 --- a/src/client/debugger/debugAdapter/serviceRegistry.ts +++ b/src/client/debugger/debugAdapter/serviceRegistry.ts @@ -5,8 +5,6 @@ import { Container } from 'inversify'; import { SocketServer } from '../../common/net/socket/socketServer'; -import { PlatformService } from '../../common/platform/platformService'; -import { IPlatformService } from '../../common/platform/types'; import { CurrentProcess } from '../../common/process/currentProcess'; import { BufferDecoder } from '../../common/process/decoder'; import { IBufferDecoder, IProcessServiceFactory } from '../../common/process/types'; @@ -36,7 +34,6 @@ function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDebugStreamProvider, DebugStreamProvider); serviceManager.addSingleton(IProtocolLogger, ProtocolLogger); serviceManager.add(IProtocolParser, ProtocolParser); - serviceManager.addSingleton(IPlatformService, PlatformService); serviceManager.addSingleton(ISocketServer, SocketServer); serviceManager.addSingleton(IProtocolMessageWriter, ProtocolMessageWriter); serviceManager.addSingleton(IBufferDecoder, BufferDecoder); From 24abfc4a44ae913195e19671007e6680ea0ec5d4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 8 Oct 2019 13:28:55 -0600 Subject: [PATCH 04/58] Group the FileSystem methods by category (and drop 2 unused methods). --- src/client/common/platform/fileSystem.ts | 139 +++++++++--------- src/client/common/platform/types.ts | 27 ++-- .../interpreterSelector.unit.test.ts | 3 - 3 files changed, 85 insertions(+), 84 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index b1413e5b422d..b1ff11407a32 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -15,11 +15,10 @@ import { IFileSystem, IPlatformService, TemporaryFile } from './types'; @injectable() export class FileSystem implements IFileSystem { - constructor(@inject(IPlatformService) private platformService: IPlatformService) {} + constructor( + @inject(IPlatformService) private platformService: IPlatformService + ) { } - public get directorySeparatorChar(): string { - return path.sep; - } public async stat(filePath: string): Promise { // Do not import vscode directly, as this isn't available in the Debugger Context. // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. @@ -28,23 +27,13 @@ export class FileSystem implements IFileSystem { return vscode.workspace.fs.stat(vscode.Uri.file(filePath)); } - public objectExists(filePath: string, statCheck: (s: fs.Stats) => boolean): Promise { - return new Promise(resolve => { - fs.stat(filePath, (error, stats) => { - if (error) { - return resolve(false); - } - return resolve(statCheck(stats)); - }); - }); - } + //**************************** + // fs-extra - public fileExists(filePath: string): Promise { - return this.objectExists(filePath, stats => stats.isFile()); - } public fileExistsSync(filePath: string): boolean { return fs.existsSync(filePath); } + /** * Reads the contents of the file using utf8 and returns the string contents. * @param {string} filePath @@ -59,10 +48,6 @@ export class FileSystem implements IFileSystem { await fs.writeFile(filePath, data, options); } - public directoryExists(filePath: string): Promise { - return this.objectExists(filePath, stats => stats.isDirectory()); - } - public createDirectory(directoryPath: string): Promise { return fs.mkdirp(directoryPath); } @@ -73,6 +58,69 @@ export class FileSystem implements IFileSystem { return deferred.promise; } + public appendFileSync(filename: string, data: {}, encoding: string): void; + public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; + // tslint:disable-next-line:unified-signatures + public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; + public appendFileSync(filename: string, data: {}, optionsOrEncoding: {}): void { + return fs.appendFileSync(filename, data, optionsOrEncoding); + } + + public deleteFile(filename: string): Promise { + const deferred = createDeferred(); + fs.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); + return deferred.promise; + } + + //**************************** + // fs + + public createWriteStream(filePath: string): fileSystem.WriteStream { + return fileSystem.createWriteStream(filePath); + } + + public chmod(filePath: string, mode: string): Promise { + return new Promise((resolve, reject) => { + fileSystem.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { + if (err) { + return reject(err); + } + resolve(); + }); + }); + } + + //**************************** + // helpers + + public arePathsSame(path1: string, path2: string): boolean { + path1 = path.normalize(path1); + path2 = path.normalize(path2); + if (this.platformService.isWindows) { + return path1.toUpperCase() === path2.toUpperCase(); + } else { + return path1 === path2; + } + } + + public objectExists(filePath: string, statCheck: (s: fs.Stats) => boolean): Promise { + return new Promise(resolve => { + fs.stat(filePath, (error, stats) => { + if (error) { + return resolve(false); + } + return resolve(statCheck(stats)); + }); + }); + } + + public fileExists(filePath: string): Promise { + return this.objectExists(filePath, (stats) => stats.isFile()); + } + public directoryExists(filePath: string): Promise { + return this.objectExists(filePath, stats => stats.isDirectory()); + } + public getSubDirectories(rootDir: string): Promise { return new Promise(resolve => { fs.readdir(rootDir, async (error, files) => { @@ -106,32 +154,6 @@ export class FileSystem implements IFileSystem { }); } - public arePathsSame(path1: string, path2: string): boolean { - path1 = path.normalize(path1); - path2 = path.normalize(path2); - if (this.platformService.isWindows) { - return path1.toUpperCase() === path2.toUpperCase(); - } else { - return path1 === path2; - } - } - - public appendFileSync(filename: string, data: {}, encoding: string): void; - public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; - // tslint:disable-next-line:unified-signatures - public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; - public appendFileSync(filename: string, data: {}, optionsOrEncoding: {}): void { - return fs.appendFileSync(filename, data, optionsOrEncoding); - } - - public getRealPath(filePath: string): Promise { - return new Promise(resolve => { - fs.realpath(filePath, (err, realPath) => { - resolve(err ? filePath : realPath); - }); - }); - } - public copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); const rs = fs.createReadStream(src).on('error', err => { @@ -149,12 +171,6 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - public deleteFile(filename: string): Promise { - const deferred = createDeferred(); - fs.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); - return deferred.promise; - } - public getFileHash(filePath: string): Promise { return new Promise((resolve, reject) => { fs.lstat(filePath, (err, stats) => { @@ -169,6 +185,7 @@ export class FileSystem implements IFileSystem { }); }); } + public search(globPattern: string): Promise { return new Promise((resolve, reject) => { glob(globPattern, (ex, files) => { @@ -179,6 +196,7 @@ export class FileSystem implements IFileSystem { }); }); } + public createTemporaryFile(extension: string): Promise { return new Promise((resolve, reject) => { tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => { @@ -189,19 +207,4 @@ export class FileSystem implements IFileSystem { }); }); } - - public createWriteStream(filePath: string): fileSystem.WriteStream { - return fileSystem.createWriteStream(filePath); - } - - public chmod(filePath: string, mode: string): Promise { - return new Promise((resolve, reject) => { - fileSystem.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { - if (err) { - return reject(err); - } - resolve(); - }); - }); - } } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 586b5557a070..6bb8151272c5 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -37,29 +37,30 @@ export type TemporaryDirectory = { path: string } & Disposable; export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { - directorySeparatorChar: string; stat(filePath: string): Promise; - objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; - fileExists(path: string): Promise; + // fs-extra fileExistsSync(path: string): boolean; - directoryExists(path: string): Promise; - createDirectory(path: string): Promise; - deleteDirectory(path: string): Promise; - getSubDirectories(rootDir: string): Promise; - getFiles(rootDir: string): Promise; - arePathsSame(path1: string, path2: string): boolean; readFile(filePath: string): Promise; writeFile(filePath: string, data: {}, options?: string | fsextra.WriteFileOptions): Promise; + createDirectory(path: string): Promise; + deleteDirectory(path: string): Promise; appendFileSync(filename: string, data: {}, encoding: string): void; appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; // tslint:disable-next-line:unified-signatures appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; - getRealPath(path: string): Promise; - copyFile(src: string, dest: string): Promise; deleteFile(filename: string): Promise; + // fs + createWriteStream(path: string): fs.WriteStream; + chmod(path: string, mode: string): Promise; + // helpers + arePathsSame(path1: string, path2: string): boolean; + objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; // XXX drop + fileExists(path: string): Promise; + directoryExists(path: string): Promise; + getSubDirectories(rootDir: string): Promise; + getFiles(rootDir: string): Promise; + copyFile(src: string, dest: string): Promise; getFileHash(filePath: string): Promise; search(globPattern: string): Promise; createTemporaryFile(extension: string): Promise; - createWriteStream(path: string): fs.WriteStream; - chmod(path: string, mode: string): Promise; } diff --git a/src/test/configuration/interpreterSelector.unit.test.ts b/src/test/configuration/interpreterSelector.unit.test.ts index 7fcf12024f67..f2ce1a8acd76 100644 --- a/src/test/configuration/interpreterSelector.unit.test.ts +++ b/src/test/configuration/interpreterSelector.unit.test.ts @@ -81,9 +81,6 @@ suite('Interpreters - selector', () => { fileSystem .setup(x => x.arePathsSame(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())) .returns((a: string, b: string) => a === b); - fileSystem - .setup(x => x.getRealPath(TypeMoq.It.isAnyString())) - .returns((a: string) => new Promise(resolve => resolve(a))); configurationService .setup(x => x.getSettings(TypeMoq.It.isAny())) .returns(() => pythonSettings.object); From 5326edf4a7a6fb021a7854b0a576e82c3abf5a9b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 9 Oct 2019 13:40:49 -0600 Subject: [PATCH 05/58] fs -> fsextra. --- src/client/common/platform/fileSystem.ts | 69 ++++++++++++------------ 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index b1ff11407a32..3bd2cd355d5a 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -4,7 +4,7 @@ import { createHash } from 'crypto'; import * as fileSystem from 'fs'; -import * as fs from 'fs-extra'; +import * as fsextra from 'fs-extra'; import * as glob from 'glob'; import { inject, injectable } from 'inversify'; import * as path from 'path'; @@ -31,7 +31,7 @@ export class FileSystem implements IFileSystem { // fs-extra public fileExistsSync(filePath: string): boolean { - return fs.existsSync(filePath); + return fsextra.existsSync(filePath); } /** @@ -41,20 +41,20 @@ export class FileSystem implements IFileSystem { * @memberof FileSystem */ public readFile(filePath: string): Promise { - return fs.readFile(filePath).then(buffer => buffer.toString()); + return fsextra.readFile(filePath).then(buffer => buffer.toString()); } - public async writeFile(filePath: string, data: {}, options: string | fs.WriteFileOptions = { encoding: 'utf8' }): Promise { - await fs.writeFile(filePath, data, options); + public async writeFile(filePath: string, data: {}, options: string | fsextra.WriteFileOptions = { encoding: 'utf8' }): Promise { + await fsextra.writeFile(filePath, data, options); } public createDirectory(directoryPath: string): Promise { - return fs.mkdirp(directoryPath); + return fsextra.mkdirp(directoryPath); } public deleteDirectory(directoryPath: string): Promise { const deferred = createDeferred(); - fs.rmdir(directoryPath, err => (err ? deferred.reject(err) : deferred.resolve())); + fsextra.rmdir(directoryPath, err => (err ? deferred.reject(err) : deferred.resolve())); return deferred.promise; } @@ -63,12 +63,12 @@ export class FileSystem implements IFileSystem { // tslint:disable-next-line:unified-signatures public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; public appendFileSync(filename: string, data: {}, optionsOrEncoding: {}): void { - return fs.appendFileSync(filename, data, optionsOrEncoding); + return fsextra.appendFileSync(filename, data, optionsOrEncoding); } public deleteFile(filename: string): Promise { const deferred = createDeferred(); - fs.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); + fsextra.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); return deferred.promise; } @@ -103,9 +103,9 @@ export class FileSystem implements IFileSystem { } } - public objectExists(filePath: string, statCheck: (s: fs.Stats) => boolean): Promise { + public objectExists(filePath: string, statCheck: (s: fsextra.Stats) => boolean): Promise { return new Promise(resolve => { - fs.stat(filePath, (error, stats) => { + fsextra.stat(filePath, (error, stats) => { if (error) { return resolve(false); } @@ -123,31 +123,33 @@ export class FileSystem implements IFileSystem { public getSubDirectories(rootDir: string): Promise { return new Promise(resolve => { - fs.readdir(rootDir, async (error, files) => { + fsextra.readdir(rootDir, async (error, files) => { if (error) { return resolve([]); } - const subDirs = (await Promise.all( - files.map(async name => { - const fullPath = path.join(rootDir, name); - try { - if ((await fs.stat(fullPath)).isDirectory()) { - return fullPath; - } - // tslint:disable-next-line:no-empty - } catch (ex) {} - }) - )).filter(dir => dir !== undefined) as string[]; + const subDirs = ( + await Promise.all( + files.map(async name => { + const fullPath = path.join(rootDir, name); + try { + if ((await fsextra.stat(fullPath)).isDirectory()) { + return fullPath; + } + // tslint:disable-next-line:no-empty + } catch (ex) { } + }) + )) + .filter(dir => dir !== undefined) as string[]; resolve(subDirs); }); }); } public async getFiles(rootDir: string): Promise { - const files = await fs.readdir(rootDir); + const files = await fsextra.readdir(rootDir); return files.filter(async f => { const fullPath = path.join(rootDir, f); - if ((await fs.stat(fullPath)).isFile()) { + if ((await fsextra.stat(fullPath)).isFile()) { return true; } return false; @@ -156,24 +158,21 @@ export class FileSystem implements IFileSystem { public copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); - const rs = fs.createReadStream(src).on('error', err => { + const rs = fsextra.createReadStream(src).on('error', (err) => { deferred.reject(err); }); - const ws = fs - .createWriteStream(dest) - .on('error', err => { - deferred.reject(err); - }) - .on('close', () => { - deferred.resolve(); - }); + const ws = fsextra.createWriteStream(dest).on('error', (err) => { + deferred.reject(err); + }).on('close', () => { + deferred.resolve(); + }); rs.pipe(ws); return deferred.promise; } public getFileHash(filePath: string): Promise { return new Promise((resolve, reject) => { - fs.lstat(filePath, (err, stats) => { + fsextra.lstat(filePath, (err, stats) => { if (err) { reject(err); } else { From f0044adc6e603422a32a51238b1f68facd472085 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 9 Oct 2019 13:41:52 -0600 Subject: [PATCH 06/58] fileSystem -> fs. --- src/client/common/platform/fileSystem.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 3bd2cd355d5a..a03f3012068f 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -3,7 +3,7 @@ 'use strict'; import { createHash } from 'crypto'; -import * as fileSystem from 'fs'; +import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import * as glob from 'glob'; import { inject, injectable } from 'inversify'; @@ -75,13 +75,13 @@ export class FileSystem implements IFileSystem { //**************************** // fs - public createWriteStream(filePath: string): fileSystem.WriteStream { - return fileSystem.createWriteStream(filePath); + public createWriteStream(filePath: string): fs.WriteStream { + return fs.createWriteStream(filePath); } public chmod(filePath: string, mode: string): Promise { return new Promise((resolve, reject) => { - fileSystem.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { + fs.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { if (err) { return reject(err); } From 6f41c9f45992b70a246be46b0b1f8d7705b02d08 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 9 Oct 2019 14:18:15 -0600 Subject: [PATCH 07/58] objectExists() -> pathExists(). --- src/client/common/platform/fileSystem.ts | 41 +++++++++++++++--------- src/client/common/platform/types.ts | 10 +++--- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index a03f3012068f..3b78686377d6 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -9,7 +9,7 @@ import * as glob from 'glob'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; -import { FileStat } from 'vscode'; +import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; import { IFileSystem, IPlatformService, TemporaryFile } from './types'; @@ -19,7 +19,7 @@ export class FileSystem implements IFileSystem { @inject(IPlatformService) private platformService: IPlatformService ) { } - public async stat(filePath: string): Promise { + public async stat(filePath: string): Promise { // Do not import vscode directly, as this isn't available in the Debugger Context. // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. // tslint:disable-next-line: no-require-imports @@ -103,22 +103,31 @@ export class FileSystem implements IFileSystem { } } - public objectExists(filePath: string, statCheck: (s: fsextra.Stats) => boolean): Promise { - return new Promise(resolve => { - fsextra.stat(filePath, (error, stats) => { - if (error) { - return resolve(false); - } - return resolve(statCheck(stats)); - }); - }); + public async pathExists( + filename: string, + fileType?: vscode.FileType + ): Promise { + let stat: fsextra.Stats; + try { + stat = await fsextra.stat(filename); + } catch { + return false; + } + if (fileType === undefined) { + return true; + } else if (fileType === vscode.FileType.File) { + return stat.isFile(); + } else if (fileType === vscode.FileType.Directory) { + return stat.isDirectory(); + } else { + return false; + } } - - public fileExists(filePath: string): Promise { - return this.objectExists(filePath, (stats) => stats.isFile()); + public async fileExists(filename: string): Promise { + return this.pathExists(filename, vscode.FileType.File); } - public directoryExists(filePath: string): Promise { - return this.objectExists(filePath, stats => stats.isDirectory()); + public async directoryExists(dirname: string): Promise { + return this.pathExists(dirname, vscode.FileType.Directory); } public getSubDirectories(rootDir: string): Promise { diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 6bb8151272c5..b3023ea2fbd5 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -4,7 +4,7 @@ import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import { SemVer } from 'semver'; -import { Disposable, FileStat } from 'vscode'; +import * as vscode from 'vscode'; import { Architecture, OSType } from '../utils/platform'; export enum RegistryHive { @@ -32,12 +32,12 @@ export interface IPlatformService { getVersion(): Promise; } -export type TemporaryFile = { filePath: string } & Disposable; -export type TemporaryDirectory = { path: string } & Disposable; +export type TemporaryFile = { filePath: string } & vscode.Disposable; +export type TemporaryDirectory = { path: string } & vscode.Disposable; export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { - stat(filePath: string): Promise; + stat(filePath: string): Promise; // fs-extra fileExistsSync(path: string): boolean; readFile(filePath: string): Promise; @@ -54,7 +54,7 @@ export interface IFileSystem { chmod(path: string, mode: string): Promise; // helpers arePathsSame(path1: string, path2: string): boolean; - objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; // XXX drop + pathExists(path: string, fileType?: vscode.FileType): Promise; fileExists(path: string): Promise; directoryExists(path: string): Promise; getSubDirectories(rootDir: string): Promise; From dc0ee44205a5b3a7d5d6ff1eb31c76021ea24e92 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 08:54:45 -0600 Subject: [PATCH 08/58] Drop appendFileSync(). --- src/client/common/platform/fileSystem.ts | 8 -------- src/client/common/platform/types.ts | 4 ---- src/test/common/platform/filesystem.unit.test.ts | 6 ------ 3 files changed, 18 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 3b78686377d6..d3a6874bd7ab 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -58,14 +58,6 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - public appendFileSync(filename: string, data: {}, encoding: string): void; - public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; - // tslint:disable-next-line:unified-signatures - public appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; - public appendFileSync(filename: string, data: {}, optionsOrEncoding: {}): void { - return fsextra.appendFileSync(filename, data, optionsOrEncoding); - } - public deleteFile(filename: string): Promise { const deferred = createDeferred(); fsextra.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index b3023ea2fbd5..c82c6effbbb9 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -44,10 +44,6 @@ export interface IFileSystem { writeFile(filePath: string, data: {}, options?: string | fsextra.WriteFileOptions): Promise; createDirectory(path: string): Promise; deleteDirectory(path: string): Promise; - appendFileSync(filename: string, data: {}, encoding: string): void; - appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; - // tslint:disable-next-line:unified-signatures - appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: string; flag?: string }): void; deleteFile(filename: string): Promise; // fs createWriteStream(path: string): fs.WriteStream; diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index b681cdcc2dad..3bdcf929d1ed 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -75,12 +75,6 @@ suite('FileSystem', () => { expect(fileSystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found'); }); - test('Test appending to file', async () => { - const dataToAppend = `Some Data\n${new Date().toString()}\nAnd another line`; - fileSystem.appendFileSync(fileToAppendTo, dataToAppend); - const fileContents = await fileSystem.readFile(fileToAppendTo); - expect(fileContents).to.be.equal(dataToAppend); - }); test('Test searching for files', async () => { const searchPattern = `${path.basename(__filename, __filename.substring(__filename.length - 3))}.*`; const files = await fileSystem.search(path.join(__dirname, searchPattern)); From 9dc2b0603e8f447cffbb86e0c40e97b2c8d889fa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 09:25:22 -0600 Subject: [PATCH 09/58] Drop unneeded "options" param from writeFile(). --- src/client/common/platform/fileSystem.ts | 9 +++++++-- src/client/common/platform/types.ts | 3 +-- .../datascience/interactive-common/interactiveBase.ts | 2 +- src/client/datascience/interactive-ipynb/nativeEditor.ts | 2 +- src/client/datascience/jupyter/kernelService.ts | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index d3a6874bd7ab..e5b1bb63d144 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -13,6 +13,8 @@ import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; import { IFileSystem, IPlatformService, TemporaryFile } from './types'; +const ENCODING = 'utf8'; + @injectable() export class FileSystem implements IFileSystem { constructor( @@ -41,10 +43,13 @@ export class FileSystem implements IFileSystem { * @memberof FileSystem */ public readFile(filePath: string): Promise { - return fsextra.readFile(filePath).then(buffer => buffer.toString()); + return fsextra.readFile(filePath, ENCODING); } - public async writeFile(filePath: string, data: {}, options: string | fsextra.WriteFileOptions = { encoding: 'utf8' }): Promise { + public async writeFile(filePath: string, data: {}): Promise { + const options: fsextra.WriteFileOptions = { + encoding: ENCODING + }; await fsextra.writeFile(filePath, data, options); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index c82c6effbbb9..ac1439edc21c 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import * as fs from 'fs'; -import * as fsextra from 'fs-extra'; import { SemVer } from 'semver'; import * as vscode from 'vscode'; import { Architecture, OSType } from '../utils/platform'; @@ -41,7 +40,7 @@ export interface IFileSystem { // fs-extra fileExistsSync(path: string): boolean; readFile(filePath: string): Promise; - writeFile(filePath: string, data: {}, options?: string | fsextra.WriteFileOptions): Promise; + writeFile(filePath: string, data: {}): Promise; createDirectory(path: string): Promise; deleteDirectory(path: string): Promise; deleteFile(filename: string): Promise; diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 387279f507a6..477a064b0024 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -642,7 +642,7 @@ export abstract class InteractiveBase extends WebViewHost { diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 83a98201383d..f848e7358bff 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -755,7 +755,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { tempFile = await this.fileSystem.createTemporaryFile('.ipynb'); // Translate the cells into a notebook - await this.fileSystem.writeFile(tempFile.filePath, await this.generateNotebookContent(cells), { encoding: 'utf-8' }); + await this.fileSystem.writeFile(tempFile.filePath, await this.generateNotebookContent(cells)); // Import this file and show it const contents = await this.importer.importFromFile(tempFile.filePath, this.file.fsPath); diff --git a/src/client/datascience/jupyter/kernelService.ts b/src/client/datascience/jupyter/kernelService.ts index 46130666232a..d2341b0f4a7c 100644 --- a/src/client/datascience/jupyter/kernelService.ts +++ b/src/client/datascience/jupyter/kernelService.ts @@ -133,7 +133,7 @@ export class KernelService { if (await this.fileSystem.fileExists(diskPath)) { const specModel: Kernel.ISpecModel = JSON.parse(await this.fileSystem.readFile(diskPath)); specModel.argv[0] = bestInterpreter.path; - await this.fileSystem.writeFile(diskPath, JSON.stringify(specModel), { flag: 'w', encoding: 'utf8' }); + await this.fileSystem.writeFile(diskPath, JSON.stringify(specModel)); } } } From 532d5449b60d2b7087ad10a44d254148e8d4cdfa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 09:44:01 -0600 Subject: [PATCH 10/58] Add some type aliases. --- src/client/common/platform/fileSystem.ts | 21 ++++++++++++--------- src/client/common/platform/types.ts | 10 ++++++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index e5b1bb63d144..9e6d361a349d 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -9,9 +9,12 @@ import * as glob from 'glob'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; -import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; -import { IFileSystem, IPlatformService, TemporaryFile } from './types'; +import { + FileStat, FileType, + IFileSystem, IPlatformService, + TemporaryFile, WriteStream +} from './types'; const ENCODING = 'utf8'; @@ -72,7 +75,7 @@ export class FileSystem implements IFileSystem { //**************************** // fs - public createWriteStream(filePath: string): fs.WriteStream { + public createWriteStream(filePath: string): WriteStream { return fs.createWriteStream(filePath); } @@ -102,9 +105,9 @@ export class FileSystem implements IFileSystem { public async pathExists( filename: string, - fileType?: vscode.FileType + fileType?: FileType ): Promise { - let stat: fsextra.Stats; + let stat: FileStat; try { stat = await fsextra.stat(filename); } catch { @@ -112,19 +115,19 @@ export class FileSystem implements IFileSystem { } if (fileType === undefined) { return true; - } else if (fileType === vscode.FileType.File) { + } else if (fileType === FileType.File) { return stat.isFile(); - } else if (fileType === vscode.FileType.Directory) { + } else if (fileType === FileType.Directory) { return stat.isDirectory(); } else { return false; } } public async fileExists(filename: string): Promise { - return this.pathExists(filename, vscode.FileType.File); + return this.pathExists(filename, FileType.File); } public async directoryExists(dirname: string): Promise { - return this.pathExists(dirname, vscode.FileType.Directory); + return this.pathExists(dirname, FileType.Directory); } public getSubDirectories(rootDir: string): Promise { diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index ac1439edc21c..dceadf4b56b9 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as fs from 'fs'; +import * as fsextra from 'fs-extra'; import { SemVer } from 'semver'; import * as vscode from 'vscode'; import { Architecture, OSType } from '../utils/platform'; @@ -34,6 +35,11 @@ export interface IPlatformService { export type TemporaryFile = { filePath: string } & vscode.Disposable; export type TemporaryDirectory = { path: string } & vscode.Disposable; +import FileType = vscode.FileType; +export { FileType }; +export type FileStat = fsextra.Stats; +export type WriteStream = fs.WriteStream; + export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { stat(filePath: string): Promise; @@ -45,11 +51,11 @@ export interface IFileSystem { deleteDirectory(path: string): Promise; deleteFile(filename: string): Promise; // fs - createWriteStream(path: string): fs.WriteStream; + createWriteStream(path: string): WriteStream; chmod(path: string, mode: string): Promise; // helpers arePathsSame(path1: string, path2: string): boolean; - pathExists(path: string, fileType?: vscode.FileType): Promise; + pathExists(path: string, fileType?: FileType): Promise; fileExists(path: string): Promise; directoryExists(path: string): Promise; getSubDirectories(rootDir: string): Promise; From 8f27657c950bd14d26ece06d625797a55dd47cd6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 11:12:02 -0600 Subject: [PATCH 11/58] Make methods properly async. --- src/client/common/platform/fileSystem.ts | 126 +++++++++++------------ 1 file changed, 58 insertions(+), 68 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 9e6d361a349d..c4b89d538478 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -9,6 +9,7 @@ import * as glob from 'glob'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; +import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; import { FileStat, FileType, @@ -18,6 +19,18 @@ import { const ENCODING = 'utf8'; +function getFileType(stat: FileStat): FileType { + if (stat.isFile()) { + return FileType.File; + } else if (stat.isDirectory()) { + return FileType.Directory; + } else if (stat.isSymbolicLink()) { + return FileType.SymbolicLink; + } else { + return FileType.Unknown; + } +} + @injectable() export class FileSystem implements IFileSystem { constructor( @@ -45,7 +58,7 @@ export class FileSystem implements IFileSystem { * @returns {Promise} * @memberof FileSystem */ - public readFile(filePath: string): Promise { + public async readFile(filePath: string): Promise { return fsextra.readFile(filePath, ENCODING); } @@ -56,20 +69,20 @@ export class FileSystem implements IFileSystem { await fsextra.writeFile(filePath, data, options); } - public createDirectory(directoryPath: string): Promise { + public async createDirectory(directoryPath: string): Promise { return fsextra.mkdirp(directoryPath); } - public deleteDirectory(directoryPath: string): Promise { - const deferred = createDeferred(); - fsextra.rmdir(directoryPath, err => (err ? deferred.reject(err) : deferred.resolve())); - return deferred.promise; + public async deleteDirectory(directoryPath: string): Promise { + return fsextra.rmdir(directoryPath); } - public deleteFile(filename: string): Promise { - const deferred = createDeferred(); - fsextra.unlink(filename, err => (err ? deferred.reject(err) : deferred.resolve())); - return deferred.promise; + public async deleteFile(filename: string): Promise { + return fsextra.unlink(filename); + } + + public async chmod(filePath: string, mode: string): Promise { + return fsextra.chmod(filePath, mode); } //**************************** @@ -79,17 +92,6 @@ export class FileSystem implements IFileSystem { return fs.createWriteStream(filePath); } - public chmod(filePath: string, mode: string): Promise { - return new Promise((resolve, reject) => { - fs.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { - if (err) { - return reject(err); - } - resolve(); - }); - }); - } - //**************************** // helpers @@ -130,42 +132,36 @@ export class FileSystem implements IFileSystem { return this.pathExists(dirname, FileType.Directory); } - public getSubDirectories(rootDir: string): Promise { - return new Promise(resolve => { - fsextra.readdir(rootDir, async (error, files) => { - if (error) { - return resolve([]); - } - const subDirs = ( - await Promise.all( - files.map(async name => { - const fullPath = path.join(rootDir, name); - try { - if ((await fsextra.stat(fullPath)).isDirectory()) { - return fullPath; - } - // tslint:disable-next-line:no-empty - } catch (ex) { } - }) - )) - .filter(dir => dir !== undefined) as string[]; - resolve(subDirs); - }); - }); + public async listdir( + dirname: string + ): Promise<[string, FileType][]> { + const filenames: string[] = await ( + fsextra.readdir(dirname) + .then(names => names.map(name => path.join(dirname, name))) + .catch(() => []) + ); + const promises = filenames + .map(filename => ( + fsextra.stat(filename) + .then(stat => [filename, getFileType(stat)] as [string, FileType]) + .catch(() => [filename, FileType.Unknown] as [string, FileType]) + )); + return Promise.all(promises); } - public async getFiles(rootDir: string): Promise { - const files = await fsextra.readdir(rootDir); - return files.filter(async f => { - const fullPath = path.join(rootDir, f); - if ((await fsextra.stat(fullPath)).isFile()) { - return true; - } - return false; - }); + public async getSubDirectories(dirname: string): Promise { + return (await this.listdir(dirname)) + .filter(([_filename, fileType]) => fileType === FileType.Directory) + .map(([filename, _fileType]) => filename); } - public copyFile(src: string, dest: string): Promise { + public async getFiles(dirname: string): Promise { + return (await this.listdir(dirname)) + .filter(([_filename, fileType]) => fileType === FileType.File) + .map(([filename, _fileType]) => filename); + } + + public async copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); const rs = fsextra.createReadStream(src).on('error', (err) => { deferred.reject(err); @@ -179,22 +175,15 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - public getFileHash(filePath: string): Promise { - return new Promise((resolve, reject) => { - fsextra.lstat(filePath, (err, stats) => { - if (err) { - reject(err); - } else { - const actual = createHash('sha512') - .update(`${stats.ctimeMs}-${stats.mtimeMs}`) - .digest('hex'); - resolve(actual); - } - }); - }); + public async getFileHash(filePath: string): Promise { + const stat = await fsextra.lstat(filePath); + const hash = createHash('sha512') + .update(`${stat.ctimeMs}-${stat.mtimeMs}`); + return hash.digest('hex'); } - public search(globPattern: string): Promise { + public async search(globPattern: string): Promise { + // We could use util.promisify() here. return new Promise((resolve, reject) => { glob(globPattern, (ex, files) => { if (ex) { @@ -205,7 +194,8 @@ export class FileSystem implements IFileSystem { }); } - public createTemporaryFile(extension: string): Promise { + public async createTemporaryFile(extension: string): Promise { + // We could use util.promisify() here. return new Promise((resolve, reject) => { tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => { if (err) { From 352a1758bc80bcc387e866b7816e4ffc87befcd8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 11:24:36 -0600 Subject: [PATCH 12/58] Clean up formatting and parameter names. --- src/client/common/platform/fileSystem.ts | 67 +++++++++++------------- src/client/common/platform/types.ts | 36 +++++++------ 2 files changed, 52 insertions(+), 51 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index c4b89d538478..fa371caab26b 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -48,48 +48,42 @@ export class FileSystem implements IFileSystem { //**************************** // fs-extra - public fileExistsSync(filePath: string): boolean { - return fsextra.existsSync(filePath); + public fileExistsSync(filename: string): boolean { + return fsextra.existsSync(filename); } - /** - * Reads the contents of the file using utf8 and returns the string contents. - * @param {string} filePath - * @returns {Promise} - * @memberof FileSystem - */ - public async readFile(filePath: string): Promise { - return fsextra.readFile(filePath, ENCODING); + public async readFile(filename: string): Promise { + return fsextra.readFile(filename, ENCODING); } - public async writeFile(filePath: string, data: {}): Promise { + public async writeFile(filename: string, data: {}): Promise { const options: fsextra.WriteFileOptions = { encoding: ENCODING }; - await fsextra.writeFile(filePath, data, options); + await fsextra.writeFile(filename, data, options); } - public async createDirectory(directoryPath: string): Promise { - return fsextra.mkdirp(directoryPath); + public async createDirectory(dirname: string): Promise { + return fsextra.mkdirp(dirname); } - public async deleteDirectory(directoryPath: string): Promise { - return fsextra.rmdir(directoryPath); + public async deleteDirectory(dirname: string): Promise { + return fsextra.rmdir(dirname); } public async deleteFile(filename: string): Promise { return fsextra.unlink(filename); } - public async chmod(filePath: string, mode: string): Promise { - return fsextra.chmod(filePath, mode); + public async chmod(filename: string, mode: string): Promise { + return fsextra.chmod(filename, mode); } //**************************** // fs - public createWriteStream(filePath: string): WriteStream { - return fs.createWriteStream(filePath); + public createWriteStream(filename: string): WriteStream { + return fs.createWriteStream(filename); } //**************************** @@ -148,13 +142,11 @@ export class FileSystem implements IFileSystem { )); return Promise.all(promises); } - public async getSubDirectories(dirname: string): Promise { return (await this.listdir(dirname)) .filter(([_filename, fileType]) => fileType === FileType.Directory) .map(([filename, _fileType]) => filename); } - public async getFiles(dirname: string): Promise { return (await this.listdir(dirname)) .filter(([_filename, fileType]) => fileType === FileType.File) @@ -163,20 +155,22 @@ export class FileSystem implements IFileSystem { public async copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); - const rs = fsextra.createReadStream(src).on('error', (err) => { - deferred.reject(err); - }); - const ws = fsextra.createWriteStream(dest).on('error', (err) => { - deferred.reject(err); - }).on('close', () => { - deferred.resolve(); - }); + const rs = fsextra.createReadStream(src) + .on('error', (err) => { + deferred.reject(err); + }); + const ws = fsextra.createWriteStream(dest) + .on('error', (err) => { + deferred.reject(err); + }).on('close', () => { + deferred.resolve(); + }); rs.pipe(ws); return deferred.promise; } - public async getFileHash(filePath: string): Promise { - const stat = await fsextra.lstat(filePath); + public async getFileHash(filename: string): Promise { + const stat = await fsextra.lstat(filename); const hash = createHash('sha512') .update(`${stat.ctimeMs}-${stat.mtimeMs}`); return hash.digest('hex'); @@ -194,14 +188,17 @@ export class FileSystem implements IFileSystem { }); } - public async createTemporaryFile(extension: string): Promise { + public async createTemporaryFile(suffix: string): Promise { // We could use util.promisify() here. return new Promise((resolve, reject) => { - tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => { + tmp.file({ postfix: suffix }, (err, tmpFile, _, cleanupCallback) => { if (err) { return reject(err); } - resolve({ filePath: tmpFile, dispose: cleanupCallback }); + resolve({ + filePath: tmpFile, + dispose: cleanupCallback + }); }); }); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index dceadf4b56b9..80f0d31d57c0 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -32,8 +32,12 @@ export interface IPlatformService { getVersion(): Promise; } -export type TemporaryFile = { filePath: string } & vscode.Disposable; -export type TemporaryDirectory = { path: string } & vscode.Disposable; +export type TemporaryFile = vscode.Disposable & { + filePath: string; +}; +export type TemporaryDirectory = vscode.Disposable & { + path: string; +}; import FileType = vscode.FileType; export { FileType }; @@ -44,24 +48,24 @@ export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { stat(filePath: string): Promise; // fs-extra - fileExistsSync(path: string): boolean; - readFile(filePath: string): Promise; - writeFile(filePath: string, data: {}): Promise; - createDirectory(path: string): Promise; - deleteDirectory(path: string): Promise; + fileExistsSync(filename: string): boolean; + readFile(filename: string): Promise; + writeFile(filename: string, data: {}): Promise; + createDirectory(dirname: string): Promise; + deleteDirectory(dirname: string): Promise; deleteFile(filename: string): Promise; // fs - createWriteStream(path: string): WriteStream; - chmod(path: string, mode: string): Promise; + createWriteStream(filename: string): WriteStream; + chmod(filename: string, mode: string): Promise; // helpers arePathsSame(path1: string, path2: string): boolean; - pathExists(path: string, fileType?: FileType): Promise; - fileExists(path: string): Promise; - directoryExists(path: string): Promise; - getSubDirectories(rootDir: string): Promise; - getFiles(rootDir: string): Promise; + pathExists(filename: string, fileType?: FileType): Promise; + fileExists(filename: string): Promise; + directoryExists(dirname: string): Promise; + getSubDirectories(dirname: string): Promise; + getFiles(dirname: string): Promise; copyFile(src: string, dest: string): Promise; - getFileHash(filePath: string): Promise; + getFileHash(filename: string): Promise; search(globPattern: string): Promise; - createTemporaryFile(extension: string): Promise; + createTemporaryFile(suffix: string): Promise; } From c7b7c5d3b515a1c0c3313282c7def79dd5891590 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 11:44:51 -0600 Subject: [PATCH 13/58] Add the isDirReadonly() method. --- .../common/installer/moduleInstaller.ts | 28 ++++++++----------- src/client/common/platform/fileSystem.ts | 16 +++++++++++ src/client/common/platform/types.ts | 1 + 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts index 8a4c58a4857a..d3e13c5ea1b0 100644 --- a/src/client/common/installer/moduleInstaller.ts +++ b/src/client/common/installer/moduleInstaller.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as fs from 'fs'; import { injectable } from 'inversify'; import * as path from 'path'; import * as vscode from 'vscode'; @@ -10,15 +9,18 @@ import { IServiceContainer } from '../../ioc/types'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { STANDARD_OUTPUT_CHANNEL } from '../constants'; +import { IFileSystem } from '../platform/types'; import { ITerminalServiceFactory } from '../terminal/types'; import { ExecutionInfo, IConfigurationService, IOutputChannel } from '../types'; -import { noop } from '../utils/misc'; @injectable() export abstract class ModuleInstaller { public abstract get name(): string; public abstract get displayName(): string - constructor(protected serviceContainer: IServiceContainer) { } + constructor( + protected serviceContainer: IServiceContainer + ) { } + public async installModule(name: string, resource?: vscode.Uri): Promise { sendTelemetryEvent(EventName.PYTHON_INSTALL_PACKAGE, undefined, { installer: this.displayName }); const executionInfo = await this.getExecutionInfo(name, resource); @@ -38,7 +40,10 @@ export abstract class ModuleInstaller { if (!currentInterpreter || currentInterpreter.type !== InterpreterType.Unknown) { await terminalService.sendCommand(pythonPath, args); } else if (settings.globalModuleInstallation) { - if (await this.isPathWritableAsync(path.dirname(pythonPath))) { + const dirname = path.dirname(pythonPath); + const fs = this.serviceContainer.get(IFileSystem); + const isWritable = ! await fs.isDirReadonly(dirname); + if (isWritable) { await terminalService.sendCommand(pythonPath, args); } else { this.elevatedInstall(pythonPath, args); @@ -50,8 +55,10 @@ export abstract class ModuleInstaller { await terminalService.sendCommand(executionInfo.execPath!, executionInfoArgs); } } + public abstract isSupported(resource?: vscode.Uri): Promise; protected abstract getExecutionInfo(moduleName: string, resource?: vscode.Uri): Promise; + private async processInstallArgs(args: string[], resource?: vscode.Uri): Promise { const indexOfPylint = args.findIndex(arg => arg.toUpperCase() === 'PYLINT'); if (indexOfPylint === -1) { @@ -69,19 +76,6 @@ export abstract class ModuleInstaller { } return args; } - private async isPathWritableAsync(directoryPath: string): Promise { - const filePath = `${directoryPath}${path.sep}___vscpTest___`; - return new Promise(resolve => { - fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => { - if (!error) { - fs.close(fd, () => { - fs.unlink(filePath, noop); - }); - } - return resolve(!error); - }); - }); - } private elevatedInstall(execPath: string, args: string[]) { const options = { diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index fa371caab26b..b0b793a6c0f9 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -11,6 +11,7 @@ import * as path from 'path'; import * as tmp from 'tmp'; import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; +import { noop } from '../utils/misc'; import { FileStat, FileType, IFileSystem, IPlatformService, @@ -153,6 +154,21 @@ export class FileSystem implements IFileSystem { .map(([filename, _fileType]) => filename); } + public async isDirReadonly(dirname: string): Promise { + // Alternative: use tmp.file(). + const filePath = path.join(dirname, '___vscpTest___'); + return new Promise(resolve => { + fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => { + if (!error) { + fs.close(fd, () => { + fs.unlink(filePath, noop); + }); + } + return resolve(!!error); + }); + }); + } + public async copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); const rs = fsextra.createReadStream(src) diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 80f0d31d57c0..4a8a22a8d051 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -64,6 +64,7 @@ export interface IFileSystem { directoryExists(dirname: string): Promise; getSubDirectories(dirname: string): Promise; getFiles(dirname: string): Promise; + isDirReadonly(dirname: string): Promise; copyFile(src: string, dest: string): Promise; getFileHash(filename: string): Promise; search(globPattern: string): Promise; From f617d712f5726bbe854408bd56e60dfd1a36beb1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 14:11:23 -0600 Subject: [PATCH 14/58] Use IFileSystem.pathExists() (almost) everywhere. --- src/client/common/envFileParser.ts | 57 +++++-- src/client/common/platform/types.ts | 2 +- src/client/common/variables/environment.ts | 14 +- .../services/windowsRegistryService.ts | 10 +- src/client/providers/jediProxy.ts | 9 +- .../envVarsProvider.multiroot.test.ts | 4 +- .../variables/envVarsService.unit.test.ts | 17 ++- .../windowsRegistryService.unit.test.ts | 141 ++++++++++++++++-- 8 files changed, 219 insertions(+), 35 deletions(-) diff --git a/src/client/common/envFileParser.ts b/src/client/common/envFileParser.ts index f1cfda52b430..4b9ec9fbea13 100644 --- a/src/client/common/envFileParser.ts +++ b/src/client/common/envFileParser.ts @@ -1,8 +1,13 @@ -import * as fs from 'fs-extra'; +import * as fsextra from 'fs-extra'; import { IS_WINDOWS } from './platform/constants'; +import { FileSystem } from './platform/fileSystem'; import { PathUtils } from './platform/pathUtils'; +import { IPlatformService } from './platform/types'; import { EnvironmentVariablesService } from './variables/environment'; -import { EnvironmentVariables } from './variables/types'; +import { + EnvironmentVariables, IEnvironmentVariablesService +} from './variables/types'; + function parseEnvironmentVariables(contents: string): EnvironmentVariables | undefined { if (typeof contents !== 'string' || contents.length === 0) { return undefined; @@ -22,10 +27,23 @@ function parseEnvironmentVariables(contents: string): EnvironmentVariables | und return env; } -export function parseEnvFile(envFile: string, mergeWithProcessEnvVars: boolean = true): EnvironmentVariables { - const buffer = fs.readFileSync(envFile, 'utf8'); +export function parseEnvFile( + envFile: string, + mergeWithProcessEnvVars: boolean = true, + service?: IEnvironmentVariablesService +): EnvironmentVariables { + const buffer = fsextra.readFileSync(envFile, 'utf8'); const env = parseEnvironmentVariables(buffer)!; - return mergeWithProcessEnvVars ? mergeEnvVariables(env, process.env) : mergePythonPath(env, process.env.PYTHONPATH as string); + if (!service) { + service = new EnvironmentVariablesService( + new PathUtils(IS_WINDOWS), + // tslint:disable-next-line:no-object-literal-type-assertion + new FileSystem({} as IPlatformService) + ); + } + return mergeWithProcessEnvVars + ? mergeEnvVariables(env, process.env, service) + : mergePythonPath(env, process.env.PYTHONPATH as string, service); } /** @@ -36,8 +54,18 @@ export function parseEnvFile(envFile: string, mergeWithProcessEnvVars: boolean = * @param {EnvironmentVariables} [sourceEnvVars=process.env] source environment variables (defaults to current process variables). * @returns {EnvironmentVariables} */ -export function mergeEnvVariables(targetEnvVars: EnvironmentVariables, sourceEnvVars: EnvironmentVariables = process.env): EnvironmentVariables { - const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS)); +export function mergeEnvVariables( + targetEnvVars: EnvironmentVariables, + sourceEnvVars: EnvironmentVariables = process.env, + service?: IEnvironmentVariablesService +): EnvironmentVariables { + if (!service) { + service = new EnvironmentVariablesService( + new PathUtils(IS_WINDOWS), + // tslint:disable-next-line:no-object-literal-type-assertion + new FileSystem({} as IPlatformService) + ); + } service.mergeVariables(sourceEnvVars, targetEnvVars); if (sourceEnvVars.PYTHONPATH) { service.appendPythonPath(targetEnvVars, sourceEnvVars.PYTHONPATH); @@ -53,11 +81,22 @@ export function mergeEnvVariables(targetEnvVars: EnvironmentVariables, sourceEnv * @param {string | undefined} [currentPythonPath] PYTHONPATH value. * @returns {EnvironmentVariables} */ -export function mergePythonPath(env: EnvironmentVariables, currentPythonPath: string | undefined): EnvironmentVariables { +export function mergePythonPath( + env: EnvironmentVariables, + currentPythonPath: string | undefined, + service?: IEnvironmentVariablesService +): EnvironmentVariables { if (typeof currentPythonPath !== 'string' || currentPythonPath.length === 0) { return env; } - const service = new EnvironmentVariablesService(new PathUtils(IS_WINDOWS)); + + if (!service) { + service = new EnvironmentVariablesService( + new PathUtils(IS_WINDOWS), + // tslint:disable-next-line:no-object-literal-type-assertion + new FileSystem({} as IPlatformService) + ); + } service.appendPythonPath(env, currentPythonPath); return env; } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 4a8a22a8d051..e9faaf74bb1e 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -58,7 +58,7 @@ export interface IFileSystem { createWriteStream(filename: string): WriteStream; chmod(filename: string, mode: string): Promise; // helpers - arePathsSame(path1: string, path2: string): boolean; + arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. pathExists(filename: string, fileType?: FileType): Promise; fileExists(filename: string): Promise; directoryExists(dirname: string): Promise; diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index 92921345a6f6..f8d295eeac65 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -1,28 +1,32 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as fs from 'fs-extra'; +import * as fsextra from 'fs-extra'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; +import { IFileSystem } from '../platform/types'; import { IPathUtils } from '../types'; import { EnvironmentVariables, IEnvironmentVariablesService } from './types'; @injectable() export class EnvironmentVariablesService implements IEnvironmentVariablesService { private readonly pathVariable: 'PATH' | 'Path'; - constructor(@inject(IPathUtils) pathUtils: IPathUtils) { + constructor( + @inject(IPathUtils) pathUtils: IPathUtils, + @inject(IFileSystem) private readonly fs: IFileSystem + ) { this.pathVariable = pathUtils.getPathVariableName(); } public async parseFile(filePath?: string, baseVars?: EnvironmentVariables): Promise { - if (!filePath || !await fs.pathExists(filePath)) { + if (!filePath || !await this.fs.pathExists(filePath)) { return; } - if (!fs.lstatSync(filePath).isFile()) { + if (!fsextra.lstatSync(filePath).isFile()) { return; } - return parseEnvFile(await fs.readFile(filePath), baseVars); + return parseEnvFile(await fsextra.readFile(filePath), baseVars); } public mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables) { if (!target) { diff --git a/src/client/interpreter/locators/services/windowsRegistryService.ts b/src/client/interpreter/locators/services/windowsRegistryService.ts index e99f29a1be66..70af04c649e8 100644 --- a/src/client/interpreter/locators/services/windowsRegistryService.ts +++ b/src/client/interpreter/locators/services/windowsRegistryService.ts @@ -1,10 +1,11 @@ // tslint:disable:no-require-imports no-var-requires underscore-consistent-invocation -import * as fs from 'fs-extra'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; import { traceError } from '../../../common/logger'; -import { IPlatformService, IRegistry, RegistryHive } from '../../../common/platform/types'; +import { + IFileSystem, IPlatformService, IRegistry, RegistryHive +} from '../../../common/platform/types'; import { IPathUtils } from '../../../common/types'; import { Architecture } from '../../../common/utils/platform'; import { parsePythonVersion } from '../../../common/utils/version'; @@ -38,7 +39,8 @@ export class WindowsRegistryService extends CacheableLocatorService { @inject(IRegistry) private registry: IRegistry, @inject(IPlatformService) private readonly platform: IPlatformService, @inject(IServiceContainer) serviceContainer: IServiceContainer, - @inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter + @inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter, + @inject(IFileSystem) private readonly fs: IFileSystem ) { super('WindowsRegistryService', serviceContainer); this.pathUtils = serviceContainer.get(IPathUtils); @@ -158,7 +160,7 @@ export class WindowsRegistryService extends CacheableLocatorService { }) .then(interpreter => interpreter - ? fs + ? this.fs .pathExists(interpreter.path) .catch(() => false) .then(exists => (exists ? interpreter : null)) diff --git a/src/client/providers/jediProxy.ts b/src/client/providers/jediProxy.ts index eea57fd58080..3e549192144c 100644 --- a/src/client/providers/jediProxy.ts +++ b/src/client/providers/jediProxy.ts @@ -3,7 +3,6 @@ // tslint:disable:no-var-requires no-require-imports no-any import { ChildProcess } from 'child_process'; -import * as fs from 'fs-extra'; import * as path from 'path'; // @ts-ignore import * as pidusage from 'pidusage'; @@ -11,6 +10,7 @@ import { CancellationToken, CancellationTokenSource, CompletionItemKind, Disposa import { isTestExecution } from '../common/constants'; import '../common/extensions'; import { IS_WINDOWS } from '../common/platform/constants'; +import { IFileSystem } from '../common/platform/types'; import { IPythonExecutionFactory } from '../common/process/types'; import { BANNER_NAME_PROPOSE_LS, IConfigurationService, ILogger, IPythonExtensionBanner, IPythonSettings } from '../common/types'; import { createDeferred, Deferred } from '../common/utils/async'; @@ -155,7 +155,11 @@ export class JediProxy implements Disposable { private readonly disposables: Disposable[] = []; private timer?: NodeJS.Timer | number; - public constructor(private extensionRootDir: string, workspacePath: string, private serviceContainer: IServiceContainer) { + public constructor( + private extensionRootDir: string, + workspacePath: string, + private serviceContainer: IServiceContainer + ) { this.workspacePath = workspacePath; const configurationService = serviceContainer.get(IConfigurationService); this.pythonSettings = configurationService.getSettings(Uri.file(workspacePath)); @@ -652,6 +656,7 @@ export class JediProxy implements Disposable { if (lines.length === 0) { return ''; } + const fs = this.serviceContainer.get(IFileSystem); const exists = await fs.pathExists(lines[0]); return exists ? lines[0] : ''; } catch { diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts index 2fce35335a72..2c36fbfda6ca 100644 --- a/src/test/common/variables/envVarsProvider.multiroot.test.ts +++ b/src/test/common/variables/envVarsProvider.multiroot.test.ts @@ -10,6 +10,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { IS_WINDOWS, NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../../../client/common/platform/constants'; import { PlatformService } from '../../../client/common/platform/platformService'; +import { IFileSystem } from '../../../client/common/platform/types'; import { IDisposableRegistry, IPathUtils } from '../../../client/common/types'; import { clearCache } from '../../../client/common/utils/cacheUtils'; import { EnvironmentVariablesService } from '../../../client/common/variables/environment'; @@ -66,8 +67,9 @@ suite('Multiroot Environment Variables Provider', () => { function getVariablesProvider(mockVariables: EnvironmentVariables = { ...process.env }) { const pathUtils = ioc.serviceContainer.get(IPathUtils); + const fs = ioc.serviceContainer.get(IFileSystem); const mockProcess = new MockProcess(mockVariables); - const variablesService = new EnvironmentVariablesService(pathUtils); + const variablesService = new EnvironmentVariablesService(pathUtils, fs); const disposables = ioc.serviceContainer.get(IDisposableRegistry); ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService()); const cfgService = new ConfigurationService(ioc.serviceContainer); diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index 54e0c61f9ac6..a21ac036582c 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -6,7 +6,9 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as path from 'path'; +import * as TypeMoq from 'typemoq'; import { PathUtils } from '../../../client/common/platform/pathUtils'; +import { IFileSystem } from '../../../client/common/platform/types'; import { IPathUtils } from '../../../client/common/types'; import { OSType } from '../../../client/common/utils/platform'; import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment'; @@ -20,10 +22,22 @@ const envFilesFolderPath = path.join(__dirname, '..', '..', '..', '..', 'src', ' // tslint:disable-next-line:max-func-body-length suite('Environment Variables Service', () => { let pathUtils: IPathUtils; + let fs: TypeMoq.IMock; let variablesService: IEnvironmentVariablesService; + + let pathExists: boolean; + setup(() => { pathUtils = new PathUtils(getOSType() === OSType.Windows); - variablesService = new EnvironmentVariablesService(pathUtils); + fs = TypeMoq.Mock.ofType(); + variablesService = new EnvironmentVariablesService( + pathUtils, + fs.object + ); + pathExists = true; + fs + .setup(s => s.pathExists(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(pathExists)); }); test('Custom variables should be undefined with no argument', async () => { @@ -32,6 +46,7 @@ suite('Environment Variables Service', () => { }); test('Custom variables should be undefined with non-existent files', async () => { + pathExists = false; const vars = await variablesService.parseFile(path.join(envFilesFolderPath, 'abcd')); expect(vars).to.equal(undefined, 'Variables should be undefined'); }); diff --git a/src/test/interpreters/windowsRegistryService.unit.test.ts b/src/test/interpreters/windowsRegistryService.unit.test.ts index e0cf7bffd056..2bdf36436351 100644 --- a/src/test/interpreters/windowsRegistryService.unit.test.ts +++ b/src/test/interpreters/windowsRegistryService.unit.test.ts @@ -1,7 +1,9 @@ import * as assert from 'assert'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; -import { IPlatformService, RegistryHive } from '../../client/common/platform/types'; +import { + IFileSystem, IPlatformService, RegistryHive +} from '../../client/common/platform/types'; import { IPathUtils, IPersistentStateFactory } from '../../client/common/types'; import { Architecture } from '../../client/common/utils/platform'; import { IInterpreterHelper, InterpreterType } from '../../client/interpreter/contracts'; @@ -19,6 +21,8 @@ suite('Interpreters from Windows Registry (unit)', () => { let interpreterHelper: TypeMoq.IMock; let platformService: TypeMoq.IMock; let windowsStoreInterpreter: TypeMoq.IMock; + let fs: TypeMoq.IMock; + setup(() => { serviceContainer = TypeMoq.Mock.ofType(); const stateFactory = TypeMoq.Mock.ofType(); @@ -26,6 +30,8 @@ suite('Interpreters from Windows Registry (unit)', () => { const pathUtils = TypeMoq.Mock.ofType(); platformService = TypeMoq.Mock.ofType(); windowsStoreInterpreter = TypeMoq.Mock.ofType(); + fs = TypeMoq.Mock.ofType(); + windowsStoreInterpreter.setup(w => w.isHiddenInterpreter(TypeMoq.It.isAny())).returns(() => false); windowsStoreInterpreter.setup(w => w.isWindowsStoreInterpreter(TypeMoq.It.isAny())).returns(() => false); serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPersistentStateFactory))).returns(() => stateFactory.object); @@ -41,16 +47,56 @@ suite('Interpreters from Windows Registry (unit)', () => { platformService.setup(ps => ps.is64bit).returns(() => is64Bit); return platformService.object; } + function setInterpreterFiles( + regValues: { key: string; value: string; name?: string }[], + skipKeys: string[] = [] + ) { + const seen: string[] = []; + const dirs: [string, boolean][] = []; + for (const regValue of regValues) { + const exists = ! skipKeys.includes(regValue.key); + if (regValue.name === undefined) { + if (regValue.value) { + dirs.push([regValue.value, exists]); + } + } else if (regValue.name === 'ExecutablePath') { + fs + .setup(s => s.pathExists(regValue.value)) + .returns(() => Promise.resolve(exists)); + seen.push(path.dirname(regValue.value)); + } + } + for (const [dirname, exists] of dirs) { + if (!seen.includes(dirname)) { + fs + .setup(s => s.pathExists(path.join(dirname, 'python.exe'))) + .returns(() => Promise.resolve(exists)); + seen.push(dirname); + } + } + } test('Must return an empty list (x86)', async () => { const registry = new MockRegistry([], []); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); const interpreters = await winRegistry.getInterpreters(); assert.equal(interpreters.length, 0, 'Incorrect number of entries'); }); test('Must return an empty list (x64)', async () => { const registry = new MockRegistry([], []); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(true), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(true), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); const interpreters = await winRegistry.getInterpreters(); assert.equal(interpreters.length, 0, 'Incorrect number of entries'); @@ -67,8 +113,15 @@ suite('Interpreters from Windows Registry (unit)', () => { { key: '\\Software\\Python\\Company One\\Tag1', hive: RegistryHive.HKCU, arch: Architecture.x86, value: '9.9.9.final', name: 'SysVersion' }, { key: '\\Software\\Python\\Company One\\Tag1', hive: RegistryHive.HKCU, arch: Architecture.x86, value: 'DisplayName.Tag1', name: 'DisplayName' } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); @@ -89,8 +142,15 @@ suite('Interpreters from Windows Registry (unit)', () => { const registryValues = [ { key: '\\Software\\Python\\PythonCore\\9.9.9-final\\InstallPath', hive: RegistryHive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); @@ -111,8 +171,15 @@ suite('Interpreters from Windows Registry (unit)', () => { const registryValues = [ { key: '\\Software\\Python\\PyLauncher\\Tag1\\InstallPath', hive: RegistryHive.HKCU, arch: Architecture.x86, value: 'c:/temp/Install Path Tag1' } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); const interpreters = await winRegistry.getInterpreters(); @@ -126,8 +193,15 @@ suite('Interpreters from Windows Registry (unit)', () => { const registryValues = [ { key: '\\Software\\Python\\Company One\\9.9.9-final\\InstallPath', hive: RegistryHive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); @@ -148,8 +222,15 @@ suite('Interpreters from Windows Registry (unit)', () => { const registryValues = [ { key: '\\Software\\Python\\Company One\\9.9.9-final\\InstallPath', hive: RegistryHive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); windowsStoreInterpreter.reset(); @@ -177,8 +258,15 @@ suite('Interpreters from Windows Registry (unit)', () => { const registryValues = [ { key: '\\Software\\Python\\Company One\\9.9.9-final\\InstallPath', hive: RegistryHive.HKCU, arch: Architecture.x86, value: path.join(environmentsPath, 'path1') } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); windowsStoreInterpreter.reset(); @@ -224,8 +312,17 @@ suite('Interpreters from Windows Registry (unit)', () => { { key: '\\Software\\Python\\Company A\\8.0.0\\InstallPath', hive: RegistryHive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'conda', 'envs', 'scipy', 'python.exe') } ]; + setInterpreterFiles(registryValues, [ + '\\Software\\Python\\Company Two\\3.0.0\\InstallPath' + ]); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); @@ -288,8 +385,15 @@ suite('Interpreters from Windows Registry (unit)', () => { { key: '\\Software\\Python\\Company A\\10.0.0\\InstallPath', hive: RegistryHive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'conda', 'envs', 'numpy') } ]; + setInterpreterFiles(registryValues); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); @@ -352,8 +456,21 @@ suite('Interpreters from Windows Registry (unit)', () => { { key: '\\Software\\Python\\Company A\\Another Tag\\InstallPath', hive: RegistryHive.HKLM, arch: Architecture.x86, value: path.join(environmentsPath, 'non-existent-path', 'envs', 'numpy') } ]; + setInterpreterFiles(registryValues, [ + '\\Software\\Python\\Company One\\Tag2\\InstallPath', + '\\Software\\Python\\Company Two\\Tag A\\InstallPath', + '\\Software\\Python\\Company Two\\Tag C\\InstallPath', + '\\Software\\Python\\Company Three\\Tag !\\InstallPath', + '\\Software\\Python\\Company A\\Another Tag\\InstallPath' + ]); const registry = new MockRegistry(registryKeys, registryValues); - const winRegistry = new WindowsRegistryService(registry, setup64Bit(false), serviceContainer.object, windowsStoreInterpreter.object); + const winRegistry = new WindowsRegistryService( + registry, + setup64Bit(false), + serviceContainer.object, + windowsStoreInterpreter.object, + fs.object + ); interpreterHelper.reset(); interpreterHelper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({ architecture: Architecture.x86 })); From aad400a96b31ab5e9c5aa3262c5661b038b1e7a0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 15:36:38 -0600 Subject: [PATCH 15/58] Stop using fs-extra outside of the FileSystem class. --- src/client/common/editor.ts | 50 +++++++++++-------- src/client/common/envFileParser.ts | 15 +++--- src/client/common/platform/fileSystem.ts | 4 ++ src/client/common/platform/types.ts | 8 +-- src/client/common/variables/environment.ts | 15 ++++-- src/client/formatters/baseFormatter.ts | 14 ++++-- src/client/providers/renameProvider.ts | 9 +++- .../variables/envVarsService.unit.test.ts | 47 +++++++++++++---- 8 files changed, 108 insertions(+), 54 deletions(-) diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index f777de96e246..234915877900 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -1,10 +1,10 @@ import { Diff, diff_match_patch } from 'diff-match-patch'; -import * as fs from 'fs-extra'; import { injectable } from 'inversify'; import * as md5 from 'md5'; import { EOL } from 'os'; import * as path from 'path'; import { Position, Range, TextDocument, TextEdit, Uri, WorkspaceEdit } from 'vscode'; +import { IFileSystem } from './platform/types'; import { IEditorUtils } from './types'; // Code borrowed from goFormat.ts (Go Extension for VS Code) @@ -80,7 +80,11 @@ export function getTextEditsFromPatch(before: string, patch: string): TextEdit[] return textEdits; } -export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot?: string): WorkspaceEdit { +export function getWorkspaceEditsFromPatch( + filePatches: string[], + fs: IFileSystem, + workspaceRoot?: string +): WorkspaceEdit { const workspaceEdit = new WorkspaceEdit(); filePatches.forEach(patch => { const indexOfAtAt = patch.indexOf('@@'); @@ -107,7 +111,7 @@ export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot? let fileName = fileNameLines[0].substring(fileNameLines[0].indexOf(' a') + 3).trim(); fileName = workspaceRoot && !path.isAbsolute(fileName) ? path.resolve(workspaceRoot, fileName) : fileName; - if (!fs.existsSync(fileName)) { + if (!fs.fileExistsSync(fileName)) { return; } @@ -123,7 +127,7 @@ export function getWorkspaceEditsFromPatch(filePatches: string[], workspaceRoot? throw new Error('Unable to parse Patch string'); } - const fileSource = fs.readFileSync(fileName).toString('utf8'); + const fileSource = fs.readFileSync(fileName); const fileUri = Uri.file(fileName); // Add line feeds and build the text edits @@ -226,24 +230,26 @@ function getTextEditsInternal(before: string, diffs: [number, string][], startLi return edits; } -export function getTempFileWithDocumentContents(document: TextDocument): Promise { - return new Promise((resolve, reject) => { - const ext = path.extname(document.uri.fsPath); - // Don't create file in temp folder since external utilities - // look into configuration files in the workspace and are not able - // to find custom rules if file is saved in a random disk location. - // This means temp file has to be created in the same folder - // as the original one and then removed. - - // tslint:disable-next-line:no-require-imports - const fileName = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`; - fs.writeFile(fileName, document.getText(), ex => { - if (ex) { - reject(`Failed to create a temporary file, ${ex.message}`); - } - resolve(fileName); - }); - }); +export async function getTempFileWithDocumentContents( + document: TextDocument, + fs: IFileSystem +): Promise { + // Don't create file in temp folder since external utilities + // look into configuration files in the workspace and are not able + // to find custom rules if file is saved in a random disk location. + // This means temp file has to be created in the same folder + // as the original one and then removed. + + const ext = path.extname(document.uri.fsPath); + // tslint:disable-next-line:no-require-imports + const filename = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`; + await ( + fs.writeFile(filename, document.getText()) + .catch(err => { + throw Error(`Failed to create a temporary file, ${err.message}`); + }) + ); + return filename; } /** diff --git a/src/client/common/envFileParser.ts b/src/client/common/envFileParser.ts index 4b9ec9fbea13..4f3b997e9451 100644 --- a/src/client/common/envFileParser.ts +++ b/src/client/common/envFileParser.ts @@ -1,8 +1,7 @@ -import * as fsextra from 'fs-extra'; import { IS_WINDOWS } from './platform/constants'; import { FileSystem } from './platform/fileSystem'; import { PathUtils } from './platform/pathUtils'; -import { IPlatformService } from './platform/types'; +import { IFileSystem, IPlatformService } from './platform/types'; import { EnvironmentVariablesService } from './variables/environment'; import { EnvironmentVariables, IEnvironmentVariablesService @@ -30,15 +29,19 @@ function parseEnvironmentVariables(contents: string): EnvironmentVariables | und export function parseEnvFile( envFile: string, mergeWithProcessEnvVars: boolean = true, - service?: IEnvironmentVariablesService + service?: IEnvironmentVariablesService, + fs?: IFileSystem ): EnvironmentVariables { - const buffer = fsextra.readFileSync(envFile, 'utf8'); + if (!fs) { + // tslint:disable-next-line:no-object-literal-type-assertion + fs = new FileSystem({} as IPlatformService); + } + const buffer = fs.readFileSync(envFile); const env = parseEnvironmentVariables(buffer)!; if (!service) { service = new EnvironmentVariablesService( new PathUtils(IS_WINDOWS), - // tslint:disable-next-line:no-object-literal-type-assertion - new FileSystem({} as IPlatformService) + fs ); } return mergeWithProcessEnvVars diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index b0b793a6c0f9..d7ea2901cbd3 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -53,6 +53,10 @@ export class FileSystem implements IFileSystem { return fsextra.existsSync(filename); } + public readFileSync(filename: string): string { + return fsextra.readFileSync(filename, ENCODING); + } + public async readFile(filename: string): Promise { return fsextra.readFile(filename, ENCODING); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index e9faaf74bb1e..39e4369b86d3 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -47,16 +47,16 @@ export type WriteStream = fs.WriteStream; export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { stat(filePath: string): Promise; - // fs-extra - fileExistsSync(filename: string): boolean; readFile(filename: string): Promise; writeFile(filename: string, data: {}): Promise; createDirectory(dirname: string): Promise; deleteDirectory(dirname: string): Promise; deleteFile(filename: string): Promise; - // fs - createWriteStream(filename: string): WriteStream; chmod(filename: string, mode: string): Promise; + // not async + fileExistsSync(filename: string): boolean; + readFileSync(filename: string): string; + createWriteStream(filename: string): WriteStream; // helpers arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. pathExists(filename: string, fileType?: FileType): Promise; diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index f8d295eeac65..fd905ad83786 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import * as fsextra from 'fs-extra'; import { inject, injectable } from 'inversify'; import * as path from 'path'; import { sendTelemetryEvent } from '../../telemetry'; @@ -19,14 +18,20 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService ) { this.pathVariable = pathUtils.getPathVariableName(); } - public async parseFile(filePath?: string, baseVars?: EnvironmentVariables): Promise { - if (!filePath || !await this.fs.pathExists(filePath)) { + public async parseFile( + filePath?: string, + baseVars?: EnvironmentVariables + ): Promise { + if (!filePath) { return; } - if (!fsextra.lstatSync(filePath).isFile()) { + if (!await this.fs.fileExists(filePath)) { return; } - return parseEnvFile(await fsextra.readFile(filePath), baseVars); + return parseEnvFile( + await this.fs.readFile(filePath), + baseVars + ); } public mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables) { if (!target) { diff --git a/src/client/formatters/baseFormatter.ts b/src/client/formatters/baseFormatter.ts index bd84428f8481..a8a2112c9d89 100644 --- a/src/client/formatters/baseFormatter.ts +++ b/src/client/formatters/baseFormatter.ts @@ -1,4 +1,3 @@ -import * as fs from 'fs-extra'; import * as path from 'path'; import * as vscode from 'vscode'; import { IApplicationShell, IWorkspaceService } from '../common/application/types'; @@ -6,6 +5,7 @@ import { STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import '../common/extensions'; import { isNotInstalledError } from '../common/helpers'; import { traceError } from '../common/logger'; +import { IFileSystem } from '../common/platform/types'; import { IPythonToolExecutionService } from '../common/process/types'; import { IDisposableRegistry, IInstaller, IOutputChannel, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; @@ -15,11 +15,17 @@ import { IFormatterHelper } from './types'; export abstract class BaseFormatter { protected readonly outputChannel: vscode.OutputChannel; protected readonly workspace: IWorkspaceService; + private readonly fs: IFileSystem; private readonly helper: IFormatterHelper; - constructor(public Id: string, private product: Product, protected serviceContainer: IServiceContainer) { + constructor( + public Id: string, + private product: Product, + protected serviceContainer: IServiceContainer + ) { this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); this.helper = serviceContainer.get(IFormatterHelper); + this.fs = serviceContainer.get(IFileSystem); this.workspace = serviceContainer.get(IWorkspaceService); } @@ -103,13 +109,13 @@ export abstract class BaseFormatter { private async createTempFile(document: vscode.TextDocument): Promise { return document.isDirty - ? getTempFileWithDocumentContents(document) + ? getTempFileWithDocumentContents(document, this.fs) : document.fileName; } private deleteTempFile(originalFile: string, tempFile: string): Promise { if (originalFile !== tempFile) { - return fs.unlink(tempFile); + return this.fs.deleteFile(tempFile); } return Promise.resolve(); } diff --git a/src/client/providers/renameProvider.ts b/src/client/providers/renameProvider.ts index a29de3281be6..481c804bf83d 100644 --- a/src/client/providers/renameProvider.ts +++ b/src/client/providers/renameProvider.ts @@ -6,6 +6,7 @@ import { import { EXTENSION_ROOT_DIR, STANDARD_OUTPUT_CHANNEL } from '../common/constants'; import { getWorkspaceEditsFromPatch } from '../common/editor'; import { traceError } from '../common/logger'; +import { IFileSystem } from '../common/platform/types'; import { IConfigurationService, IInstaller, IOutputChannel, Product } from '../common/types'; import { IServiceContainer } from '../ioc/types'; import { RefactorProxy } from '../refactor/proxy'; @@ -19,9 +20,13 @@ type RenameResponse = { export class PythonRenameProvider implements RenameProvider { private readonly outputChannel: OutputChannel; private readonly configurationService: IConfigurationService; - constructor(private serviceContainer: IServiceContainer) { + private readonly fs: IFileSystem; + constructor( + private serviceContainer: IServiceContainer + ) { this.outputChannel = serviceContainer.get(IOutputChannel, STANDARD_OUTPUT_CHANNEL); this.configurationService = serviceContainer.get(IConfigurationService); + this.fs = serviceContainer.get(IFileSystem); } @captureTelemetry(EventName.REFACTOR_RENAME) public provideRenameEdits(document: TextDocument, position: Position, newName: string, _token: CancellationToken): ProviderResult { @@ -57,7 +62,7 @@ export class PythonRenameProvider implements RenameProvider { const proxy = new RefactorProxy(EXTENSION_ROOT_DIR, pythonSettings, workspaceRoot, this.serviceContainer); return proxy.rename(document, newName, document.uri.fsPath, range).then(response => { const fileDiffs = response.results.map(fileChanges => fileChanges.diff); - return getWorkspaceEditsFromPatch(fileDiffs, workspaceRoot); + return getWorkspaceEditsFromPatch(fileDiffs, this.fs, workspaceRoot); }).catch(reason => { if (reason === 'Not installed') { const installer = this.serviceContainer.get(IInstaller); diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index a21ac036582c..aa526825ea00 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -5,6 +5,7 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; +import * as nodeFS from 'fs'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; import { PathUtils } from '../../../client/common/platform/pathUtils'; @@ -18,6 +19,9 @@ import { getOSType } from '../../common'; use(chaiAsPromised); const envFilesFolderPath = path.join(__dirname, '..', '..', '..', '..', 'src', 'testMultiRootWkspc', 'workspace4'); +const ENV1_TEXT = nodeFS.readFileSync(path.join(envFilesFolderPath, '.env'), 'utf8'); +const ENV5_TEXT = nodeFS.readFileSync(path.join(envFilesFolderPath, '.env5'), 'utf8'); +const ENV6_TEXT = nodeFS.readFileSync(path.join(envFilesFolderPath, '.env6'), 'utf8'); // tslint:disable-next-line:max-func-body-length suite('Environment Variables Service', () => { @@ -25,7 +29,8 @@ suite('Environment Variables Service', () => { let fs: TypeMoq.IMock; let variablesService: IEnvironmentVariablesService; - let pathExists: boolean; + let fileExists: boolean; + let text: string; setup(() => { pathUtils = new PathUtils(getOSType() === OSType.Windows); @@ -34,35 +39,50 @@ suite('Environment Variables Service', () => { pathUtils, fs.object ); - pathExists = true; + fileExists = true; + text = ''; fs - .setup(s => s.pathExists(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(pathExists)); + .setup(s => s.fileExists(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(fileExists)); + fs + .setup(s => s.readFile(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(text)); }); test('Custom variables should be undefined with no argument', async () => { const vars = await variablesService.parseFile(undefined); + expect(vars).to.equal(undefined, 'Variables should be undefined'); }); test('Custom variables should be undefined with non-existent files', async () => { - pathExists = false; - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, 'abcd')); + fileExists = false; + + const vars = await variablesService.parseFile('spam'); + expect(vars).to.equal(undefined, 'Variables should be undefined'); }); test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { - const vars = await variablesService.parseFile(envFilesFolderPath); + fileExists = false; + + const vars = await variablesService.parseFile('spam/'); + expect(vars).to.equal(undefined, 'Variables should be undefined'); }); test('Custom variables should be not undefined with a valid environment file', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); + text = ENV1_TEXT; + + const vars = await variablesService.parseFile('spam/.env'); + expect(vars).to.not.equal(undefined, 'Variables should be undefined'); }); test('Custom variables should be parsed from env file', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); + text = ENV1_TEXT; + + const vars = await variablesService.parseFile('spam/.env'); expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); @@ -71,7 +91,10 @@ suite('Environment Variables Service', () => { }); test('PATH and PYTHONPATH from env file should be returned as is', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env5')); + text = ENV5_TEXT; + + const vars = await variablesService.parseFile('spam/.env'); + const expectedPythonPath = '/usr/one/three:/usr/one/four'; const expectedPath = '/usr/x:/usr/y'; expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); @@ -83,8 +106,10 @@ suite('Environment Variables Service', () => { }); test('Simple variable substitution is supported', async () => { + text = ENV6_TEXT; + const vars = await variablesService.parseFile( - path.join(envFilesFolderPath, '.env6'), + 'spam/.env', { BINDIR: '/usr/bin' } ); From 7ea1b4bc8f6a51f769e1939a08727f7da6ae8e0f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Oct 2019 16:50:37 -0600 Subject: [PATCH 16/58] Stop using fs outside of the FileSystem class (mostly). --- src/client/common/net/fileDownloader.ts | 3 +- src/client/common/utils/fs.ts | 66 ------------------- src/client/common/utils/localize.ts | 17 +++-- src/client/interpreter/locators/helpers.ts | 35 +++++++--- .../locators/services/KnownPathsService.ts | 14 ++-- .../services/baseVirtualEnvService.ts | 2 +- src/client/sourceMapSupport.ts | 3 + .../managers/testConfigurationManager.ts | 12 +++- src/client/workspaceSymbols/parser.ts | 12 +++- .../process/pythonDaemon.functional.test.ts | 9 ++- .../pythonDaemonPool.functional.test.ts | 9 ++- .../nativeEditor.functional.test.tsx | 15 ++--- 12 files changed, 88 insertions(+), 109 deletions(-) delete mode 100644 src/client/common/utils/fs.ts diff --git a/src/client/common/net/fileDownloader.ts b/src/client/common/net/fileDownloader.ts index 53162c965304..281538a47a5d 100644 --- a/src/client/common/net/fileDownloader.ts +++ b/src/client/common/net/fileDownloader.ts @@ -3,12 +3,11 @@ 'use strict'; -import { WriteStream } from 'fs'; import { inject, injectable } from 'inversify'; import * as requestTypes from 'request'; import { Progress, ProgressLocation } from 'vscode'; import { IApplicationShell } from '../application/types'; -import { IFileSystem } from '../platform/types'; +import { IFileSystem, WriteStream } from '../platform/types'; import { DownloadOptions, IFileDownloader, IHttpClient } from '../types'; import { Http } from '../utils/localize'; import { noop } from '../utils/misc'; diff --git a/src/client/common/utils/fs.ts b/src/client/common/utils/fs.ts deleted file mode 100644 index 4ab1e19686c8..000000000000 --- a/src/client/common/utils/fs.ts +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import * as fs from 'fs'; -import * as path from 'path'; -import * as tmp from 'tmp'; - -export function fsExistsAsync(filePath: string): Promise { - return new Promise(resolve => { - fs.exists(filePath, exists => { - return resolve(exists); - }); - }); -} -export function fsReaddirAsync(root: string): Promise { - return new Promise(resolve => { - // Now look for Interpreters in this directory - fs.readdir(root, (err, subDirs) => { - if (err) { - return resolve([]); - } - resolve(subDirs.map(subDir => path.join(root, subDir))); - }); - }); -} - -export function getSubDirectories(rootDir: string): Promise { - return new Promise(resolve => { - fs.readdir(rootDir, (error, files) => { - if (error) { - return resolve([]); - } - const subDirs: string[] = []; - files.forEach(name => { - const fullPath = path.join(rootDir, name); - try { - if (fs.statSync(fullPath).isDirectory()) { - subDirs.push(fullPath); - } - } - // tslint:disable-next-line:no-empty one-line - catch (ex) { } - }); - resolve(subDirs); - }); - }); -} - -export function createTemporaryFile(extension: string, temporaryDirectory?: string): Promise<{ filePath: string; cleanupCallback: Function }> { - // tslint:disable-next-line:no-any - const options: any = { postfix: extension }; - if (temporaryDirectory) { - options.dir = temporaryDirectory; - } - - return new Promise<{ filePath: string; cleanupCallback: Function }>((resolve, reject) => { - tmp.file(options, (err, tmpFile, _fd, cleanupCallback) => { - if (err) { - return reject(err); - } - resolve({ filePath: tmpFile, cleanupCallback: cleanupCallback }); - }); - }); -} diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ba1648f0ef4e..b734e36116b8 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -3,9 +3,10 @@ 'use strict'; -import * as fs from 'fs'; import * as path from 'path'; import { EXTENSION_ROOT_DIR } from '../../constants'; +import { FileSystem } from '../platform/fileSystem'; +import { IFileSystem, IPlatformService } from '../platform/types'; // External callers of localize use these tables to retrieve localized values. export namespace Diagnostics { @@ -490,14 +491,18 @@ function getString(key: string, defValue?: string) { return result; } -function load() { +function load(fs?: IFileSystem) { + if (!fs) { + // tslint:disable-next-line:no-object-literal-type-assertion + fs = new FileSystem({} as IPlatformService); + } // Figure out our current locale. loadedLocale = parseLocale(); // Find the nls file that matches (if there is one) const nlsFile = path.join(EXTENSION_ROOT_DIR, `package.nls.${loadedLocale}.json`); - if (fs.existsSync(nlsFile)) { - const contents = fs.readFileSync(nlsFile, 'utf8'); + if (fs.fileExistsSync(nlsFile)) { + const contents = fs.readFileSync(nlsFile); loadedCollection = JSON.parse(contents); } else { // If there isn't one, at least remember that we looked so we don't try to load a second time @@ -507,8 +512,8 @@ function load() { // Get the default collection if necessary. Strings may be in the default or the locale json if (!defaultCollection) { const defaultNlsFile = path.join(EXTENSION_ROOT_DIR, 'package.nls.json'); - if (fs.existsSync(defaultNlsFile)) { - const contents = fs.readFileSync(defaultNlsFile, 'utf8'); + if (fs.fileExistsSync(defaultNlsFile)) { + const contents = fs.readFileSync(defaultNlsFile); defaultCollection = JSON.parse(contents); } else { defaultCollection = {}; diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index bf1a0a8424dd..d747d2fec2a2 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -2,20 +2,37 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { traceError } from '../../common/logger'; import { IS_WINDOWS } from '../../common/platform/constants'; -import { IFileSystem } from '../../common/platform/types'; -import { fsReaddirAsync } from '../../common/utils/fs'; +import { FileSystem } from '../../common/platform/fileSystem'; +import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IInterpreterLocatorHelper, InterpreterType, PythonInterpreter } from '../contracts'; import { IPipEnvServiceHelper } from './types'; const CheckPythonInterpreterRegEx = IS_WINDOWS ? /^python(\d+(.\d+)?)?\.exe$/ : /^python(\d+(.\d+)?)?$/; -export function lookForInterpretersInDirectory(pathToCheck: string): Promise { - return fsReaddirAsync(pathToCheck) - .then(subDirs => subDirs.filter(fileName => CheckPythonInterpreterRegEx.test(path.basename(fileName)))) - .catch(err => { - traceError('Python Extension (lookForInterpretersInDirectory.fsReaddirAsync):', err); - return [] as string[]; - }); +export async function lookForInterpretersInDirectory( + pathToCheck: string, + fs?: IFileSystem +): Promise { + if (!fs) { + // tslint:disable-next-line:no-object-literal-type-assertion + fs = new FileSystem({} as IPlatformService); + } + const files = await ( + fs.getFiles(pathToCheck) + .catch(err => { + traceError('Python Extension (lookForInterpretersInDirectory.fsReaddirAsync):', err); + return [] as string[]; + }) + ); + const result: string[] = []; + for (const filename of files) { + const name = path.basename(filename); + if (!CheckPythonInterpreterRegEx.test(name)) { + continue; + } + result.push(filename); + } + return result; } @injectable() diff --git a/src/client/interpreter/locators/services/KnownPathsService.ts b/src/client/interpreter/locators/services/KnownPathsService.ts index ee033e322bc7..1458d4a52ba9 100644 --- a/src/client/interpreter/locators/services/KnownPathsService.ts +++ b/src/client/interpreter/locators/services/KnownPathsService.ts @@ -2,9 +2,8 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { Uri } from 'vscode'; -import { IPlatformService } from '../../../common/platform/types'; +import { IFileSystem, IPlatformService } from '../../../common/platform/types'; import { ICurrentProcess, IPathUtils } from '../../../common/types'; -import { fsExistsAsync } from '../../../common/utils/fs'; import { IServiceContainer } from '../../../ioc/types'; import { IInterpreterHelper, IKnownSearchPathsForInterpreters, InterpreterType, PythonInterpreter } from '../../contracts'; import { lookForInterpretersInDirectory } from '../helpers'; @@ -16,12 +15,14 @@ const flatten = require('lodash/flatten') as typeof import('lodash/flatten'); */ @injectable() export class KnownPathsService extends CacheableLocatorService { + private readonly fs: IFileSystem; public constructor( @inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: IKnownSearchPathsForInterpreters, @inject(IInterpreterHelper) private helper: IInterpreterHelper, @inject(IServiceContainer) serviceContainer: IServiceContainer ) { super('KnownPathsService', serviceContainer); + this.fs = serviceContainer.get(IFileSystem); } /** @@ -72,9 +73,12 @@ export class KnownPathsService extends CacheableLocatorService { /** * Return the interpreters in the given directory. */ - private getInterpretersInDirectory(dir: string) { - return fsExistsAsync(dir) - .then(exists => exists ? lookForInterpretersInDirectory(dir) : Promise.resolve([])); + private async getInterpretersInDirectory(dir: string): Promise { + if (await this.fs.fileExists(dir)) { + return lookForInterpretersInDirectory(dir, this.fs); + } else { + return []; + } } } diff --git a/src/client/interpreter/locators/services/baseVirtualEnvService.ts b/src/client/interpreter/locators/services/baseVirtualEnvService.ts index 4b46de4a2261..1dec370789fd 100644 --- a/src/client/interpreter/locators/services/baseVirtualEnvService.ts +++ b/src/client/interpreter/locators/services/baseVirtualEnvService.ts @@ -40,7 +40,7 @@ export class BaseVirtualEnvService extends CacheableLocatorService { return this.fileSystem.getSubDirectories(pathToCheck) .then(subDirs => Promise.all(this.getProspectiveDirectoriesForLookup(subDirs))) .then(dirs => dirs.filter(dir => dir.length > 0)) - .then(dirs => Promise.all(dirs.map(lookForInterpretersInDirectory))) + .then(dirs => Promise.all(dirs.map(d => lookForInterpretersInDirectory(d, this.fileSystem)))) .then(pathsWithInterpreters => flatten(pathsWithInterpreters)) .then(interpreters => Promise.all(interpreters.map(interpreter => this.getVirtualEnvDetails(interpreter, resource)))) .then(interpreters => interpreters.filter(interpreter => !!interpreter).map(interpreter => interpreter!)) diff --git a/src/client/sourceMapSupport.ts b/src/client/sourceMapSupport.ts index 02907eaaf5ae..b878df21a217 100644 --- a/src/client/sourceMapSupport.ts +++ b/src/client/sourceMapSupport.ts @@ -59,6 +59,9 @@ export class SourceMapSupport { } } protected async rename(sourceFile: string, targetFile: string) { + // SourceMapSupport is initialized before the extension, so we + // do not have access to IFileSystem yet and have to use Node's + // "fs" directly. const fsExists = promisify(fs.exists); const fsRename = promisify(fs.rename); if (await fsExists(targetFile)) { diff --git a/src/client/testing/common/managers/testConfigurationManager.ts b/src/client/testing/common/managers/testConfigurationManager.ts index ca4d7ce43ebb..ad693d5b9a0a 100644 --- a/src/client/testing/common/managers/testConfigurationManager.ts +++ b/src/client/testing/common/managers/testConfigurationManager.ts @@ -1,9 +1,9 @@ import * as path from 'path'; import { OutputChannel, QuickPickItem, Uri } from 'vscode'; import { IApplicationShell } from '../../../common/application/types'; +import { IFileSystem } from '../../../common/platform/types'; import { IInstaller, ILogger, IOutputChannel } from '../../../common/types'; import { createDeferred } from '../../../common/utils/async'; -import { getSubDirectories } from '../../../common/utils/fs'; import { IServiceContainer } from '../../../ioc/types'; import { ITestConfigSettingsService, ITestConfigurationManager } from '../../types'; import { TEST_OUTPUT_CHANNEL, UNIT_TEST_PRODUCTS } from '../constants'; @@ -13,7 +13,10 @@ export abstract class TestConfigurationManager implements ITestConfigurationMana protected readonly outputChannel: OutputChannel; protected readonly installer: IInstaller; protected readonly testConfigSettingsService: ITestConfigSettingsService; - constructor(protected workspace: Uri, + private readonly fs: IFileSystem; + + constructor( + protected workspace: Uri, protected product: UnitTestProduct, protected readonly serviceContainer: IServiceContainer, cfg?: ITestConfigSettingsService @@ -21,9 +24,12 @@ export abstract class TestConfigurationManager implements ITestConfigurationMana this.outputChannel = serviceContainer.get(IOutputChannel, TEST_OUTPUT_CHANNEL); this.installer = serviceContainer.get(IInstaller); this.testConfigSettingsService = cfg ? cfg : serviceContainer.get(ITestConfigSettingsService); + this.fs = serviceContainer.get(IFileSystem); } + public abstract configure(wkspace: Uri): Promise; public abstract requiresUserToConfigure(wkspace: Uri): Promise; + public async enable() { // Disable other test frameworks. await Promise.all(UNIT_TEST_PRODUCTS @@ -101,7 +107,7 @@ export abstract class TestConfigurationManager implements ITestConfigurationMana return def.promise; } protected getTestDirs(rootDir: string): Promise { - return getSubDirectories(rootDir).then(subDirs => { + return this.fs.getSubDirectories(rootDir).then(subDirs => { subDirs.sort(); // Find out if there are any dirs with the name test and place them on the top. diff --git a/src/client/workspaceSymbols/parser.ts b/src/client/workspaceSymbols/parser.ts index 54bd50571360..37ea90e4f336 100644 --- a/src/client/workspaceSymbols/parser.ts +++ b/src/client/workspaceSymbols/parser.ts @@ -1,6 +1,7 @@ import * as path from 'path'; import * as vscode from 'vscode'; -import { fsExistsAsync } from '../common/utils/fs'; +import { FileSystem } from '../common/platform/fileSystem'; +import { IFileSystem, IPlatformService } from '../common/platform/types'; import { ITag } from './contracts'; // tslint:disable:no-require-imports no-var-requires no-suspicious-comment @@ -107,9 +108,14 @@ export function parseTags( workspaceFolder: string, tagFile: string, query: string, - token: vscode.CancellationToken + token: vscode.CancellationToken, + fs?: IFileSystem ): Promise { - return fsExistsAsync(tagFile).then(exists => { + if (!fs) { + // tslint:disable-next-line:no-object-literal-type-assertion + fs = new FileSystem({} as IPlatformService); + } + return fs.fileExists(tagFile).then(exists => { if (!exists) { return Promise.resolve([]); } diff --git a/src/test/common/process/pythonDaemon.functional.test.ts b/src/test/common/process/pythonDaemon.functional.test.ts index 7f6a865d9c82..cdcd5c471dd5 100644 --- a/src/test/common/process/pythonDaemon.functional.test.ts +++ b/src/test/common/process/pythonDaemon.functional.test.ts @@ -12,11 +12,12 @@ import * as os from 'os'; import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import { createMessageConnection, MessageConnection, RequestType, StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { PlatformService } from '../../../client/common/platform/platformService'; import { PythonDaemonExecutionService } from '../../../client/common/process/pythonDaemon'; import { PythonExecutionService } from '../../../client/common/process/pythonProcess'; import { IPythonExecutionService, PythonVersionInfo } from '../../../client/common/process/types'; import { IDisposable } from '../../../client/common/types'; -import { createTemporaryFile } from '../../../client/common/utils/fs'; import { Architecture } from '../../../client/common/utils/platform'; import { parsePythonVersion } from '../../../client/common/utils/version'; import { EXTENSION_ROOT_DIR } from '../../../client/constants'; @@ -34,6 +35,7 @@ suite('Daemon', () => { let pythonDaemon: PythonDaemonExecutionService; let pythonExecutionService: IPythonExecutionService; let disposables: IDisposable[] = []; + let fsUtils: FileSystem; suiteSetup(() => { // When running locally. if (PYTHON_PATH.toLowerCase() === 'python') { @@ -41,6 +43,7 @@ suite('Daemon', () => { .stdout.toString() .trim(); } + fsUtils = new FileSystem(new PlatformService()); }); setup(async function () { if (isPythonVersion('2.7')){ @@ -66,8 +69,8 @@ suite('Daemon', () => { }); async function createPythonFile(source: string): Promise { - const tmpFile = await createTemporaryFile('.py'); - disposables.push({ dispose: () => tmpFile.cleanupCallback() }); + const tmpFile = await fsUtils.createTemporaryFile('.py'); + disposables.push({ dispose: () => tmpFile.dispose() }); await fs.writeFile(tmpFile.filePath, source, { encoding: 'utf8' }); return tmpFile.filePath; } diff --git a/src/test/common/process/pythonDaemonPool.functional.test.ts b/src/test/common/process/pythonDaemonPool.functional.test.ts index 190f42d7b4ed..51f5a1e45599 100644 --- a/src/test/common/process/pythonDaemonPool.functional.test.ts +++ b/src/test/common/process/pythonDaemonPool.functional.test.ts @@ -15,13 +15,14 @@ import { Observable } from 'rxjs/Observable'; import * as sinon from 'sinon'; import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { createMessageConnection, StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { PlatformService } from '../../../client/common/platform/platformService'; import { ProcessLogger } from '../../../client/common/process/logger'; import { PythonDaemonExecutionServicePool } from '../../../client/common/process/pythonDaemonPool'; import { PythonExecutionService } from '../../../client/common/process/pythonProcess'; import { IProcessLogger, IPythonDaemonExecutionService, IPythonExecutionService, ObservableExecutionResult, Output, PythonVersionInfo } from '../../../client/common/process/types'; import { IDisposable } from '../../../client/common/types'; import { sleep } from '../../../client/common/utils/async'; -import { createTemporaryFile } from '../../../client/common/utils/fs'; import { noop } from '../../../client/common/utils/misc'; import { Architecture } from '../../../client/common/utils/platform'; import { parsePythonVersion } from '../../../client/common/utils/version'; @@ -41,6 +42,7 @@ suite('Daemon - Python Daemon Pool', () => { let disposables: IDisposable[] = []; let createDaemonServicesSpy: sinon.SinonSpy<[], Promise>; let logger: IProcessLogger; + let fsUtils: FileSystem; class DaemonPool extends PythonDaemonExecutionServicePool { // tslint:disable-next-line: no-unnecessary-override public createDaemonServices(): Promise { @@ -54,6 +56,7 @@ suite('Daemon - Python Daemon Pool', () => { .stdout.toString() .trim(); } + fsUtils = new FileSystem(new PlatformService()); }); setup(async function () { if (isPythonVersion('2.7')){ @@ -94,8 +97,8 @@ suite('Daemon - Python Daemon Pool', () => { } async function createPythonFile(source: string): Promise { - const tmpFile = await createTemporaryFile('.py'); - disposables.push({ dispose: () => tmpFile.cleanupCallback() }); + const tmpFile = await fsUtils.createTemporaryFile('.py'); + disposables.push({ dispose: () => tmpFile.dispose() }); await fs.writeFile(tmpFile.filePath, source, { encoding: 'utf8' }); return tmpFile.filePath; } diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 23f124fd4344..1baa7fc1c78d 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -14,9 +14,10 @@ import { anything, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode'; import { IApplicationShell, IDocumentManager } from '../../client/common/application/types'; -import { IFileSystem } from '../../client/common/platform/types'; +import { FileSystem } from '../../client/common/platform/fileSystem'; +import { PlatformService } from '../../client/common/platform/platformService'; +import { IFileSystem, TemporaryFile } from '../../client/common/platform/types'; import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async'; -import { createTemporaryFile } from '../../client/common/utils/fs'; import { noop } from '../../client/common/utils/misc'; import { Identifiers } from '../../client/datascience/constants'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; @@ -486,10 +487,7 @@ for _ in range(50): }); const addedJSONFile = JSON.stringify(addedJSON, null, ' '); - let notebookFile: { - filePath: string; - cleanupCallback: Function; - }; + let notebookFile: TemporaryFile; function initIoc() { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); @@ -503,7 +501,8 @@ for _ in range(50): addMockData(ioc, 'c=3\nc', 3); // Use a real file so we can save notebook to a file. // This is used in some tests (saving). - notebookFile = await createTemporaryFile('.ipynb'); + const filesystem = new FileSystem(new PlatformService()); + notebookFile = await filesystem.createTemporaryFile('.ipynb'); await fs.writeFile(notebookFile.filePath, baseFile); await Promise.all([waitForUpdate(wrapper, NativeEditor, 1), openEditor(ioc, baseFile, notebookFile.filePath)]); } else { @@ -525,7 +524,7 @@ for _ in range(50): } await ioc.dispose(); try { - notebookFile.cleanupCallback(); + notebookFile.dispose(); } catch { noop(); } From de5f872782461c628a7f77ef5d768b5d871e3a3d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 12:56:47 -0600 Subject: [PATCH 17/58] Drop IPlatformService as a dependency of FileSystem. --- src/client/common/envFileParser.ts | 13 +-- src/client/common/platform/fileSystem.ts | 9 +- src/client/common/utils/localize.ts | 7 +- src/client/interpreter/locators/helpers.ts | 9 +- src/client/workspaceSymbols/parser.ts | 7 +- src/test/common/crypto.unit.test.ts | 3 +- .../common/net/fileDownloader.unit.test.ts | 2 +- .../common/platform/filesystem.unit.test.ts | 108 +++++++++++------- .../process/pythonDaemon.functional.test.ts | 3 +- .../pythonDaemonPool.functional.test.ts | 3 +- .../terminalActivation.testvirtualenvs.ts | 3 +- .../nativeEditor.functional.test.tsx | 3 +- .../interpreters/condaService.unit.test.ts | 3 +- .../workspaceVirtualEnvService.test.ts | 3 +- src/test/linters/common.ts | 6 +- src/test/linters/lint.functional.test.ts | 5 +- src/test/linters/lint.unit.test.ts | 1 - src/test/sourceMapSupport.test.ts | 3 +- .../pytest/pytest.testMessageService.test.ts | 4 +- 19 files changed, 97 insertions(+), 98 deletions(-) diff --git a/src/client/common/envFileParser.ts b/src/client/common/envFileParser.ts index 4f3b997e9451..8c341363365b 100644 --- a/src/client/common/envFileParser.ts +++ b/src/client/common/envFileParser.ts @@ -1,7 +1,7 @@ import { IS_WINDOWS } from './platform/constants'; import { FileSystem } from './platform/fileSystem'; import { PathUtils } from './platform/pathUtils'; -import { IFileSystem, IPlatformService } from './platform/types'; +import { IFileSystem } from './platform/types'; import { EnvironmentVariablesService } from './variables/environment'; import { EnvironmentVariables, IEnvironmentVariablesService @@ -32,10 +32,7 @@ export function parseEnvFile( service?: IEnvironmentVariablesService, fs?: IFileSystem ): EnvironmentVariables { - if (!fs) { - // tslint:disable-next-line:no-object-literal-type-assertion - fs = new FileSystem({} as IPlatformService); - } + fs = fs ? fs : new FileSystem(); const buffer = fs.readFileSync(envFile); const env = parseEnvironmentVariables(buffer)!; if (!service) { @@ -65,8 +62,7 @@ export function mergeEnvVariables( if (!service) { service = new EnvironmentVariablesService( new PathUtils(IS_WINDOWS), - // tslint:disable-next-line:no-object-literal-type-assertion - new FileSystem({} as IPlatformService) + new FileSystem() ); } service.mergeVariables(sourceEnvVars, targetEnvVars); @@ -96,8 +92,7 @@ export function mergePythonPath( if (!service) { service = new EnvironmentVariablesService( new PathUtils(IS_WINDOWS), - // tslint:disable-next-line:no-object-literal-type-assertion - new FileSystem({} as IPlatformService) + new FileSystem() ); } service.appendPythonPath(env, currentPythonPath); diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index d7ea2901cbd3..393e59791a6d 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -6,15 +6,16 @@ import { createHash } from 'crypto'; import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import * as glob from 'glob'; -import { inject, injectable } from 'inversify'; +import { injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; +import { getOSType, OSType } from '../utils/platform'; import { FileStat, FileType, - IFileSystem, IPlatformService, + IFileSystem, TemporaryFile, WriteStream } from './types'; @@ -35,7 +36,7 @@ function getFileType(stat: FileStat): FileType { @injectable() export class FileSystem implements IFileSystem { constructor( - @inject(IPlatformService) private platformService: IPlatformService + private readonly isWindows = (getOSType() === OSType.Windows) ) { } public async stat(filePath: string): Promise { @@ -97,7 +98,7 @@ export class FileSystem implements IFileSystem { public arePathsSame(path1: string, path2: string): boolean { path1 = path.normalize(path1); path2 = path.normalize(path2); - if (this.platformService.isWindows) { + if (this.isWindows) { return path1.toUpperCase() === path2.toUpperCase(); } else { return path1 === path2; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index b734e36116b8..a4ab229ace9a 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import { EXTENSION_ROOT_DIR } from '../../constants'; import { FileSystem } from '../platform/fileSystem'; -import { IFileSystem, IPlatformService } from '../platform/types'; +import { IFileSystem } from '../platform/types'; // External callers of localize use these tables to retrieve localized values. export namespace Diagnostics { @@ -492,10 +492,7 @@ function getString(key: string, defValue?: string) { } function load(fs?: IFileSystem) { - if (!fs) { - // tslint:disable-next-line:no-object-literal-type-assertion - fs = new FileSystem({} as IPlatformService); - } + fs = fs ? fs : new FileSystem(); // Figure out our current locale. loadedLocale = parseLocale(); diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index d747d2fec2a2..98741a0536bd 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { traceError } from '../../common/logger'; import { IS_WINDOWS } from '../../common/platform/constants'; import { FileSystem } from '../../common/platform/fileSystem'; -import { IFileSystem, IPlatformService } from '../../common/platform/types'; +import { IFileSystem } from '../../common/platform/types'; import { IInterpreterLocatorHelper, InterpreterType, PythonInterpreter } from '../contracts'; import { IPipEnvServiceHelper } from './types'; @@ -13,12 +13,9 @@ export async function lookForInterpretersInDirectory( pathToCheck: string, fs?: IFileSystem ): Promise { - if (!fs) { - // tslint:disable-next-line:no-object-literal-type-assertion - fs = new FileSystem({} as IPlatformService); - } + fs = fs ? fs : new FileSystem(); const files = await ( - fs.getFiles(pathToCheck) + fs!.getFiles(pathToCheck) .catch(err => { traceError('Python Extension (lookForInterpretersInDirectory.fsReaddirAsync):', err); return [] as string[]; diff --git a/src/client/workspaceSymbols/parser.ts b/src/client/workspaceSymbols/parser.ts index 37ea90e4f336..3522e66c6a66 100644 --- a/src/client/workspaceSymbols/parser.ts +++ b/src/client/workspaceSymbols/parser.ts @@ -1,7 +1,7 @@ import * as path from 'path'; import * as vscode from 'vscode'; import { FileSystem } from '../common/platform/fileSystem'; -import { IFileSystem, IPlatformService } from '../common/platform/types'; +import { IFileSystem } from '../common/platform/types'; import { ITag } from './contracts'; // tslint:disable:no-require-imports no-var-requires no-suspicious-comment @@ -111,10 +111,7 @@ export function parseTags( token: vscode.CancellationToken, fs?: IFileSystem ): Promise { - if (!fs) { - // tslint:disable-next-line:no-object-literal-type-assertion - fs = new FileSystem({} as IPlatformService); - } + fs = fs ? fs : new FileSystem(); return fs.fileExists(tagFile).then(exists => { if (!exists) { return Promise.resolve([]); diff --git a/src/test/common/crypto.unit.test.ts b/src/test/common/crypto.unit.test.ts index 2d1ea5257cf9..dfa6f1801dcd 100644 --- a/src/test/common/crypto.unit.test.ts +++ b/src/test/common/crypto.unit.test.ts @@ -7,13 +7,12 @@ import { assert, expect } from 'chai'; import * as path from 'path'; import { CryptoUtils } from '../../client/common/crypto'; import { FileSystem } from '../../client/common/platform/fileSystem'; -import { PlatformService } from '../../client/common/platform/platformService'; import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants'; // tslint:disable-next-line: max-func-body-length suite('Crypto Utils', async () => { let crypto: CryptoUtils; - const fs = new FileSystem(new PlatformService()); + const fs = new FileSystem(); const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'common', 'randomWords.txt'); setup(() => { crypto = new CryptoUtils(); diff --git a/src/test/common/net/fileDownloader.unit.test.ts b/src/test/common/net/fileDownloader.unit.test.ts index d3b31e1ffc86..c82ddd023cc2 100644 --- a/src/test/common/net/fileDownloader.unit.test.ts +++ b/src/test/common/net/fileDownloader.unit.test.ts @@ -95,7 +95,7 @@ suite('File Downloader', () => { httpClient = mock(HttpClient); appShell = mock(ApplicationShell); when(httpClient.downloadFile(anything())).thenCall(request); - fs = new FileSystem(new PlatformService()); + fs = new FileSystem(); }); teardown(() => { rewiremock.disable(); diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 3bdcf929d1ed..d0f69ef4d427 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -4,9 +4,8 @@ import { expect, use } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; -import * as TypeMoq from 'typemoq'; import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { IFileSystem, IPlatformService, TemporaryFile } from '../../../client/common/platform/types'; +import { TemporaryFile } from '../../../client/common/platform/types'; // tslint:disable:no-require-imports no-var-requires const assertArrays = require('chai-arrays'); use(require('chai-as-promised')); @@ -14,13 +13,9 @@ use(assertArrays); // tslint:disable-next-line:max-func-body-length suite('FileSystem', () => { - let platformService: TypeMoq.IMock; - let fileSystem: IFileSystem; const fileToAppendTo = path.join(__dirname, 'created_for_testing_dummy.txt'); setup(() => { - platformService = TypeMoq.Mock.ofType(); - fileSystem = new FileSystem(platformService.object); - cleanTestFiles(); + cleanTestFiles(); // This smells like functional testing... }); teardown(cleanTestFiles); function cleanTestFiles() { @@ -28,71 +23,95 @@ suite('FileSystem', () => { fs.unlinkSync(fileToAppendTo); } } + test('ReadFile returns contents of a file', async () => { const file = __filename; + const filesystem = new FileSystem(); const expectedContents = await fs.readFile(file).then(buffer => buffer.toString()); - const content = await fileSystem.readFile(file); + + const content = await filesystem.readFile(file); expect(content).to.be.equal(expectedContents); }); test('ReadFile throws an exception if file does not exist', async () => { - const readPromise = fs.readFile('xyz', { encoding: 'utf8' }); + const filesystem = new FileSystem(); + + const readPromise = filesystem.readFile('xyz'); + await expect(readPromise).to.be.rejectedWith(); }); - function caseSensitivityFileCheck(isWindows: boolean, isOsx: boolean, isLinux: boolean) { - platformService.setup(p => p.isWindows).returns(() => isWindows); - platformService.setup(p => p.isMac).returns(() => isOsx); - platformService.setup(p => p.isLinux).returns(() => isLinux); + suite('Case sensitivity', () => { const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt'; const path2 = 'c:\\USERS\\Peter Smith\\my documents\\test.TXT'; const path3 = 'c:\\USERS\\Peter Smith\\my documents\\test.exe'; - if (isWindows) { - expect(fileSystem.arePathsSame(path1, path2)).to.be.equal(true, 'file paths do not match (windows)'); - } else { - expect(fileSystem.arePathsSame(path1, path2)).to.be.equal(false, 'file match (non windows)'); - } + test('Case sensitivity is ignored when comparing file names on windows', () => { + const isWindows = true; + const filesystem = new FileSystem(isWindows); - expect(fileSystem.arePathsSame(path1, path1)).to.be.equal(true, '1. file paths do not match'); - expect(fileSystem.arePathsSame(path2, path2)).to.be.equal(true, '2. file paths do not match'); - expect(fileSystem.arePathsSame(path1, path3)).to.be.equal(false, '2. file paths do not match'); - } + const same12 = filesystem.arePathsSame(path1, path2); + const same11 = filesystem.arePathsSame(path1, path1); + const same22 = filesystem.arePathsSame(path2, path2); + const same13 = filesystem.arePathsSame(path1, path3); - test('Case sensitivity is ignored when comparing file names on windows', async () => { - caseSensitivityFileCheck(true, false, false); - }); + expect(same12).to.be.equal(true, 'file paths do not match (windows)'); + expect(same11).to.be.equal(true, '1. file paths do not match'); + expect(same22).to.be.equal(true, '2. file paths do not match'); + expect(same13).to.be.equal(false, '2. file paths do not match'); + }); - test('Case sensitivity is not ignored when comparing file names on osx', async () => { - caseSensitivityFileCheck(false, true, false); - }); + test('Case sensitivity is not ignored when comparing file names on non-Windows', () => { + const isWindows = false; + const filesystem = new FileSystem(isWindows); + + const same12 = filesystem.arePathsSame(path1, path2); + const same11 = filesystem.arePathsSame(path1, path1); + const same22 = filesystem.arePathsSame(path2, path2); + const same13 = filesystem.arePathsSame(path1, path3); - test('Case sensitivity is not ignored when comparing file names on linux', async () => { - caseSensitivityFileCheck(false, false, true); + expect(same12).to.be.equal(false, 'file match (non windows)'); + expect(same11).to.be.equal(true, '1. file paths do not match'); + expect(same22).to.be.equal(true, '2. file paths do not match'); + expect(same13).to.be.equal(false, '2. file paths do not match'); + }); }); + test('Check existence of files synchronously', async () => { - expect(fileSystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found'); + const filesystem = new FileSystem(); + + expect(filesystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found'); }); test('Test searching for files', async () => { const searchPattern = `${path.basename(__filename, __filename.substring(__filename.length - 3))}.*`; - const files = await fileSystem.search(path.join(__dirname, searchPattern)); + const filesystem = new FileSystem(); + + const files = await filesystem.search(path.join(__dirname, searchPattern)); + expect(files).to.be.array(); expect(files.length).to.be.at.least(1); const expectedFileName = __filename.replace(/\\/g, '/'); const fileName = files[0].replace(/\\/g, '/'); expect(fileName).to.equal(expectedFileName); }); + test('Ensure creating a temporary file results in a unique temp file path', async () => { - const tempFile = await fileSystem.createTemporaryFile('.tmp'); - const tempFile2 = await fileSystem.createTemporaryFile('.tmp'); + const filesystem = new FileSystem(); + + const tempFile = await filesystem.createTemporaryFile('.tmp'); + const tempFile2 = await filesystem.createTemporaryFile('.tmp'); + expect(tempFile.filePath).to.not.equal(tempFile2.filePath, 'Temp files must be unique, implementation of createTemporaryFile is off.'); }); + test('Ensure writing to a temp file is supported via file stream', async () => { - await fileSystem.createTemporaryFile('.tmp').then((tf: TemporaryFile) => { + const filesystem = new FileSystem(); + + await filesystem.createTemporaryFile('.tmp').then((tf: TemporaryFile) => { expect(tf).to.not.equal(undefined, 'Error trying to create a temporary file'); - const writeStream = fileSystem.createWriteStream(tf.filePath); + const writeStream = filesystem.createWriteStream(tf.filePath); writeStream.write('hello', 'utf8', (err: Error | null | undefined) => { expect(err).to.equal(undefined, `Failed to write to a temp file, error is ${err}`); }); @@ -100,9 +119,12 @@ suite('FileSystem', () => { expect(failReason).to.equal('No errors occurred', `Failed to create a temporary file with error ${failReason}`); }); }); + test('Ensure chmod works against a temporary file', async () => { - await fileSystem.createTemporaryFile('.tmp').then(async (fl: TemporaryFile) => { - await fileSystem.chmod(fl.filePath, '7777').then( + const filesystem = new FileSystem(); + + await filesystem.createTemporaryFile('.tmp').then(async (fl: TemporaryFile) => { + await filesystem.chmod(fl.filePath, '7777').then( (_success: void) => { // cannot check for success other than we got here, chmod in Windows won't have any effect on the file itself. }, @@ -111,13 +133,19 @@ suite('FileSystem', () => { }); }); }); + test('Getting hash for non existent file should throw error', async () => { - const promise = fileSystem.getFileHash('some unknown file'); + const filesystem = new FileSystem(); + + const promise = filesystem.getFileHash('some unknown file'); await expect(promise).to.eventually.be.rejected; }); + test('Getting hash for a file should return non-empty string', async () => { - const hash = await fileSystem.getFileHash(__filename); + const filesystem = new FileSystem(); + + const hash = await filesystem.getFileHash(__filename); expect(hash).to.be.length.greaterThan(0); }); diff --git a/src/test/common/process/pythonDaemon.functional.test.ts b/src/test/common/process/pythonDaemon.functional.test.ts index cdcd5c471dd5..d1a51d89bd5b 100644 --- a/src/test/common/process/pythonDaemon.functional.test.ts +++ b/src/test/common/process/pythonDaemon.functional.test.ts @@ -13,7 +13,6 @@ import * as path from 'path'; import { instance, mock } from 'ts-mockito'; import { createMessageConnection, MessageConnection, RequestType, StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc'; import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { PlatformService } from '../../../client/common/platform/platformService'; import { PythonDaemonExecutionService } from '../../../client/common/process/pythonDaemon'; import { PythonExecutionService } from '../../../client/common/process/pythonProcess'; import { IPythonExecutionService, PythonVersionInfo } from '../../../client/common/process/types'; @@ -43,7 +42,7 @@ suite('Daemon', () => { .stdout.toString() .trim(); } - fsUtils = new FileSystem(new PlatformService()); + fsUtils = new FileSystem(); }); setup(async function () { if (isPythonVersion('2.7')){ diff --git a/src/test/common/process/pythonDaemonPool.functional.test.ts b/src/test/common/process/pythonDaemonPool.functional.test.ts index 51f5a1e45599..05e235b6f8f8 100644 --- a/src/test/common/process/pythonDaemonPool.functional.test.ts +++ b/src/test/common/process/pythonDaemonPool.functional.test.ts @@ -16,7 +16,6 @@ import * as sinon from 'sinon'; import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import { createMessageConnection, StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc'; import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { PlatformService } from '../../../client/common/platform/platformService'; import { ProcessLogger } from '../../../client/common/process/logger'; import { PythonDaemonExecutionServicePool } from '../../../client/common/process/pythonDaemonPool'; import { PythonExecutionService } from '../../../client/common/process/pythonProcess'; @@ -56,7 +55,7 @@ suite('Daemon - Python Daemon Pool', () => { .stdout.toString() .trim(); } - fsUtils = new FileSystem(new PlatformService()); + fsUtils = new FileSystem(); }); setup(async function () { if (isPythonVersion('2.7')){ diff --git a/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts b/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts index f09314612262..e462c2d44792 100644 --- a/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts +++ b/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts @@ -8,7 +8,6 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import * as vscode from 'vscode'; import { FileSystem } from '../../../../client/common/platform/fileSystem'; -import { PlatformService } from '../../../../client/common/platform/platformService'; import { PYTHON_VIRTUAL_ENVS_LOCATION } from '../../../ciConstants'; import { PYTHON_PATH, restorePythonPathInWorkspaceRoot, setPythonPathInWorkspaceRoot, updateSetting, waitForCondition } from '../../../common'; import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../constants'; @@ -20,7 +19,7 @@ suite('Activation of Environments in Terminal', () => { const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', 'testExecInTerminal.py'); let outputFile = ''; let outputFileCounter = 0; - const fileSystem = new FileSystem(new PlatformService()); + const fileSystem = new FileSystem(); const outputFilesCreated: string[] = []; const envsLocation = PYTHON_VIRTUAL_ENVS_LOCATION !== undefined ? path.join(EXTENSION_ROOT_DIR_FOR_TESTS, PYTHON_VIRTUAL_ENVS_LOCATION) : path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'tmp', 'envPaths.json'); diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 1baa7fc1c78d..7dfb0ebe228a 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -15,7 +15,6 @@ import * as TypeMoq from 'typemoq'; import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode'; import { IApplicationShell, IDocumentManager } from '../../client/common/application/types'; import { FileSystem } from '../../client/common/platform/fileSystem'; -import { PlatformService } from '../../client/common/platform/platformService'; import { IFileSystem, TemporaryFile } from '../../client/common/platform/types'; import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async'; import { noop } from '../../client/common/utils/misc'; @@ -501,7 +500,7 @@ for _ in range(50): addMockData(ioc, 'c=3\nc', 3); // Use a real file so we can save notebook to a file. // This is used in some tests (saving). - const filesystem = new FileSystem(new PlatformService()); + const filesystem = new FileSystem(); notebookFile = await filesystem.createTemporaryFile('.ipynb'); await fs.writeFile(notebookFile.filePath, baseFile); await Promise.all([waitForUpdate(wrapper, NativeEditor, 1), openEditor(ioc, baseFile, notebookFile.filePath)]); diff --git a/src/test/interpreters/condaService.unit.test.ts b/src/test/interpreters/condaService.unit.test.ts index cfe953fc6e6f..3dfdfc6705c2 100644 --- a/src/test/interpreters/condaService.unit.test.ts +++ b/src/test/interpreters/condaService.unit.test.ts @@ -94,7 +94,8 @@ suite('Interpreters Conda Service', () => { config.setup(c => c.getSettings(TypeMoq.It.isValue(undefined))).returns(() => settings.object); settings.setup(p => p.condaPath).returns(() => condaPathSetting); fileSystem.setup(fs => fs.arePathsSame(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p1, p2) => { - return new FileSystem(platformService.object).arePathsSame(p1, p2); + return new FileSystem(platformService.object.isWindows) + .arePathsSame(p1, p2); }); condaService = new CondaService( diff --git a/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts b/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts index 76d8af54a136..473f20e24ab8 100644 --- a/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts +++ b/src/test/interpreters/locators/workspaceVirtualEnvService.test.ts @@ -11,7 +11,6 @@ import { promisify } from 'util'; import { Uri } from 'vscode'; import '../../../client/common/extensions'; import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { PlatformService } from '../../../client/common/platform/platformService'; import { createDeferredFromPromise, Deferred } from '../../../client/common/utils/async'; import { StopWatch } from '../../../client/common/utils/stopWatch'; import { @@ -93,7 +92,7 @@ class Venvs { ? this.pipInstaller : path.join(this.topDir, 'get-pip.py'); if (!this.pipInstaller) { - const fs = new FileSystem(new PlatformService()); + const fs = new FileSystem(); if (!await fs.fileExists(script)) { await this.run([ 'curl', diff --git a/src/test/linters/common.ts b/src/test/linters/common.ts index b9967be94351..8aa903c3cd92 100644 --- a/src/test/linters/common.ts +++ b/src/test/linters/common.ts @@ -17,8 +17,7 @@ import { import { Product } from '../../client/common/installer/productInstaller'; import { ProductNames } from '../../client/common/installer/productNames'; import { - IFileSystem, - IPlatformService + IFileSystem } from '../../client/common/platform/types'; import { IPythonExecutionFactory, @@ -209,7 +208,6 @@ export class BaseTestFixture { public logged: string[]; constructor( - platformService: IPlatformService, filesystem: IFileSystem, pythonToolExecService: IPythonToolExecutionService, pythonExecFactory: IPythonExecutionFactory, @@ -236,8 +234,6 @@ export class BaseTestFixture { .returns(() => this.logger.object); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInstaller), TypeMoq.It.isAny())) .returns(() => this.installer.object); - this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService), TypeMoq.It.isAny())) - .returns(() => platformService); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonToolExecutionService), TypeMoq.It.isAny())) .returns(() => pythonToolExecService); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory), TypeMoq.It.isAny())) diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index 4bcba475a609..983726e3c0de 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -16,7 +16,6 @@ import { } from 'vscode'; import { Product } from '../../client/common/installer/productInstaller'; import { FileSystem } from '../../client/common/platform/fileSystem'; -import { PlatformService } from '../../client/common/platform/platformService'; import { BufferDecoder } from '../../client/common/process/decoder'; import { ProcessServiceFactory } from '../../client/common/process/processFactory'; import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory'; @@ -197,11 +196,9 @@ class TestFixture extends BaseTestFixture { processLogger.setup(p => p.logProcess(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { return; }); serviceContainer.setup(s => s.get(TypeMoq.It.isValue(IProcessLogger), TypeMoq.It.isAny())).returns(() => processLogger.object); - const platformService = new PlatformService(); - const filesystem = new FileSystem(platformService); + const filesystem = new FileSystem(); super( - platformService, filesystem, TestFixture.newPythonToolExecService( serviceContainer.object diff --git a/src/test/linters/lint.unit.test.ts b/src/test/linters/lint.unit.test.ts index 7fb81f4aa582..af98e6b561f5 100644 --- a/src/test/linters/lint.unit.test.ts +++ b/src/test/linters/lint.unit.test.ts @@ -115,7 +115,6 @@ class TestFixture extends BaseTestFixture { const pythonToolExecService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const pythonExecFactory = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); super( - platformService.object, filesystem.object, pythonToolExecService.object, pythonExecFactory.object, diff --git a/src/test/sourceMapSupport.test.ts b/src/test/sourceMapSupport.test.ts index 67295bd7157d..af63a1bd58b5 100644 --- a/src/test/sourceMapSupport.test.ts +++ b/src/test/sourceMapSupport.test.ts @@ -9,7 +9,6 @@ import { expect } from 'chai'; import * as fs from 'fs'; import { ConfigurationTarget, Disposable } from 'vscode'; import { FileSystem } from '../client/common/platform/fileSystem'; -import { PlatformService } from '../client/common/platform/platformService'; import { Diagnostics } from '../client/common/utils/localize'; import { SourceMapSupport } from '../client/sourceMapSupport'; import { noop } from './core'; @@ -66,7 +65,7 @@ suite('Source Map Support', () => { }); }); test('When disabling source maps, the map file is renamed and vice versa', async () => { - const fileSystem = new FileSystem(new PlatformService()); + const fileSystem = new FileSystem(); const jsFile = await fileSystem.createTemporaryFile('.js'); disposables.push(jsFile); const mapFile = `${jsFile.filePath}.map`; diff --git a/src/test/testing/pytest/pytest.testMessageService.test.ts b/src/test/testing/pytest/pytest.testMessageService.test.ts index a745a8a0c0d2..2202b45721ea 100644 --- a/src/test/testing/pytest/pytest.testMessageService.test.ts +++ b/src/test/testing/pytest/pytest.testMessageService.test.ts @@ -13,7 +13,6 @@ import { IWorkspaceService } from '../../../client/common/application/types'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { ProductNames } from '../../../client/common/installer/productNames'; import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { PlatformService } from '../../../client/common/platform/platformService'; import { Product } from '../../../client/common/types'; import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts'; import { InterpreterService } from '../../../client/interpreter/interpreterService'; @@ -101,8 +100,7 @@ async function getExpectedLocationStackFromTestDetails(testDetails: ITestDetails suite('Unit Tests - PyTest - TestMessageService', () => { let ioc: UnitTestIocContainer; - const platformService = new PlatformService(); - const filesystem = new FileSystem(platformService); + const filesystem = new FileSystem(); const configTarget = IS_MULTI_ROOT_TEST ? vscode.ConfigurationTarget.WorkspaceFolder : vscode.ConfigurationTarget.Workspace; suiteSetup(async () => { await initialize(); From e3b362328cf488402e6362a3be202e50f8019d9a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 15:06:55 -0600 Subject: [PATCH 18/58] Require strict mode. --- src/client/common/platform/fileSystem.ts | 1 + src/client/common/platform/types.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 393e59791a6d..6f8be7b90cc7 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -1,5 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. + 'use strict'; import { createHash } from 'crypto'; diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 39e4369b86d3..80975534c531 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +'use strict'; + import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import { SemVer } from 'semver'; @@ -39,6 +41,8 @@ export type TemporaryDirectory = vscode.Disposable & { path: string; }; +//export { FileType } from 'vscode'; +//import { FileType } from 'vscode'; import FileType = vscode.FileType; export { FileType }; export type FileStat = fsextra.Stats; From ad0f8570ae520a7cfb91e2d6ba3e95c01da02c69 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 15:08:07 -0600 Subject: [PATCH 19/58] Fix some tests. --- src/test/linters/common.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/linters/common.ts b/src/test/linters/common.ts index 8aa903c3cd92..94534209bfa3 100644 --- a/src/test/linters/common.ts +++ b/src/test/linters/common.ts @@ -17,7 +17,8 @@ import { import { Product } from '../../client/common/installer/productInstaller'; import { ProductNames } from '../../client/common/installer/productNames'; import { - IFileSystem + IFileSystem, + IPlatformService } from '../../client/common/platform/types'; import { IPythonExecutionFactory, @@ -194,6 +195,7 @@ export class BaseTestFixture { public logger: TypeMoq.IMock; public installer: TypeMoq.IMock; public appShell: TypeMoq.IMock; + public platform: TypeMoq.IMock; // config public configService: TypeMoq.IMock; @@ -225,6 +227,7 @@ export class BaseTestFixture { this.logger = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); this.installer = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); this.appShell = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + this.platform = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny())) .returns(() => filesystem); @@ -234,6 +237,8 @@ export class BaseTestFixture { .returns(() => this.logger.object); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInstaller), TypeMoq.It.isAny())) .returns(() => this.installer.object); + this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService), TypeMoq.It.isAny())) + .returns(() => this.platform.object); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonToolExecutionService), TypeMoq.It.isAny())) .returns(() => pythonToolExecService); this.serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory), TypeMoq.It.isAny())) From 2fdc4819c5ea0a518d2b75a014aeff5378f43917 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 15:28:51 -0600 Subject: [PATCH 20/58] Add a mock vscode enum. --- src/client/common/platform/types.ts | 5 +---- src/test/mocks/vsc/index.ts | 7 +++++++ src/test/vscode-mock.ts | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 80975534c531..32b4303b512a 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -41,10 +41,7 @@ export type TemporaryDirectory = vscode.Disposable & { path: string; }; -//export { FileType } from 'vscode'; -//import { FileType } from 'vscode'; -import FileType = vscode.FileType; -export { FileType }; +export import FileType = vscode.FileType; export type FileStat = fsextra.Stats; export type WriteStream = fs.WriteStream; diff --git a/src/test/mocks/vsc/index.ts b/src/test/mocks/vsc/index.ts index c7df9e7c2148..8e5edfb699d1 100644 --- a/src/test/mocks/vsc/index.ts +++ b/src/test/mocks/vsc/index.ts @@ -183,4 +183,11 @@ export namespace vscMock { export class DebugAdapterExecutable { constructor(public readonly command: string, public readonly args: string[] = [], public readonly options?: DebugAdapterExecutableOptions) { } } + + export enum FileType { + Unknown = 0, + File = 1, + Directory = 2, + SymbolicLink = 64 + } } diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index be2f0c367624..18e56f318b40 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -82,6 +82,7 @@ mockedVSCode.CodeActionKind = vscodeMocks.vscMock.CodeActionKind; mockedVSCode.DebugAdapterExecutable = vscodeMocks.vscMock.DebugAdapterExecutable; mockedVSCode.DebugAdapterServer = vscodeMocks.vscMock.DebugAdapterServer; mockedVSCode.QuickInputButtons = vscodeMocks.vscMockExtHostedTypes.QuickInputButtons; +mockedVSCode.FileType = vscodeMocks.vscMock.FileType; // This API is used in src/client/telemetry/telemetry.ts const extensions = TypeMoq.Mock.ofType(); From 4a66d39e3833a556c38d7a158378dac76ae41ccd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 17:54:43 -0600 Subject: [PATCH 21/58] Factor out IRawFileSystem. --- src/client/common/platform/fileSystem.ts | 236 +++++++++++++++++------ src/client/common/platform/types.ts | 34 +++- 2 files changed, 208 insertions(+), 62 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 6f8be7b90cc7..4471bc97eece 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -12,15 +12,14 @@ import * as path from 'path'; import * as tmp from 'tmp'; import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; -import { noop } from '../utils/misc'; import { getOSType, OSType } from '../utils/platform'; import { FileStat, FileType, - IFileSystem, + IFileSystem, IRawFileSystem, TemporaryFile, WriteStream } from './types'; -const ENCODING = 'utf8'; +const ENCODING: string = 'utf8'; function getFileType(stat: FileStat): FileType { if (stat.isFile()) { @@ -34,12 +33,153 @@ function getFileType(stat: FileStat): FileType { } } +interface IRawFS { + //tslint:disable-next-line:no-any + open(filename: string, flags: number, callback: any): void; + //tslint:disable-next-line:no-any + close(fd: number, callback: any): void; + //tslint:disable-next-line:no-any + unlink(filename: string, callback: any): void; + + // non-async + createWriteStream(filePath: string): fs.WriteStream; +} + +interface IRawFSExtra { + chmod(filePath: string, mode: string): Promise; + readFile(path: string, encoding: string): Promise; + //tslint:disable-next-line:no-any + writeFile(path: string, data: any, options: any): Promise; + unlink(filename: string): Promise; + stat(filename: string): Promise; + lstat(filename: string): Promise; + mkdirp(dirname: string): Promise; + rmdir(dirname: string): Promise; + readdir(dirname: string): Promise; + + // non-async + statSync(filename: string): fsextra.Stats; + readFileSync(path: string, encoding: string): string; + createReadStream(src: string): fsextra.ReadStream; + createWriteStream(dest: string): fsextra.WriteStream; +} + +// Later we will rename "FileSystem" to "FileSystemUtils" and +// "RawFileSystem" to "FileSystem". + +@injectable() +class RawFileSystem { + constructor( + private readonly nodefs: IRawFS = fs, + private readonly fsExtra: IRawFSExtra = fsextra + ) { } + + //**************************** + // fs-extra + + public async readText(filename: string): Promise { + return this.fsExtra.readFile(filename, ENCODING); + } + + public async writeText(filename: string, data: {}): Promise { + const options: fsextra.WriteFileOptions = { + encoding: ENCODING + }; + await this.fsExtra.writeFile(filename, data, options); + } + + public async mkdirp(dirname: string): Promise { + return this.fsExtra.mkdirp(dirname); + } + + public async rmtree(dirname: string): Promise { + return this.fsExtra.rmdir(dirname); + } + + public async rmfile(filename: string): Promise { + return this.fsExtra.unlink(filename); + } + + public async chmod(filename: string, mode: string): Promise { + return this.fsExtra.chmod(filename, mode); + } + + public async stat(filename: string): Promise { + return this.fsExtra.stat(filename); + } + + public async lstat(filename: string): Promise { + return this.fsExtra.lstat(filename); + } + + public async listdir(dirname: string): Promise { + return this.fsExtra.readdir(dirname); + } + + public async copyFile(src: string, dest: string): Promise { + const deferred = createDeferred(); + const rs = this.fsExtra.createReadStream(src) + .on('error', (err) => { + deferred.reject(err); + }); + const ws = this.fsExtra.createWriteStream(dest) + .on('error', (err) => { + deferred.reject(err); + }).on('close', () => { + deferred.resolve(); + }); + rs.pipe(ws); + return deferred.promise; + } + + //**************************** + // fs + + public async touch(filename: string): Promise { + const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; + const raw = this.nodefs; + return new Promise((resolve, reject) => { + raw.open(filename, flags, (error: string, fd: number) => { + if (error) { + return reject(error); + } + raw.close(fd, () => { + return resolve(); + }); + }); + }); + } + + //**************************** + // non-async (fs-extra) + + public statSync(filename: string): FileStat { + return this.fsExtra.statSync(filename); + } + + public readTextSync(filename: string): string { + return this.fsExtra.readFileSync(filename, ENCODING); + } + + //**************************** + // non-async (fs) + + public createWriteStream(filename: string): WriteStream { + return this.nodefs.createWriteStream(filename); + } +} + @injectable() export class FileSystem implements IFileSystem { constructor( - private readonly isWindows = (getOSType() === OSType.Windows) + private readonly isWindows = (getOSType() === OSType.Windows), + //public readonly raw: IFileSystem = {} + public readonly raw: IRawFileSystem = new RawFileSystem() ) { } + //**************************** + // aliases + public async stat(filePath: string): Promise { // Do not import vscode directly, as this isn't available in the Debugger Context. // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. @@ -48,49 +188,40 @@ export class FileSystem implements IFileSystem { return vscode.workspace.fs.stat(vscode.Uri.file(filePath)); } - //**************************** - // fs-extra - - public fileExistsSync(filename: string): boolean { - return fsextra.existsSync(filename); - } - - public readFileSync(filename: string): string { - return fsextra.readFileSync(filename, ENCODING); - } - public async readFile(filename: string): Promise { - return fsextra.readFile(filename, ENCODING); + return this.raw.readText(filename); } public async writeFile(filename: string, data: {}): Promise { - const options: fsextra.WriteFileOptions = { - encoding: ENCODING - }; - await fsextra.writeFile(filename, data, options); + return this.raw.writeText(filename, data); } public async createDirectory(dirname: string): Promise { - return fsextra.mkdirp(dirname); + return this.raw.mkdirp(dirname); } public async deleteDirectory(dirname: string): Promise { - return fsextra.rmdir(dirname); + return this.raw.rmtree(dirname); } public async deleteFile(filename: string): Promise { - return fsextra.unlink(filename); + return this.raw.rmfile(filename); } public async chmod(filename: string, mode: string): Promise { - return fsextra.chmod(filename, mode); + return this.raw.chmod(filename, mode); } - //**************************** - // fs + public async copyFile(src: string, dest: string): Promise { + return this.raw.copyFile(src, dest); + } + + public readFileSync(filename: string): string { + return this.raw.readTextSync(filename); + } public createWriteStream(filename: string): WriteStream { - return fs.createWriteStream(filename); + return this.raw.createWriteStream(filename); } //**************************** @@ -112,7 +243,7 @@ export class FileSystem implements IFileSystem { ): Promise { let stat: FileStat; try { - stat = await fsextra.stat(filename); + stat = await this.raw.stat(filename); } catch { return false; } @@ -132,18 +263,26 @@ export class FileSystem implements IFileSystem { public async directoryExists(dirname: string): Promise { return this.pathExists(dirname, FileType.Directory); } + public fileExistsSync(filename: string): boolean { + try { + this.raw.statSync(filename); + } catch { + return false; + } + return true; + } public async listdir( dirname: string ): Promise<[string, FileType][]> { const filenames: string[] = await ( - fsextra.readdir(dirname) + this.raw.listdir(dirname) .then(names => names.map(name => path.join(dirname, name))) .catch(() => []) ); const promises = filenames .map(filename => ( - fsextra.stat(filename) + this.raw.stat(filename) .then(stat => [filename, getFileType(stat)] as [string, FileType]) .catch(() => [filename, FileType.Unknown] as [string, FileType]) )); @@ -162,37 +301,18 @@ export class FileSystem implements IFileSystem { public async isDirReadonly(dirname: string): Promise { // Alternative: use tmp.file(). - const filePath = path.join(dirname, '___vscpTest___'); - return new Promise(resolve => { - fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => { - if (!error) { - fs.close(fd, () => { - fs.unlink(filePath, noop); - }); - } - return resolve(!!error); - }); - }); - } - - public async copyFile(src: string, dest: string): Promise { - const deferred = createDeferred(); - const rs = fsextra.createReadStream(src) - .on('error', (err) => { - deferred.reject(err); - }); - const ws = fsextra.createWriteStream(dest) - .on('error', (err) => { - deferred.reject(err); - }).on('close', () => { - deferred.resolve(); - }); - rs.pipe(ws); - return deferred.promise; + const filename = path.join(dirname, '___vscpTest___'); + try { + await this.raw.touch(filename); + } catch { + return false; + } + await this.raw.rmfile(filename); + return true; } public async getFileHash(filename: string): Promise { - const stat = await fsextra.lstat(filename); + const stat = await this.raw.lstat(filename); const hash = createHash('sha512') .update(`${stat.ctimeMs}-${stat.mtimeMs}`); return hash.digest('hex'); diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 32b4303b512a..d63009544270 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -45,8 +45,33 @@ export import FileType = vscode.FileType; export type FileStat = fsextra.Stats; export type WriteStream = fs.WriteStream; +// Later we will rename "IFileSystem" to "IFileSystemUtils" and +// "IRawFileSystem" to "IFileSystem". + +export interface IRawFileSystem { + stat(filename: string): Promise; + lstat(filename: string): Promise; + chmod(filename: string, mode: string): Promise; + // files + readText(filename: string): Promise; + writeText(filename: string, data: {}): Promise; + touch(filename: string): Promise; + copyFile(src: string, dest: string): Promise; + rmfile(filename: string): Promise; + // directories + mkdirp(dirname: string): Promise; + rmtree(dirname: string): Promise; + listdir(dirname: string): Promise; + // not async + statSync(filename: string): FileStat; + readTextSync(filename: string): string; + createWriteStream(filename: string): WriteStream; +} + export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { + raw: IRawFileSystem; + // aliases (to cause less churn) stat(filePath: string): Promise; readFile(filename: string): Promise; writeFile(filename: string, data: {}): Promise; @@ -54,20 +79,21 @@ export interface IFileSystem { deleteDirectory(dirname: string): Promise; deleteFile(filename: string): Promise; chmod(filename: string, mode: string): Promise; - // not async - fileExistsSync(filename: string): boolean; + copyFile(src: string, dest: string): Promise; + // aliases (non-async) readFileSync(filename: string): string; createWriteStream(filename: string): WriteStream; // helpers - arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. pathExists(filename: string, fileType?: FileType): Promise; fileExists(filename: string): Promise; directoryExists(dirname: string): Promise; getSubDirectories(dirname: string): Promise; getFiles(dirname: string): Promise; isDirReadonly(dirname: string): Promise; - copyFile(src: string, dest: string): Promise; getFileHash(filename: string): Promise; search(globPattern: string): Promise; createTemporaryFile(suffix: string): Promise; + // helpers (non-async) + arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. + fileExistsSync(filename: string): boolean; } From a9fdb3502fb2f557cc2175c8e571c7c172e10c52 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 18:06:01 -0600 Subject: [PATCH 22/58] Factor out IFileSystemUtils. --- src/client/common/platform/fileSystem.ts | 77 ++++++++++--------- src/client/common/platform/serviceRegistry.ts | 5 +- src/client/common/platform/types.ts | 31 ++++---- 3 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 4471bc97eece..69f4a7fd91d9 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -15,7 +15,7 @@ import { createDeferred } from '../utils/async'; import { getOSType, OSType } from '../utils/platform'; import { FileStat, FileType, - IFileSystem, IRawFileSystem, + IFileSystem, IFileSystemUtils, IRawFileSystem, TemporaryFile, WriteStream } from './types'; @@ -64,10 +64,9 @@ interface IRawFSExtra { createWriteStream(dest: string): fsextra.WriteStream; } -// Later we will rename "FileSystem" to "FileSystemUtils" and -// "RawFileSystem" to "FileSystem". +// Later we will drop "FileSystem", switching usage to +// "FileSystemUtils" and then rename "RawFileSystem" to "FileSystem". -@injectable() class RawFileSystem { constructor( private readonly nodefs: IRawFS = fs, @@ -169,8 +168,9 @@ class RawFileSystem { } } +// more aliases (to cause less churn) @injectable() -export class FileSystem implements IFileSystem { +export class FileSystemUtils implements IFileSystemUtils { constructor( private readonly isWindows = (getOSType() === OSType.Windows), //public readonly raw: IFileSystem = {} @@ -180,22 +180,6 @@ export class FileSystem implements IFileSystem { //**************************** // aliases - public async stat(filePath: string): Promise { - // Do not import vscode directly, as this isn't available in the Debugger Context. - // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. - // tslint:disable-next-line: no-require-imports - const vscode = require('vscode'); - return vscode.workspace.fs.stat(vscode.Uri.file(filePath)); - } - - public async readFile(filename: string): Promise { - return this.raw.readText(filename); - } - - public async writeFile(filename: string, data: {}): Promise { - return this.raw.writeText(filename, data); - } - public async createDirectory(dirname: string): Promise { return this.raw.mkdirp(dirname); } @@ -208,22 +192,6 @@ export class FileSystem implements IFileSystem { return this.raw.rmfile(filename); } - public async chmod(filename: string, mode: string): Promise { - return this.raw.chmod(filename, mode); - } - - public async copyFile(src: string, dest: string): Promise { - return this.raw.copyFile(src, dest); - } - - public readFileSync(filename: string): string { - return this.raw.readTextSync(filename); - } - - public createWriteStream(filename: string): WriteStream { - return this.raw.createWriteStream(filename); - } - //**************************** // helpers @@ -345,3 +313,38 @@ export class FileSystem implements IFileSystem { }); } } + +@injectable() +export class FileSystem extends FileSystemUtils implements IFileSystem { + public async stat(filePath: string): Promise { + // Do not import vscode directly, as this isn't available in the Debugger Context. + // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. + // tslint:disable-next-line: no-require-imports + const vscode = require('vscode'); + return vscode.workspace.fs.stat(vscode.Uri.file(filePath)); + } + + public async readFile(filename: string): Promise { + return this.raw.readText(filename); + } + + public async writeFile(filename: string, data: {}): Promise { + return this.raw.writeText(filename, data); + } + + public async chmod(filename: string, mode: string): Promise { + return this.raw.chmod(filename, mode); + } + + public async copyFile(src: string, dest: string): Promise { + return this.raw.copyFile(src, dest); + } + + public readFileSync(filename: string): string { + return this.raw.readTextSync(filename); + } + + public createWriteStream(filename: string): WriteStream { + return this.raw.createWriteStream(filename); + } +} diff --git a/src/client/common/platform/serviceRegistry.ts b/src/client/common/platform/serviceRegistry.ts index d15edf5fc388..7c1f0df30666 100644 --- a/src/client/common/platform/serviceRegistry.ts +++ b/src/client/common/platform/serviceRegistry.ts @@ -3,13 +3,14 @@ 'use strict'; import { IServiceManager } from '../../ioc/types'; -import { FileSystem } from './fileSystem'; +import { FileSystem, FileSystemUtils } from './fileSystem'; import { PlatformService } from './platformService'; import { RegistryImplementation } from './registry'; -import { IFileSystem, IPlatformService, IRegistry } from './types'; +import { IFileSystem, IFileSystemUtils, IPlatformService, IRegistry } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IPlatformService, PlatformService); serviceManager.addSingleton(IFileSystem, FileSystem); + serviceManager.addSingleton(IFileSystemUtils, FileSystemUtils); serviceManager.addSingleton(IRegistry, RegistryImplementation); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index d63009544270..b82dffdf6023 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -45,8 +45,8 @@ export import FileType = vscode.FileType; export type FileStat = fsextra.Stats; export type WriteStream = fs.WriteStream; -// Later we will rename "IFileSystem" to "IFileSystemUtils" and -// "IRawFileSystem" to "IFileSystem". +// Later we will drop "IFileSystem", switching usage to +// "IFileSystemUtils" and then rename "IRawFileSystem" to "IFileSystem". export interface IRawFileSystem { stat(filename: string): Promise; @@ -68,21 +68,13 @@ export interface IRawFileSystem { createWriteStream(filename: string): WriteStream; } -export const IFileSystem = Symbol('IFileSystem'); -export interface IFileSystem { +export const IFileSystemUtils = Symbol('IFileSystemUtils'); +export interface IFileSystemUtils { raw: IRawFileSystem; - // aliases (to cause less churn) - stat(filePath: string): Promise; - readFile(filename: string): Promise; - writeFile(filename: string, data: {}): Promise; + // aliases createDirectory(dirname: string): Promise; deleteDirectory(dirname: string): Promise; deleteFile(filename: string): Promise; - chmod(filename: string, mode: string): Promise; - copyFile(src: string, dest: string): Promise; - // aliases (non-async) - readFileSync(filename: string): string; - createWriteStream(filename: string): WriteStream; // helpers pathExists(filename: string, fileType?: FileType): Promise; fileExists(filename: string): Promise; @@ -97,3 +89,16 @@ export interface IFileSystem { arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. fileExistsSync(filename: string): boolean; } + +// more aliases (to cause less churn) +export const IFileSystem = Symbol('IFileSystem'); +export interface IFileSystem extends IFileSystemUtils { + stat(filePath: string): Promise; + readFile(filename: string): Promise; + writeFile(filename: string, data: {}): Promise; + chmod(filename: string, mode: string): Promise; + copyFile(src: string, dest: string): Promise; + // non-async + readFileSync(filename: string): string; + createWriteStream(filename: string): WriteStream; +} From 7dbb147b37a577662584ca1329fbde6f937012a2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:31:00 -0600 Subject: [PATCH 23/58] Drop an outdated tslint comment. --- src/client/common/editor.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/editor.ts b/src/client/common/editor.ts index 234915877900..15927c59194c 100644 --- a/src/client/common/editor.ts +++ b/src/client/common/editor.ts @@ -241,7 +241,6 @@ export async function getTempFileWithDocumentContents( // as the original one and then removed. const ext = path.extname(document.uri.fsPath); - // tslint:disable-next-line:no-require-imports const filename = `${document.uri.fsPath}.${md5(document.uri.fsPath)}${ext}`; await ( fs.writeFile(filename, document.getText()) From db3b9ac586d1efd6ea197f26e822fd2d78e663e6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:32:06 -0600 Subject: [PATCH 24/58] Drop an entire unused module. --- src/client/common/envFileParser.ts | 100 ----------------------------- 1 file changed, 100 deletions(-) delete mode 100644 src/client/common/envFileParser.ts diff --git a/src/client/common/envFileParser.ts b/src/client/common/envFileParser.ts deleted file mode 100644 index 8c341363365b..000000000000 --- a/src/client/common/envFileParser.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { IS_WINDOWS } from './platform/constants'; -import { FileSystem } from './platform/fileSystem'; -import { PathUtils } from './platform/pathUtils'; -import { IFileSystem } from './platform/types'; -import { EnvironmentVariablesService } from './variables/environment'; -import { - EnvironmentVariables, IEnvironmentVariablesService -} from './variables/types'; - -function parseEnvironmentVariables(contents: string): EnvironmentVariables | undefined { - if (typeof contents !== 'string' || contents.length === 0) { - return undefined; - } - - const env: EnvironmentVariables = {}; - contents.split('\n').forEach(line => { - const match = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/); - if (match !== null) { - let value = typeof match[2] === 'string' ? match[2] : ''; - if (value.length > 0 && value.charAt(0) === '"' && value.charAt(value.length - 1) === '"') { - value = value.replace(/\\n/gm, '\n'); - } - env[match[1]] = value.replace(/(^['"]|['"]$)/g, ''); - } - }); - return env; -} - -export function parseEnvFile( - envFile: string, - mergeWithProcessEnvVars: boolean = true, - service?: IEnvironmentVariablesService, - fs?: IFileSystem -): EnvironmentVariables { - fs = fs ? fs : new FileSystem(); - const buffer = fs.readFileSync(envFile); - const env = parseEnvironmentVariables(buffer)!; - if (!service) { - service = new EnvironmentVariablesService( - new PathUtils(IS_WINDOWS), - fs - ); - } - return mergeWithProcessEnvVars - ? mergeEnvVariables(env, process.env, service) - : mergePythonPath(env, process.env.PYTHONPATH as string, service); -} - -/** - * Merge the target environment variables into the source. - * Note: The source variables are modified and returned (i.e. it modifies value passed in). - * @export - * @param {EnvironmentVariables} targetEnvVars target environment variables. - * @param {EnvironmentVariables} [sourceEnvVars=process.env] source environment variables (defaults to current process variables). - * @returns {EnvironmentVariables} - */ -export function mergeEnvVariables( - targetEnvVars: EnvironmentVariables, - sourceEnvVars: EnvironmentVariables = process.env, - service?: IEnvironmentVariablesService -): EnvironmentVariables { - if (!service) { - service = new EnvironmentVariablesService( - new PathUtils(IS_WINDOWS), - new FileSystem() - ); - } - service.mergeVariables(sourceEnvVars, targetEnvVars); - if (sourceEnvVars.PYTHONPATH) { - service.appendPythonPath(targetEnvVars, sourceEnvVars.PYTHONPATH); - } - return targetEnvVars; -} - -/** - * Merge the target PYTHONPATH value into the env variables passed. - * Note: The env variables passed in are modified and returned (i.e. it modifies value passed in). - * @export - * @param {EnvironmentVariables} env target environment variables. - * @param {string | undefined} [currentPythonPath] PYTHONPATH value. - * @returns {EnvironmentVariables} - */ -export function mergePythonPath( - env: EnvironmentVariables, - currentPythonPath: string | undefined, - service?: IEnvironmentVariablesService -): EnvironmentVariables { - if (typeof currentPythonPath !== 'string' || currentPythonPath.length === 0) { - return env; - } - - if (!service) { - service = new EnvironmentVariablesService( - new PathUtils(IS_WINDOWS), - new FileSystem() - ); - } - service.appendPythonPath(env, currentPythonPath); - return env; -} From b32b05406b1a0560eba319f38dddc1e719091d64 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:39:14 -0600 Subject: [PATCH 25/58] Simplify lookForInterpretersInDirectory(). --- src/client/interpreter/locators/helpers.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index 98741a0536bd..b3d3a7a5bc6f 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -11,25 +11,19 @@ const CheckPythonInterpreterRegEx = IS_WINDOWS ? /^python(\d+(.\d+)?)?\.exe$/ : export async function lookForInterpretersInDirectory( pathToCheck: string, - fs?: IFileSystem + fs: IFileSystem = new FileSystem() ): Promise { - fs = fs ? fs : new FileSystem(); const files = await ( - fs!.getFiles(pathToCheck) + fs.getFiles(pathToCheck) .catch(err => { traceError('Python Extension (lookForInterpretersInDirectory.fsReaddirAsync):', err); return [] as string[]; }) ); - const result: string[] = []; - for (const filename of files) { + return files.filter(filename => { const name = path.basename(filename); - if (!CheckPythonInterpreterRegEx.test(name)) { - continue; - } - result.push(filename); - } - return result; + return CheckPythonInterpreterRegEx.test(name); + }); } @injectable() From 9eb24f27241021379f4329bc63afadefc653f704 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:55:27 -0600 Subject: [PATCH 26/58] Drop an unused variable. --- src/test/linters/lint.unit.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/linters/lint.unit.test.ts b/src/test/linters/lint.unit.test.ts index af98e6b561f5..f5cd699b2693 100644 --- a/src/test/linters/lint.unit.test.ts +++ b/src/test/linters/lint.unit.test.ts @@ -14,8 +14,7 @@ import { Product } from '../../client/common/installer/productInstaller'; import { ProductNames } from '../../client/common/installer/productNames'; import { ProductService } from '../../client/common/installer/productService'; import { - IFileSystem, - IPlatformService + IFileSystem } from '../../client/common/platform/types'; import { IPythonExecutionFactory, @@ -100,7 +99,6 @@ const pydocstyleMessagesToBeReturned: ILintMessage[] = [ ]; class TestFixture extends BaseTestFixture { - public platformService: TypeMoq.IMock; public filesystem: TypeMoq.IMock; public pythonToolExecService: TypeMoq.IMock; public pythonExecService: TypeMoq.IMock; @@ -110,7 +108,6 @@ class TestFixture extends BaseTestFixture { workspaceDir = '.', printLogs = false ) { - const platformService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const filesystem = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const pythonToolExecService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); const pythonExecFactory = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); @@ -125,7 +122,6 @@ class TestFixture extends BaseTestFixture { printLogs ); - this.platformService = platformService; this.filesystem = filesystem; this.pythonToolExecService = pythonToolExecService; this.pythonExecService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); From 1acf2ff8060b97cdd5c256bf961f39a8df50f5a0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 13:17:09 -0600 Subject: [PATCH 27/58] Make the existing filesystem tests "functional". --- .../{filesystem.unit.test.ts => filesystem.functional.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/test/common/platform/{filesystem.unit.test.ts => filesystem.functional.test.ts} (100%) diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.functional.test.ts similarity index 100% rename from src/test/common/platform/filesystem.unit.test.ts rename to src/test/common/platform/filesystem.functional.test.ts From 71c1d549bc093dfd454d2651a79170402b803a5d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 24 Oct 2019 13:56:01 -0600 Subject: [PATCH 28/58] Add unit tests (and make dependencies explicit). --- src/client/common/platform/fileSystem.ts | 157 ++- src/client/common/platform/types.ts | 15 +- .../common/platform/filesystem.unit.test.ts | 1072 +++++++++++++++++ 3 files changed, 1193 insertions(+), 51 deletions(-) create mode 100644 src/test/common/platform/filesystem.unit.test.ts diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 69f4a7fd91d9..4e00d5e6d328 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -8,17 +8,19 @@ import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import * as glob from 'glob'; import { injectable } from 'inversify'; -import * as path from 'path'; +import * as fspath from 'path'; import * as tmp from 'tmp'; import * as vscode from 'vscode'; import { createDeferred } from '../utils/async'; import { getOSType, OSType } from '../utils/platform'; import { FileStat, FileType, - IFileSystem, IFileSystemUtils, IRawFileSystem, + IFileSystem, IFileSystemPath, IFileSystemUtils, IRawFileSystem, TemporaryFile, WriteStream } from './types'; +// tslint:disable:max-classes-per-file + const ENCODING: string = 'utf8'; function getFileType(stat: FileStat): FileType { @@ -46,7 +48,7 @@ interface IRawFS { } interface IRawFSExtra { - chmod(filePath: string, mode: string): Promise; + chmod(filePath: string, mode: string | number): Promise; readFile(path: string, encoding: string): Promise; //tslint:disable-next-line:no-any writeFile(path: string, data: any, options: any): Promise; @@ -56,6 +58,7 @@ interface IRawFSExtra { mkdirp(dirname: string): Promise; rmdir(dirname: string): Promise; readdir(dirname: string): Promise; + remove(dirname: string): Promise; // non-async statSync(filename: string): fsextra.Stats; @@ -67,7 +70,7 @@ interface IRawFSExtra { // Later we will drop "FileSystem", switching usage to // "FileSystemUtils" and then rename "RawFileSystem" to "FileSystem". -class RawFileSystem { +export class RawFileSystem implements IRawFileSystem { constructor( private readonly nodefs: IRawFS = fs, private readonly fsExtra: IRawFSExtra = fsextra @@ -88,18 +91,23 @@ class RawFileSystem { } public async mkdirp(dirname: string): Promise { - return this.fsExtra.mkdirp(dirname); + return this.fsExtra.mkdirp(dirname) + .catch(_err => { + //throw err; + }); } public async rmtree(dirname: string): Promise { - return this.fsExtra.rmdir(dirname); + return this.fsExtra.stat(dirname) + .then(() => this.fsExtra.remove(dirname)); + //.catch((err) => this.fsExtra.rmdir(dirname)); } public async rmfile(filename: string): Promise { return this.fsExtra.unlink(filename); } - public async chmod(filename: string, mode: string): Promise { + public async chmod(filename: string, mode: string | number): Promise { return this.fsExtra.chmod(filename, mode); } @@ -111,8 +119,18 @@ class RawFileSystem { return this.fsExtra.lstat(filename); } - public async listdir(dirname: string): Promise { - return this.fsExtra.readdir(dirname); + // Once we move to the VS Code API, this method becomes a trivial + // wrapper and The "path" parameter can go away. + public async listdir(dirname: string, path: IFileSystemPath): Promise<[string, FileType][]> { + const names: string[] = await this.fsExtra.readdir(dirname); + const promises = names + .map(name => { + const filename = path.join(dirname, name); + return this.lstat(filename) + .then(stat => [name, getFileType(stat)] as [string, FileType]) + .catch(() => [name, FileType.Unknown] as [string, FileType]); + }); + return Promise.all(promises); } public async copyFile(src: string, dest: string): Promise { @@ -168,13 +186,49 @@ class RawFileSystem { } } -// more aliases (to cause less churn) +interface INodePath { + join(...filenames: string[]): string; + normalize(filename: string): string; +} + +// Eventually we will merge PathUtils into FileSystemPath. + +export class FileSystemPath implements IFileSystemPath { + constructor( + private readonly isWindows = (getOSType() === OSType.Windows), + private readonly raw: INodePath = fspath + ) { } + + public join(...filenames: string[]): string { + return this.raw.join(...filenames); + } + + public normCase(filename: string): string { + filename = this.raw.normalize(filename); + return this.isWindows ? filename.toUpperCase() : filename; + } +} + +// We *could* use ICryptUtils, but it's a bit overkill. +function getHashString(data: string): string { + const hash = createHash('sha512') + .update(data); + return hash.digest('hex'); +} + +type GlobCallback = (err: Error | null, matches: string[]) => void; +//tslint:disable-next-line:no-any +type TempCallback = (err: any, path: string, fd: number, cleanupCallback: () => void) => void; + @injectable() export class FileSystemUtils implements IFileSystemUtils { constructor( - private readonly isWindows = (getOSType() === OSType.Windows), - //public readonly raw: IFileSystem = {} - public readonly raw: IRawFileSystem = new RawFileSystem() + public readonly raw: IRawFileSystem = new RawFileSystem(), + public readonly path: IFileSystemPath = new FileSystemPath(), + private readonly getHash = getHashString, + // tslint:disable-next-line:no-unnecessary-callback-wrapper + private readonly globFile = ((pat: string, cb: GlobCallback) => glob(pat, cb)), + private readonly makeTempFile = ((s: string, cb: TempCallback) => tmp.file({ postfix: s }, cb)) ) { } //**************************** @@ -196,13 +250,12 @@ export class FileSystemUtils implements IFileSystemUtils { // helpers public arePathsSame(path1: string, path2: string): boolean { - path1 = path.normalize(path1); - path2 = path.normalize(path2); - if (this.isWindows) { - return path1.toUpperCase() === path2.toUpperCase(); - } else { - return path1 === path2; + if (path1 === path2) { + return true; } + path1 = this.path.normCase(path1); + path2 = this.path.normCase(path2); + return path1 === path2; } public async pathExists( @@ -212,7 +265,7 @@ export class FileSystemUtils implements IFileSystemUtils { let stat: FileStat; try { stat = await this.raw.stat(filename); - } catch { + } catch (err) { return false; } if (fileType === undefined) { @@ -231,7 +284,7 @@ export class FileSystemUtils implements IFileSystemUtils { public async directoryExists(dirname: string): Promise { return this.pathExists(dirname, FileType.Directory); } - public fileExistsSync(filename: string): boolean { + public pathExistsSync(filename: string): boolean { try { this.raw.statSync(filename); } catch { @@ -240,56 +293,47 @@ export class FileSystemUtils implements IFileSystemUtils { return true; } - public async listdir( - dirname: string - ): Promise<[string, FileType][]> { - const filenames: string[] = await ( - this.raw.listdir(dirname) - .then(names => names.map(name => path.join(dirname, name))) - .catch(() => []) - ); - const promises = filenames - .map(filename => ( - this.raw.stat(filename) - .then(stat => [filename, getFileType(stat)] as [string, FileType]) - .catch(() => [filename, FileType.Unknown] as [string, FileType]) - )); - return Promise.all(promises); + public async listdir(dirname: string): Promise<[string, FileType][]> { + try { + return await this.raw.listdir(dirname, this.path); + } catch { + return []; + } } public async getSubDirectories(dirname: string): Promise { return (await this.listdir(dirname)) - .filter(([_filename, fileType]) => fileType === FileType.Directory) - .map(([filename, _fileType]) => filename); + .filter(([_name, fileType]) => fileType === FileType.Directory) + .map(([name, _fileType]) => this.path.join(dirname, name)); } public async getFiles(dirname: string): Promise { return (await this.listdir(dirname)) - .filter(([_filename, fileType]) => fileType === FileType.File) - .map(([filename, _fileType]) => filename); + .filter(([_name, fileType]) => fileType === FileType.File) + .map(([name, _fileType]) => this.path.join(dirname, name)); } public async isDirReadonly(dirname: string): Promise { // Alternative: use tmp.file(). - const filename = path.join(dirname, '___vscpTest___'); + const filename = this.path.join(dirname, '___vscpTest___'); try { await this.raw.touch(filename); } catch { - return false; + await this.raw.stat(dirname); // fails if does not exist + return true; } await this.raw.rmfile(filename); - return true; + return false; } public async getFileHash(filename: string): Promise { const stat = await this.raw.lstat(filename); - const hash = createHash('sha512') - .update(`${stat.ctimeMs}-${stat.mtimeMs}`); - return hash.digest('hex'); + const data = `${stat.ctimeMs}-${stat.mtimeMs}`; + return this.getHash(data); } public async search(globPattern: string): Promise { // We could use util.promisify() here. return new Promise((resolve, reject) => { - glob(globPattern, (ex, files) => { + this.globFile(globPattern, (ex, files) => { if (ex) { return reject(ex); } @@ -301,7 +345,7 @@ export class FileSystemUtils implements IFileSystemUtils { public async createTemporaryFile(suffix: string): Promise { // We could use util.promisify() here. return new Promise((resolve, reject) => { - tmp.file({ postfix: suffix }, (err, tmpFile, _, cleanupCallback) => { + this.makeTempFile(suffix, (err, tmpFile, _, cleanupCallback) => { if (err) { return reject(err); } @@ -314,8 +358,21 @@ export class FileSystemUtils implements IFileSystemUtils { } } +// more aliases (to cause less churn) @injectable() export class FileSystem extends FileSystemUtils implements IFileSystem { + constructor( + isWindows: boolean = (getOSType() === OSType.Windows) + ) { + super( + new RawFileSystem(), + new FileSystemPath(isWindows) + ); + } + + //**************************** + // aliases + public async stat(filePath: string): Promise { // Do not import vscode directly, as this isn't available in the Debugger Context. // If stat is used in debugger context, it will fail, however theres a separate PR that will resolve this. @@ -340,6 +397,10 @@ export class FileSystem extends FileSystemUtils implements IFileSystem { return this.raw.copyFile(src, dest); } + public fileExistsSync(filename: string): boolean { + return this.pathExistsSync(filename); + } + public readFileSync(filename: string): string { return this.raw.readTextSync(filename); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index b82dffdf6023..b35694486856 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -51,7 +51,7 @@ export type WriteStream = fs.WriteStream; export interface IRawFileSystem { stat(filename: string): Promise; lstat(filename: string): Promise; - chmod(filename: string, mode: string): Promise; + chmod(filename: string, mode: string | number): Promise; // files readText(filename: string): Promise; writeText(filename: string, data: {}): Promise; @@ -61,16 +61,24 @@ export interface IRawFileSystem { // directories mkdirp(dirname: string): Promise; rmtree(dirname: string): Promise; - listdir(dirname: string): Promise; + listdir(dirname: string, path: IFileSystemPath): Promise<[string, FileType][]>; // not async statSync(filename: string): FileStat; readTextSync(filename: string): string; createWriteStream(filename: string): WriteStream; } +// Eventually we will merge IPathUtils into IFileSystemPath. + +export interface IFileSystemPath { + join(...filenames: string[]): string; + normCase(filename: string): string; +} + export const IFileSystemUtils = Symbol('IFileSystemUtils'); export interface IFileSystemUtils { raw: IRawFileSystem; + path: IFileSystemPath; // aliases createDirectory(dirname: string): Promise; deleteDirectory(dirname: string): Promise; @@ -87,7 +95,7 @@ export interface IFileSystemUtils { createTemporaryFile(suffix: string): Promise; // helpers (non-async) arePathsSame(path1: string, path2: string): boolean; // Move to IPathUtils. - fileExistsSync(filename: string): boolean; + pathExistsSync(filename: string): boolean; } // more aliases (to cause less churn) @@ -99,6 +107,7 @@ export interface IFileSystem extends IFileSystemUtils { chmod(filename: string, mode: string): Promise; copyFile(src: string, dest: string): Promise; // non-async + fileExistsSync(filename: string): boolean; readFileSync(filename: string): string; createWriteStream(filename: string): WriteStream; } diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts new file mode 100644 index 000000000000..ccbd15a294b9 --- /dev/null +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -0,0 +1,1072 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as fs from 'fs'; +import * as fsextra from 'fs-extra'; +import * as TypeMoq from 'typemoq'; +import { + FileSystemPath, FileSystemUtils, RawFileSystem +} from '../../../client/common/platform/fileSystem'; +import { + FileStat, FileType, + IFileSystemPath, IFileSystemUtils, IRawFileSystem, + TemporaryFile, WriteStream +} from '../../../client/common/platform/types'; + +// tslint:disable:max-func-body-length chai-vague-errors + +interface IRawFS { + // node "fs" + //tslint:disable-next-line:no-any + open(filename: string, flags: number, callback: any): void; + //tslint:disable-next-line:no-any + close(fd: number, callback: any): void; + + // "fs-extra" + chmod(filePath: string, mode: string): Promise; + readFile(path: string, encoding: string): Promise; + //tslint:disable-next-line:no-any + writeFile(path: string, data: any, options: any): Promise; + unlink(filename: string): Promise; + stat(filename: string): Promise; + lstat(filename: string): Promise; + mkdirp(dirname: string): Promise; + rmdir(dirname: string): Promise; + readdir(dirname: string): Promise; + remove(dirname: string): Promise; + statSync(filename: string): fsextra.Stats; + readFileSync(path: string, encoding: string): string; + createReadStream(src: string): fsextra.ReadStream; + createWriteStream(dest: string): fsextra.WriteStream; + + // node "path" + join(...filenames: string[]): string; + normalize(filename: string): string; +} + +suite('Raw FileSystem', () => { + let raw: TypeMoq.IMock; + let filesystem: IRawFileSystem; + setup(() => { + raw = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + filesystem = new RawFileSystem( + raw.object, + raw.object + ); + }); + function verifyAll() { + raw.verifyAll(); + } + + suite('readText', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + const expected = ''; + raw.setup(r => r.readFile(filename, TypeMoq.It.isAny())) + .returns(() => Promise.resolve(expected)); + + const text = await filesystem.readText(filename); + + expect(text).to.equal(expected); + verifyAll(); + }); + + test('always UTF-8', async () => { + const filename = 'x/y/z/spam.py'; + const expected = ''; + raw.setup(r => r.readFile(filename, 'utf8')) + .returns(() => Promise.resolve(expected)); + + const text = await filesystem.readText(filename); + + expect(text).to.equal(expected); + verifyAll(); + }); + }); + + suite('writeText', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + const data = ''; + raw.setup(r => r.writeFile(filename, data, { encoding: 'utf8' })) + .returns(() => Promise.resolve()); + + await filesystem.writeText(filename, data); + + verifyAll(); + }); + }); + + suite('mkdirp', () => { + test('wraps the low-level function', async () => { + const dirname = 'x/y/z/spam'; + raw.setup(r => r.mkdirp(dirname)) + .returns(() => Promise.resolve()); + + await filesystem.mkdirp(dirname); + + verifyAll(); + }); + }); + + suite('rmtree', () => { + test('wraps the low-level function', async () => { + const dirname = 'x/y/z/spam'; + raw.setup(r => r.stat(dirname)) + //tslint:disable-next-line:no-any + .returns(() => Promise.resolve({} as any as FileStat)); + raw.setup(r => r.remove(dirname)) + .returns(() => Promise.resolve()); + + await filesystem.rmtree(dirname); + + verifyAll(); + }); + + test('fails if the directory does not exist', async () => { + const dirname = 'x/y/z/spam'; + raw.setup(r => r.stat(dirname)) + .throws(new Error('file not found')); + + const promise = filesystem.rmtree(dirname); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('rmfile', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + raw.setup(r => r.unlink(filename)) + .returns(() => Promise.resolve()); + + await filesystem.rmfile(filename); + + verifyAll(); + }); + }); + + suite('chmod', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + const mode = 'w'; + raw.setup(r => r.chmod(filename, mode)) + .returns(() => Promise.resolve()); + + await filesystem.chmod(filename, mode); + + verifyAll(); + }); + }); + + suite('stat', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + //tslint:disable-next-line:no-any + const expected: FileStat = {} as any; + raw.setup(r => r.stat(filename)) + .returns(() => Promise.resolve(expected)); + + const stat = await filesystem.stat(filename); + + expect(stat).to.equal(expected); + verifyAll(); + }); + }); + + suite('lstat', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + //tslint:disable-next-line:no-any + const expected: FileStat = {} as any; + raw.setup(r => r.lstat(filename)) + .returns(() => Promise.resolve(expected)); + + const stat = await filesystem.lstat(filename); + + expect(stat).to.equal(expected); + verifyAll(); + }); + }); + + suite('listdir', () => { + let fspath: TypeMoq.IMock; + setup(() => { + fspath = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + }); + function setupStat(filename: string, ft: FileType) { + const stat = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + if (ft === FileType.File) { + stat.setup(s => s.isFile()) + .returns(() => true); + } else { + stat.setup(s => s.isFile()) + .returns(() => false); + if (ft === FileType.Directory) { + stat.setup(s => s.isDirectory()) + .returns(() => true); + } else { + stat.setup(s => s.isDirectory()) + .returns(() => false); + if (ft === FileType.SymbolicLink) { + stat.setup(s => s.isSymbolicLink()) + .returns(() => true); + } else { + stat.setup(s => s.isSymbolicLink()) + .returns(() => false); + } + } + } + // This is necessary because passing "stat.object" to + // Promise.resolve() triggers the lookup. + //tslint:disable-next-line:no-any + stat.setup((s: any) => s.then) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeast(0)); + raw.setup(r => r.lstat(filename)) + .returns(() => Promise.resolve(stat.object)); + return stat; + } + + test('mixed', async () => { + const dirname = 'x/y/z/spam'; + const expected: [string, FileType][] = [ + ['dev1', FileType.Unknown], + ['w', FileType.Directory], + ['spam.py', FileType.File], + ['other', FileType.SymbolicLink] + ]; + const names = expected.map(([name, _ft]) => name); + raw.setup(r => r.readdir(dirname)) + .returns(() => Promise.resolve(names)); + const stats: TypeMoq.IMock[] = []; + expected.forEach(([name, ft]) => { + const filename = `${dirname}/${name}`; + fspath.setup(p => p.join(dirname, name)) + .returns(() => filename); + stats.push( + setupStat(filename, ft)); + }); + + const entries = await filesystem.listdir(dirname, fspath.object); + + expect(entries).to.deep.equal(expected); + verifyAll(); + stats.forEach(stat => stat.verifyAll()); + }); + + test('empty', async () => { + const dirname = 'x/y/z/spam'; + const names: string[] = []; + raw.setup(r => r.readdir(dirname)) + .returns(() => Promise.resolve(names)); + + const entries = await filesystem.listdir(dirname, fspath.object); + + expect(entries).to.deep.equal([]); + verifyAll(); + }); + + test('fails if the low-level call fails', async () => { + const dirname = 'x/y/z/spam'; + raw.setup(r => r.readdir(dirname)) + .throws(new Error('file not found')); + + const promise = filesystem.listdir(dirname, fspath.object); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('copyFile', () => { + let rs: TypeMoq.IMock; + let ws: TypeMoq.IMock; + let done: () => void; + let finished: boolean; + setup(() => { + rs = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + ws = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + + rs.setup(s => s.on('error', TypeMoq.It.isAny())) + .returns(() => rs.object); + finished = false; + done = () => { + throw Error(); + }; + rs.setup(s => s.pipe(TypeMoq.It.isAny())) + .callback(_r => { + done(); + finished = true; + }); + + ws.setup(s => s.on('error', TypeMoq.It.isAny())) + .returns(() => ws.object); + ws.setup(s => s.on('close', TypeMoq.It.isAny())) + .callback((_e, cb) => { + done = cb; + }) + .returns(() => ws.object); + }); + + test('read/write streams are used properly', async () => { + const src = 'x/y/z/spam.py'; + const dest = 'x/y/z/spam.py.bak'; + raw.setup(r => r.createReadStream(src)) + .returns(() => rs.object); + raw.setup(r => r.createWriteStream(dest)) + .returns(() => ws.object); + + await filesystem.copyFile(src, dest); + + expect(finished).to.equal(true); + rs.verifyAll(); + ws.verifyAll(); + verifyAll(); + }); + }); + + suite('touch', () => { + test('open() and closed() are used properly', async () => { + const filename = 'x/y/z/spam.py'; + const fd = 1; + const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; + raw.setup(r => r.open(filename, flags, TypeMoq.It.isAny())) + .callback((_f, _fl, cb) => { + cb(undefined, fd); + }); + raw.setup(r => r.close(fd, TypeMoq.It.isAny())) + .callback((_fd, cb) => { + cb(); + }); + + await filesystem.touch(filename); + + verifyAll(); + }); + }); + + suite('statSync', () => { + test('wraps the low-level function', () => { + const filename = 'x/y/z/spam.py'; + //tslint:disable-next-line:no-any + const expected: FileStat = {} as any; + raw.setup(r => r.statSync(filename)) + .returns(() => expected); + + const stat = filesystem.statSync(filename); + + expect(stat).to.equal(expected); + verifyAll(); + }); + }); + + suite('readTextSync', () => { + test('wraps the low-level function', () => { + const filename = 'x/y/z/spam.py'; + const expected = ''; + raw.setup(r => r.readFileSync(filename, TypeMoq.It.isAny())) + .returns(() => expected); + + const text = filesystem.readTextSync(filename); + + expect(text).to.equal(expected); + verifyAll(); + }); + + test('always UTF-8', async () => { + const filename = 'x/y/z/spam.py'; + const expected = ''; + raw.setup(r => r.readFileSync(filename, 'utf8')) + .returns(() => expected); + + const text = filesystem.readTextSync(filename); + + expect(text).to.equal(expected); + verifyAll(); + }); + }); + + suite('createWriteStream', () => { + test('wraps the low-level function', () => { + const filename = 'x/y/z/spam.py'; + //tslint:disable-next-line:no-any + const expected: WriteStream = {} as any; + raw.setup(r => r.createWriteStream(filename)) + .returns(() => expected); + + const stream = filesystem.createWriteStream(filename); + + expect(stream).to.equal(expected); + verifyAll(); + }); + }); +}); + +suite('FileSystem paths', () => { + let raw: TypeMoq.IMock; + let path: IFileSystemPath; + setup(() => { + raw = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + path = new FileSystemPath( + false, // isWindows + raw.object + ); + }); + function verifyAll() { + raw.verifyAll(); + } + + suite('join', () => { + test('wraps low-level function', () => { + const expected = 'x/y/z/spam.py'; + raw.setup(r => r.join('x', 'y/z', 'spam.py')) + .returns(() => expected); + + const result = path.join('x', 'y/z', 'spam.py'); + + expect(result).to.equal(expected); + }); + }); + + suite('normCase', () => { + test('wraps low-level function', () => { + const filename = 'x/y/z/spam.py'; + raw.setup(r => r.normalize(filename)) + .returns(() => filename); + path = new FileSystemPath( + false, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(filename); + verifyAll(); + }); + + test('path separators get normalized on Windows', () => { + const filename = 'X/Y/Z/SPAM.PY'; + const expected = 'X\\Y\\Z\\SPAM.PY'; + raw.setup(r => r.normalize(filename)) + .returns(() => expected); + path = new FileSystemPath( + true, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + + test('path separators stay the same on non-Windows', () => { + const filename = 'x\\y\\z\\spam.py'; + const expected = filename; + raw.setup(r => r.normalize(filename)) + .returns(() => expected); + path = new FileSystemPath( + false, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + + test('on Windows, lower-case is made upper-case', () => { + const filename = 'x\\y\\z\\spam.py'; + const expected = 'X\\Y\\Z\\SPAM.PY'; + raw.setup(r => r.normalize(filename)) + .returns(() => filename); + path = new FileSystemPath( + true, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + + test('on Windows, upper-case stays upper-case', () => { + const filename = 'X\\Y\\Z\\SPAM.PY'; + const expected = 'X\\Y\\Z\\SPAM.PY'; + raw.setup(r => r.normalize(filename)) + .returns(() => expected); + path = new FileSystemPath( + true, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + + test('on non-Windows, lower-case stays lower-case', () => { + const filename = 'x/y/z/spam.py'; + const expected = 'x/y/z/spam.py'; + raw.setup(r => r.normalize(filename)) + .returns(() => filename); + path = new FileSystemPath( + false, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + + test('on non-Windows, upper-case stays upper-case', () => { + const filename = 'X/Y/Z/SPAM.PY'; + const expected = 'X/Y/Z/SPAM.PY'; + raw.setup(r => r.normalize(filename)) + .returns(() => expected); + path = new FileSystemPath( + false, // isWindows + raw.object + ); + + const result = path.normCase(filename); + + expect(result).to.equal(expected); + verifyAll(); + }); + }); +}); + +interface IDeps { + getHashString(data: string): string; + glob(pattern: string, cb: (err: Error | null, matches: string[]) => void): void; + //tslint:disable-next-line:no-any + mktemp(suffix: string, callback: ((err: any, path: string, fd: number, cleanupCallback: () => void) => void)): void; +} + +suite('FileSystem Utils', () => { + let stat: TypeMoq.IMock; + let filesystem: TypeMoq.IMock; + let path: TypeMoq.IMock; + let deps: TypeMoq.IMock; + let utils: IFileSystemUtils; + setup(() => { + stat = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + filesystem = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + path = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + deps = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + utils = new FileSystemUtils( + filesystem.object, + path.object, + ((data: string) => deps.object.getHashString(data)), + ((p, cb) => deps.object.glob(p, cb)), + ((s, cb) => deps.object.mktemp(s, cb)) + ); + + // This is necessary because passing "stat.object" to + // Promise.resolve() triggers the lookup. + //tslint:disable-next-line:no-any + stat.setup((s: any) => s.then) + .returns(() => undefined) + .verifiable(TypeMoq.Times.atLeast(0)); + }); + function verifyAll() { + filesystem.verifyAll(); + path.verifyAll(); + stat.verifyAll(); + } + + suite('arePathsSame', () => { + function setFakes( + path1: string, path2: string, + norm1: string, norm2: string + ) { + if (path1 === path2) { + throw Error('must be different filenames'); + } + path.setup(p => p.normCase(path1)) + .returns(() => norm1); + path.setup(p => p.normCase(path2)) + .returns(() => norm2); + } + + test('identical', () => { + const filename = 'x/y/z/spam.py'; + // No calls get made. + + const result = utils.arePathsSame(filename, filename); + + expect(result).to.equal(true); + verifyAll(); + }); + + test('not the same', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'a/b/c/spam.py'; + setFakes( + file1, file2, + 'x/y/z/spam.py', 'a/b/c/spam.py' + ); + + const result = utils.arePathsSame(file1, file2); + + expect(result).to.equal(false); + verifyAll(); + }); + + test('equal with different separators', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'x\\y\\z\\spam.py'; + setFakes( + file1, file2, + 'x/y/z/spam.py', 'x/y/z/spam.py' + ); + + const result = utils.arePathsSame(file1, file2); + + expect(result).to.equal(true); + verifyAll(); + }); + + test('equal with different case', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'x/Y/z/Spam.py'; + setFakes( + file1, file2, + 'x/y/z/spam.py', 'x/y/z/spam.py' + ); + + const result = utils.arePathsSame(file1, file2); + + expect(result).to.equal(true); + verifyAll(); + }); + }); + + suite('pathExists', () => { + test('file missing (any})', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .throws(new Error('file not found')); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(false); + verifyAll(); + }); + + Object.keys(FileType).forEach(ft => { + test(`file missing (${ft})`, async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .throws(new Error('file not found')); + + //tslint:disable-next-line:no-any + const exists = await utils.pathExists(filename, ft as any as FileType); + + expect(exists).to.equal(false); + verifyAll(); + }); + }); + + test('any', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('want file, got file', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isFile()) + .returns(() => true); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('want file, not file', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isFile()) + .returns(() => false); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('want directory, got directory', async () => { + const dirname = 'x/y/z/spam'; + filesystem.setup(f => f.stat(dirname)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isDirectory()) + .returns(() => true); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('want directory, not directory', async () => { + const dirname = 'x/y/z/spam'; + filesystem.setup(f => f.stat(dirname)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isDirectory()) + .returns(() => false); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('symlink', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename, FileType.SymbolicLink); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('unknown', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename, FileType.Unknown); + + expect(exists).to.equal(false); + verifyAll(); + }); + }); + + suite('fileExists', () => { + test('want file, got file', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.stat(filename)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isFile()) + .returns(() => true); + + const exists = await utils.fileExists(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + }); + + suite('directoryExists', () => { + test('want directory, got directory', async () => { + const dirname = 'x/y/z/spam'; + filesystem.setup(f => f.stat(dirname)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.isDirectory()) + .returns(() => true); + + const exists = await utils.directoryExists(dirname); + + expect(exists).to.equal(true); + verifyAll(); + }); + }); + + suite('pathExistsSync', () => { + test('exists', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.statSync(filename)) + .returns(() => stat.object); + + const exists = utils.pathExistsSync(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('not found', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.statSync(filename)) + .throws(new Error('file not found')); + + const exists = utils.pathExistsSync(filename); + + expect(exists).to.equal(false); + verifyAll(); + }); + }); + + suite('getSubDirectories', () => { + test('mixed types', async () => { + const dirname = 'x/y/z/spam'; + const files: [string, FileType][] = [ + ['w', FileType.Directory], + ['spam.py', FileType.File], + ['v', FileType.Directory], + ['eggs.py', FileType.File], + ['dev1', FileType.Unknown], + ['other', FileType.SymbolicLink], + ['data.json', FileType.File] + ]; + filesystem.setup(f => f.listdir(dirname, TypeMoq.It.isAny())) + .returns(() => Promise.resolve(files)); + ['w', 'v'].forEach(name => { + path.setup(p => p.join(dirname, name)) + .returns(() => `${dirname}/${name}`); + }); + + const results = await utils.getSubDirectories(dirname); + + expect(results).to.deep.equal([ + 'x/y/z/spam/w', + 'x/y/z/spam/v' + ]); + verifyAll(); + }); + + test('empty if the raw call fails', async () => { + const dirname = 'x/y/z/spam'; + filesystem.setup(f => f.listdir(dirname, TypeMoq.It.isAny())) + .throws(new Error('directory not found')); + + const entries = await utils.getSubDirectories(dirname); + + expect(entries).to.deep.equal([]); + verifyAll(); + }); + }); + + suite('getFiles', () => { + test('mixed types', async () => { + const dirname = 'x/y/z/spam'; + const files: [string, FileType][] = [ + ['w', FileType.Directory], + ['spam.py', FileType.File], + ['v', FileType.Directory], + ['eggs.py', FileType.File], + ['dev1', FileType.Unknown], + ['other', FileType.SymbolicLink], + ['data.json', FileType.File] + ]; + filesystem.setup(f => f.listdir(dirname, TypeMoq.It.isAny())) + .returns(() => Promise.resolve(files)); + ['spam.py', 'eggs.py', 'data.json'].forEach(name => { + path.setup(p => p.join(dirname, name)) + .returns(() => `${dirname}/${name}`); + }); + + const results = await utils.getFiles(dirname); + + expect(results).to.deep.equal([ + 'x/y/z/spam/spam.py', + 'x/y/z/spam/eggs.py', + 'x/y/z/spam/data.json' + ]); + verifyAll(); + }); + + test('empty if the raw call fails', async () => { + const dirname = 'x/y/z/spam'; + filesystem.setup(f => f.listdir(dirname, TypeMoq.It.isAny())) + .throws(new Error('directory not found')); + + const entries = await utils.getFiles(dirname); + + expect(entries).to.deep.equal([]); + verifyAll(); + }); + }); + + suite('isDirReadonly', () => { + test('is readonly', async () => { + const dirname = 'x/y/z/spam'; + const filename = 'x/y/z/spam/___vscpTest___'; + path.setup(p => p.join(dirname, '___vscpTest___')) + .returns(() => filename); + filesystem.setup(f => f.touch(filename)) + .throws(new Error('operation not permitted')); + filesystem.setup(f => f.stat(dirname)) + //tslint:disable-next-line:no-any + .returns(() => Promise.resolve({} as any as FileStat)); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(true); + verifyAll(); + }); + + test('is not readonly', async () => { + const dirname = 'x/y/z/spam'; + const filename = 'x/y/z/spam/___vscpTest___'; + path.setup(p => p.join(dirname, '___vscpTest___')) + .returns(() => filename); + filesystem.setup(f => f.touch(filename)) + .returns(() => Promise.resolve()); + filesystem.setup(f => f.rmfile(filename)) + .returns(() => Promise.resolve()); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(false); + verifyAll(); + }); + + test('file does not exist', async () => { + const dirname = 'x/y/z/spam'; + const filename = 'x/y/z/spam/___vscpTest___'; + path.setup(p => p.join(dirname, '___vscpTest___')) + .returns(() => filename); + filesystem.setup(f => f.touch(filename)) + .throws(new Error('file not found')); + filesystem.setup(f => f.stat(dirname)) + .throws(new Error('file not found')); + + const promise = utils.isDirReadonly(dirname); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('getFileHash', () => { + test('Getting hash for non existent file should throw error', async () => { + const filename = 'some unknown file'; + filesystem.setup(f => f.lstat(filename)) + .throws(new Error('file not found')); + + const promise = utils.getFileHash(filename); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + + test('Getting hash for a file should return non-empty string', async () => { + const filename = 'x/y/z/spam.py'; + filesystem.setup(f => f.lstat(filename)) + .returns(() => Promise.resolve(stat.object)); + stat.setup(s => s.ctimeMs) + .returns(() => 101); + stat.setup(s => s.mtimeMs) + .returns(() => 102); + const expected = ''; + deps.setup(d => d.getHashString('101-102')) + .returns(() => expected); + + const hash = await utils.getFileHash(filename); + + expect(hash).to.equal(expected); + verifyAll(); + }); + }); + + suite('search', () => { + test('found matches', async () => { + const pattern = `x/y/z/spam.*`; + const expected: string[] = [ + 'x/y/z/spam.py', + 'x/y/z/spam.pyc', + 'x/y/z/spam.so' + ]; + deps.setup(d => d.glob(pattern, TypeMoq.It.isAny())) + .callback((_p, cb) => { + cb(undefined, expected); + }); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal(expected); + verifyAll(); + }); + + test('no matches (empty)', async () => { + const pattern = `x/y/z/spam.*`; + deps.setup(d => d.glob(pattern, TypeMoq.It.isAny())) + .callback((_p, cb) => { + cb(undefined, []); + }); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal([]); + verifyAll(); + }); + + test('no matches (undefined)', async () => { + const pattern = `x/y/z/spam.*`; + deps.setup(d => d.glob(pattern, TypeMoq.It.isAny())) + .callback((_p, cb) => { + cb(undefined, undefined); + }); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal([]); + verifyAll(); + }); + + test('failed', async () => { + const pattern = `x/y/z/spam.*`; + const err = new Error('something went wrong'); + deps.setup(d => d.glob(pattern, TypeMoq.It.isAny())) + .callback((_p, cb) => { + cb(err); + }); + + const promise = utils.search(pattern); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('createTemporaryFile', () => { + test('TemporaryFile is populated properly', async () => { + const expected: TemporaryFile = { + filePath: '/tmp/xyz.tmp', + dispose: (() => undefined) + }; + deps.setup(d => d.mktemp('.tmp', TypeMoq.It.isAny())) + .callback((_s, cb) => { + cb(undefined, expected.filePath, undefined, expected.dispose); + }); + + const tempfile = await utils.createTemporaryFile('.tmp'); + + expect(tempfile).to.deep.equal(expected); + verifyAll(); + }); + + test('failure', async () => { + const err = new Error('something went wrong'); + deps.setup(d => d.mktemp('.tmp', TypeMoq.It.isAny())) + .callback((_s, cb) => { + cb(err); + }); + + const promise = utils.createTemporaryFile('.tmp'); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); +}); From 67eafb0cd76a06a738b0a8dee602b4aab0a33f7f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Oct 2019 11:53:19 -0600 Subject: [PATCH 29/58] Add functional tests. --- .../platform/filesystem.functional.test.ts | 1074 ++++++++++++++++- 1 file changed, 1066 insertions(+), 8 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index d0f69ef4d427..22807abafcb7 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -2,32 +2,1088 @@ // Licensed under the MIT License. import { expect, use } from 'chai'; -import * as fs from 'fs-extra'; +import * as fsextra from 'fs-extra'; +import * as net from 'net'; import * as path from 'path'; -import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { TemporaryFile } from '../../../client/common/platform/types'; +import * as tmp from 'tmp'; +import { + FileSystem, FileSystemPath, FileSystemUtils, RawFileSystem +} from '../../../client/common/platform/fileSystem'; +import { + FileType, + IFileSystemPath, IFileSystemUtils, IRawFileSystem, + TemporaryFile +} from '../../../client/common/platform/types'; +import { sleep } from '../../../client/common/utils/async'; // tslint:disable:no-require-imports no-var-requires const assertArrays = require('chai-arrays'); use(require('chai-as-promised')); use(assertArrays); -// tslint:disable-next-line:max-func-body-length -suite('FileSystem', () => { +// tslint:disable:max-func-body-length chai-vague-errors + +const DOES_NOT_EXIST = 'this file does not exist'; + +async function ensureDoesNotExist(filename: string) { + await expect( + fsextra.stat(filename) + ).to.eventually.be.rejected; +} + +class FSFixture { + public tempDir: tmp.SynchrounousResult | undefined; + public sockServer: net.Server | undefined; + + public async cleanUp() { + if (this.tempDir) { + try { + await fsextra.remove(this.tempDir.name); + //tempDir.removeCallback(); + } catch { + // do nothing + } + this.tempDir = undefined; + } + if (this.sockServer) { + const srv = this.sockServer; + await new Promise(resolve => srv.close(resolve)); + this.sockServer = undefined; + } + } + + public async resolve(relname: string, mkdirs = true): Promise { + if (!this.tempDir) { + this.tempDir = tmp.dirSync({ prefix: 'pyvsc-fs-tests-' }); + } + relname = path.normalize(relname); + const filename = path.join(this.tempDir.name, relname); + if (mkdirs) { + await fsextra.mkdirp( + path.dirname(filename)); + } + return filename; + } + + public async createFile(relname: string, text = ''): Promise { + const filename = await this.resolve(relname); + await fsextra.writeFile(filename, text); + return filename; + } + + public async createDirectory(relname: string): Promise { + const dirname = await this.resolve(relname); + await fsextra.mkdir(dirname); + return dirname; + } + + public async createSymlink(relname: string, source: string): Promise { + const symlink = await this.resolve(relname); + await fsextra.ensureSymlink(source, symlink); + return symlink; + } + + public async createSocket(relname: string): Promise { + if (!this.sockServer) { + this.sockServer = net.createServer(); + } + const srv = this.sockServer!; + const filename = await this.resolve(relname); + await new Promise(resolve => srv!.listen(filename, 0, resolve)); + return filename; + } +} + +suite('Raw FileSystem', () => { + let filesystem: IRawFileSystem; + let fix: FSFixture; + setup(() => { + filesystem = new RawFileSystem(); + fix = new FSFixture(); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('readText', () => { + test('returns contents of a file', async () => { + const expected = ''; + const filename = await fix.createFile('x/y/z/spam.py', expected); + + const content = await filesystem.readText(filename); + + expect(content).to.be.equal(expected); + }); + + test('always UTF-8', async () => { + const expected = '... 😁 ...'; + const filename = await fix.createFile('x/y/z/spam.py', expected); + + const text = await filesystem.readText(filename); + + expect(text).to.equal(expected); + }); + + test('throws an exception if file does not exist', async () => { + const promise = filesystem.readText(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('writeText', () => { + test('creates the file if missing', async () => { + const filename = await fix.resolve('x/y/z/spam.py'); + await ensureDoesNotExist(filename); + const data = 'line1\nline2\n'; + + await filesystem.writeText(filename, data); + + const actual = await fsextra.readFile(filename) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); + }); + + test('always UTF-8', async () => { + const filename = await fix.resolve('x/y/z/spam.py'); + const data = '... 😁 ...'; + + await filesystem.writeText(filename, data); + + const actual = await fsextra.readFile(filename) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); + }); + + test('overwrites existing file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const data = 'line1\nline2\n'; + + await filesystem.writeText(filename, data); + + const actual = await fsextra.readFile(filename) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); + }); + }); + + suite('mkdirp', () => { + test('creates the directory and all missing parents', async () => { + await fix.createDirectory('x'); + // x/y, x/y/z, and x/y/z/spam are all missing. + const dirname = await fix.resolve('x/y/z/spam', false); + await ensureDoesNotExist(dirname); + + await filesystem.mkdirp(dirname); + + await fsextra.stat(dirname); // This should not fail. + }); + + test('works if the directory already exists', async () => { + const dirname = await fix.createDirectory('spam'); + await fsextra.stat(dirname); // This should not fail. + + await filesystem.mkdirp(dirname); + + await fsextra.stat(dirname); // This should not fail. + }); + }); + + suite('rmtree', () => { + test('deletes the directory and everything in it', async () => { + const dirname = await fix.createDirectory('x'); + const filename = await fix.createFile('x/y/z/spam.py'); + await fsextra.stat(filename); // This should not fail. + + await filesystem.rmtree(dirname); + + await ensureDoesNotExist(dirname); + }); + + test('fails if the directory does not exist', async () => { + const promise = filesystem.rmtree(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('rmfile', () => { + test('deletes the file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + await fsextra.stat(filename); // This should not fail. + + await filesystem.rmfile(filename); + + await ensureDoesNotExist(filename); + }); + + test('fails if the file does not exist', async () => { + const promise = filesystem.rmfile(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('chmod', () => { + async function checkMode(filename: string, expected: number) { + const stat = await fsextra.stat(filename); + if (/^win/.test(process.platform)) { + // On Windows, chmod won't have any effect on the file itself. + return; + } + expect(stat.mode & 0o777).to.equal(expected); + } + + test('the file mode gets updated (string)', async () => { + const filename = await fix.createFile('spam.py', '...'); + await fsextra.chmod(filename, 0o644); + + await filesystem.chmod(filename, '755'); + + await checkMode(filename, 0o755); + }); + + test('the file mode gets updated (number)', async () => { + const filename = await fix.createFile('spam.py', '...'); + await fsextra.chmod(filename, 0o644); + + await filesystem.chmod(filename, 0o755); + + await checkMode(filename, 0o755); + }); + + test('the file mode gets updated for a directory', async () => { + const dirname = await fix.createDirectory('spam'); + await fsextra.chmod(dirname, 0o755); + + await filesystem.chmod(dirname, 0o700); + + await checkMode(dirname, 0o700); + }); + + test('nothing happens if the file mode already matches', async () => { + const filename = await fix.createFile('spam.py', '...'); + await fsextra.chmod(filename, 0o644); + + await filesystem.chmod(filename, 0o644); + + await checkMode(filename, 0o644); + }); + + test('fails if the file does not exist', async () => { + const promise = filesystem.chmod(DOES_NOT_EXIST, 0o755); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('stat', () => { + test('gets the info for an existing file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const expected = await fsextra.stat(filename); + + const stat = await filesystem.stat(filename); + + expect(stat).to.deep.equal(expected); + }); + + test('gets the info for an existing directory', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + const expected = await fsextra.stat(dirname); + + const stat = await filesystem.stat(dirname); + + expect(stat).to.deep.equal(expected); + }); + + test('for symlinks, gets the info for the linked file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); + const expected = await fsextra.stat(filename); + + const stat = await filesystem.stat(symlink); + + expect(stat).to.deep.equal(expected); + }); + + test('fails if the file does not exist', async () => { + const promise = filesystem.stat(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('lstat', () => { + test('for symlinks, gives the link info', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); + const expected = await fsextra.lstat(symlink); + + const stat = await filesystem.lstat(symlink); + + expect(stat).to.deep.equal(expected); + }); + + test('for normal files, gives the file info', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const expected = await fsextra.stat(filename); + + const stat = await filesystem.lstat(filename); + + expect(stat).to.deep.equal(expected); + }); + + test('fails if the file does not exist', async () => { + const promise = filesystem.lstat(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('listdir', () => { + let fspath: IFileSystemPath; + setup(() => { + fspath = new FileSystemPath(); + }); + + test('mixed', async () => { + // Create the target directory and its contents. + const dirname = await fix.createDirectory('x/y/z'); + await fix.createFile('x/y/z/__init__.py', ''); + const script = await fix.createFile('x/y/z/__main__.py', '