From 97846b0ecd437157854b1812ea5540313ff2a09c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 9 Jan 2020 13:22:18 -0700 Subject: [PATCH 01/17] Factor out FileSystemUtils. --- src/client/common/platform/fileSystem.ts | 301 ++++++++++-------- src/client/common/platform/types.ts | 45 +++ .../platform/filesystem.functional.test.ts | 293 +++++++++++++++-- src/test/common/platform/filesystem.test.ts | 211 +++++++++++- .../common/platform/filesystem.unit.test.ts | 94 +++++- src/test/serviceRegistry.ts | 5 +- 6 files changed, 779 insertions(+), 170 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index dbde159cfc7c..39d02349e1a6 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -13,17 +13,22 @@ import { createDeferred } from '../utils/async'; import { isFileNotFoundError, isNoPermissionsError } from './errors'; import { FileSystemPaths, FileSystemPathUtils } from './fs-paths'; import { TemporaryFileSystem } from './fs-temp'; -// prettier-ignore import { - FileStat, FileType, - IFileSystem, IFileSystemPaths, IRawFileSystem, - ReadStream, TemporaryFile, WriteStream + FileStat, + FileType, + IFileSystem, + IFileSystemPaths, + IFileSystemPathUtils, + IFileSystemUtils, + IRawFileSystem, + ITempFileSystem, + ReadStream, + TemporaryFile, + WriteStream } from './types'; const ENCODING: string = 'utf8'; -const globAsync = promisify(glob); - // This helper function determines the file type of the given stats // object. The type follows the convention of node's fs module, where // a file has exactly one type. Symlinks are not resolved. @@ -272,62 +277,39 @@ export class RawFileSystem implements IRawFileSystem { } //========================================== -// filesystem "utils" (& legacy aliases) - -@injectable() -export class FileSystem implements IFileSystem { - // We expose this for the sake of functional tests that do not have - // access to the actual "vscode" namespace. - protected raw: RawFileSystem; - private readonly paths: IFileSystemPaths; - private readonly pathUtils: FileSystemPathUtils; - private readonly tmp: TemporaryFileSystem; - constructor() { - this.paths = FileSystemPaths.withDefaults(); - this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); - this.tmp = TemporaryFileSystem.withDefaults(); - this.raw = RawFileSystem.withDefaults(this.paths); - } - - //================================= - // path-related - - public get directorySeparatorChar(): string { - return this.paths.sep; - } - - public arePathsSame(path1: string, path2: string): boolean { - return this.pathUtils.arePathsSame(path1, path2); - } - - //================================= - // "raw" operations - - public async stat(filename: string): Promise { - return this.raw.stat(filename); - } - - public async lstat(filename: string): Promise { - return this.raw.lstat(filename); - } +// filesystem "utils" - public async readFile(filePath: string): Promise { - return this.raw.readText(filePath); - } - public readFileSync(filePath: string): string { - return this.raw.readTextSync(filePath); - } - public async readData(filePath: string): Promise { - return this.raw.readData(filePath); +// High-level filesystem operations used by the extension. +export class FileSystemUtils implements IFileSystemUtils { + constructor( + public readonly raw: IRawFileSystem, + public readonly pathUtils: IFileSystemPathUtils, + public readonly paths: IFileSystemPaths, + public readonly tmp: ITempFileSystem, + protected readonly getHash: (data: string) => string, + protected readonly globFile: (pat: string, options?: { cwd: string }) => Promise + ) {} + // Create a new object using common-case default values. + public static withDefaults( + raw?: IRawFileSystem, + pathUtils?: IFileSystemPathUtils, + tmp?: ITempFileSystem, + getHash?: (data: string) => string, + globFile?: (pat: string, options?: { cwd: string }) => Promise + ): FileSystemUtils { + pathUtils = pathUtils || FileSystemPathUtils.withDefaults(); + return new FileSystemUtils( + raw || RawFileSystem.withDefaults(pathUtils.paths), + pathUtils, + pathUtils.paths, + tmp || TemporaryFileSystem.withDefaults(), + getHash || getHashString, + globFile || promisify(glob) + ); } - public async writeFile(filePath: string, text: string, _options: string | fs.WriteFileOptions = { encoding: 'utf8' }): Promise { - // tslint:disable-next-line:no-suspicious-comment - // TODO (GH-8542) For now we ignore the options, since all call - // sites already match the defaults. Later we will fix the call - // sites. - return this.raw.writeText(filePath, text); - } + //**************************** + // aliases public async createDirectory(directoryPath: string): Promise { return this.raw.mkdirp(directoryPath); @@ -337,48 +319,12 @@ export class FileSystem implements IFileSystem { return this.raw.rmtree(directoryPath); } - public async listdir(dirname: string): Promise<[string, FileType][]> { - // prettier-ignore - return this.raw.listdir(dirname) - .catch(async err => { - // We're only preserving pre-existng behavior here... - if (!(await this.pathExists(dirname))) { - return []; - } - throw err; // re-throw - }); - } - - public async appendFile(filename: string, text: string): Promise { - return this.raw.appendText(filename, text); - } - - public async copyFile(src: string, dest: string): Promise { - return this.raw.copyFile(src, dest); - } - public async deleteFile(filename: string): Promise { return this.raw.rmfile(filename); } - public async chmod(filePath: string, mode: string | number): Promise { - return this.raw.chmod(filePath, mode); - } - - public async move(src: string, tgt: string) { - await this.raw.move(src, tgt); - } - - public createReadStream(filePath: string): ReadStream { - return this.raw.createReadStream(filePath); - } - - public createWriteStream(filePath: string): WriteStream { - return this.raw.createWriteStream(filePath); - } - - //================================= - // utils + //**************************** + // helpers // prettier-ignore public async pathExists( @@ -409,13 +355,21 @@ export class FileSystem implements IFileSystem { public async fileExists(filename: string): Promise { return this.pathExists(filename, FileType.File); } - public fileExistsSync(filePath: string): boolean { - return fs.existsSync(filePath); - } public async directoryExists(dirname: string): Promise { return this.pathExists(dirname, FileType.Directory); } + public async listdir(dirname: string): Promise<[string, FileType][]> { + // prettier-ignore + return this.raw.listdir(dirname) + .catch(async err => { + // We're only preserving pre-existng behavior here... + if (!(await this.pathExists(dirname))) { + return []; + } + throw err; // re-throw + }); + } public async getSubDirectories(dirname: string): Promise { // prettier-ignore return filterByFileType( @@ -431,11 +385,30 @@ export class FileSystem implements IFileSystem { ).map(([filename, _fileType]) => filename); } + public async isDirReadonly(dirname: string): Promise { + const filePath = `${dirname}${this.paths.sep}___vscpTest___`; + const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; + let fd: number; + try { + fd = await fs.open(filePath, flags); + } catch (err) { + if (isNoPermissionsError(err)) { + return true; + } + throw err; // re-throw + } + // Clean resources in the background. + fs.close(fd) + .finally(() => fs.unlink(filePath)) + .ignoreErrors(); + return false; + } + public async getFileHash(filename: string): Promise { // The reason for lstat rather than stat is not clear... const stat = await this.raw.lstat(filename); const data = `${stat.ctime}-${stat.mtime}`; - return getHashString(data); + return this.getHash(data); } public async search(globPattern: string, cwd?: string): Promise { @@ -444,42 +417,118 @@ export class FileSystem implements IFileSystem { const options = { cwd: cwd }; - found = await globAsync(globPattern, options); + found = await this.globFile(globPattern, options); } else { - found = await globAsync(globPattern); + found = await this.globFile(globPattern); } return Array.isArray(found) ? found : []; } - public createTemporaryFile(extension: string): Promise { - return this.tmp.createFile(extension); - } + //**************************** + // helpers (non-async) - public async isDirReadonly(dirname: string): Promise { - const filePath = `${dirname}${this.paths.sep}___vscpTest___`; - const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; - let fd: number; - try { - fd = await fs.open(filePath, flags); - // Clean resources in the background. - fs.close(fd) - .finally(() => fs.unlink(filePath)) - .ignoreErrors(); - } catch (err) { - if (isNoPermissionsError(err)) { - return true; - } - throw err; // re-throw - } - return false; + public fileExistsSync(filePath: string): boolean { + return fs.existsSync(filePath); } } // We *could* use ICryptoUtils, but it's a bit overkill, issue tracked // in https://github.com/microsoft/vscode-python/issues/8438. function getHashString(data: string): string { - // prettier-ignore - const hash = createHash('sha512') - .update(data); + const hash = createHash('sha512'); + hash.update(data); return hash.digest('hex'); } + +//========================================== +// legacy filesystem API + +// more aliases (to cause less churn) +@injectable() +export class FileSystem implements IFileSystem { + // We expose this for the sake of functional tests that do not have + // access to the actual "vscode" namespace. + protected utils: FileSystemUtils; + constructor() { + this.utils = FileSystemUtils.withDefaults(); + } + + public get directorySeparatorChar(): string { + return this.utils.paths.sep; + } + public arePathsSame(path1: string, path2: string): boolean { + return this.utils.pathUtils.arePathsSame(path1, path2); + } + public async stat(filename: string): Promise { + return this.utils.raw.stat(filename); + } + public async createDirectory(dirname: string): Promise { + return this.utils.createDirectory(dirname); + } + public async deleteDirectory(dirname: string): Promise { + return this.utils.deleteDirectory(dirname); + } + public async listdir(dirname: string): Promise<[string, FileType][]> { + return this.utils.listdir(dirname); + } + public async readFile(filePath: string): Promise { + return this.utils.raw.readText(filePath); + } + public async readData(filePath: string): Promise { + return this.utils.raw.readData(filePath); + } + public async writeFile(filename: string, data: {}): Promise { + return this.utils.raw.writeText(filename, data); + } + public async appendFile(filename: string, text: string): Promise { + return this.utils.raw.appendText(filename, text); + } + public async copyFile(src: string, dest: string): Promise { + return this.utils.raw.copyFile(src, dest); + } + public async deleteFile(filename: string): Promise { + return this.utils.deleteFile(filename); + } + public async chmod(filename: string, mode: string): Promise { + return this.utils.raw.chmod(filename, mode); + } + public async move(src: string, tgt: string) { + await this.utils.raw.move(src, tgt); + } + public readFileSync(filePath: string): string { + return this.utils.raw.readTextSync(filePath); + } + public createReadStream(filePath: string): ReadStream { + return this.utils.raw.createReadStream(filePath); + } + public createWriteStream(filePath: string): WriteStream { + return this.utils.raw.createWriteStream(filePath); + } + public async fileExists(filename: string): Promise { + return this.utils.fileExists(filename); + } + public fileExistsSync(filename: string): boolean { + return this.utils.fileExistsSync(filename); + } + public async directoryExists(dirname: string): Promise { + return this.utils.directoryExists(dirname); + } + public async getSubDirectories(dirname: string): Promise { + return this.utils.getSubDirectories(dirname); + } + public async getFiles(dirname: string): Promise { + return this.utils.getFiles(dirname); + } + public async getFileHash(filename: string): Promise { + return this.utils.getFileHash(filename); + } + public async search(globPattern: string): Promise { + return this.utils.search(globPattern); + } + public async createTemporaryFile(suffix: string): Promise { + return this.utils.tmp.createFile(suffix); + } + public async isDirReadonly(dirname: string): Promise { + return this.utils.isDirReadonly(dirname); + } +} diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 3776253b182e..f94f08eda1d3 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -142,6 +142,51 @@ export interface IRawFileSystem { createWriteStream(filename: string): WriteStream; } +// High-level filesystem operations used by the extension. +export interface IFileSystemUtils { + readonly raw: IRawFileSystem; + readonly paths: IFileSystemPaths; + readonly pathUtils: IFileSystemPathUtils; + readonly tmp: ITempFileSystem; + + //*********************** + // aliases + + createDirectory(dirname: string): Promise; + deleteDirectory(dirname: string): Promise; + deleteFile(filename: string): Promise; + + //*********************** + // helpers + + // Determine if the file exists, optionally requiring the type. + pathExists(filename: string, fileType?: FileType): Promise; + // Determine if the regular file exists. + fileExists(filename: string): Promise; + // Determine if the directory exists. + directoryExists(dirname: string): Promise; + // Get all the directory's entries. + listdir(dirname: string): Promise<[string, FileType][]>; + // Get the paths of all immediate subdirectories. + getSubDirectories(dirname: string): Promise; + // Get the paths of all immediately contained files. + getFiles(dirname: string): Promise; + // Determine if the directory is read-only. + isDirReadonly(dirname: string): Promise; + // Generate the sha512 hash for the file (based on timestamps). + getFileHash(filename: string): Promise; + // Get the paths of all files matching the pattern. + search(globPattern: string): Promise; + + //*********************** + // helpers (non-async) + + fileExistsSync(path: string): boolean; +} + +// tslint:disable-next-line:no-suspicious-comment +// TODO(GH-8542): Later we will drop IFileSystem, switching usage to IFileSystemUtils. + export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { // path-related diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index b28b1437c97d..75ef299404cb 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -6,7 +6,7 @@ import { expect, use } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; -import { convertStat, FileSystem, RawFileSystem } from '../../../client/common/platform/fileSystem'; +import { convertStat, FileSystem, FileSystemUtils, RawFileSystem } from '../../../client/common/platform/fileSystem'; import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; import { FileType } from '../../../client/common/platform/types'; import { sleep } from '../../../client/common/utils/async'; @@ -790,6 +790,268 @@ suite('FileSystem - raw', () => { }); }); +suite('FileSystem - utils', () => { + let utils: FileSystemUtils; + let fix: FSFixture; + setup(async () => { + // prettier-ignore + utils = FileSystemUtils.withDefaults(); + fix = new FSFixture(); + + await assertDoesNotExist(DOES_NOT_EXIST); + }); + teardown(async () => { + await fix.cleanUp(); + await fix.ensureDeleted(DOES_NOT_EXIST); + }); + + suite('createDirectory', () => { + test('wraps the low-level impl', async () => { + await fix.createDirectory('x'); + // x/y, x/y/z, and x/y/z/spam are all missing. + const dirname = await fix.resolve('x/spam', false); + await assertDoesNotExist(dirname); + + await utils.createDirectory(dirname); + + await assertExists(dirname); + }); + }); + + suite('deleteDirectory', () => { + test('wraps the low-level impl', async () => { + const dirname = await fix.createDirectory('x'); + await assertExists(dirname); + + await utils.deleteDirectory(dirname); + + await assertDoesNotExist(dirname); + }); + }); + + suite('deleteFile', () => { + test('wraps the low-level impl', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + await assertExists(filename); + + await utils.deleteFile(filename); + + await assertDoesNotExist(filename); + }); + }); + + suite('listdir', () => { + test('wraps the low-level impl', async () => { + test('mixed', async () => { + // Create the target directory and its contents. + const dirname = await fix.createDirectory('x/y/z'); + const file = await fix.createFile('x/y/z/__init__.py', ''); + const subdir = await fix.createDirectory('x/y/z/w'); + + const entries = await utils.listdir(dirname); + + expect(entries.sort()).to.deep.equal([ + [file, FileType.File], + [subdir, FileType.Directory] + ]); + }); + }); + }); + + suite('isDirReadonly', () => { + suite('non-Windows', () => { + suiteSetup(function() { + if (WINDOWS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + }); + + // On Windows, chmod won't have any effect on the file itself. + test('is readonly', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + await fs.chmod(dirname, 0o444); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(false); + }); + }); + + test('is not readonly', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(true); + }); + + // Failing may be more sensible, but for now we are sticking + // with the existing behavior. + test('false if the directory does not exist', async () => { + const isReadonly = await utils.isDirReadonly(DOES_NOT_EXIST); + + expect(isReadonly).to.equal(false); + }); + }); + + suite('getFileHash', () => { + // Since getFileHash() relies on timestamps, we have to take + // into account filesystem timestamp resolution. For instance + // on FAT and HFS it is 1 second. + // See: https://nodejs.org/api/fs.html#fs_stat_time_values + + test('Getting hash for a file should return non-empty string', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const hash = await utils.getFileHash(filename); + + expect(hash).to.not.equal(''); + }); + + test('the returned hash is stable', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const hash1 = await utils.getFileHash(filename); + const hash2 = await utils.getFileHash(filename); + await sleep(2_000); // just in case + const hash3 = await utils.getFileHash(filename); + + expect(hash1).to.equal(hash2); + expect(hash1).to.equal(hash3); + expect(hash2).to.equal(hash3); + }); + + test('the returned hash changes with modification', async () => { + const filename = await fix.createFile('x/y/z/spam.py', 'original text'); + + const hash1 = await utils.getFileHash(filename); + await sleep(2_000); // for filesystems with 1s resolution + await fs.writeFile(filename, 'new text'); + const hash2 = await utils.getFileHash(filename); + + expect(hash1).to.not.equal(hash2); + }); + + test('the returned hash is unique', async () => { + const file1 = await fix.createFile('spam.py'); + await sleep(2_000); // for filesystems with 1s resolution + const file2 = await fix.createFile('x/y/z/spam.py'); + await sleep(2_000); // for filesystems with 1s resolution + const file3 = await fix.createFile('eggs.py'); + + const hash1 = await utils.getFileHash(file1); + const hash2 = await utils.getFileHash(file2); + const hash3 = await utils.getFileHash(file3); + + expect(hash1).to.not.equal(hash2); + expect(hash1).to.not.equal(hash3); + expect(hash2).to.not.equal(hash3); + }); + + test('Getting hash for non existent file should throw error', async () => { + const promise = utils.getFileHash(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('search', () => { + test('found matches', async () => { + const pattern = await fix.resolve(`x/y/z/spam.*`); + const expected: string[] = [ + await fix.createFile('x/y/z/spam.py'), + await fix.createFile('x/y/z/spam.pyc'), + await fix.createFile('x/y/z/spam.so'), + await fix.createDirectory('x/y/z/spam.data') + ]; + // non-matches + await fix.createFile('x/spam.py'); + await fix.createFile('x/y/z/eggs.py'); + await fix.createFile('x/y/z/spam-all.py'); + await fix.createFile('x/y/z/spam'); + await fix.createFile('x/spam.py'); + + let files = await utils.search(pattern); + + // For whatever reason, on Windows "search()" is + // returning filenames with forward slasshes... + files = files.map(fixPath); + expect(files.sort()).to.deep.equal(expected.sort()); + }); + + test('no matches', async () => { + const pattern = await fix.resolve(`x/y/z/spam.*`); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal([]); + }); + }); + + suite('fileExistsSync', () => { + test('want file, got file', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const exists = utils.fileExistsSync(filename); + + expect(exists).to.equal(true); + }); + + test('want file, not file', async () => { + const filename = await fix.createDirectory('x/y/z/spam.py'); + + const exists = utils.fileExistsSync(filename); + + // Note that currently the "file" can be *anything*. It + // doesn't have to be just a regular file. This is the + // way it already worked, so we're keeping it that way + // for now. + expect(exists).to.equal(true); + }); + + test('symlink', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); + + const exists = utils.fileExistsSync(symlink); + + // Note that currently the "file" can be *anything*. It + // doesn't have to be just a regular file. This is the + // way it already worked, so we're keeping it that way + // for now. + expect(exists).to.equal(true); + }); + + test('unknown', async function() { + if (WINDOWS) { + // tslint:disable-next-line:no-suspicious-comment + // TODO(GH-8995) These tests are failing on Windows, + // so we are // temporarily disabling it. + // tslint:disable-next-line:no-invalid-this + return this.skip(); + } + if (!SUPPORTS_SOCKETS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = utils.fileExistsSync(sockFile); + + // Note that currently the "file" can be *anything*. It + // doesn't have to be just a regular file. This is the + // way it already worked, so we're keeping it that way + // for now. + expect(exists).to.equal(true); + }); + }); +}); + suite('FileSystem', () => { let fileSystem: FileSystem; let fix: FSFixture; @@ -837,24 +1099,6 @@ suite('FileSystem', () => { }); suite('raw', () => { - suite('lstat', () => { - test('wraps the low-level impl', async () => { - const filename = await fix.createFile('x/y/z/spam.py', '...'); - // Ideally we would compare to the result of - // fileSystem.stat(). However, we do not have access - // to the VS Code API here. - // prettier-ignore - const expected = convertStat( - await fs.lstat(filename), - FileType.File - ); - - const stat = await fileSystem.lstat(filename); - - expect(stat).to.deep.equal(expected); - }); - }); - suite('createDirectory', () => { test('wraps the low-level impl', async () => { await fix.createDirectory('x'); @@ -967,17 +1211,6 @@ suite('FileSystem', () => { }); }); - suite('deleteFile', () => { - test('wraps the low-level impl', async () => { - const filename = await fix.createFile('x/y/z/spam.py', '...'); - await assertExists(filename); - - await fileSystem.deleteFile(filename); - - await assertDoesNotExist(filename); - }); - }); - suite('chmod (non-Windows)', () => { suiteSetup(function() { // On Windows, chmod won't have any effect on the file itself. diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 60f49512a636..65405e01123f 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/src/test/common/platform/filesystem.test.ts @@ -8,11 +8,11 @@ import { expect } from 'chai'; import * as fsextra from 'fs-extra'; // prettier-ignore import { - convertStat, FileSystem, RawFileSystem + convertStat, FileSystem, FileSystemUtils, RawFileSystem } from '../../../client/common/platform/fileSystem'; // prettier-ignore import { - FileType, IFileSystem, IRawFileSystem + FileType, IFileSystem, IFileSystemUtils, IRawFileSystem } from '../../../client/common/platform/types'; // prettier-ignore import { @@ -94,6 +94,213 @@ suite('FileSystem - raw', () => { }); }); +suite('FileSystem - utils', () => { + let utils: IFileSystemUtils; + let fix: FSFixture; + setup(async () => { + utils = FileSystemUtils.withDefaults(); + fix = new FSFixture(); + + await assertDoesNotExist(DOES_NOT_EXIST); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('pathExists', () => { + test('exists (without type)', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(true); + }); + + test('does not exist (without type)', async () => { + const exists = await utils.pathExists(DOES_NOT_EXIST); + + expect(exists).to.equal(false); + }); + + test('matches (type: file)', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(true); + }); + + test('mismatch (type: file)', async () => { + const filename = await fix.createDirectory('x/y/z/spam.py'); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(false); + }); + + test('matches (type: directory)', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(true); + }); + + test('mismatch (type: directory)', async () => { + const dirname = await fix.createFile('x/y/z/spam'); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(false); + }); + + test('symlinks are followed', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); + + const exists = await utils.pathExists(symlink, FileType.SymbolicLink); + const destIsFile = await utils.pathExists(symlink, FileType.File); + const destIsDir = await utils.pathExists(symlink, FileType.Directory); + + expect(exists).to.equal(true); + expect(destIsFile).to.equal(true); + expect(destIsDir).to.equal(false); + }); + + test('mismatch (type: symlink)', async () => { + const dirname = await fix.createFile('x/y/z/spam.py'); + + const exists = await utils.pathExists(dirname, FileType.SymbolicLink); + + expect(exists).to.equal(false); + }); + + test('matches (type: unknown)', async function() { + if (!SUPPORTS_SOCKETS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = await utils.pathExists(sockFile, FileType.Unknown); + + expect(exists).to.equal(true); + }); + + test('mismatch (type: unknown)', async () => { + const dirname = await fix.createFile('x/y/z/spam.py'); + + const exists = await utils.pathExists(dirname, FileType.Unknown); + + expect(exists).to.equal(false); + }); + }); + + suite('fileExists', () => { + test('want file, got file', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const exists = await utils.fileExists(filename); + + expect(exists).to.equal(true); + }); + + test('want file, not file', async () => { + const filename = await fix.createDirectory('x/y/z/spam.py'); + + const exists = await utils.fileExists(filename); + + expect(exists).to.equal(false); + }); + + test('symlink', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); + + const exists = await utils.fileExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + }); + + test('unknown', async function() { + if (!SUPPORTS_SOCKETS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = await utils.fileExists(sockFile); + + expect(exists).to.equal(false); + }); + }); + + suite('directoryExists', () => { + test('want directory, got directory', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const exists = await utils.directoryExists(dirname); + + expect(exists).to.equal(true); + }); + + test('want directory, not directory', async () => { + const dirname = await fix.createFile('x/y/z/spam'); + + const exists = await utils.directoryExists(dirname); + + expect(exists).to.equal(false); + }); + + test('symlink', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + const symlink = await fix.createSymlink('x/y/z/eggs', dirname); + + const exists = await utils.directoryExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + }); + + test('unknown', async function() { + if (!SUPPORTS_SOCKETS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = await utils.directoryExists(sockFile); + + expect(exists).to.equal(false); + }); + }); + + suite('getSubDirectories', () => { + test('empty if the directory does not exist', async () => { + const entries = await utils.getSubDirectories(DOES_NOT_EXIST); + + expect(entries).to.deep.equal([]); + }); + }); + + suite('getFiles', () => { + test('empty if the directory does not exist', async () => { + const entries = await utils.getFiles(DOES_NOT_EXIST); + + expect(entries).to.deep.equal([]); + }); + }); +}); + suite('FileSystem', () => { let filesystem: IFileSystem; let fix: FSFixture; diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 7939763a50ab..57629d239d17 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -6,10 +6,17 @@ import * as fs from 'fs'; import * as fsextra from 'fs-extra'; import * as TypeMoq from 'typemoq'; import * as vscode from 'vscode'; -import { RawFileSystem } from '../../../client/common/platform/fileSystem'; -// prettier-ignore +import { FileSystemUtils, RawFileSystem } from '../../../client/common/platform/fileSystem'; import { - FileStat, FileType, ReadStream, WriteStream + FileStat, + FileType, + // These interfaces are needed for FileSystemUtils deps. + IFileSystemPaths, + IFileSystemPathUtils, + IRawFileSystem, + ITempFileSystem, + ReadStream, + WriteStream } from '../../../client/common/platform/types'; // tslint:disable:max-func-body-length chai-vague-errors @@ -19,7 +26,13 @@ function createDummyStat(filetype: FileType): FileStat { return { type: filetype } as any; } -interface IRawFS { +interface IPaths { + // fs paths (IFileSystemPaths) + sep: string; + join(...filenames: string[]): string; +} + +interface IRawFS extends IPaths { // vscode.workspace.fs stat(uri: vscode.Uri): Thenable; @@ -39,9 +52,6 @@ interface IRawFS { readFileSync(path: string, encoding: string): string; createReadStream(filename: string): ReadStream; createWriteStream(filename: string): WriteStream; - - // fs paths (IFileSystemPaths) - join(...filenames: string[]): string; } suite('Raw FileSystem', () => { @@ -661,6 +671,70 @@ suite('Raw FileSystem', () => { }); }); -// tslint:disable-next-line:no-suspicious-comment -// TODO(GH-8995): The FileSystem isn't unit-tesstable currently. Once -// we address that, all its methods should have tests here. +interface IUtilsDeps extends IRawFileSystem, IFileSystemPaths, IFileSystemPathUtils, ITempFileSystem { + // helpers + getHash(data: string): string; + globFile(pat: string, options?: { cwd: string }): Promise; +} + +suite('FileSystemUtils', () => { + let deps: TypeMoq.IMock; + let stats: TypeMoq.IMock[]; + let utils: FileSystemUtils; + setup(() => { + deps = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + + stats = []; + utils = new FileSystemUtils( + // Since it's a mock we can just use it for all 3 values. + deps.object, + deps.object, + deps.object, + deps.object, + (data: string) => deps.object.getHash(data), + (pat: string, options?: { cwd: string }) => deps.object.globFile(pat, options) + ); + }); + function verifyAll() { + deps.verifyAll(); + stats.forEach(stat => { + stat.verifyAll(); + }); + } + + suite('createDirectory', () => { + test('wraps the low-level function', async () => { + const dirname = 'x/y/z/spam'; + deps.setup(d => d.mkdirp(dirname)) // expect the specific filename + .returns(() => Promise.resolve()); + + await utils.createDirectory(dirname); + + verifyAll(); + }); + }); + + suite('deleteDirectory', () => { + test('wraps the low-level function', async () => { + const dirname = 'x/y/z/spam'; + deps.setup(d => d.rmtree(dirname)) // expect the specific filename + .returns(() => Promise.resolve()); + + await utils.deleteDirectory(dirname); + + verifyAll(); + }); + }); + + suite('deleteFile', () => { + test('wraps the low-level function', async () => { + const filename = 'x/y/z/spam.py'; + deps.setup(d => d.rmfile(filename)) // expect the specific filename + .returns(() => Promise.resolve()); + + await utils.deleteFile(filename); + + verifyAll(); + }); + }); +}); diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 9609a97649ae..6cb983e1e3fc 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -9,7 +9,7 @@ import { Disposable, Memento, OutputChannel, Uri } from 'vscode'; import { STANDARD_OUTPUT_CHANNEL } from '../client/common/constants'; import { Logger } from '../client/common/logger'; import { IS_WINDOWS } from '../client/common/platform/constants'; -import { convertStat, FileSystem, RawFileSystem } from '../client/common/platform/fileSystem'; +import { convertStat, FileSystem, FileSystemUtils, RawFileSystem } from '../client/common/platform/fileSystem'; import { PathUtils } from '../client/common/platform/pathUtils'; import { PlatformService } from '../client/common/platform/platformService'; import { RegistryImplementation } from '../client/common/platform/registry'; @@ -98,7 +98,8 @@ class LegacyFileSystem extends FileSystem { constructor() { super(); const vscfs = new FakeVSCodeFileSystemAPI(); - this.raw = RawFileSystem.withDefaults(undefined, vscfs); + const raw = RawFileSystem.withDefaults(undefined, vscfs); + this.utils = FileSystemUtils.withDefaults(raw); } } From c2927ff53ba72be35e87261671113ae289749e66 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 28 Jan 2020 11:19:16 -0700 Subject: [PATCH 02/17] Add unit tests. --- src/client/common/platform/fileSystem.ts | 24 +- .../platform/filesystem.functional.test.ts | 4 +- src/test/common/platform/filesystem.test.ts | 8 +- .../common/platform/filesystem.unit.test.ts | 626 +++++++++++++++++- 4 files changed, 646 insertions(+), 16 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 39d02349e1a6..890bef00846a 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -279,6 +279,14 @@ export class RawFileSystem implements IRawFileSystem { //========================================== // filesystem "utils" +// This is the parts of the 'fs-extra' module that we use in RawFileSystem. +interface IFSExtraForUtils { + open(path: string, flags: string | number, mode?: string | number | null): Promise; + close(fd: number): Promise; + unlink(path: string): Promise; + existsSync(path: string): boolean; +} + // High-level filesystem operations used by the extension. export class FileSystemUtils implements IFileSystemUtils { constructor( @@ -286,14 +294,17 @@ export class FileSystemUtils implements IFileSystemUtils { public readonly pathUtils: IFileSystemPathUtils, public readonly paths: IFileSystemPaths, public readonly tmp: ITempFileSystem, - protected readonly getHash: (data: string) => string, - protected readonly globFile: (pat: string, options?: { cwd: string }) => Promise + // tslint:disable-next-line:no-shadowed-variable + private readonly fs: IFSExtraForUtils, + private readonly getHash: (data: string) => string, + private readonly globFile: (pat: string, options?: { cwd: string }) => Promise ) {} // Create a new object using common-case default values. public static withDefaults( raw?: IRawFileSystem, pathUtils?: IFileSystemPathUtils, tmp?: ITempFileSystem, + fsDeps?: IFSExtraForUtils, getHash?: (data: string) => string, globFile?: (pat: string, options?: { cwd: string }) => Promise ): FileSystemUtils { @@ -303,6 +314,7 @@ export class FileSystemUtils implements IFileSystemUtils { pathUtils, pathUtils.paths, tmp || TemporaryFileSystem.withDefaults(), + fsDeps || fs, getHash || getHashString, globFile || promisify(glob) ); @@ -390,7 +402,7 @@ export class FileSystemUtils implements IFileSystemUtils { const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; let fd: number; try { - fd = await fs.open(filePath, flags); + fd = await this.fs.open(filePath, flags); } catch (err) { if (isNoPermissionsError(err)) { return true; @@ -398,8 +410,8 @@ export class FileSystemUtils implements IFileSystemUtils { throw err; // re-throw } // Clean resources in the background. - fs.close(fd) - .finally(() => fs.unlink(filePath)) + this.fs.close(fd) + .finally(() => this.fs.unlink(filePath)) .ignoreErrors(); return false; } @@ -428,7 +440,7 @@ export class FileSystemUtils implements IFileSystemUtils { // helpers (non-async) public fileExistsSync(filePath: string): boolean { - return fs.existsSync(filePath); + return this.fs.existsSync(filePath); } } diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 75ef299404cb..2190005bdaec 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -874,7 +874,7 @@ suite('FileSystem - utils', () => { const isReadonly = await utils.isDirReadonly(dirname); - expect(isReadonly).to.equal(false); + expect(isReadonly).to.equal(true); }); }); @@ -883,7 +883,7 @@ suite('FileSystem - utils', () => { const isReadonly = await utils.isDirReadonly(dirname); - expect(isReadonly).to.equal(true); + expect(isReadonly).to.equal(false); }); // Failing may be more sensible, but for now we are sticking diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 65405e01123f..3b4c9a69f2b9 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/src/test/common/platform/filesystem.test.ts @@ -172,9 +172,9 @@ suite('FileSystem - utils', () => { }); test('mismatch (type: symlink)', async () => { - const dirname = await fix.createFile('x/y/z/spam.py'); + const filename = await fix.createFile('x/y/z/spam.py'); - const exists = await utils.pathExists(dirname, FileType.SymbolicLink); + const exists = await utils.pathExists(filename, FileType.SymbolicLink); expect(exists).to.equal(false); }); @@ -192,9 +192,9 @@ suite('FileSystem - utils', () => { }); test('mismatch (type: unknown)', async () => { - const dirname = await fix.createFile('x/y/z/spam.py'); + const filename = await fix.createFile('x/y/z/spam.py'); - const exists = await utils.pathExists(dirname, FileType.Unknown); + const exists = await utils.pathExists(filename, FileType.Unknown); expect(exists).to.equal(false); }); diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 57629d239d17..1764add2cd30 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -672,6 +672,12 @@ suite('Raw FileSystem', () => { }); interface IUtilsDeps extends IRawFileSystem, IFileSystemPaths, IFileSystemPathUtils, ITempFileSystem { + // fs + open(path: string, flags: string | number, mode?: string | number | null): Promise; + close(fd: number): Promise; + unlink(path: string): Promise; + existsSync(path: string): boolean; + // helpers getHash(data: string): string; globFile(pat: string, options?: { cwd: string }): Promise; @@ -687,10 +693,11 @@ suite('FileSystemUtils', () => { stats = []; utils = new FileSystemUtils( // Since it's a mock we can just use it for all 3 values. - deps.object, - deps.object, - deps.object, - deps.object, + deps.object, // rawFS + deps.object, // pathUtils + deps.object, // paths + deps.object, // tempFS + deps.object, // fs (data: string) => deps.object.getHash(data), (pat: string, options?: { cwd: string }) => deps.object.globFile(pat, options) ); @@ -701,6 +708,17 @@ suite('FileSystemUtils', () => { stat.verifyAll(); }); } + function createMockStat(): TypeMoq.IMock { + const stat = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + // This is necessary because passing "mock.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)); + stats.push(stat); + return stat; + } suite('createDirectory', () => { test('wraps the low-level function', async () => { @@ -737,4 +755,604 @@ suite('FileSystemUtils', () => { verifyAll(); }); }); + + suite('pathExists', () => { + test('exists (without type)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('does not exist', async () => { + const filename = 'x/y/z/spam.py'; + const err = vscode.FileSystemError.FileNotFound(filename); + deps.setup(d => d.stat(filename)) // The file does not exist. + .throws(err); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('fails if stat fails', async () => { + const filename = 'x/y/z/spam.py'; + const err = new Error('oops!'); + deps.setup(d => d.stat(filename)) // There was a problem while stat'ing the file. + .throws(err); + + const promise = utils.pathExists(filename); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + + test('matches (type: undefined)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('matches (type: file)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a file. + .returns(() => FileType.File); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('mismatch (type: file)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a directory. + .returns(() => FileType.Directory); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename, FileType.File); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('matches (type: directory)', async () => { + const dirname = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a directory. + .returns(() => FileType.Directory); + deps.setup(d => d.stat(dirname)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('mismatch (type: directory)', async () => { + const dirname = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a file. + .returns(() => FileType.File); + deps.setup(d => d.stat(dirname)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(dirname, FileType.Directory); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('symlinks are followed', async () => { + const symlink = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a symlink to a file. + .returns(() => FileType.File | FileType.SymbolicLink) + .verifiable(TypeMoq.Times.exactly(3)); + deps.setup(d => d.stat(symlink)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)) + .verifiable(TypeMoq.Times.exactly(3)); + + const exists = await utils.pathExists(symlink, FileType.SymbolicLink); + const destIsFile = await utils.pathExists(symlink, FileType.File); + const destIsDir = await utils.pathExists(symlink, FileType.Directory); + + expect(exists).to.equal(true); + expect(destIsFile).to.equal(true); + expect(destIsDir).to.equal(false); + verifyAll(); + }); + + test('mismatch (type: symlink)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a file. + .returns(() => FileType.File); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(filename, FileType.SymbolicLink); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('matches (type: unknown)', async () => { + const sockFile = 'x/y/z/ipc.sock'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a socket. + .returns(() => FileType.Unknown); + deps.setup(d => d.stat(sockFile)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.pathExists(sockFile, FileType.Unknown); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('mismatch (type: unknown)', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a file. + .returns(() => FileType.File); + deps.setup(d => d.stat(filename)) // The "file" exists. + .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'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a File. + .returns(() => FileType.File); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.fileExists(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('want file, not file', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a directory. + .returns(() => FileType.Directory); + deps.setup(d => d.stat(filename)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.fileExists(filename); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('symlink', async () => { + const symlink = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a symlink to a File. + .returns(() => FileType.File | FileType.SymbolicLink); + deps.setup(d => d.stat(symlink)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.fileExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + verifyAll(); + }); + + test('unknown', async () => { + const sockFile = 'x/y/z/ipc.sock'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a socket. + .returns(() => FileType.Unknown); + deps.setup(d => d.stat(sockFile)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.fileExists(sockFile); + + expect(exists).to.equal(false); + verifyAll(); + }); + }); + + suite('directoryExists', () => { + test('want directory, got directory', async () => { + const dirname = 'x/y/z/spam'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a directory. + .returns(() => FileType.Directory); + deps.setup(d => d.stat(dirname)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.directoryExists(dirname); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('want directory, not directory', async () => { + const dirname = 'x/y/z/spam'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a file. + .returns(() => FileType.File); + deps.setup(d => d.stat(dirname)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.directoryExists(dirname); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('symlink', async () => { + const symlink = 'x/y/z/spam'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a symlink to a directory. + .returns(() => FileType.Directory | FileType.SymbolicLink); + deps.setup(d => d.stat(symlink)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.directoryExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + verifyAll(); + }); + + test('unknown', async () => { + const sockFile = 'x/y/z/ipc.sock'; + const stat = createMockStat(); + stat.setup(s => s.type) // It's a socket. + .returns(() => FileType.Unknown); + deps.setup(d => d.stat(sockFile)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const exists = await utils.directoryExists(sockFile); + + expect(exists).to.equal(false); + verifyAll(); + }); + }); + + suite('listdir', () => { + test('wraps the raw call on success', async () => { + const dirname = 'x/y/z/spam'; + const expected: [string, FileType][] = [ + ['x/y/z/spam/dev1', FileType.Unknown], + ['x/y/z/spam/w', FileType.Directory], + ['x/y/z/spam/spam.py', FileType.File], + ['x/y/z/spam/other', FileType.SymbolicLink | FileType.File] + ]; + deps.setup(d => d.listdir(dirname)) // Full results get returned from RawFileSystem.listdir(). + .returns(() => Promise.resolve(expected)); + + const entries = await utils.listdir(dirname); + + expect(entries).to.deep.equal(expected); + verifyAll(); + }); + + test('returns [] if the directory does not exist', async () => { + const dirname = 'x/y/z/spam'; + const err = vscode.FileSystemError.FileNotFound(dirname); + deps.setup(d => d.listdir(dirname)) // The "file" does not exist. + .returns(() => Promise.reject(err)); + deps.setup(d => d.stat(dirname)) // The "file" does not exist. + .returns(() => Promise.reject(err)); + + const entries = await utils.listdir(dirname); + + expect(entries).to.deep.equal([]); + verifyAll(); + }); + + test('fails if not a directory', async () => { + const dirname = 'x/y/z/spam'; + const err = vscode.FileSystemError.FileNotADirectory(dirname); + deps.setup(d => d.listdir(dirname)) // Fail (async) with not-a-directory. + .returns(() => Promise.reject(err)); + const stat = createMockStat(); + deps.setup(d => d.stat(dirname)) // The "file" exists. + .returns(() => Promise.resolve(stat.object)); + + const promise = utils.listdir(dirname); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + + test('fails if the raw call promise fails', async () => { + const dirname = 'x/y/z/spam'; + const err = new Error('oops!'); + deps.setup(d => d.listdir(dirname)) // Fail (async) with an arbitrary error. + .returns(() => Promise.reject(err)); + deps.setup(d => d.stat(dirname)) // Fail with file-not-found. + .throws(vscode.FileSystemError.FileNotFound(dirname)); + + const entries = await utils.listdir(dirname); + + expect(entries).to.deep.equal([]); + verifyAll(); + }); + + test('fails if the raw call fails', async () => { + const dirname = 'x/y/z/spam'; + const err = new Error('oops!'); + deps.setup(d => d.listdir(dirname)) // Fail with an arbirary error. + .throws(err); + + const promise = utils.listdir(dirname); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('getSubDirectories', () => { + test('filters out non-subdirs', async () => { + const dirname = 'x/y/z/spam'; + const entries: [string, FileType][] = [ + ['x/y/z/spam/dev1', FileType.Unknown], + ['x/y/z/spam/w', FileType.Directory], + ['x/y/z/spam/spam.py', FileType.File], + ['x/y/z/spam/v', FileType.Directory], + ['x/y/z/spam/eggs.py', FileType.File], + ['x/y/z/spam/other1', FileType.SymbolicLink | FileType.File], + ['x/y/z/spam/other2', FileType.SymbolicLink | FileType.Directory] + ]; + const expected = [ + // only entries with FileType.Directory + 'x/y/z/spam/w', + 'x/y/z/spam/v', + 'x/y/z/spam/other2' + ]; + deps.setup(d => d.listdir(dirname)) // Full results get returned from RawFileSystem.listdir(). + .returns(() => Promise.resolve(entries)); + + const filtered = await utils.getSubDirectories(dirname); + + expect(filtered).to.deep.equal(expected); + verifyAll(); + }); + }); + + suite('getFiles', () => { + test('filters out non-files', async () => { + const filename = 'x/y/z/spam'; + const entries: [string, FileType][] = [ + ['x/y/z/spam/dev1', FileType.Unknown], + ['x/y/z/spam/w', FileType.Directory], + ['x/y/z/spam/spam.py', FileType.File], + ['x/y/z/spam/v', FileType.Directory], + ['x/y/z/spam/eggs.py', FileType.File], + ['x/y/z/spam/other1', FileType.SymbolicLink | FileType.File], + ['x/y/z/spam/other2', FileType.SymbolicLink | FileType.Directory] + ]; + const expected = [ + // only entries with FileType.File + 'x/y/z/spam/spam.py', + 'x/y/z/spam/eggs.py', + 'x/y/z/spam/other1' + ]; + deps.setup(d => d.listdir(filename)) // Full results get returned from RawFileSystem.listdir(). + .returns(() => Promise.resolve(entries)); + + const filtered = await utils.getFiles(filename); + + expect(filtered).to.deep.equal(expected); + verifyAll(); + }); + }); + + suite('isDirReadonly', () => { + const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; + setup(() => { + deps.setup(d => d.sep) // The value really doesn't matter. + .returns(() => '/'); + }); + + test('is readonly', async () => { + const dirname = 'x/y/z/spam'; + const fd = 10; + const filename = `${dirname}/___vscpTest___`; + deps.setup(d => d.open(filename, flags)) // Success! + .returns(() => Promise.resolve(fd)); + deps.setup(d => d.close(fd)) // Success! + .returns(() => Promise.resolve()); + deps.setup(d => d.unlink(filename)) // Success! + .returns(() => Promise.resolve()); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(false); + verifyAll(); + }); + + test('is not readonly', async () => { + const dirname = 'x/y/z/spam'; + const filename = `${dirname}/___vscpTest___`; + const err = new Error('not permitted'); + // tslint:disable-next-line:no-any + (err as any).code = 'EACCES'; // errno + deps.setup(d => d.open(filename, flags)) // not permitted + .returns(() => Promise.reject(err)); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(true); + verifyAll(); + }); + + test('fails if the directory does not exist', async () => { + const dirname = 'x/y/z/spam'; + const filename = `${dirname}/___vscpTest___`; + const err = new Error('not found'); + // tslint:disable-next-line:no-any + (err as any).code = 'ENOENT'; // errno + deps.setup(d => d.open(filename, flags)) // file-not-found + .returns(() => Promise.reject(err)); + + const promise = utils.isDirReadonly(dirname); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('getFileHash', () => { + test('Getting hash for a file should return non-empty string', async () => { + const filename = 'x/y/z/spam.py'; + const stat = createMockStat(); + stat.setup(s => s.ctime) // created + .returns(() => 100); + stat.setup(s => s.mtime) // modified + .returns(() => 120); + deps.setup(d => d.lstat(filename)) // file exists + .returns(() => Promise.resolve(stat.object)); + deps.setup(d => d.getHash('100-120')) // built from ctime and mtime + .returns(() => 'deadbeef'); + + const hash = await utils.getFileHash(filename); + + expect(hash).to.equal('deadbeef'); + verifyAll(); + }); + + test('Getting hash for non existent file should throw error', async () => { + const filename = 'x/y/z/spam.py'; + const err = vscode.FileSystemError.FileNotFound(filename); + deps.setup(d => d.lstat(filename)) // file-not-found + .returns(() => Promise.reject(err)); + + const promise = utils.getFileHash(filename); + + await expect(promise).to.eventually.be.rejected; + verifyAll(); + }); + }); + + suite('search', () => { + test('found matches (without cwd)', async () => { + const pattern = `x/y/z/spam.*`; + const expected: string[] = [ + // We can pretend that there were other files + // that were ignored. + 'x/y/z/spam.py', + 'x/y/z/spam.pyc', + 'x/y/z/spam.so', + 'x/y/z/spam.data' + ]; + deps.setup(d => d.globFile(pattern, undefined)) // found some + .returns(() => Promise.resolve(expected)); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal(expected); + verifyAll(); + }); + + test('found matches (with cwd)', async () => { + const pattern = `x/y/z/spam.*`; + const cwd = 'a/b/c'; + const expected: string[] = [ + // We can pretend that there were other files + // that were ignored. + 'x/y/z/spam.py', + 'x/y/z/spam.pyc', + 'x/y/z/spam.so', + 'x/y/z/spam.data' + ]; + deps.setup(d => d.globFile(pattern, { cwd: cwd })) // found some + .returns(() => Promise.resolve(expected)); + + const files = await utils.search(pattern, cwd); + + expect(files).to.deep.equal(expected); + verifyAll(); + }); + + test('no matches (empty)', async () => { + const pattern = `x/y/z/spam.*`; + deps.setup(d => d.globFile(pattern, undefined)) // found none + .returns(() => Promise.resolve([])); + + 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.globFile(pattern, undefined)) // found none + .returns(() => Promise.resolve((undefined as unknown) as string[])); + + const files = await utils.search(pattern); + + expect(files).to.deep.equal([]); + verifyAll(); + }); + }); + + suite('fileExistsSync', () => { + test('file exists', async () => { + const filename = 'x/y/z/spam.py'; + deps.setup(d => d.existsSync(filename)) // The file exists. + .returns(() => true); + + const exists = utils.fileExistsSync(filename); + + expect(exists).to.equal(true); + verifyAll(); + }); + + test('file does not exist', async () => { + const filename = 'x/y/z/spam.py'; + deps.setup(d => d.existsSync(filename)) // The file does not exist. + .returns(() => false); + + const exists = utils.fileExistsSync(filename); + + expect(exists).to.equal(false); + verifyAll(); + }); + + test('fails if low-level call fails', async () => { + const filename = 'x/y/z/spam.py'; + const err = new Error('oops!'); + deps.setup(d => d.existsSync(filename)) // big badda boom + .throws(err); + + expect(() => utils.fileExistsSync(filename)).to.throw(err); + verifyAll(); + }); + }); }); From 9d74247b9e937c4601172c49c19c3beacd6b973a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 28 Jan 2020 15:08:59 -0700 Subject: [PATCH 03/17] Fix a functional test. --- src/test/common/platform/filesystem.functional.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 2190005bdaec..3c5497ac88f3 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -886,12 +886,10 @@ suite('FileSystem - utils', () => { expect(isReadonly).to.equal(false); }); - // Failing may be more sensible, but for now we are sticking - // with the existing behavior. - test('false if the directory does not exist', async () => { - const isReadonly = await utils.isDirReadonly(DOES_NOT_EXIST); + test('fail if the directory does not exist', async () => { + const promise = utils.isDirReadonly(DOES_NOT_EXIST); - expect(isReadonly).to.equal(false); + await expect(promise).to.eventually.be.rejected; }); }); From 4ebde478da1ecf6e384d9e2af41b34fd45c7048f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:06:37 -0700 Subject: [PATCH 04/17] Plug the old FileSystem impl back in. --- src/client/common/platform/fileSystem.ts | 219 ++++++++++++++++++++++- src/test/serviceRegistry.ts | 1 + 2 files changed, 214 insertions(+), 6 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 890bef00846a..2f3ea4b0f3f9 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -297,7 +297,7 @@ export class FileSystemUtils implements IFileSystemUtils { // tslint:disable-next-line:no-shadowed-variable private readonly fs: IFSExtraForUtils, private readonly getHash: (data: string) => string, - private readonly globFile: (pat: string, options?: { cwd: string }) => Promise + private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise ) {} // Create a new object using common-case default values. public static withDefaults( @@ -306,7 +306,7 @@ export class FileSystemUtils implements IFileSystemUtils { tmp?: ITempFileSystem, fsDeps?: IFSExtraForUtils, getHash?: (data: string) => string, - globFile?: (pat: string, options?: { cwd: string }) => Promise + globFiles?: (pat: string, options?: { cwd: string }) => Promise ): FileSystemUtils { pathUtils = pathUtils || FileSystemPathUtils.withDefaults(); return new FileSystemUtils( @@ -316,7 +316,7 @@ export class FileSystemUtils implements IFileSystemUtils { tmp || TemporaryFileSystem.withDefaults(), fsDeps || fs, getHash || getHashString, - globFile || promisify(glob) + globFiles || promisify(glob) ); } @@ -429,9 +429,9 @@ export class FileSystemUtils implements IFileSystemUtils { const options = { cwd: cwd }; - found = await this.globFile(globPattern, options); + found = await this.globFiles(globPattern, options); } else { - found = await this.globFile(globPattern); + found = await this.globFiles(globPattern); } return Array.isArray(found) ? found : []; } @@ -457,7 +457,8 @@ function getHashString(data: string): string { // more aliases (to cause less churn) @injectable() -export class FileSystem implements IFileSystem { +//export class FileSystem implements IFileSystem { +class FileSystemBase { // We expose this for the sake of functional tests that do not have // access to the actual "vscode" namespace. protected utils: FileSystemUtils; @@ -544,3 +545,209 @@ export class FileSystem implements IFileSystem { return this.utils.isDirReadonly(dirname); } } + +// tslint:disable:max-classes-per-file + +@injectable() +export class FileSystem extends FileSystemBase implements IFileSystem { + // We expose this for the sake of functional tests that do not have + // access to the actual "vscode" namespace. + protected raw: RawFileSystem; + private readonly paths: IFileSystemPaths; + private readonly pathUtils: FileSystemPathUtils; + private readonly tmp: TemporaryFileSystem; + private readonly getHash: (data: string) => string; + private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; + constructor() { + super(); + this.paths = FileSystemPaths.withDefaults(); + this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); + this.tmp = TemporaryFileSystem.withDefaults(); + this.raw = RawFileSystem.withDefaults(this.paths); + this.getHash = getHashString; + this.globFiles = promisify(glob); + } + + //================================= + // path-related + + public get directorySeparatorChar(): string { + return this.paths.sep; + } + + public arePathsSame(path1: string, path2: string): boolean { + return this.pathUtils.arePathsSame(path1, path2); + } + + //================================= + // "raw" operations + + public async stat(filename: string): Promise { + return this.raw.stat(filename); + } + + public async lstat(filename: string): Promise { + return this.raw.lstat(filename); + } + + public async readFile(filePath: string): Promise { + return this.raw.readText(filePath); + } + public readFileSync(filePath: string): string { + return this.raw.readTextSync(filePath); + } + public async readData(filePath: string): Promise { + return this.raw.readData(filePath); + } + + public async writeFile(filePath: string, text: string, _options: string | fs.WriteFileOptions = { encoding: 'utf8' }): Promise { + // tslint:disable-next-line:no-suspicious-comment + // TODO (GH-8542) For now we ignore the options, since all call + // sites already match the defaults. Later we will fix the call + // sites. + return this.raw.writeText(filePath, text); + } + + public async createDirectory(directoryPath: string): Promise { + return this.raw.mkdirp(directoryPath); + } + + public async deleteDirectory(directoryPath: string): Promise { + return this.raw.rmtree(directoryPath); + } + + public async listdir(dirname: string): Promise<[string, FileType][]> { + // prettier-ignore + return this.raw.listdir(dirname) + .catch(async err => { + // We're only preserving pre-existng behavior here... + if (!(await this.pathExists(dirname))) { + return []; + } + throw err; // re-throw + }); + } + + public async appendFile(filename: string, text: string): Promise { + return this.raw.appendText(filename, text); + } + + public async copyFile(src: string, dest: string): Promise { + return this.raw.copyFile(src, dest); + } + + public async deleteFile(filename: string): Promise { + return this.raw.rmfile(filename); + } + + public async chmod(filePath: string, mode: string | number): Promise { + return this.raw.chmod(filePath, mode); + } + + public async move(src: string, tgt: string) { + await this.raw.move(src, tgt); + } + + public createReadStream(filePath: string): ReadStream { + return this.raw.createReadStream(filePath); + } + + public createWriteStream(filePath: string): WriteStream { + return this.raw.createWriteStream(filePath); + } + + //================================= + // utils + + // prettier-ignore + public async pathExists( + filename: string, + fileType?: FileType + ): Promise { + let stat: FileStat; + try { + // Note that we are using stat() rather than lstat(). This + // means that any symlinks are getting resolved. + stat = await this.raw.stat(filename); + } catch (err) { + if (isFileNotFoundError(err)) { + return false; + } + throw err; + } + + if (fileType === undefined) { + return true; + } + if (fileType === FileType.Unknown) { + // FileType.Unknown == 0, hence do not use bitwise operations. + return stat.type === FileType.Unknown; + } + return (stat.type & fileType) === fileType; + } + public async fileExists(filename: string): Promise { + return this.pathExists(filename, FileType.File); + } + public fileExistsSync(filePath: string): boolean { + return fs.existsSync(filePath); + } + public async directoryExists(dirname: string): Promise { + return this.pathExists(dirname, FileType.Directory); + } + + public async getSubDirectories(dirname: string): Promise { + // prettier-ignore + return filterByFileType( + (await this.listdir(dirname)), + FileType.Directory + ).map(([filename, _fileType]) => filename); + } + public async getFiles(dirname: string): Promise { + // prettier-ignore + return filterByFileType( + (await this.listdir(dirname)), + FileType.File + ).map(([filename, _fileType]) => filename); + } + + public async getFileHash(filename: string): Promise { + // The reason for lstat rather than stat is not clear... + const stat = await this.raw.lstat(filename); + const data = `${stat.ctime}-${stat.mtime}`; + return this.getHash(data); + } + + public async search(globPattern: string, cwd?: string): Promise { + let found: string[]; + if (cwd) { + const options = { + cwd: cwd + }; + found = await this.globFiles(globPattern, options); + } else { + found = await this.globFiles(globPattern); + } + return Array.isArray(found) ? found : []; + } + + public createTemporaryFile(extension: string): Promise { + return this.tmp.createFile(extension); + } + + public async isDirReadonly(dirname: string): Promise { + const filePath = `${dirname}${this.paths.sep}___vscpTest___`; + const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; + let fd: number; + try { + fd = await fs.open(filePath, flags); + } catch (err) { + if (isNoPermissionsError(err)) { + return true; + } + throw err; // re-throw + } + await fs.close(fd); + await fs.unlink(filePath); + return false; + } +} diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 6cb983e1e3fc..45433f15e569 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -100,6 +100,7 @@ class LegacyFileSystem extends FileSystem { const vscfs = new FakeVSCodeFileSystemAPI(); const raw = RawFileSystem.withDefaults(undefined, vscfs); this.utils = FileSystemUtils.withDefaults(raw); + this.raw = raw; } } From 0bd0c3da0f08837e0e2038453021e27fc2aa8813 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:18:32 -0700 Subject: [PATCH 05/17] Use FileSystemUtils.getFileHash(). --- src/client/common/platform/fileSystem.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 2f3ea4b0f3f9..63aba6244247 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -556,7 +556,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { private readonly paths: IFileSystemPaths; private readonly pathUtils: FileSystemPathUtils; private readonly tmp: TemporaryFileSystem; - private readonly getHash: (data: string) => string; private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; constructor() { super(); @@ -564,7 +563,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); this.tmp = TemporaryFileSystem.withDefaults(); this.raw = RawFileSystem.withDefaults(this.paths); - this.getHash = getHashString; this.globFiles = promisify(glob); } @@ -710,13 +708,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { ).map(([filename, _fileType]) => filename); } - public async getFileHash(filename: string): Promise { - // The reason for lstat rather than stat is not clear... - const stat = await this.raw.lstat(filename); - const data = `${stat.ctime}-${stat.mtime}`; - return this.getHash(data); - } - public async search(globPattern: string, cwd?: string): Promise { let found: string[]; if (cwd) { From c4f788076e3f42a6df9c6acf3af6a3ea71b3ff01 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:23:18 -0700 Subject: [PATCH 06/17] Use the new raw wrappers. --- src/client/common/platform/fileSystem.ts | 62 ------------------------ 1 file changed, 62 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 63aba6244247..8316ef6449bb 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -580,40 +580,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { //================================= // "raw" operations - public async stat(filename: string): Promise { - return this.raw.stat(filename); - } - - public async lstat(filename: string): Promise { - return this.raw.lstat(filename); - } - - public async readFile(filePath: string): Promise { - return this.raw.readText(filePath); - } - public readFileSync(filePath: string): string { - return this.raw.readTextSync(filePath); - } - public async readData(filePath: string): Promise { - return this.raw.readData(filePath); - } - - public async writeFile(filePath: string, text: string, _options: string | fs.WriteFileOptions = { encoding: 'utf8' }): Promise { - // tslint:disable-next-line:no-suspicious-comment - // TODO (GH-8542) For now we ignore the options, since all call - // sites already match the defaults. Later we will fix the call - // sites. - return this.raw.writeText(filePath, text); - } - - public async createDirectory(directoryPath: string): Promise { - return this.raw.mkdirp(directoryPath); - } - - public async deleteDirectory(directoryPath: string): Promise { - return this.raw.rmtree(directoryPath); - } - public async listdir(dirname: string): Promise<[string, FileType][]> { // prettier-ignore return this.raw.listdir(dirname) @@ -626,34 +592,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { }); } - public async appendFile(filename: string, text: string): Promise { - return this.raw.appendText(filename, text); - } - - public async copyFile(src: string, dest: string): Promise { - return this.raw.copyFile(src, dest); - } - - public async deleteFile(filename: string): Promise { - return this.raw.rmfile(filename); - } - - public async chmod(filePath: string, mode: string | number): Promise { - return this.raw.chmod(filePath, mode); - } - - public async move(src: string, tgt: string) { - await this.raw.move(src, tgt); - } - - public createReadStream(filePath: string): ReadStream { - return this.raw.createReadStream(filePath); - } - - public createWriteStream(filePath: string): WriteStream { - return this.raw.createWriteStream(filePath); - } - //================================= // utils From d3c1a23542e8122284eb861cf72d4f7aea84472c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:25:15 -0700 Subject: [PATCH 07/17] Use FileSystemUtils.tmp.createFile(). --- src/client/common/platform/fileSystem.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 8316ef6449bb..0e5ce6a2ee64 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -555,13 +555,11 @@ export class FileSystem extends FileSystemBase implements IFileSystem { protected raw: RawFileSystem; private readonly paths: IFileSystemPaths; private readonly pathUtils: FileSystemPathUtils; - private readonly tmp: TemporaryFileSystem; private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; constructor() { super(); this.paths = FileSystemPaths.withDefaults(); this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); - this.tmp = TemporaryFileSystem.withDefaults(); this.raw = RawFileSystem.withDefaults(this.paths); this.globFiles = promisify(glob); } @@ -659,10 +657,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { return Array.isArray(found) ? found : []; } - public createTemporaryFile(extension: string): Promise { - return this.tmp.createFile(extension); - } - public async isDirReadonly(dirname: string): Promise { const filePath = `${dirname}${this.paths.sep}___vscpTest___`; const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; From 04f608bbc41a83f5267b20f2aa5976ffcf1443c1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:27:17 -0700 Subject: [PATCH 08/17] Use the path helpers from FileSystemUtils. --- src/client/common/platform/fileSystem.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 0e5ce6a2ee64..4f5ca31ad95f 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -554,27 +554,14 @@ export class FileSystem extends FileSystemBase implements IFileSystem { // access to the actual "vscode" namespace. protected raw: RawFileSystem; private readonly paths: IFileSystemPaths; - private readonly pathUtils: FileSystemPathUtils; private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; constructor() { super(); this.paths = FileSystemPaths.withDefaults(); - this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); this.raw = RawFileSystem.withDefaults(this.paths); this.globFiles = promisify(glob); } - //================================= - // path-related - - public get directorySeparatorChar(): string { - return this.paths.sep; - } - - public arePathsSame(path1: string, path2: string): boolean { - return this.pathUtils.arePathsSame(path1, path2); - } - //================================= // "raw" operations @@ -658,7 +645,7 @@ export class FileSystem extends FileSystemBase implements IFileSystem { } public async isDirReadonly(dirname: string): Promise { - const filePath = `${dirname}${this.paths.sep}___vscpTest___`; + const filePath = `${dirname}${this.utils.paths.sep}___vscpTest___`; const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; let fd: number; try { From ca66d2a40f52630db146c58a768c1006bf9e582d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:28:57 -0700 Subject: [PATCH 09/17] Use FileSystemUtils.listdir(). --- src/client/common/platform/fileSystem.ts | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 4f5ca31ad95f..cfe0462df8fe 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -562,21 +562,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { this.globFiles = promisify(glob); } - //================================= - // "raw" operations - - public async listdir(dirname: string): Promise<[string, FileType][]> { - // prettier-ignore - return this.raw.listdir(dirname) - .catch(async err => { - // We're only preserving pre-existng behavior here... - if (!(await this.pathExists(dirname))) { - return []; - } - throw err; // re-throw - }); - } - //================================= // utils @@ -619,14 +604,14 @@ export class FileSystem extends FileSystemBase implements IFileSystem { public async getSubDirectories(dirname: string): Promise { // prettier-ignore return filterByFileType( - (await this.listdir(dirname)), + (await this.utils.listdir(dirname)), FileType.Directory ).map(([filename, _fileType]) => filename); } public async getFiles(dirname: string): Promise { // prettier-ignore return filterByFileType( - (await this.listdir(dirname)), + (await this.utils.listdir(dirname)), FileType.File ).map(([filename, _fileType]) => filename); } From 594e1a04b429c9e285e00db05e64d857436e1182 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:31:31 -0700 Subject: [PATCH 10/17] Use FileSystemUtils.fileExistsSync(). --- src/client/common/platform/fileSystem.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index cfe0462df8fe..f4addcf87cb5 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -594,9 +594,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { public async fileExists(filename: string): Promise { return this.pathExists(filename, FileType.File); } - public fileExistsSync(filePath: string): boolean { - return fs.existsSync(filePath); - } public async directoryExists(dirname: string): Promise { return this.pathExists(dirname, FileType.Directory); } From 027fc1dbf44d08a8523bf35dc1fd4605a77b3db4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:39:04 -0700 Subject: [PATCH 11/17] Use FileSystemUtils.getFiles() and .getSubDirectories(). --- src/client/common/platform/fileSystem.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index f4addcf87cb5..f573314d5bad 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -598,21 +598,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { return this.pathExists(dirname, FileType.Directory); } - public async getSubDirectories(dirname: string): Promise { - // prettier-ignore - return filterByFileType( - (await this.utils.listdir(dirname)), - FileType.Directory - ).map(([filename, _fileType]) => filename); - } - public async getFiles(dirname: string): Promise { - // prettier-ignore - return filterByFileType( - (await this.utils.listdir(dirname)), - FileType.File - ).map(([filename, _fileType]) => filename); - } - public async search(globPattern: string, cwd?: string): Promise { let found: string[]; if (cwd) { From 01a845cef778500a092e40b1902565a006b5e48a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:40:01 -0700 Subject: [PATCH 12/17] Use FileSystemUtils.isDirReadOnly(). --- src/client/common/platform/fileSystem.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index f573314d5bad..859cea2bbcb6 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -610,21 +610,4 @@ export class FileSystem extends FileSystemBase implements IFileSystem { } return Array.isArray(found) ? found : []; } - - public async isDirReadonly(dirname: string): Promise { - const filePath = `${dirname}${this.utils.paths.sep}___vscpTest___`; - const flags = fs.constants.O_CREAT | fs.constants.O_RDWR; - let fd: number; - try { - fd = await fs.open(filePath, flags); - } catch (err) { - if (isNoPermissionsError(err)) { - return true; - } - throw err; // re-throw - } - await fs.close(fd); - await fs.unlink(filePath); - return false; - } } From 0f0e6355dd5e68d3cecbe00afe5e92aea1dec940 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:43:02 -0700 Subject: [PATCH 13/17] Use FileSystemUtils.pathExists(), etc. --- src/client/common/platform/fileSystem.ts | 33 ------------------------ 1 file changed, 33 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 859cea2bbcb6..0bfc1ceed591 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -565,39 +565,6 @@ export class FileSystem extends FileSystemBase implements IFileSystem { //================================= // utils - // prettier-ignore - public async pathExists( - filename: string, - fileType?: FileType - ): Promise { - let stat: FileStat; - try { - // Note that we are using stat() rather than lstat(). This - // means that any symlinks are getting resolved. - stat = await this.raw.stat(filename); - } catch (err) { - if (isFileNotFoundError(err)) { - return false; - } - throw err; - } - - if (fileType === undefined) { - return true; - } - if (fileType === FileType.Unknown) { - // FileType.Unknown == 0, hence do not use bitwise operations. - return stat.type === FileType.Unknown; - } - return (stat.type & fileType) === fileType; - } - public async fileExists(filename: string): Promise { - return this.pathExists(filename, FileType.File); - } - public async directoryExists(dirname: string): Promise { - return this.pathExists(dirname, FileType.Directory); - } - public async search(globPattern: string, cwd?: string): Promise { let found: string[]; if (cwd) { From 8dbe6c38708bb4e481385de57e40f891f42b80c5 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:46:31 -0700 Subject: [PATCH 14/17] Drop superflous props. --- src/client/common/platform/fileSystem.ts | 6 ------ src/test/serviceRegistry.ts | 1 - 2 files changed, 7 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 0bfc1ceed591..c70f2ee1c04d 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -550,15 +550,9 @@ class FileSystemBase { @injectable() export class FileSystem extends FileSystemBase implements IFileSystem { - // We expose this for the sake of functional tests that do not have - // access to the actual "vscode" namespace. - protected raw: RawFileSystem; - private readonly paths: IFileSystemPaths; private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; constructor() { super(); - this.paths = FileSystemPaths.withDefaults(); - this.raw = RawFileSystem.withDefaults(this.paths); this.globFiles = promisify(glob); } diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 45433f15e569..6cb983e1e3fc 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -100,7 +100,6 @@ class LegacyFileSystem extends FileSystem { const vscfs = new FakeVSCodeFileSystemAPI(); const raw = RawFileSystem.withDefaults(undefined, vscfs); this.utils = FileSystemUtils.withDefaults(raw); - this.raw = raw; } } From 79594bb8782517cf8f00d63408b1d56a9dabcbcf Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:53:19 -0700 Subject: [PATCH 15/17] Use the new FileSystem.search(). --- src/client/common/platform/fileSystem.ts | 30 +----------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index c70f2ee1c04d..7b6568e8f383 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -457,8 +457,7 @@ function getHashString(data: string): string { // more aliases (to cause less churn) @injectable() -//export class FileSystem implements IFileSystem { -class FileSystemBase { +export class FileSystem implements IFileSystem { // We expose this for the sake of functional tests that do not have // access to the actual "vscode" namespace. protected utils: FileSystemUtils; @@ -545,30 +544,3 @@ class FileSystemBase { return this.utils.isDirReadonly(dirname); } } - -// tslint:disable:max-classes-per-file - -@injectable() -export class FileSystem extends FileSystemBase implements IFileSystem { - private readonly globFiles: (pat: string, options?: { cwd: string }) => Promise; - constructor() { - super(); - this.globFiles = promisify(glob); - } - - //================================= - // utils - - public async search(globPattern: string, cwd?: string): Promise { - let found: string[]; - if (cwd) { - const options = { - cwd: cwd - }; - found = await this.globFiles(globPattern, options); - } else { - found = await this.globFiles(globPattern); - } - return Array.isArray(found) ? found : []; - } -} From cf3abbae44e5a012d59a773c1489a0cf7462561d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 13:54:08 -0700 Subject: [PATCH 16/17] Do not forget to support the "cwd" arg to FileSystem.search(). --- src/client/common/platform/fileSystem.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 7b6568e8f383..5cb0ff242bd0 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -534,8 +534,8 @@ export class FileSystem implements IFileSystem { public async getFileHash(filename: string): Promise { return this.utils.getFileHash(filename); } - public async search(globPattern: string): Promise { - return this.utils.search(globPattern); + public async search(globPattern: string, cwd?: string): Promise { + return this.utils.search(globPattern, cwd); } public async createTemporaryFile(suffix: string): Promise { return this.utils.tmp.createFile(suffix); From df07eb59bb078e6c6461fac66f7565e16de2b479 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 30 Jan 2020 14:26:18 -0700 Subject: [PATCH 17/17] lint --- src/client/common/platform/fileSystem.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 5cb0ff242bd0..0217163ce37c 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -410,7 +410,8 @@ export class FileSystemUtils implements IFileSystemUtils { throw err; // re-throw } // Clean resources in the background. - this.fs.close(fd) + this.fs + .close(fd) .finally(() => this.fs.unlink(filePath)) .ignoreErrors(); return false;