From 5f4ea1ce9770187203cb494a122d51ecf3cc60e8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Dec 2019 13:10:56 -0700 Subject: [PATCH 01/31] Group the IFileSystem methods. --- src/client/common/platform/types.ts | 38 +++++++++++++++++------------ 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index cadd7b0ae1a1..e52a62726159 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -39,36 +39,42 @@ export type WriteStream = fs.WriteStream; export const IFileSystem = Symbol('IFileSystem'); export interface IFileSystem { + // path-related directorySeparatorChar: string; + arePathsSame(path1: string, path2: string): boolean; + getRealPath(path: string): Promise; + + // "raw" operations stat(filePath: string): Promise; - objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; - fileExists(path: string): Promise; - fileExistsSync(path: string): boolean; - directoryExists(path: string): Promise; createDirectory(path: string): Promise; deleteDirectory(path: string): Promise; listdir(dirname: string): Promise; - getSubDirectories(rootDir: string): Promise; - getFiles(rootDir: string): Promise; - arePathsSame(path1: string, path2: string): boolean; - readData(filePath: string): Promise; readFile(filePath: string): Promise; + readData(filePath: string): Promise; writeFile(filePath: string, data: {}, options?: string | fsextra.WriteFileOptions): Promise; - readFileSync(filename: string): string; appendFile(filename: string, data: {}): Promise; + copyFile(src: string, dest: string): Promise; + deleteFile(filename: string): Promise; + chmod(path: string, mode: string): Promise; + move(src: string, tgt: string): Promise; + // sync + readFileSync(filename: string): string; 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; + createReadStream(path: string): fs.ReadStream; + createWriteStream(path: string): fs.WriteStream; + + // utils + objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; + fileExists(path: string): Promise; + fileExistsSync(path: string): boolean; + directoryExists(path: string): Promise; + getSubDirectories(rootDir: string): Promise; + getFiles(rootDir: string): Promise; getFileHash(filePath: string): Promise; search(globPattern: string, cwd?: string): Promise; createTemporaryFile(extension: string): Promise; - createReadStream(path: string): fs.ReadStream; - createWriteStream(path: string): fs.WriteStream; - chmod(path: string, mode: string): Promise; - move(src: string, tgt: string): Promise; isDirReadonly(dirname: string): Promise; } From adc6b119c4db295a657006939dfa81ba824e5131 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Dec 2019 13:18:59 -0700 Subject: [PATCH 02/31] Group FileSystem methods. --- src/client/common/platform/fileSystem.ts | 217 ++++++++++++----------- 1 file changed, 111 insertions(+), 106 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 3ed20df830f4..82181b402f0b 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -19,11 +19,38 @@ const globAsync = promisify(glob); @injectable() export class FileSystem implements IFileSystem { - constructor(@inject(IPlatformService) private platformService: IPlatformService) {} + constructor( + @inject(IPlatformService) private platformService: IPlatformService + ) { } + + //================================= + // path-related public get directorySeparatorChar(): string { return path.sep; } + + 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 getRealPath(filePath: string): Promise { + return new Promise(resolve => { + fs.realpath(filePath, (err, realPath) => { + resolve(err ? filePath : realPath); + }); + }); + } + + //================================= + // "raw" operations + 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. @@ -32,32 +59,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)); - }); - }); - } - - 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 - * @returns {Promise} - * @memberof FileSystem - */ + // Return the UTF8-decoded text of the file. public readFile(filePath: string): Promise { return fs.readFile(filePath, 'utf8'); } + public readFileSync(filePath: string): string { + return fs.readFileSync(filePath, 'utf8'); + } public readData(filePath: string): Promise { return fs.readFile(filePath); } @@ -66,10 +74,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); } @@ -92,49 +96,6 @@ export class FileSystem implements IFileSystem { }); } - public getSubDirectories(rootDir: string): Promise { - return new Promise(resolve => { - fs.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[]; - resolve(subDirs); - }); - }); - } - - public async getFiles(rootDir: string): Promise { - const files = await fs.readdir(rootDir); - return files.filter(async f => { - const fullPath = path.join(rootDir, f); - if ((await fs.stat(fullPath)).isFile()) { - return true; - } - return false; - }); - } - - 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 appendFile(filename: string, data: {}): Promise { return fs.appendFile(filename, data); } @@ -146,14 +107,6 @@ export class FileSystem implements IFileSystem { 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 => { @@ -177,6 +130,85 @@ export class FileSystem implements IFileSystem { return deferred.promise; } + 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(); + }); + }); + } + + public async move(src: string, tgt: string) { + await fs.rename(src, tgt); + } + + public createReadStream(filePath: string): fileSystem.ReadStream { + return fileSystem.createReadStream(filePath); + } + + public createWriteStream(filePath: string): fileSystem.WriteStream { + return fileSystem.createWriteStream(filePath); + } + + //================================= + // utils + + 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 fileExistsSync(filePath: string): boolean { + return fs.existsSync(filePath); + } + 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) => { + 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[]; + resolve(subDirs); + }); + }); + } + + public async getFiles(rootDir: string): Promise { + const files = await fs.readdir(rootDir); + return files.filter(async f => { + const fullPath = path.join(rootDir, f); + if ((await fs.stat(fullPath)).isFile()) { + return true; + } + return false; + }); + } + public getFileHash(filePath: string): Promise { return new Promise((resolve, reject) => { fs.lstat(filePath, (err, stats) => { @@ -214,33 +246,6 @@ export class FileSystem implements IFileSystem { }); } - public createReadStream(filePath: string): fileSystem.ReadStream { - return fileSystem.createReadStream(filePath); - } - - 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(); - }); - }); - } - - public readFileSync(filePath: string): string { - return fs.readFileSync(filePath, 'utf8'); - } - - public async move(src: string, tgt: string) { - await fs.rename(src, tgt); - } - public async isDirReadonly(dirname: string): Promise { const filePath = `${dirname}${path.sep}___vscpTest___`; return new Promise(resolve => { From 7911537b5afe94c9ed4795dc9982b2d19f49fb56 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Dec 2019 13:22:20 -0700 Subject: [PATCH 03/31] Make the FS 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 2069cefacded4ec70c389dbcfaaede1fcb731201 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Dec 2019 14:37:46 -0700 Subject: [PATCH 04/31] Drop IFileSystem.objectExists. --- src/client/common/platform/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index e52a62726159..ebb2d8b3ed71 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -67,7 +67,6 @@ export interface IFileSystem { createWriteStream(path: string): fs.WriteStream; // utils - objectExists(path: string, statCheck: (s: fs.Stats) => boolean): Promise; fileExists(path: string): Promise; fileExistsSync(path: string): boolean; directoryExists(path: string): Promise; From 64171ffdf403733ec29c19e0ad391a1c63435ed7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Dec 2019 15:33:22 -0700 Subject: [PATCH 05/31] Re-organize tests. --- .../platform/filesystem.functional.test.ts | 359 +++++++++++++----- src/test/common/platform/filesystem.test.ts | 95 +++++ .../common/platform/filesystem.unit.test.ts | 57 +++ src/test/common/platform/utils.ts | 96 +++++ 4 files changed, 507 insertions(+), 100 deletions(-) create mode 100644 src/test/common/platform/filesystem.test.ts create mode 100644 src/test/common/platform/filesystem.unit.test.ts create mode 100644 src/test/common/platform/utils.ts diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index b681cdcc2dad..755b920b8a64 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -1,130 +1,289 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// tslint:disable:max-func-body-length + 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 { PlatformService } from '../../../client/common/platform/platformService'; +import { IFileSystem, TemporaryFile } from '../../../client/common/platform/types'; // 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', () => { - 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(); - }); - teardown(cleanTestFiles); - function cleanTestFiles() { - if (fs.existsSync(fileToAppendTo)) { - fs.unlinkSync(fileToAppendTo); - } - } - test('ReadFile returns contents of a file', async () => { - const file = __filename; - const expectedContents = await fs.readFile(file).then(buffer => buffer.toString()); - const content = await fileSystem.readFile(file); - - expect(content).to.be.equal(expectedContents); + fileSystem = new FileSystem( + new PlatformService() + ); }); - test('ReadFile throws an exception if file does not exist', async () => { - const readPromise = fs.readFile('xyz', { encoding: 'utf8' }); - await expect(readPromise).to.be.rejectedWith(); - }); + suite('path-related', () => { + suite('directorySeparatorChar', () => { + test('value', () => { + const sep = fileSystem.directorySeparatorChar; - 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); - 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)'); - } - - 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'); - } - - test('Case sensitivity is ignored when comparing file names on windows', async () => { - caseSensitivityFileCheck(true, false, false); - }); + expect(sep).to.equal(path.sep); + }); + }); - test('Case sensitivity is not ignored when comparing file names on osx', async () => { - caseSensitivityFileCheck(false, true, false); - }); + suite('arePathsSame', () => { + test('', () => { + // XXX + }); + }); - test('Case sensitivity is not ignored when comparing file names on linux', async () => { - caseSensitivityFileCheck(false, false, true); - }); - test('Check existence of files synchronously', async () => { - expect(fileSystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found'); + suite('getRealPath', () => { + test('', async () => { + // XXX + }); + }); }); - 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)); - 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'); - 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) => { - expect(tf).to.not.equal(undefined, 'Error trying to create a temporary file'); - 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}`); + suite('raw', () => { + suite('createDirectory', () => { + test('', async () => { + // XXX }); - }, (failReason) => { - 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( - (_success: void) => { - // cannot check for success other than we got here, chmod in Windows won't have any effect on the file itself. - }, - (failReason) => { - expect(failReason).to.equal('There was no error using chmod', `Failed to perform chmod operation successfully, got error ${failReason}`); - }); + + suite('deleteDirectory', () => { + test('', async () => { + // XXX + }); }); - }); - test('Getting hash for non existent file should throw error', async () => { - const promise = fileSystem.getFileHash('some unknown file'); - await expect(promise).to.eventually.be.rejected; + suite('readFile', () => { + test('returns contents of a file', async () => { + const filename = __filename; + const expected = await fs.readFile(filename, 'utf8'); + + const text = await fileSystem.readFile(filename); + + expect(text).to.be.equal(expected); + }); + + test('throws an exception if file does not exist', async () => { + const readPromise = fileSystem.readFile('xyz'); + + await expect(readPromise).to.be.rejectedWith(); + }); + + test('', async () => { + // XXX + }); + }); + + suite('writeFile', () => { + test('', async () => { + // XXX + }); + }); + + suite('appendFile', () => { + test('', async () => { + // XXX + }); + }); + + suite('copyFile', () => { + test('', async () => { + // XXX + }); + }); + + suite('deleteFile', () => { + test('', async () => { + // XXX + }); + }); + + suite('chmod', () => { + test('', async () => { + // XXX + }); + }); + + suite('move', () => { + test('', async () => { + // XXX + }); + }); + + //============================= + // sync methods + + suite('readFileSync', () => { + test('', async () => { + // XXX + }); + }); + + suite('appendFileSync', () => { + const fileToAppendTo = path.join(__dirname, 'created_for_testing_dummy.txt'); + setup(cleanTestFiles); + teardown(cleanTestFiles); + function cleanTestFiles() { + if (fs.existsSync(fileToAppendTo)) { + fs.unlinkSync(fileToAppendTo); + } + } + + test('appending to a file works', async () => { + const dataToAppend = `Some Data\n${new Date().toString()}\nAnd another line`; + + fileSystem.appendFileSync(fileToAppendTo, dataToAppend); + + const actual = await fs.readFile(fileToAppendTo, 'utf8'); + expect(actual).to.be.equal(dataToAppend); + }); + + test('', async () => { + // XXX + }); + }); + + suite('createReadStream', () => { + test('', async () => { + // XXX + }); + }); + + suite('createWriteStream', () => { + test('', async () => { + // XXX + }); + }); }); - test('Getting hash for a file should return non-empty string', async () => { - const hash = await fileSystem.getFileHash(__filename); - expect(hash).to.be.length.greaterThan(0); + suite('utils', () => { + suite('fileExists', () => { + test('', async () => { + // XXX + }); + }); + + suite('directoryExists', () => { + test('', async () => { + // XXX + }); + }); + + suite('getSubDirectories', () => { + test('', async () => { + // XXX + }); + }); + + suite('getFiles', () => { + test('', async () => { + // XXX + }); + }); + + suite('getFileHash', () => { + test('Getting hash for non existent file should throw error', async () => { + 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); + + expect(hash).to.be.length.greaterThan(0); // weak + }); + + test('', async () => { + // XXX + }); + }); + + suite('search', () => { + test('Test searching for files', async () => { + const expected = __filename.replace(/\\/g, '/'); + const searchPattern = `${path.basename(__filename, __filename.substring(__filename.length - 3))}.*`; + + const files = await fileSystem.search(path.join(__dirname, searchPattern)); + + expect(files).to.be.array(); + expect(files.length).to.be.at.least(1); + const fileName = files[0].replace(/\\/g, '/'); + expect(fileName).to.equal(expected); + }); + + test('', async () => { + // XXX + }); + }); + + suite('createTemporaryFile', () => { + 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'); + + expect(tempFile.filePath).to.not.equal(tempFile2.filePath, 'Temp files must be unique, implementation of createTemporaryFile is off.'); + }); + + test('Ensure chmod works against a temporary file', async () => { + 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. + }, + (failReason) => { + expect(failReason).to.equal('There was no error using chmod', `Failed to perform chmod operation successfully, got error ${failReason}`); + } + ); + }); + }); + + test('Ensure writing to a temp file is supported via file stream', async () => { + 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); + writeStream.write('hello', 'utf8', (err: Error | null | undefined) => { + expect(err).to.equal(undefined, `Failed to write to a temp file, error is ${err}`); + }); + }, + (failReason) => { + expect(failReason).to.equal('No errors occurred', `Failed to create a temporary file with error ${failReason}`); + } + ); + }); + + test('', async () => { + // XXX + }); + }); + + suite('isDirReadonly', () => { + test('', async () => { + // XXX + }); + }); + + //============================= + // sync methods + + suite('fileExistsSync', () => { + test('Check existence of files synchronously', async () => { + const exists = fileSystem.fileExistsSync(__filename); + + expect(exists).to.be.equal(true, 'file not found'); + }); + + test('', async () => { + // XXX + }); + }); }); }); diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts new file mode 100644 index 000000000000..551a74ddb9eb --- /dev/null +++ b/src/test/common/platform/filesystem.test.ts @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-func-body-length chai-vague-errors +// tslint:disable:no-suspicious-comment + +import { expect } from 'chai'; +import * as fsextra from 'fs-extra'; +import * as vscode from 'vscode'; +import { + FileSystem +} from '../../../client/common/platform/fileSystem'; +import { PlatformService } from '../../../client/common/platform/platformService'; +import { + IFileSystem +} from '../../../client/common/platform/types'; +import { + assertDoesNotExist, DOES_NOT_EXIST, FSFixture +} from './utils'; + +// Note: all functional tests that do not trigger the VS Code "fs" API +// are found in filesystem.functional.test.ts. + +import FileType = vscode.FileType; +type FileStat = vscode.FileStat; + +suite('FileSystem', () => { + let filesystem: IFileSystem; + let fix: FSFixture; + setup(async () => { + filesystem = new FileSystem( + new PlatformService() + ); + fix = new FSFixture(); + + await assertDoesNotExist(DOES_NOT_EXIST); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('stat', () => { + function convertStat(old: fsextra.Stats, filetype: FileType): FileStat { + return { + type: filetype, + size: old.size, + // TODO (https://github.com/microsoft/vscode/issues/84177) + // FileStat.ctime and FileStat.mtime only have 1-second resolution. + // So for now we round to the nearest integer. + // TODO (https://github.com/microsoft/vscode/issues/84177) + // FileStat.ctime is consistently 0 instead of the actual ctime. + ctime: 0, + //ctime: Math.round(old.ctimeMs), + mtime: Math.round(old.mtimeMs) + }; + } + + test('gets the info for an existing file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const old = await fsextra.stat(filename); + const expected = convertStat(old, FileType.File); + + 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 old = await fsextra.stat(dirname); + const expected = convertStat(old, FileType.Directory); + + 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 old = await fsextra.stat(filename); + const expected = convertStat(old, FileType.SymbolicLink | FileType.File); + + 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; + }); + }); +}); 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..02bef9e62733 --- /dev/null +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-func-body-length + +import { expect } from 'chai'; +import * as TypeMoq from 'typemoq'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { IPlatformService } from '../../../client/common/platform/types'; + +suite('FileSystem', () => { + let platformService: TypeMoq.IMock; + let fileSystem: FileSystem; + setup(() => { + platformService = TypeMoq.Mock.ofType(); + fileSystem = new FileSystem(platformService.object); + }); + + suite('path-related', () => { + suite('arePathsSame', () => { + 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); + 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'; + + const res11 = fileSystem.arePathsSame(path1, path1); + const res12 = fileSystem.arePathsSame(path1, path2); + const res13 = fileSystem.arePathsSame(path1, path3); + const res22 = fileSystem.arePathsSame(path2, path2); + + if (isWindows) { + expect(res12).to.be.equal(true, 'file paths do not match (windows)'); + } else { + expect(res12).to.be.equal(false, 'file match (non windows)'); + } + expect(res11).to.be.equal(true, '1. file paths do not match'); + expect(res22).to.be.equal(true, '2. file paths do not match'); + expect(res13).to.be.equal(false, '2. file paths do not match'); + } + + test('Case sensitivity is ignored when comparing file names on windows', async () => { + caseSensitivityFileCheck(true, false, false); + }); + + 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 linux', async () => { + caseSensitivityFileCheck(false, false, true); + }); + }); + }); +}); diff --git a/src/test/common/platform/utils.ts b/src/test/common/platform/utils.ts new file mode 100644 index 000000000000..d4041f065d5b --- /dev/null +++ b/src/test/common/platform/utils.ts @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as fsextra from 'fs-extra'; +import * as net from 'net'; +import * as path from 'path'; +import * as tmpMod from 'tmp'; + +// Note: all functional tests that trigger the VS Code "fs" API are +// found in filesystem.test.ts. + +export const WINDOWS = /^win/.test(process.platform); + +export const DOES_NOT_EXIST = 'this file does not exist'; + +export async function assertDoesNotExist(filename: string) { + await expect( + fsextra.stat(filename) + ).to.eventually.be.rejected; +} + +export async function assertExists(filename: string) { + await expect( + fsextra.stat(filename) + ).to.not.eventually.be.rejected; +} + +export class FSFixture { + public tempDir: tmpMod.SynchrounousResult | undefined; + public sockServer: net.Server | undefined; + + public async cleanUp() { + if (this.tempDir) { + const tempDir = this.tempDir; + this.tempDir = undefined; + try { + tempDir.removeCallback(); + } catch { + // The "unsafeCleanup: true" option is supposed + // to support a non-empty directory, but apparently + // that isn't always the case. (see #8804) + await fsextra.remove(tempDir.name); + } + } + 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 = tmpMod.dirSync({ + prefix: 'pyvsc-fs-tests-', + unsafeCleanup: true + }); + } + 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; + } +} From 3917ce5cb17cffcfdb73a8e9a0fe661e02a23863 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 11:30:53 -0700 Subject: [PATCH 06/31] Fix up the unit tests. --- .../common/platform/filesystem.unit.test.ts | 93 +++++++++++++------ 1 file changed, 63 insertions(+), 30 deletions(-) diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index 02bef9e62733..80235951e3e7 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -7,50 +7,83 @@ import { expect } from 'chai'; import * as TypeMoq from 'typemoq'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IPlatformService } from '../../../client/common/platform/types'; +import { getNamesAndValues } from '../../../client/common/utils/enum'; +import { OSType } from '../../../client/common/utils/platform'; suite('FileSystem', () => { let platformService: TypeMoq.IMock; let fileSystem: FileSystem; setup(() => { - platformService = TypeMoq.Mock.ofType(); + platformService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); fileSystem = new FileSystem(platformService.object); }); + function verifyAll() { + platformService.verifyAll(); + } suite('path-related', () => { + const caseInsensitive = [OSType.Windows]; + suite('arePathsSame', () => { - 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); - 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'; - - const res11 = fileSystem.arePathsSame(path1, path1); - const res12 = fileSystem.arePathsSame(path1, path2); - const res13 = fileSystem.arePathsSame(path1, path3); - const res22 = fileSystem.arePathsSame(path2, path2); - - if (isWindows) { - expect(res12).to.be.equal(true, 'file paths do not match (windows)'); - } else { - expect(res12).to.be.equal(false, 'file match (non windows)'); + getNamesAndValues(OSType).forEach(item => { + const osType = item.value; + + function setPlatform(numCalls = 1) { + platformService.setup(p => p.isWindows) + .returns(() => osType === OSType.Windows) + .verifiable(TypeMoq.Times.exactly(numCalls)); } - expect(res11).to.be.equal(true, '1. file paths do not match'); - expect(res22).to.be.equal(true, '2. file paths do not match'); - expect(res13).to.be.equal(false, '2. file paths do not match'); - } - test('Case sensitivity is ignored when comparing file names on windows', async () => { - caseSensitivityFileCheck(true, false, false); - }); + test(`True if paths are identical (type: ${item.name})`, () => { + setPlatform(2); + const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt'; + const path2 = 'c:\\USERS\\Peter Smith\\my documents\\test.TXT'; - test('Case sensitivity is not ignored when comparing file names on osx', async () => { - caseSensitivityFileCheck(false, true, false); - }); + const areSame11 = fileSystem.arePathsSame(path1, path1); + const areSame22 = fileSystem.arePathsSame(path2, path2); + + expect(areSame11).to.be.equal(true, '1. file paths do not match'); + expect(areSame22).to.be.equal(true, '2. file paths do not match'); + verifyAll(); + }); + + test(`False if paths are completely different (type: ${item.name})`, () => { + setPlatform(); + const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt'; + const path2 = 'c:\\users\\Peter Smith\\my documents\\test.exe'; + + const areSame = fileSystem.arePathsSame(path1, path2); + + expect(areSame).to.be.equal(false, 'file paths do not match'); + verifyAll(); + }); + + if (caseInsensitive.includes(osType)) { + test(`True if paths only differ by case (type: ${item.name})`, () => { + setPlatform(); + const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt'; + const path2 = 'c:\\USERS\\Peter Smith\\my documents\\test.TXT'; + + const areSame = fileSystem.arePathsSame(path1, path2); + + expect(areSame).to.be.equal(true, 'file paths match'); + verifyAll(); + }); + } else { + test(`False if paths only differ by case (type: ${item.name})`, () => { + setPlatform(); + const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt'; + const path2 = 'c:\\USERS\\Peter Smith\\my documents\\test.TXT'; + + const areSame = fileSystem.arePathsSame(path1, path2); + + expect(areSame).to.be.equal(false, 'file paths do not match'); + verifyAll(); + }); + } - test('Case sensitivity is not ignored when comparing file names on linux', async () => { - caseSensitivityFileCheck(false, false, true); + // Missing tests: + // * exercize normalization }); }); }); From 5ebb73702cb62181b004080aa0509d5de3cc64a9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 15:30:42 -0700 Subject: [PATCH 07/31] Support numbers in chmod(). --- src/client/common/platform/fileSystem.ts | 2 +- src/client/common/platform/types.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 82181b402f0b..0aac747ec4bc 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -130,7 +130,7 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - public chmod(filePath: string, mode: string): Promise { + public chmod(filePath: string, mode: string | number): Promise { return new Promise((resolve, reject) => { fileSystem.chmod(filePath, mode, (err: NodeJS.ErrnoException | null) => { if (err) { diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index ebb2d8b3ed71..f7c598604e77 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -55,7 +55,7 @@ export interface IFileSystem { appendFile(filename: string, data: {}): Promise; copyFile(src: string, dest: string): Promise; deleteFile(filename: string): Promise; - chmod(path: string, mode: string): Promise; + chmod(path: string, mode: string | number): Promise; move(src: string, tgt: string): Promise; // sync readFileSync(filename: string): string; From 8db3cdb0fc8c8c788b25805dc176aa6e1f516f5e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 15:32:57 -0700 Subject: [PATCH 08/31] Add a CleanupFixture and use it in FSFixture. --- src/test/common/platform/utils.ts | 156 ++++++++++++++++++++++++------ 1 file changed, 127 insertions(+), 29 deletions(-) diff --git a/src/test/common/platform/utils.ts b/src/test/common/platform/utils.ts index d4041f065d5b..049fb5aed24a 100644 --- a/src/test/common/platform/utils.ts +++ b/src/test/common/platform/utils.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// tslint:disable:no-console + import { expect } from 'chai'; import * as fsextra from 'fs-extra'; import * as net from 'net'; @@ -12,6 +14,18 @@ import * as tmpMod from 'tmp'; export const WINDOWS = /^win/.test(process.platform); +export const SUPPORTS_SYMLINKS = (() => { + const source = fsextra.readdirSync('.')[0]; + const symlink = `${source}.symlink`; + try { + fsextra.symlinkSync(source, symlink); + } catch { + return false; + } + fsextra.unlinkSync(symlink); + return true; +})(); + export const DOES_NOT_EXIST = 'this file does not exist'; export async function assertDoesNotExist(filename: string) { @@ -26,39 +40,46 @@ export async function assertExists(filename: string) { ).to.not.eventually.be.rejected; } -export class FSFixture { - public tempDir: tmpMod.SynchrounousResult | undefined; - public sockServer: net.Server | undefined; +export class CleanupFixture { + private cleanups: (() => void | Promise)[]; + constructor() { + this.cleanups = []; + } + + public addCleanup(cleanup: () => void | Promise) { + this.cleanups.push(cleanup); + } public async cleanUp() { - if (this.tempDir) { - const tempDir = this.tempDir; - this.tempDir = undefined; + const cleanups = this.cleanups; + this.cleanups = []; + + return Promise.all(cleanups.map(async (cleanup, i) => { try { - tempDir.removeCallback(); - } catch { - // The "unsafeCleanup: true" option is supposed - // to support a non-empty directory, but apparently - // that isn't always the case. (see #8804) - await fsextra.remove(tempDir.name); + const res = cleanup(); + if (res) { + await res; + } + } catch (err) { + console.log(`cleanup ${i + 1} failed: ${err}`); + console.log('moving on...'); } - } - if (this.sockServer) { - const srv = this.sockServer; - await new Promise(resolve => srv.close(resolve)); - this.sockServer = undefined; - } + })); + } +} + +export class FSFixture extends CleanupFixture { + private tempDir: string | undefined; + private sockServer: net.Server | undefined; + + public addFSCleanup(filename: string, dispose?: () => void) { + this.addCleanup(() => this.ensureDeleted(filename, dispose)); } public async resolve(relname: string, mkdirs = true): Promise { - if (!this.tempDir) { - this.tempDir = tmpMod.dirSync({ - prefix: 'pyvsc-fs-tests-', - unsafeCleanup: true - }); - } + const tempDir = this.ensureTempDir(); relname = path.normalize(relname); - const filename = path.join(this.tempDir.name, relname); + const filename = path.join(tempDir, relname); if (mkdirs) { await fsextra.mkdirp( path.dirname(filename)); @@ -79,18 +100,95 @@ export class FSFixture { } public async createSymlink(relname: string, source: string): Promise { + if (!SUPPORTS_SYMLINKS) { + throw Error('this platform does not support symlinks'); + } 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 srv = this.ensureSocketServer(); const filename = await this.resolve(relname); await new Promise(resolve => srv!.listen(filename, 0, resolve)); return filename; } + + public async ensureDeleted(filename: string, dispose?: () => void) { + if (dispose) { + try { + dispose(); + return; // Trust that dispose() did what it's supposed to. + } catch (err) { + // For temp directories, the "unsafeCleanup: true" + // option of the "tmp" module is supposed to support + // a non-empty directory, but apparently that isn't + // always the case. + // (see #8804) + if (!await fsextra.pathExists(filename)) { + return; + } + console.log(`failure during dispose() for ${filename}: ${err}`); + console.log('...manually deleting'); + // Fall back to fsextra. + } + } + + try { + await fsextra.remove(filename); + } catch (err) { + if (!await fsextra.pathExists(filename)) { + return; + } + console.log(`failure while deleting ${filename}: ${err}`); + } + } + + private ensureTempDir(): string { + if (this.tempDir) { + return this.tempDir; + } + + const tempDir = tmpMod.dirSync({ + prefix: 'pyvsc-fs-tests-', + unsafeCleanup: true + }); + this.tempDir = tempDir.name; + + this.addFSCleanup(tempDir.name, async () => { + if (!this.tempDir) { + return; + } + this.tempDir = undefined; + + await this.ensureDeleted(tempDir.name, tempDir.removeCallback); + //try { + // tempDir.removeCallback(); + //} catch { + // // The "unsafeCleanup: true" option is supposed + // // to support a non-empty directory, but apparently + // // that isn't always the case. (see #8804) + // await fsextra.remove(tempDir.name); + //} + }); + return tempDir.name; + } + + private ensureSocketServer(): net.Server { + if (this.sockServer) { + return this.sockServer; + } + + const srv = net.createServer(); + this.sockServer = srv; + this.addCleanup(async () => { + try { + await new Promise(resolve => srv.close(resolve)); + } catch (err) { + console.log(`failure while closing socket server: ${err}`); + } + }); + return srv; + } } From f0f565a3e074496f2ee300a59dd82c2a81bd8d74 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 15:35:08 -0700 Subject: [PATCH 09/31] Unrevert a bunch of functional tests. --- .../platform/filesystem.functional.test.ts | 673 ++++++++++++++++-- src/test/common/platform/filesystem.test.ts | 8 +- 2 files changed, 600 insertions(+), 81 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 755b920b8a64..612bcbda5d9f 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// tslint:disable:max-func-body-length +// tslint:disable:max-func-body-length chai-vague-errors import { expect, use } from 'chai'; import * as fs from 'fs-extra'; @@ -9,6 +9,12 @@ import * as path from 'path'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { PlatformService } from '../../../client/common/platform/platformService'; import { IFileSystem, TemporaryFile } from '../../../client/common/platform/types'; +import { sleep } from '../../../client/common/utils/async'; +import { + assertDoesNotExist, assertExists, DOES_NOT_EXIST, FSFixture, + SUPPORTS_SYMLINKS, WINDOWS +} from './utils'; + // tslint:disable:no-require-imports no-var-requires const assertArrays = require('chai-arrays'); use(require('chai-as-promised')); @@ -16,10 +22,17 @@ use(assertArrays); suite('FileSystem', () => { let fileSystem: IFileSystem; - setup(() => { + let fix: FSFixture; + setup(async () => { fileSystem = new FileSystem( new PlatformService() ); + fix = new FSFixture(); + + await assertDoesNotExist(DOES_NOT_EXIST); + }); + teardown(async () => { + await fix.cleanUp(); }); suite('path-related', () => { @@ -32,8 +45,41 @@ suite('FileSystem', () => { }); suite('arePathsSame', () => { - test('', () => { - // XXX + test('identical', () => { + const filename = 'x/y/z/spam.py'; + + const result = fileSystem.arePathsSame(filename, filename); + + expect(result).to.equal(true); + }); + + test('not the same', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'a/b/c/spam.py'; + + const result = fileSystem.arePathsSame(file1, file2); + + expect(result).to.equal(false); + }); + + test('with different separators', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'x\\y\\z\\spam.py'; + const expected = WINDOWS; + + const result = fileSystem.arePathsSame(file1, file2); + + expect(result).to.equal(expected); + }); + + test('with different case', () => { + const file1 = 'x/y/z/spam.py'; + const file2 = 'x/Y/z/Spam.py'; + const expected = WINDOWS; + + const result = fileSystem.arePathsSame(file1, file2); + + expect(result).to.equal(expected); }); }); @@ -46,41 +92,126 @@ suite('FileSystem', () => { suite('raw', () => { suite('createDirectory', () => { - test('', async () => { - // XXX + 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 assertDoesNotExist(dirname); + + await fileSystem.createDirectory(dirname); + + await assertExists(dirname); + }); + + test('works if the directory already exists', async () => { + const dirname = await fix.createDirectory('spam'); + await assertExists(dirname); + + await fileSystem.createDirectory(dirname); + + await assertExists(dirname); }); }); suite('deleteDirectory', () => { - test('', async () => { - // XXX + test('deletes the directory if empty', async () => { + const dirname = await fix.createDirectory('x'); + await assertExists(dirname); + + await fileSystem.deleteDirectory(dirname); + + await assertDoesNotExist(dirname); + }); + + test('fails if the directory is not empty', async () => { + const dirname = await fix.createDirectory('x'); + const filename = await fix.createFile('x/y/z/spam.py'); + await assertExists(filename); + + const promise = fileSystem.deleteDirectory(dirname); + + await expect(promise).to.eventually.be.rejected; + }); + + test('fails if the directory does not exist', async () => { + const promise = fileSystem.deleteDirectory(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; }); }); suite('readFile', () => { test('returns contents of a file', async () => { - const filename = __filename; - const expected = await fs.readFile(filename, 'utf8'); + const expected = ''; + const filename = await fix.createFile('x/y/z/spam.py', expected); + + const content = await fileSystem.readFile(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.readFile(filename); - expect(text).to.be.equal(expected); + expect(text).to.equal(expected); }); - test('throws an exception if file does not exist', async () => { - const readPromise = fileSystem.readFile('xyz'); + test('returns garbage if encoding is UCS-2', async () => { + const filename = await fix.resolve('spam.py'); + // There are probably cases where this would fail too. + // However, the extension never has to deal with non-UTF8 + // cases, so it doesn't matter too much. + const original = '... 😁 ...'; + await fs.writeFile(filename, original, { encoding: 'ucs2' }); - await expect(readPromise).to.be.rejectedWith(); + const text = await fileSystem.readFile(filename); + + expect(text).to.equal('.\u0000.\u0000.\u0000 \u0000=�\u0001� \u0000.\u0000.\u0000.\u0000'); }); - test('', async () => { - // XXX + test('throws an exception if file does not exist', async () => { + const promise = fileSystem.readFile(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; }); }); suite('writeFile', () => { - test('', async () => { - // XXX + test('creates the file if missing', async () => { + const filename = await fix.resolve('x/y/z/spam.py'); + await assertDoesNotExist(filename); + const data = 'line1\nline2\n'; + + await fileSystem.writeFile(filename, data); + + const actual = await fs.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.writeFile(filename, data); + + const actual = await fs.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.writeFile(filename, data); + + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); }); }); @@ -91,20 +222,128 @@ suite('FileSystem', () => { }); suite('copyFile', () => { - test('', async () => { - // XXX + test('the source file gets copied (same directory)', async () => { + const data = ''; + const src = await fix.createFile('x/y/z/spam.py', data); + const dest = await fix.resolve('x/y/z/spam.py.bak'); + await assertDoesNotExist(dest); + + await fileSystem.copyFile(src, dest); + + const actual = await fs.readFile(dest) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); + const original = await fs.readFile(src) + .then(buffer => buffer.toString()); + expect(original).to.equal(data); + }); + + test('the source file gets copied (different directory)', async () => { + const data = ''; + const src = await fix.createFile('x/y/z/spam.py', data); + const dest = await fix.resolve('x/y/eggs.py'); + await assertDoesNotExist(dest); + + await fileSystem.copyFile(src, dest); + + const actual = await fs.readFile(dest) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); + const original = await fs.readFile(src) + .then(buffer => buffer.toString()); + expect(original).to.equal(data); + }); + + test('fails if the source does not exist', async () => { + const dest = await fix.resolve('x/spam.py'); + + const promise = fileSystem.copyFile(DOES_NOT_EXIST, dest); + + await expect(promise).to.eventually.be.rejected; + }); + + test('fails if the target parent directory does not exist', async () => { + const src = await fix.createFile('x/spam.py', '...'); + const dest = await fix.resolve('y/eggs.py', false); + await assertDoesNotExist(path.dirname(dest)); + + const promise = fileSystem.copyFile(src, dest); + + await expect(promise).to.eventually.be.rejected; }); }); suite('deleteFile', () => { - test('', async () => { - // XXX + test('deletes the file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + await assertExists(filename); + + await fileSystem.deleteFile(filename); + + await assertDoesNotExist(filename); + }); + + test('fails if the file does not exist', async () => { + const promise = fileSystem.deleteFile(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; }); }); - suite('chmod', () => { - test('', async () => { - // XXX + suite('chmod (non-Windows)', () => { + suiteSetup(function () { + // On Windows, chmod won't have any effect on the file itself. + if (WINDOWS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + }); + + async function checkMode(filename: string, expected: number) { + const stat = await fs.stat(filename); + expect(stat.mode & 0o777).to.equal(expected); + } + + test('the file mode gets updated (string)', async () => { + const filename = await fix.createFile('spam.py', '...'); + await fs.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 fs.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 fs.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 fs.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; }); }); @@ -118,8 +357,28 @@ suite('FileSystem', () => { // sync methods suite('readFileSync', () => { - test('', async () => { - // XXX + test('returns contents of a file', async () => { + const expected = ''; + const filename = await fix.createFile('x/y/z/spam.py', expected); + + const text = fileSystem.readFileSync(filename); + + expect(text).to.be.equal(expected); + }); + + test('always UTF-8', async () => { + const expected = '... 😁 ...'; + const filename = await fix.createFile('x/y/z/spam.py', expected); + + const text = fileSystem.readFileSync(filename); + + expect(text).to.equal(expected); + }); + + test('throws an exception if file does not exist', async () => { + expect( + () => fileSystem.readFileSync(DOES_NOT_EXIST) + ).to.throw(Error); }); }); @@ -154,110 +413,337 @@ suite('FileSystem', () => { }); suite('createWriteStream', () => { - test('', async () => { - // XXX + test('returns the correct WriteStream', async () => { + const filename = await fix.resolve('x/y/z/spam.py'); + const expected = fs.createWriteStream(filename); + expected.destroy(); + + const stream = fileSystem.createWriteStream(filename); + stream.destroy(); + + expect(stream.path).to.deep.equal(expected.path); + }); + + test('creates the file if missing', async () => { + const filename = await fix.resolve('x/y/z/spam.py'); + await assertDoesNotExist(filename); + const data = 'line1\nline2\n'; + + const stream = fileSystem.createWriteStream(filename); + stream.write(data); + stream.destroy(); + + const actual = await fs.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 = '... 😁 ...'; + + const stream = fileSystem.createWriteStream(filename); + stream.write(data); + stream.destroy(); + + const actual = await fs.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'; + + const stream = fileSystem.createWriteStream(filename); + stream.write(data); + stream.destroy(); + + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); + expect(actual).to.equal(data); }); }); }); suite('utils', () => { suite('fileExists', () => { + test('want file, got file', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const exists = await fileSystem.fileExists(filename); + + expect(exists).to.equal(true); + }); + test('', async () => { // XXX }); }); suite('directoryExists', () => { + test('want directory, got directory', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const exists = await fileSystem.directoryExists(dirname); + + expect(exists).to.equal(true); + }); + test('', async () => { // XXX }); }); suite('getSubDirectories', () => { + if (SUPPORTS_SYMLINKS) { + test('mixed types', async () => { + const symlinkSource = await fix.createFile('x/info.py'); + const dirname = await fix.createDirectory('x/y/z/scripts'); + const subdir1 = await fix.createDirectory('x/y/z/scripts/w'); + await fix.createFile('x/y/z/scripts/spam.py'); + const subdir2 = await fix.createDirectory('x/y/z/scripts/v'); + await fix.createFile('x/y/z/scripts/eggs.py'); + await fix.createSocket('x/y/z/scripts/spam.sock'); + await fix.createSymlink('x/y/z/scripts/other', symlinkSource); + await fix.createFile('x/y/z/scripts/data.json'); + + const results = await fileSystem.getSubDirectories(dirname); + + expect(results.sort()).to.deep.equal([ + subdir2, + subdir1 + ]); + }); + } else { + test('mixed types', async () => { + const dirname = await fix.createDirectory('x/y/z/scripts'); + const subdir1 = await fix.createDirectory('x/y/z/scripts/w'); + await fix.createFile('x/y/z/scripts/spam.py'); + const subdir2 = await fix.createDirectory('x/y/z/scripts/v'); + await fix.createFile('x/y/z/scripts/eggs.py'); + await fix.createSocket('x/y/z/scripts/spam.sock'); + await fix.createFile('x/y/z/scripts/data.json'); + + const results = await fileSystem.getSubDirectories(dirname); + + expect(results.sort()).to.deep.equal([ + subdir2, + subdir1 + ]); + }); + } + + test('empty if the directory does not exist', async () => { + const entries = await fileSystem.getSubDirectories(DOES_NOT_EXIST); + + expect(entries).to.deep.equal([]); + }); + test('', async () => { // XXX }); }); suite('getFiles', () => { + if (SUPPORTS_SYMLINKS) { + test('mixed types', async () => { + const symlinkSource = await fix.createFile('x/info.py'); + const dirname = await fix.createDirectory('x/y/z/scripts'); + await fix.createDirectory('x/y/z/scripts/w'); + const file1 = await fix.createFile('x/y/z/scripts/spam.py'); + await fix.createDirectory('x/y/z/scripts/v'); + const file2 = await fix.createFile('x/y/z/scripts/eggs.py'); + const sock = await fix.createSocket('x/y/z/scripts/spam.sock'); + const symlink = await fix.createSymlink('x/y/z/scripts/other', symlinkSource); + const file3 = await fix.createFile('x/y/z/scripts/data.json'); + + const results = await fileSystem.getFiles(dirname); + + expect(results.sort()).to.deep.equal([ + file3, + file2, + symlink, + file1, + sock + ]); + }); + } else { + test('mixed types', async () => { + const dirname = await fix.createDirectory('x/y/z/scripts'); + await fix.createDirectory('x/y/z/scripts/w'); + const file1 = await fix.createFile('x/y/z/scripts/spam.py'); + await fix.createDirectory('x/y/z/scripts/v'); + const file2 = await fix.createFile('x/y/z/scripts/eggs.py'); + const sock = await fix.createSocket('x/y/z/scripts/spam.sock'); + const file3 = await fix.createFile('x/y/z/scripts/data.json'); + + const results = await fileSystem.getFiles(dirname); + + expect(results.sort()).to.deep.equal([ + file3, + file2, + file1, + sock + ]); + }); + } + + test('empty if the directory does not exist', async () => { + const entries = await fileSystem.getFiles(DOES_NOT_EXIST); + + expect(entries).to.deep.equal([]); + }); + test('', async () => { // XXX }); }); suite('getFileHash', () => { - test('Getting hash for non existent file should throw error', async () => { - const promise = fileSystem.getFileHash('some unknown file'); + // 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 - await expect(promise).to.eventually.be.rejected; + 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 fileSystem.getFileHash(filename); + + expect(hash).to.not.equal(''); }); - test('Getting hash for a file should return non-empty string', async () => { - const hash = await fileSystem.getFileHash(__filename); + test('the returned hash is stable', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); + + const hash1 = await fileSystem.getFileHash(filename); + const hash2 = await fileSystem.getFileHash(filename); + await sleep(2_000); // just in case + const hash3 = await fileSystem.getFileHash(filename); - expect(hash).to.be.length.greaterThan(0); // weak + expect(hash1).to.equal(hash2); + expect(hash1).to.equal(hash3); + expect(hash2).to.equal(hash3); }); - test('', async () => { - // XXX + test('the returned hash changes with modification', async () => { + const filename = await fix.createFile('x/y/z/spam.py', 'original text'); + + const hash1 = await fileSystem.getFileHash(filename); + await sleep(2_000); // for filesystems with 1s resolution + await fs.writeFile(filename, 'new text'); + const hash2 = await fileSystem.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 fileSystem.getFileHash(file1); + const hash2 = await fileSystem.getFileHash(file2); + const hash3 = await fileSystem.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 = fileSystem.getFileHash(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; }); }); suite('search', () => { - test('Test searching for files', async () => { - const expected = __filename.replace(/\\/g, '/'); - const searchPattern = `${path.basename(__filename, __filename.substring(__filename.length - 3))}.*`; + 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'); - const files = await fileSystem.search(path.join(__dirname, searchPattern)); + const files = await fileSystem.search(pattern); - expect(files).to.be.array(); - expect(files.length).to.be.at.least(1); - const fileName = files[0].replace(/\\/g, '/'); - expect(fileName).to.equal(expected); + expect(files.sort()).to.deep.equal(expected.sort()); }); - test('', async () => { - // XXX + test('no matches', async () => { + const pattern = await fix.resolve(`x/y/z/spam.*`); + + const files = await fileSystem.search(pattern); + + expect(files).to.deep.equal([]); }); }); suite('createTemporaryFile', () => { - 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'); + async function createTemporaryFile(suffix: string): Promise { + const tempfile = await fileSystem.createTemporaryFile(suffix); + fix.addFSCleanup(tempfile.filePath, tempfile.dispose); + return tempfile; + } + + test('TemporaryFile is created properly', async () => { + const tempfile = await fileSystem.createTemporaryFile('.tmp'); + fix.addFSCleanup(tempfile.filePath, tempfile.dispose); + await assertExists(tempfile.filePath); - expect(tempFile.filePath).to.not.equal(tempFile2.filePath, 'Temp files must be unique, implementation of createTemporaryFile is off.'); + expect(tempfile.filePath.endsWith('.tmp')).to.equal(true, `bad suffix on ${tempfile.filePath}`); }); - test('Ensure chmod works against a temporary file', async () => { - 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. - }, - (failReason) => { - expect(failReason).to.equal('There was no error using chmod', `Failed to perform chmod operation successfully, got error ${failReason}`); - } - ); - }); + test('TemporaryFile is disposed properly', async () => { + const tempfile = await createTemporaryFile('.tmp'); + await assertExists(tempfile.filePath); + + tempfile.dispose(); + + await assertDoesNotExist(tempfile.filePath); + }); + + test('Ensure creating a temporary file results in a unique temp file path', async () => { + const tempFile = await createTemporaryFile('.tmp'); + const tempFile2 = await createTemporaryFile('.tmp'); + + const filename1 = tempFile.filePath; + const filename2 = tempFile2.filePath; + + expect(filename1).to.not.equal(filename2); }); test('Ensure writing to a temp file is supported via file stream', async () => { - 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); - writeStream.write('hello', 'utf8', (err: Error | null | undefined) => { - expect(err).to.equal(undefined, `Failed to write to a temp file, error is ${err}`); - }); - }, - (failReason) => { - expect(failReason).to.equal('No errors occurred', `Failed to create a temporary file with error ${failReason}`); - } - ); + const tempfile = await createTemporaryFile('.tmp'); + const stream = fileSystem.createWriteStream(tempfile.filePath); + fix.addCleanup(() => stream.destroy()); + const data = '...'; + + stream.write(data, 'utf8'); + + const actual = await fs.readFile(tempfile.filePath, 'utf8'); + expect(actual).to.equal(data); + }); + + test('Ensure chmod works against a temporary file', async () => { + // Note that on Windows chmod is a noop. + const tempfile = await createTemporaryFile('.tmp'); + + await expect( + fs.chmod(tempfile.filePath, '7777') + ).to.not.eventually.be.rejected; }); test('', async () => { @@ -266,8 +752,37 @@ suite('FileSystem', () => { }); suite('isDirReadonly', () => { - test('', async () => { - // XXX + 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 fileSystem.isDirReadonly(dirname); + + expect(isReadonly).to.equal(false); + }); + }); + + test('is not readonly', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const isReadonly = await fileSystem.isDirReadonly(dirname); + + expect(isReadonly).to.equal(true); + }); + + test('fails if the directory does not exist', async () => { + const promise = fileSystem.isDirReadonly(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; }); }); diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 551a74ddb9eb..070bc48180b0 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/src/test/common/platform/filesystem.test.ts @@ -15,7 +15,7 @@ import { IFileSystem } from '../../../client/common/platform/types'; import { - assertDoesNotExist, DOES_NOT_EXIST, FSFixture + assertDoesNotExist, DOES_NOT_EXIST, FSFixture, SUPPORTS_SYMLINKS } from './utils'; // Note: all functional tests that do not trigger the VS Code "fs" API @@ -75,7 +75,11 @@ suite('FileSystem', () => { expect(stat).to.deep.equal(expected); }); - test('for symlinks, gets the info for the linked file', async () => { + test('for symlinks, gets the info for the linked file', 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 old = await fsextra.stat(filename); From bf21a4369a65e57f7212a8f00138b53c38258842 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 15:36:29 -0700 Subject: [PATCH 10/31] Add some missing blank lines. --- src/client/common/platform/fileSystem.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 0aac747ec4bc..86feddd711bc 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -223,6 +223,7 @@ export class FileSystem implements IFileSystem { }); }); } + public async search(globPattern: string, cwd?: string): Promise { let found: string[]; if (cwd) { @@ -235,6 +236,7 @@ export class FileSystem implements IFileSystem { } return Array.isArray(found) ? found : []; } + public createTemporaryFile(extension: string): Promise { return new Promise((resolve, reject) => { tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => { From e28dea14ea29c52033f2e1d341c5f23d43a07d9f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 16:44:15 -0700 Subject: [PATCH 11/31] Alias vscode's FileType and FileStat locally. --- src/client/common/platform/fileSystem.ts | 3 +-- src/client/common/platform/types.ts | 8 +++++--- src/test/common/platform/filesystem.test.ts | 6 +----- src/test/mocks/vsc/index.ts | 7 +++++++ src/test/vscode-mock.ts | 1 + 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 86feddd711bc..6f360158edde 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -10,10 +10,9 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import * as tmp from 'tmp'; import { promisify } from 'util'; -import { FileStat } from 'vscode'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; -import { IFileSystem, IPlatformService, TemporaryFile } from './types'; +import { FileStat, FileType, IFileSystem, IPlatformService, TemporaryFile } from './types'; const globAsync = promisify(glob); diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index f7c598604e77..aa5a60a3bfd4 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,9 +32,11 @@ 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 import FileType = vscode.FileType; +export import FileStat = vscode.FileStat; export type WriteStream = fs.WriteStream; export const IFileSystem = Symbol('IFileSystem'); diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 070bc48180b0..86c3168e9709 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/src/test/common/platform/filesystem.test.ts @@ -6,13 +6,12 @@ import { expect } from 'chai'; import * as fsextra from 'fs-extra'; -import * as vscode from 'vscode'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { PlatformService } from '../../../client/common/platform/platformService'; import { - IFileSystem + FileStat, FileType, IFileSystem } from '../../../client/common/platform/types'; import { assertDoesNotExist, DOES_NOT_EXIST, FSFixture, SUPPORTS_SYMLINKS @@ -21,9 +20,6 @@ import { // Note: all functional tests that do not trigger the VS Code "fs" API // are found in filesystem.functional.test.ts. -import FileType = vscode.FileType; -type FileStat = vscode.FileStat; - suite('FileSystem', () => { let filesystem: IFileSystem; let fix: FSFixture; 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 a5f0fef1851ecc73e0a567696ee3471bfa3b6fb4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 22 Oct 2019 15:47:30 -0600 Subject: [PATCH 12/31] 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 f9337be584b9..dc46d70bf1af 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 @@ export 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 b441de6b5523770130dd701efbbc8463f167dd72 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 23 Oct 2019 09:20:31 -0600 Subject: [PATCH 13/31] 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 dc46d70bf1af..140c1b0fe0dd 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 @@ export 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 fb4a2d22aeef26d139a03ff3ed9ca293d3d80c16 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 16:47:00 -0700 Subject: [PATCH 14/31] Fix the failing tests. --- src/client/common/platform/fileSystem.ts | 87 ++++++++++++------- .../platform/filesystem.functional.test.ts | 8 +- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 6f360158edde..e57b88fafe85 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -16,6 +16,27 @@ import { FileStat, FileType, IFileSystem, IPlatformService, TemporaryFile } from const globAsync = promisify(glob); +async function getFileType(filename: string): Promise { + let stat: fs.Stats; + try { + // Note that we use stat() instead of lstat(). This is solely + // to preserve the pre-existing behavior. It may make more + // sense to switch to lstat() in the future. + stat = await fs.stat(filename); + } catch { + return FileType.Unknown; + } + 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( @@ -175,37 +196,45 @@ export class FileSystem implements IFileSystem { return this.objectExists(filePath, stats => stats.isDirectory()); } - public getSubDirectories(rootDir: string): Promise { - return new Promise(resolve => { - fs.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[]; - resolve(subDirs); + public async listDir(dirname: string): Promise<[string, FileType][]> { + let files: string[]; + try { + files = await fs.readdir(dirname); + } catch (err) { + if (!await fs.pathExists(dirname)) { + return []; + } + throw err; // re-throw + } + const promises = files + .map(async basename => { + const filename = path.join(dirname, basename); + const fileType = await getFileType(filename); + return [filename, fileType] as [string, FileType]; }); - }); + return Promise.all(promises); } - - public async getFiles(rootDir: string): Promise { - const files = await fs.readdir(rootDir); - return files.filter(async f => { - const fullPath = path.join(rootDir, f); - if ((await fs.stat(fullPath)).isFile()) { - return true; - } - return false; - }); + public async getSubDirectories(dirname: string): Promise { + let files: [string, FileType][]; + try { + files = await this.listDir(dirname); + } catch { + // We're only preserving pre-existng behavior here... + return []; + } + return files + .filter(([_file, fileType]) => { + return fileType === FileType.Directory; + }) + .map(([filename, _ft]) => filename); + } + public async getFiles(dirname: string): Promise { + const files = await this.listDir(dirname); + return files + .filter(([_file, fileType]) => { + return fileType !== FileType.Directory; + }) + .map(([filename, _ft]) => filename); } public getFileHash(filePath: string): Promise { diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 612bcbda5d9f..43658ff165ce 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -779,10 +779,12 @@ suite('FileSystem', () => { expect(isReadonly).to.equal(true); }); - test('fails if the directory does not exist', async () => { - const promise = fileSystem.isDirReadonly(DOES_NOT_EXIST); + // 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 fileSystem.isDirReadonly(DOES_NOT_EXIST); - await expect(promise).to.eventually.be.rejected; + expect(isReadonly).to.equal(false); }); }); From f32d5f312d36816ea36344da1e5ebbcb5b584a19 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 16:59:45 -0700 Subject: [PATCH 15/31] Drop appendFileSync(). --- src/client/common/platform/fileSystem.ts | 7 ------ src/client/common/platform/types.ts | 4 ---- .../platform/filesystem.functional.test.ts | 24 ------------------- 3 files changed, 35 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index e57b88fafe85..157264bb1671 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -119,13 +119,6 @@ export class FileSystem implements IFileSystem { public appendFile(filename: string, data: {}): Promise { return fs.appendFile(filename, data); } - 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 copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index aa5a60a3bfd4..e61fcf3cc70c 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -61,10 +61,6 @@ export interface IFileSystem { move(src: string, tgt: string): Promise; // sync readFileSync(filename: string): string; - 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; createReadStream(path: string): fs.ReadStream; createWriteStream(path: string): fs.WriteStream; diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 43658ff165ce..7122194dc100 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -382,30 +382,6 @@ suite('FileSystem', () => { }); }); - suite('appendFileSync', () => { - const fileToAppendTo = path.join(__dirname, 'created_for_testing_dummy.txt'); - setup(cleanTestFiles); - teardown(cleanTestFiles); - function cleanTestFiles() { - if (fs.existsSync(fileToAppendTo)) { - fs.unlinkSync(fileToAppendTo); - } - } - - test('appending to a file works', async () => { - const dataToAppend = `Some Data\n${new Date().toString()}\nAnd another line`; - - fileSystem.appendFileSync(fileToAppendTo, dataToAppend); - - const actual = await fs.readFile(fileToAppendTo, 'utf8'); - expect(actual).to.be.equal(dataToAppend); - }); - - test('', async () => { - // XXX - }); - }); - suite('createReadStream', () => { test('', async () => { // XXX From 5a41b81a71028a7ac0e8789f7c421854252c79f3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 17:07:50 -0700 Subject: [PATCH 16/31] Add tests for appendFile(). --- .../platform/filesystem.functional.test.ts | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 7122194dc100..d4a963e9a96b 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -33,6 +33,7 @@ suite('FileSystem', () => { }); teardown(async () => { await fix.cleanUp(); + await fix.ensureDeleted(DOES_NOT_EXIST); }); suite('path-related', () => { @@ -216,8 +217,42 @@ suite('FileSystem', () => { }); suite('appendFile', () => { - test('', async () => { - // XXX + test('existing file', async () => { + const orig = 'spamspamspam\n\n'; + const dataToAppend = `Some Data\n${new Date().toString()}\nAnd another line`; + const filename = await fix.createFile('spam.txt', orig); + const expected = `${orig}${dataToAppend}`; + + await fileSystem.appendFile(filename, dataToAppend); + + const actual = await fs.readFile(filename, 'utf8'); + expect(actual).to.be.equal(expected); + }); + + test('existing empty file', async () => { + const filename = await fix.createFile('spam.txt'); + const dataToAppend = `Some Data\n${new Date().toString()}\nAnd another line`; + const expected = dataToAppend; + + await fileSystem.appendFile(filename, dataToAppend); + + const actual = await fs.readFile(filename, 'utf8'); + expect(actual).to.be.equal(expected); + }); + + test('creates the file if it does not already exist', async () => { + await fileSystem.appendFile(DOES_NOT_EXIST, 'spam'); + + const actual = await fs.readFile(DOES_NOT_EXIST, 'utf8'); + expect(actual).to.be.equal('spam'); + }); + + test('fails if not a file', async () => { + const dirname = await fix.createDirectory('spam'); + + const promise = fileSystem.appendFile(dirname, 'spam'); + + await expect(promise).to.eventually.be.rejected; }); }); From 2b5e1b6826b68e8c42f581362d793d9b971b9474 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 17:10:21 -0700 Subject: [PATCH 17/31] Drop unneeded TODO comments. --- .../common/platform/filesystem.functional.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index d4a963e9a96b..9790d53d1f19 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -550,10 +550,6 @@ suite('FileSystem', () => { expect(entries).to.deep.equal([]); }); - - test('', async () => { - // XXX - }); }); suite('getFiles', () => { @@ -605,10 +601,6 @@ suite('FileSystem', () => { expect(entries).to.deep.equal([]); }); - - test('', async () => { - // XXX - }); }); suite('getFileHash', () => { @@ -756,10 +748,6 @@ suite('FileSystem', () => { fs.chmod(tempfile.filePath, '7777') ).to.not.eventually.be.rejected; }); - - test('', async () => { - // XXX - }); }); suite('isDirReadonly', () => { From 4f9060e20b935ef432765642f2f8ccb39769616b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 11 Dec 2019 17:27:19 -0700 Subject: [PATCH 18/31] Add tests for *Exists(). --- src/client/common/platform/fileSystem.ts | 2 + .../platform/filesystem.functional.test.ts | 105 ++++++++++++++++-- 2 files changed, 98 insertions(+), 9 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 157264bb1671..d8f59f402891 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -171,6 +171,8 @@ export class FileSystem implements IFileSystem { public objectExists(filePath: string, statCheck: (s: fs.Stats) => boolean): Promise { return new Promise(resolve => { + // Note that we are using stat() rather than lstat(). This + // means that any symlinks are getting resolved. fs.stat(filePath, (error, stats) => { if (error) { return resolve(false); diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 9790d53d1f19..4fcfa6b166b6 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -487,8 +487,34 @@ suite('FileSystem', () => { expect(exists).to.equal(true); }); - test('', async () => { - // XXX + test('want file, not file', async () => { + const filename = await fix.createDirectory('x/y/z/spam.py'); + + const exists = await fileSystem.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 fileSystem.fileExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + }); + + test('unknown', async () => { + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = await fileSystem.fileExists(sockFile); + + expect(exists).to.equal(false); }); }); @@ -501,8 +527,30 @@ suite('FileSystem', () => { expect(exists).to.equal(true); }); - test('', async () => { - // XXX + test('want directory, not directory', async () => { + const dirname = await fix.createFile('x/y/z/spam'); + + const exists = await fileSystem.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 fileSystem.directoryExists(symlink); + + // This is because we currently use stat() and not lstat(). + expect(exists).to.equal(true); + }); + + test('unknown', async () => { + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = await fileSystem.directoryExists(sockFile); + + expect(exists).to.equal(false); }); }); @@ -791,14 +839,53 @@ suite('FileSystem', () => { // sync methods suite('fileExistsSync', () => { - test('Check existence of files synchronously', async () => { - const exists = fileSystem.fileExistsSync(__filename); + test('want file, got file', async () => { + const filename = await fix.createFile('x/y/z/spam.py'); - expect(exists).to.be.equal(true, 'file not found'); + const exists = fileSystem.fileExistsSync(filename); + + expect(exists).to.equal(true); }); - test('', async () => { - // XXX + test('want file, not file', async () => { + const filename = await fix.createDirectory('x/y/z/spam.py'); + + const exists = fileSystem.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 = fileSystem.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 () => { + const sockFile = await fix.createSocket('x/y/z/ipc.sock'); + + const exists = fileSystem.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); }); }); }); From 7a11004c5fa15c139914bb3f69b698d6a422c6a1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 12 Dec 2019 08:48:02 -0700 Subject: [PATCH 19/31] Add tests for createReadStream(). --- .../platform/filesystem.functional.test.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 4fcfa6b166b6..795b457c7027 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -410,7 +410,7 @@ suite('FileSystem', () => { expect(text).to.equal(expected); }); - test('throws an exception if file does not exist', async () => { + test('throws an exception if file does not exist', () => { expect( () => fileSystem.readFileSync(DOES_NOT_EXIST) ).to.throw(Error); @@ -418,9 +418,21 @@ suite('FileSystem', () => { }); suite('createReadStream', () => { - test('', async () => { - // XXX + test('returns the correct ReadStream', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + const expected = fs.createReadStream(filename); + expected.destroy(); + + const stream = fileSystem.createReadStream(filename); + stream.destroy(); + + expect(stream.path).to.deep.equal(expected.path); }); + + // Missing tests: + // * creation fails if the file does not exist + // * .read() works as expected + // * .pipe() works as expected }); suite('createWriteStream', () => { From 1043045c94b247166a5755aaebcd7ecee1c3605d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 12 Dec 2019 09:33:55 -0700 Subject: [PATCH 20/31] Add tests for listdir(). --- src/client/common/platform/fileSystem.ts | 78 +++++++------ src/client/common/platform/types.ts | 2 +- src/client/interpreter/locators/helpers.ts | 17 +-- src/test/.vscode/settings.json | 3 +- .../platform/filesystem.functional.test.ts | 106 +++++++++++++++++- src/test/common/platform/utils.ts | 4 +- 6 files changed, 162 insertions(+), 48 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index d8f59f402891..c448ec2397a0 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -19,10 +19,10 @@ const globAsync = promisify(glob); async function getFileType(filename: string): Promise { let stat: fs.Stats; try { - // Note that we use stat() instead of lstat(). This is solely - // to preserve the pre-existing behavior. It may make more - // sense to switch to lstat() in the future. - stat = await fs.stat(filename); + // Note that we used to use stat() here instead of lstat(). + // This shouldn't matter because the only consumers were + // internal methods that have been updated appropriately. + stat = await fs.lstat(filename); } catch { return FileType.Unknown; } @@ -31,7 +31,20 @@ async function getFileType(filename: string): Promise { } else if (stat.isDirectory()) { return FileType.Directory; } else if (stat.isSymbolicLink()) { - return FileType.SymbolicLink; + // We emulate the behavior of vscode.workspace.fs.listdir(). + // See: https://code.visualstudio.com/api/references/vscode-api#FileType + try { + stat = await fs.stat(filename); + } catch { + return FileType.SymbolicLink; + } + if (stat.isFile()) { + return FileType.SymbolicLink | FileType.File; + } else if (stat.isDirectory()) { + return FileType.SymbolicLink | FileType.Directory; + } else { + return FileType.SymbolicLink; + } } else { return FileType.Unknown; } @@ -104,16 +117,15 @@ export class FileSystem implements IFileSystem { return deferred.promise; } - public async listdir(root: string): Promise { - return new Promise(resolve => { - // Now look for Interpreters in this directory - fs.readdir(root, (err, names) => { - if (err) { - return resolve([]); - } - resolve(names.map(name => path.join(root, name))); + public async listdir(dirname: string): Promise<[string, FileType][]> { + const files = await fs.readdir(dirname); + const promises = files + .map(async basename => { + const filename = path.join(dirname, basename); + const fileType = await getFileType(filename); + return [filename, fileType] as [string, FileType]; }); - }); + return Promise.all(promises); } public appendFile(filename: string, data: {}): Promise { @@ -191,43 +203,37 @@ export class FileSystem implements IFileSystem { return this.objectExists(filePath, stats => stats.isDirectory()); } - public async listDir(dirname: string): Promise<[string, FileType][]> { - let files: string[]; - try { - files = await fs.readdir(dirname); - } catch (err) { - if (!await fs.pathExists(dirname)) { - return []; - } - throw err; // re-throw - } - const promises = files - .map(async basename => { - const filename = path.join(dirname, basename); - const fileType = await getFileType(filename); - return [filename, fileType] as [string, FileType]; - }); - return Promise.all(promises); - } public async getSubDirectories(dirname: string): Promise { let files: [string, FileType][]; try { - files = await this.listDir(dirname); + files = await this.listdir(dirname); } catch { // We're only preserving pre-existng behavior here... return []; } return files .filter(([_file, fileType]) => { - return fileType === FileType.Directory; + // We preserve the pre-existing behavior of following + // symlinks. + return (fileType & FileType.Directory) > 0; }) .map(([filename, _ft]) => filename); } public async getFiles(dirname: string): Promise { - const files = await this.listDir(dirname); + let files: [string, FileType][]; + try { + files = await this.listdir(dirname); + } catch (err) { + if (!await fs.pathExists(dirname)) { + return []; + } + throw err; // re-throw + } return files .filter(([_file, fileType]) => { - return fileType !== FileType.Directory; + // We preserve the pre-existing behavior of following + // symlinks. + return (fileType & FileType.Directory) === 0; }) .map(([filename, _ft]) => filename); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index e61fcf3cc70c..95deb9065c01 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -50,7 +50,7 @@ export interface IFileSystem { stat(filePath: string): Promise; createDirectory(path: string): Promise; deleteDirectory(path: string): Promise; - listdir(dirname: string): Promise; + listdir(dirname: string): Promise<[string, FileType][]>; readFile(filePath: string): Promise; readData(filePath: string): Promise; writeFile(filePath: string, data: {}, options?: string | fsextra.WriteFileOptions): Promise; diff --git a/src/client/interpreter/locators/helpers.ts b/src/client/interpreter/locators/helpers.ts index 096437ac624f..e7264d765ba1 100644 --- a/src/client/interpreter/locators/helpers.ts +++ b/src/client/interpreter/locators/helpers.ts @@ -8,15 +8,18 @@ import { IPipEnvServiceHelper } from './types'; const CheckPythonInterpreterRegEx = IS_WINDOWS ? /^python(\d+(.\d+)?)?\.exe$/ : /^python(\d+(.\d+)?)?$/; -export function lookForInterpretersInDirectory(pathToCheck: string, fs: IFileSystem): Promise { +export async function lookForInterpretersInDirectory(pathToCheck: string, fs: IFileSystem): Promise { // Technically, we should be able to use fs.getFiles(). However, // that breaks some tests. So we stick with the broader behavior. - return fs.listdir(pathToCheck) - .then(subDirs => subDirs.filter(fileName => CheckPythonInterpreterRegEx.test(path.basename(fileName)))) - .catch(err => { - traceError('Python Extension (lookForInterpretersInDirectory.fs.listdir):', err); - return [] as string[]; - }); + try { + const subDirs = await fs.listdir(pathToCheck); + return subDirs + .map(([filename, _ft]) => filename) + .filter(fileName => CheckPythonInterpreterRegEx.test(path.basename(fileName))); + } catch (err) { + traceError('Python Extension (lookForInterpretersInDirectory.fs.listdir):', err); + return [] as string[]; + }; } @injectable() diff --git a/src/test/.vscode/settings.json b/src/test/.vscode/settings.json index f78fa68d8fd7..f18683828501 100644 --- a/src/test/.vscode/settings.json +++ b/src/test/.vscode/settings.json @@ -18,7 +18,8 @@ "python.linting.mypyEnabled": false, "python.linting.banditEnabled": false, "python.formatting.provider": "yapf", - "python.linting.pylintUseMinimalCheckers": false + "python.linting.pylintUseMinimalCheckers": false, + "python.pythonPath": "/usr/bin/python3" // Do not set this to true/false even when LS is the default, else // it will result in LS being downloaded on CI and slow down tests significantly. // We have other tests on CI for testing downloading of CI with this setting enabled. diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 795b457c7027..14909cd4f962 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -8,7 +8,7 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { PlatformService } from '../../../client/common/platform/platformService'; -import { IFileSystem, TemporaryFile } from '../../../client/common/platform/types'; +import { FileType, TemporaryFile } from '../../../client/common/platform/types'; import { sleep } from '../../../client/common/utils/async'; import { assertDoesNotExist, assertExists, DOES_NOT_EXIST, FSFixture, @@ -21,7 +21,7 @@ use(require('chai-as-promised')); use(assertArrays); suite('FileSystem', () => { - let fileSystem: IFileSystem; + let fileSystem: FileSystem; let fix: FSFixture; setup(async () => { fileSystem = new FileSystem( @@ -141,6 +141,108 @@ suite('FileSystem', () => { }); }); + suite('listdir', () => { + if (SUPPORTS_SYMLINKS) { + test('mixed', async () => { + // Create the target directory and its contents. + const dirname = await fix.createDirectory('x/y/z'); + const file1 = await fix.createFile('x/y/z/__init__.py', ''); + const script = await fix.createFile('x/y/z/__main__.py', '