From e7d4ee0a6581b5737a44ceccae61dd2d43bae0a8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 15:52:22 -0700 Subject: [PATCH 01/15] Factor out IFileSystemPathUtils and friends. --- src/client/common/platform/constants.ts | 4 +- src/client/common/platform/fs-paths.ts | 96 +++++++++++++++++++++++++ src/client/common/platform/pathUtils.ts | 61 +++++++++++----- src/client/common/platform/types.ts | 33 +++++++++ src/client/common/types.ts | 3 +- 5 files changed, 176 insertions(+), 21 deletions(-) create mode 100644 src/client/common/platform/fs-paths.ts diff --git a/src/client/common/platform/constants.ts b/src/client/common/platform/constants.ts index a1c33dd0a605..d361eefcbd3c 100644 --- a/src/client/common/platform/constants.ts +++ b/src/client/common/platform/constants.ts @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// TO DO: Deprecate in favor of IPlatformService +// tslint:disable-next-line:no-suspicious-comment +// TODO (GH-8542): Drop all these in favor of IPlatformService + export const WINDOWS_PATH_VARIABLE_NAME = 'Path'; export const NON_WINDOWS_PATH_VARIABLE_NAME = 'PATH'; export const IS_WINDOWS = /^win/.test(process.platform); diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts new file mode 100644 index 000000000000..cc9d9536244b --- /dev/null +++ b/src/client/common/platform/fs-paths.ts @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as nodepath from 'path'; +import { getOSType, OSType } from '../utils/platform'; +import { + IExecutables, + IFileSystemPaths +} from './types'; +// tslint:disable-next-line:no-var-requires no-require-imports +const untildify = require('untildify'); + +// The parts of node's 'path' module used by FileSystemPaths. +interface INodePath { + sep: string; + basename(filename: string, ext?: string): string; +} + +// The file path operations used by the extension. +export class FileSystemPaths { + constructor( + private raw: INodePath + ) { } + // Create a new object using common-case default values. + // We do not use an alternate constructor because defaults in the + // constructor runs counter to our typical approach. + public static withDefaults(): FileSystemPaths { + return new FileSystemPaths( + nodepath + ); + } + + public get sep(): string { + return this.raw.sep; + } + + public basename(filename: string, suffix?: string): string { + return this.raw.basename(filename, suffix); + } +} + +export class Executables { + constructor( + public readonly delimiter: string, + private readonly osType: OSType + ) { } + // Create a new object using common-case default values. + // We do not use an alternate constructor because defaults in the + // constructor runs counter to our typical approach. + public static withDefaults(): Executables { + return new Executables( + nodepath.delimiter, + getOSType() + ); + } + + public get envVar(): string { + return this.osType === OSType.Windows + ? 'Path' + : 'PATH'; + } +} + +interface IRawPaths { + relative(relpath: string, rootpath: string): string; +} + +export class FileSystemPathUtils { + constructor( + public readonly home: string, + public readonly paths: IFileSystemPaths, + public readonly executables: IExecutables, + private readonly raw: IRawPaths + ) { } + // Create a new object using common-case default values. + // We do not use an alternate constructor because defaults in the + // constructor runs counter to our typical approach. + public static withDefaults(): FileSystemPathUtils { + return new FileSystemPathUtils( + untildify('~'), + FileSystemPaths.withDefaults(), + Executables.withDefaults(), + nodepath + ); + } + + public getDisplayName(pathValue: string, cwd?: string): string { + if (cwd && pathValue.startsWith(cwd)) { + return `.${this.paths.sep}${this.raw.relative(cwd, pathValue)}`; + } else if (pathValue.startsWith(this.home)) { + return `~${this.paths.sep}${this.raw.relative(this.home, pathValue)}`; + } else { + return pathValue; + } + } +} diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 87b2b5c22354..b92f72c577ff 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -1,36 +1,59 @@ +// tslint:disable-next-line:no-suspicious-comment +// TODO(GH-8542): Drop this file. + import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IPathUtils, IsWindows } from '../types'; -import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants'; +import { OSType } from '../utils/platform'; +import { + Executables, + FileSystemPaths, + FileSystemPathUtils +} from './fs-paths'; // tslint:disable-next-line:no-var-requires no-require-imports const untildify = require('untildify'); @injectable() export class PathUtils implements IPathUtils { - public readonly home = ''; - constructor(@inject(IsWindows) private isWindows: boolean) { - this.home = untildify('~'); + private readonly utils: FileSystemPathUtils; + constructor( + @inject(IsWindows) isWindows: boolean + ) { + this.utils = new FileSystemPathUtils( + untildify('~'), + FileSystemPaths.withDefaults(), + new Executables( + path.delimiter, + isWindows ? OSType.Windows : OSType.Unknown + ), + path + ); + } + + public get home(): string { + return this.utils.home; } + public get delimiter(): string { - return path.delimiter; + return this.utils.executables.delimiter; } + public get separator(): string { - return path.sep; - } - // TO DO: Deprecate in favor of IPlatformService - public getPathVariableName() { - return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME; + return this.utils.paths.sep; } - public basename(pathValue: string, ext?: string): string { - return path.basename(pathValue, ext); + + // tslint:disable-next-line:no-suspicious-comment + // TODO: Deprecate in favor of IPlatformService? + public getPathVariableName(): 'Path' | 'PATH' { + // tslint:disable-next-line:no-any + return this.utils.executables.envVar as any; } + public getDisplayName(pathValue: string, cwd?: string): string { - if (cwd && pathValue.startsWith(cwd)) { - return `.${path.sep}${path.relative(cwd, pathValue)}`; - } else if (pathValue.startsWith(this.home)) { - return `~${path.sep}${path.relative(this.home, pathValue)}`; - } else { - return pathValue; - } + return this.utils.getDisplayName(pathValue, cwd); + } + + public basename(pathValue: string, ext?: string): string { + return this.utils.paths.basename(pathValue, ext); } } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 479f3deed079..aecdecec3a44 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -7,6 +7,9 @@ import { SemVer } from 'semver'; import * as vscode from 'vscode'; import { Architecture, OSType } from '../utils/platform'; +//=========================== +// registry + export enum RegistryHive { HKCU, HKLM @@ -18,6 +21,11 @@ export interface IRegistry { getValue(key: string, hive: RegistryHive, arch?: Architecture, name?: string): Promise; } +//=========================== +// platform + +export const IsWindows = Symbol('IS_WINDOWS'); + export const IPlatformService = Symbol('IPlatformService'); export interface IPlatformService { readonly osType: OSType; @@ -33,9 +41,34 @@ export interface IPlatformService { getVersion(): Promise; } +//=========================== +// temp FS + export type TemporaryFile = { filePath: string } & vscode.Disposable; export type TemporaryDirectory = { path: string } & vscode.Disposable; +//=========================== +// FS paths + +export interface IFileSystemPaths { + readonly sep: string; + basename(filename: string, suffix?: string): string; +} + +export interface IExecutables { + delimiter: string; + envVar: string; +} + +export interface IFileSystemPathUtils { + readonly paths: IFileSystemPaths; + readonly executables: IExecutables; + getDisplayName(pathValue: string, cwd?: string): string; +} + +//=========================== +// filesystem operations + export import FileType = vscode.FileType; export import FileStat = vscode.FileStat; export type WriteStream = fs.WriteStream; diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 95880a6d369c..2dc8bd13c58e 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -133,8 +133,9 @@ export interface IInstaller { translateProductToModuleName(product: Product, purpose: ModuleNamePurpose): string; } +// tslint:disable-next-line:no-suspicious-comment +// TODO(GH-8542): Drop IPathUtils in favor of IFileSystemPathUtils. export const IPathUtils = Symbol('IPathUtils'); - export interface IPathUtils { readonly delimiter: string; readonly home: string; From 1e05c64bf74276580b53d4a3977b8d0d3d82d605 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 16:55:05 -0700 Subject: [PATCH 02/15] Add missing functional tests. --- .../platform/fs-paths.functional.test.ts | 128 ++++++++++++++++++ .../platform/pathUtils.functional.test.ts | 120 ++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 src/test/common/platform/fs-paths.functional.test.ts create mode 100644 src/test/common/platform/pathUtils.functional.test.ts diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts new file mode 100644 index 000000000000..58be81995f36 --- /dev/null +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -0,0 +1,128 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:max-func-body-length + +import { expect } from 'chai'; +import * as os from 'os'; +import * as path from 'path'; +import { + Executables, + FileSystemPaths, + FileSystemPathUtils +} from '../../../client/common/platform/fs-paths'; + +const IS_WINDOWS = /^win/.test(process.platform); + +suite('FileSystem - Paths', () => { + let paths: FileSystemPaths; + setup(() => { + paths = FileSystemPaths.withDefaults(); + }); + + suite('separator', () => { + test('matches node', () => { + expect(paths.sep).to.be.equal(path.sep); + }); + }); + + suite('basename', () => { + test('with dirname', () => { + const filename = path.join('spam', 'eggs', 'spam.py'); + const expected = 'spam.py'; + + const basename = paths.basename(filename); + + expect(basename).to.equal(expected); + }); + + test('without dirname', () => { + const filename = 'spam.py'; + const expected = filename; + + const basename = paths.basename(filename); + + expect(basename).to.equal(expected); + }); + }); +}); + +suite('FileSystem - Executables', () => { + let execs: Executables; + setup(() => { + execs = Executables.withDefaults(); + }); + + suite('delimiter', () => { + test('matches node', () => { + expect(execs.delimiter).to.be.equal(path.delimiter); + }); + }); + + suite('getPathVariableName', () => { + const expected = IS_WINDOWS ? 'Path' : 'PATH'; + + test('matches platform', () => { + expect(execs.envVar).to.equal(expected); + }); + }); +}); + +suite('FileSystem - Path Utils', () => { + let utils: FileSystemPathUtils; + setup(() => { + utils = FileSystemPathUtils.withDefaults(); + }); + + suite('getDisplayName', () => { + const relname = path.join('spam', 'eggs', 'spam.py'); + const cwd = path.resolve(path.sep, 'x', 'y', 'z'); + + test('filename matches CWD', () => { + const filename = path.join(cwd, relname); + const expected = `.${path.sep}${relname}`; + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename does not match CWD', () => { + const filename = path.resolve(cwd, '..', relname); + const expected = filename; + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename matches home dir, not cwd', () => { + const filename = path.join(os.homedir(), relname); + const expected = path.join('~', relname); + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename matches home dir', () => { + const filename = path.join(os.homedir(), relname); + const expected = path.join('~', relname); + + const display = utils.getDisplayName(filename); + + expect(display).to.equal(expected); + }); + + test('filename does not match home dir', () => { + const filename = relname; + const expected = filename; + + const display = utils.getDisplayName(filename); + + expect(display).to.equal(expected); + }); + }); +}); diff --git a/src/test/common/platform/pathUtils.functional.test.ts b/src/test/common/platform/pathUtils.functional.test.ts new file mode 100644 index 000000000000..ad4f5d554ed9 --- /dev/null +++ b/src/test/common/platform/pathUtils.functional.test.ts @@ -0,0 +1,120 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:max-func-body-length + +import { expect } from 'chai'; +import * as os from 'os'; +import * as path from 'path'; +import { PathUtils } from '../../../client/common/platform/pathUtils'; + +const IS_WINDOWS = /^win/.test(process.platform); + +suite('FileSystem - PathUtils', () => { + let utils: PathUtils; + setup(() => { + utils = new PathUtils(IS_WINDOWS); + }); + + suite('home', () => { + const expected = os.homedir(); + + test('matches node', () => { + expect(utils.home).to.equal(expected); + }); + }); + + suite('delimiter', () => { + test('matches node', () => { + expect(utils.delimiter).to.be.equal(path.delimiter); + }); + }); + + suite('separator', () => { + test('matches node', () => { + expect(utils.separator).to.be.equal(path.sep); + }); + }); + + suite('getPathVariableName', () => { + const expected = IS_WINDOWS ? 'Path' : 'PATH'; + + test('matches platform', () => { + const envVar = utils.getPathVariableName(); + + expect(envVar).to.equal(expected); + }); + }); + + suite('getDisplayName', () => { + const relname = path.join('spam', 'eggs', 'spam.py'); + const cwd = path.resolve(path.sep, 'x', 'y', 'z'); + + test('filename matches CWD', () => { + const filename = path.join(cwd, relname); + const expected = `.${path.sep}${relname}`; + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename does not match CWD', () => { + const filename = path.resolve(cwd, '..', relname); + const expected = filename; + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename matches home dir, not cwd', () => { + const filename = path.join(os.homedir(), relname); + const expected = path.join('~', relname); + + const display = utils.getDisplayName(filename, cwd); + + expect(display).to.equal(expected); + }); + + test('filename matches home dir', () => { + const filename = path.join(os.homedir(), relname); + const expected = path.join('~', relname); + + const display = utils.getDisplayName(filename); + + expect(display).to.equal(expected); + }); + + test('filename does not match home dir', () => { + const filename = relname; + const expected = filename; + + const display = utils.getDisplayName(filename); + + expect(display).to.equal(expected); + }); + }); + + suite('basename', () => { + test('with dirname', () => { + const filename = path.join('spam', 'eggs', 'spam.py'); + const expected = 'spam.py'; + + const basename = utils.basename(filename); + + expect(basename).to.equal(expected); + }); + + test('without dirname', () => { + const filename = 'spam.py'; + const expected = filename; + + const basename = utils.basename(filename); + + expect(basename).to.equal(expected); + }); + }); +}); From 3ab7ec78c787b74b7937514b448273ac4dc080e6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:11:22 -0700 Subject: [PATCH 03/15] Add FileSystemPaths.normalize(). --- src/client/common/platform/fs-paths.ts | 5 +++ src/client/common/platform/types.ts | 1 + .../platform/fs-paths.functional.test.ts | 38 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index cc9d9536244b..89dae262cfb1 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -14,6 +14,7 @@ const untildify = require('untildify'); interface INodePath { sep: string; basename(filename: string, ext?: string): string; + normalize(filename: string): string; } // The file path operations used by the extension. @@ -37,6 +38,10 @@ export class FileSystemPaths { public basename(filename: string, suffix?: string): string { return this.raw.basename(filename, suffix); } + + public normalize(filename: string): string { + return this.raw.normalize(filename); + } } export class Executables { diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index aecdecec3a44..bd17c95858de 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -53,6 +53,7 @@ export type TemporaryDirectory = { path: string } & vscode.Disposable; export interface IFileSystemPaths { readonly sep: string; basename(filename: string, suffix?: string): string; + normalize(filename: string): string; } export interface IExecutables { diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 58be81995f36..69fb5b592dd9 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -47,6 +47,44 @@ suite('FileSystem - Paths', () => { expect(basename).to.equal(expected); }); }); + + suite('normalize', () => { + test('noop', () => { + const filename = path.join('spam', 'eggs', 'spam.py'); + const expected = filename; + + const norm = paths.normalize(filename); + + expect(norm).to.equal(expected); + }); + + test('pathological', () => { + const filename = path.join(path.sep, 'spam', '..', 'eggs', '.', 'spam.py'); + const expected = path.join(path.sep, 'eggs', 'spam.py'); + + const norm = paths.normalize(filename); + + expect(norm).to.equal(expected); + }); + + test('relative to CWD', () => { + const filename = path.join('..', 'spam', 'eggs', 'spam.py'); + const expected = filename; + + const norm = paths.normalize(filename); + + expect(norm).to.equal(expected); + }); + + test('parent of root fails', () => { + const filename = path.join(path.sep, '..'); + const expected = filename; + + const norm = paths.normalize(filename); + + expect(norm).to.equal(expected); + }); + }); }); suite('FileSystem - Executables', () => { From b83f20fb135cee6fc99637368948aa03767098da Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:18:35 -0700 Subject: [PATCH 04/15] Add FileSystemPaths.join(). --- src/client/common/platform/fs-paths.ts | 5 +++++ src/client/common/platform/types.ts | 1 + .../common/platform/fs-paths.functional.test.ts | 14 ++++++++++++++ 3 files changed, 20 insertions(+) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 89dae262cfb1..04a9b521296a 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -13,6 +13,7 @@ const untildify = require('untildify'); // The parts of node's 'path' module used by FileSystemPaths. interface INodePath { sep: string; + join(...filenames: string[]): string; basename(filename: string, ext?: string): string; normalize(filename: string): string; } @@ -35,6 +36,10 @@ export class FileSystemPaths { return this.raw.sep; } + public join(...filenames: string[]): string { + return this.raw.join(...filenames); + } + public basename(filename: string, suffix?: string): string { return this.raw.basename(filename, suffix); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index bd17c95858de..9956a52ee7a3 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -52,6 +52,7 @@ export type TemporaryDirectory = { path: string } & vscode.Disposable; export interface IFileSystemPaths { readonly sep: string; + join(...filenames: string[]): string; basename(filename: string, suffix?: string): string; normalize(filename: string): string; } diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 69fb5b592dd9..4b4530805244 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -85,6 +85,20 @@ suite('FileSystem - Paths', () => { expect(norm).to.equal(expected); }); }); + + suite('join', () => { + test('parts get joined by path.sep', () => { + const expected = path.join('x', 'y', 'z', 'spam.py'); + + const result = paths.join( + 'x', + path.sep === '\\' ? 'y\\z' : 'y/z', + 'spam.py' + ); + + expect(result).to.equal(expected); + }); + }); }); suite('FileSystem - Executables', () => { From 7c4c44a536e7729ea76836771a34e6fa70072711 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:21:04 -0700 Subject: [PATCH 05/15] Use FileSystemPaths in the FileSystem class. --- src/client/common/platform/fileSystem.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index bf99bc0204e4..c63ed1a2ced8 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -7,11 +7,11 @@ import * as fileSystem from 'fs'; import * as fs from 'fs-extra'; import * as glob from 'glob'; import { inject, injectable } from 'inversify'; -import * as path from 'path'; import * as tmp from 'tmp'; import { promisify } from 'util'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; +import { FileSystemPaths } from './fs-paths'; import { FileStat, FileType, IFileSystem, IPlatformService, TemporaryFile } from './types'; const globAsync = promisify(glob); @@ -78,18 +78,23 @@ export function convertStat(old: fs.Stats, filetype: FileType): FileStat { @injectable() export class FileSystem implements IFileSystem { - constructor(@inject(IPlatformService) private platformService: IPlatformService) {} + private readonly paths: FileSystemPaths; + constructor( + @inject(IPlatformService) private platformService: IPlatformService + ) { + this.paths = FileSystemPaths.withDefaults(); + } //================================= // path-related public get directorySeparatorChar(): string { - return path.sep; + return this.paths.sep; } public arePathsSame(path1: string, path2: string): boolean { - path1 = path.normalize(path1); - path2 = path.normalize(path2); + path1 = this.paths.normalize(path1); + path2 = this.paths.normalize(path2); if (this.platformService.isWindows) { return path1.toUpperCase() === path2.toUpperCase(); } else { @@ -154,7 +159,7 @@ export class FileSystem implements IFileSystem { 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 filename = this.paths.join(dirname, basename); const fileType = await getFileType(filename); return [filename, fileType] as [string, FileType]; }); @@ -312,7 +317,7 @@ export class FileSystem implements IFileSystem { } public async isDirReadonly(dirname: string): Promise { - const filePath = `${dirname}${path.sep}___vscpTest___`; + const filePath = `${dirname}${this.paths.sep}___vscpTest___`; return new Promise(resolve => { fs.open(filePath, fs.constants.O_CREAT | fs.constants.O_RDWR, (error, fd) => { if (!error) { From a390c4532fd1ba8e7e0600e11df7803bb017a01d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:28:36 -0700 Subject: [PATCH 06/15] Add FileSystemPaths.normCase(). --- src/client/common/platform/fs-paths.ts | 11 +++++- .../platform/fs-paths.functional.test.ts | 38 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 04a9b521296a..059ab4a83507 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -21,13 +21,15 @@ interface INodePath { // The file path operations used by the extension. export class FileSystemPaths { constructor( - private raw: INodePath + private readonly isCaseInsensitive: boolean, + private readonly raw: INodePath ) { } // Create a new object using common-case default values. // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. public static withDefaults(): FileSystemPaths { return new FileSystemPaths( + (getOSType() === OSType.Windows), nodepath ); } @@ -47,6 +49,13 @@ export class FileSystemPaths { public normalize(filename: string): string { return this.raw.normalize(filename); } + + public normCase(filename: string): string { + filename = this.raw.normalize(filename); + return this.isCaseInsensitive + ? filename.toUpperCase() + : filename; + } } export class Executables { diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 4b4530805244..10bc60d8399f 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -99,6 +99,44 @@ suite('FileSystem - Paths', () => { expect(result).to.equal(expected); }); }); + + suite('normCase', () => { + test('forward-slash', () => { + const filename = 'X/Y/Z/SPAM.PY'; + const expected = IS_WINDOWS ? 'X\\Y\\Z\\SPAM.PY' : filename; + + const result = paths.normCase(filename); + + expect(result).to.equal(expected); + }); + + test('backslash is not changed', () => { + const filename = 'X\\Y\\Z\\SPAM.PY'; + const expected = filename; + + const result = paths.normCase(filename); + + expect(result).to.equal(expected); + }); + + test('lower-case', () => { + const filename = 'x\\y\\z\\spam.py'; + const expected = IS_WINDOWS ? 'X\\Y\\Z\\SPAM.PY' : filename; + + const result = paths.normCase(filename); + + expect(result).to.equal(expected); + }); + + test('upper-case stays upper-case', () => { + const filename = 'X\\Y\\Z\\SPAM.PY'; + const expected = 'X\\Y\\Z\\SPAM.PY'; + + const result = paths.normCase(filename); + + expect(result).to.equal(expected); + }); + }); }); suite('FileSystem - Executables', () => { From 806fc1756149005cd702a1910f05d96e7796aa75 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:32:17 -0700 Subject: [PATCH 07/15] Add FileSystemPaths.dirname(). --- src/client/common/platform/fs-paths.ts | 5 +++++ src/client/common/platform/types.ts | 1 + .../platform/fs-paths.functional.test.ts | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 059ab4a83507..61e87f999743 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -14,6 +14,7 @@ const untildify = require('untildify'); interface INodePath { sep: string; join(...filenames: string[]): string; + dirname(filename: string): string; basename(filename: string, ext?: string): string; normalize(filename: string): string; } @@ -42,6 +43,10 @@ export class FileSystemPaths { return this.raw.join(...filenames); } + public dirname(filename: string): string { + return this.raw.dirname(filename); + } + public basename(filename: string, suffix?: string): string { return this.raw.basename(filename, suffix); } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 9956a52ee7a3..1af78a568131 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -53,6 +53,7 @@ export type TemporaryDirectory = { path: string } & vscode.Disposable; export interface IFileSystemPaths { readonly sep: string; join(...filenames: string[]): string; + dirname(filename: string): string; basename(filename: string, suffix?: string): string; normalize(filename: string): string; } diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 10bc60d8399f..bd3080d60c3a 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -28,6 +28,26 @@ suite('FileSystem - Paths', () => { }); }); + suite('dirname', () => { + test('with dirname', () => { + const filename = path.join('spam', 'eggs', 'spam.py'); + const expected = path.join('spam', 'eggs'); + + const basename = paths.dirname(filename); + + expect(basename).to.equal(expected); + }); + + test('without dirname', () => { + const filename = 'spam.py'; + const expected = '.'; + + const basename = paths.dirname(filename); + + expect(basename).to.equal(expected); + }); + }); + suite('basename', () => { test('with dirname', () => { const filename = path.join('spam', 'eggs', 'spam.py'); From 1634f92399bcbec31b54ffdfe43eaa98e3be7f0d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 17:47:01 -0700 Subject: [PATCH 08/15] Move arePathsSame() over to FileSystemPathUtils. --- src/client/common/platform/fileSystem.ts | 26 ++-- src/client/common/platform/fs-paths.ts | 24 +++- src/client/common/platform/types.ts | 2 + .../platform/filesystem.functional.test.ts | 39 +----- .../common/platform/filesystem.unit.test.ts | 91 -------------- .../platform/fs-paths.functional.test.ts | 41 +++++- .../common/platform/fs-paths.unit.test.ts | 117 ++++++++++++++++++ 7 files changed, 194 insertions(+), 146 deletions(-) delete mode 100644 src/test/common/platform/filesystem.unit.test.ts create mode 100644 src/test/common/platform/fs-paths.unit.test.ts diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index c63ed1a2ced8..603291ae8922 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -11,8 +11,12 @@ import * as tmp from 'tmp'; import { promisify } from 'util'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; -import { FileSystemPaths } from './fs-paths'; -import { FileStat, FileType, IFileSystem, IPlatformService, TemporaryFile } from './types'; +import { FileSystemPaths, FileSystemPathUtils } from './fs-paths'; +import { + FileStat, FileType, + IFileSystem, IFileSystemPaths, IPlatformService, + TemporaryFile +} from './types'; const globAsync = promisify(glob); @@ -78,11 +82,15 @@ export function convertStat(old: fs.Stats, filetype: FileType): FileStat { @injectable() export class FileSystem implements IFileSystem { - private readonly paths: FileSystemPaths; + private readonly paths: IFileSystemPaths; + private readonly pathUtils: FileSystemPathUtils; constructor( - @inject(IPlatformService) private platformService: IPlatformService + @inject(IPlatformService) platformService: IPlatformService ) { - this.paths = FileSystemPaths.withDefaults(); + this.paths = FileSystemPaths.withDefaults( + platformService.isWindows + ); + this.pathUtils = FileSystemPathUtils.withDefaults(this.paths); } //================================= @@ -93,13 +101,7 @@ export class FileSystem implements IFileSystem { } public arePathsSame(path1: string, path2: string): boolean { - path1 = this.paths.normalize(path1); - path2 = this.paths.normalize(path2); - if (this.platformService.isWindows) { - return path1.toUpperCase() === path2.toUpperCase(); - } else { - return path1 === path2; - } + return this.pathUtils.arePathsSame(path1, path2); } public getRealPath(filePath: string): Promise { diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 61e87f999743..6980bde9e610 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -28,9 +28,14 @@ export class FileSystemPaths { // Create a new object using common-case default values. // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. - public static withDefaults(): FileSystemPaths { + public static withDefaults( + isCaseInsensitive?: boolean + ): FileSystemPaths { + if (isCaseInsensitive === undefined) { + isCaseInsensitive = (getOSType() === OSType.Windows); + } return new FileSystemPaths( - (getOSType() === OSType.Windows), + isCaseInsensitive, nodepath ); } @@ -99,15 +104,26 @@ export class FileSystemPathUtils { // Create a new object using common-case default values. // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. - public static withDefaults(): FileSystemPathUtils { + public static withDefaults( + paths?: IFileSystemPaths + ): FileSystemPathUtils { + if (paths === undefined) { + paths = FileSystemPaths.withDefaults(); + } return new FileSystemPathUtils( untildify('~'), - FileSystemPaths.withDefaults(), + paths, Executables.withDefaults(), nodepath ); } + public arePathsSame(path1: string, path2: string): boolean { + path1 = this.paths.normCase(path1); + path2 = this.paths.normCase(path2); + return path1 === path2; + } + public getDisplayName(pathValue: string, cwd?: string): string { if (cwd && pathValue.startsWith(cwd)) { return `.${this.paths.sep}${this.raw.relative(cwd, pathValue)}`; diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 1af78a568131..c0541fb8298c 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -56,6 +56,7 @@ export interface IFileSystemPaths { dirname(filename: string): string; basename(filename: string, suffix?: string): string; normalize(filename: string): string; + normCase(filename: string): string; } export interface IExecutables { @@ -66,6 +67,7 @@ export interface IExecutables { export interface IFileSystemPathUtils { readonly paths: IFileSystemPaths; readonly executables: IExecutables; + arePathsSame(path1: string, path2: string): boolean; getDisplayName(pathValue: string, cwd?: string): string; } diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index a2179c38430a..c351bc0be17b 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -40,44 +40,7 @@ suite('FileSystem', () => { }); }); - suite('arePathsSame', () => { - 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); - }); - }); + // arePathsSame() is tested in the FileSystemPathUtils tests. suite('getRealPath', () => { const prevCwd = process.cwd(); diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts deleted file mode 100644 index a7b1f8de15aa..000000000000 --- a/src/test/common/platform/filesystem.unit.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -// 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'; -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(undefined, TypeMoq.MockBehavior.Strict); - fileSystem = new FileSystem(platformService.object); - }); - function verifyAll() { - platformService.verifyAll(); - } - - suite('path-related', () => { - const caseInsensitive = [OSType.Windows]; - - suite('arePathsSame', () => { - 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)); - } - - 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'; - - 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(); - }); - } - - // Missing tests: - // * exercize normalization - }); - }); - }); -}); diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index bd3080d60c3a..7dcf5eb0fd78 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -3,7 +3,7 @@ 'use strict'; -// tslint:disable:max-func-body-length +// tslint:disable:max-func-body-length chai-vague-errors import { expect } from 'chai'; import * as os from 'os'; @@ -186,6 +186,45 @@ suite('FileSystem - Path Utils', () => { utils = FileSystemPathUtils.withDefaults(); }); + suite('arePathsSame', () => { + test('identical', () => { + const filename = 'x/y/z/spam.py'; + + const result = utils.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 = utils.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 = IS_WINDOWS; + + const result = utils.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 = IS_WINDOWS; + + const result = utils.arePathsSame(file1, file2); + + expect(result).to.equal(expected); + }); + }); + suite('getDisplayName', () => { const relname = path.join('spam', 'eggs', 'spam.py'); const cwd = path.resolve(path.sep, 'x', 'y', 'z'); diff --git a/src/test/common/platform/fs-paths.unit.test.ts b/src/test/common/platform/fs-paths.unit.test.ts new file mode 100644 index 000000000000..e64ee47ea6d5 --- /dev/null +++ b/src/test/common/platform/fs-paths.unit.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-func-body-length + +import { expect } from 'chai'; +import * as path from 'path'; +import * as TypeMoq from 'typemoq'; +import { FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; +import { getNamesAndValues } from '../../../client/common/utils/enum'; +import { OSType } from '../../../client/common/utils/platform'; + +interface IUtilsDeps { + // executables + delimiter: string; + envVar: string; + // paths + readonly sep: string; + join(...filenames: string[]): string; + dirname(filename: string): string; + basename(filename: string, suffix?: string): string; + normalize(filename: string): string; + normCase(filename: string): string; + // node "path" + relative(relpath: string, rootpath: string): string; +} + +suite('FileSystem - Path Utils', () => { + let deps: TypeMoq.IMock; + let utils: FileSystemPathUtils; + setup(() => { + deps = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + utils = new FileSystemPathUtils( + 'my-home', + deps.object, + deps.object, + deps.object + ); + }); + function verifyAll() { + deps.verifyAll(); + } + + suite('path-related', () => { + const caseInsensitive = [OSType.Windows]; + + suite('arePathsSame', () => { + getNamesAndValues(OSType).forEach(item => { + const osType = item.value; + + function setNormCase(filename: string, numCalls = 1): string { + //if (numCalls !== 1) { + // console.log(filename); + // return filename; + //} + const norm = (osType === OSType.Windows) + ? path.normalize(filename).toUpperCase() + : filename; + deps.setup(d => d.normCase(filename)) + .returns(() => norm) + .verifiable(TypeMoq.Times.exactly(numCalls)); + return filename; + } + + [ + 'c:\\users\\Peter Smith\\my documents\\test.txt', + + 'c:\\USERS\\Peter Smith\\my documents\\test.TXT' + ].forEach(path1 => { + test(`True if paths are identical (type: ${item.name}) - ${path1}`, () => { + path1 = setNormCase(path1, 2); + + const areSame = utils.arePathsSame(path1, path1); + + expect(areSame).to.be.equal(true, 'file paths do not match'); + verifyAll(); + }); + }); + + test(`False if paths are completely different (type: ${item.name})`, () => { + const path1 = setNormCase('c:\\users\\Peter Smith\\my documents\\test.txt'); + const path2 = setNormCase('c:\\users\\Peter Smith\\my documents\\test.exe'); + + const areSame = utils.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})`, () => { + const path1 = setNormCase('c:\\users\\Peter Smith\\my documents\\test.txt'); + const path2 = setNormCase('c:\\USERS\\Peter Smith\\my documents\\test.TXT'); + + const areSame = utils.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})`, () => { + const path1 = setNormCase('c:\\users\\Peter Smith\\my documents\\test.txt'); + const path2 = setNormCase('c:\\USERS\\Peter Smith\\my documents\\test.TXT'); + + const areSame = utils.arePathsSame(path1, path2); + + expect(areSame).to.be.equal(false, 'file paths do not match'); + verifyAll(); + }); + } + + // Missing tests: + // * exercize normalization + }); + }); + }); +}); From ff554d11af95fc02f31e5db2151972f7aa1e3501 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 19 Dec 2019 18:30:47 -0700 Subject: [PATCH 09/15] Move getRealPath() to FileSystemPathUtils. --- src/client/common/platform/fileSystem.ts | 6 +- src/client/common/platform/fs-paths.ts | 10 ++ .../platform/filesystem.functional.test.ts | 133 +---------------- .../platform/fs-paths.functional.test.ts | 140 +++++++++++++++++- .../platform/pathUtils.functional.test.ts | 3 +- 5 files changed, 151 insertions(+), 141 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 603291ae8922..d050ccb3ea34 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -105,11 +105,7 @@ export class FileSystem implements IFileSystem { } public getRealPath(filePath: string): Promise { - return new Promise(resolve => { - fs.realpath(filePath, (err, realPath) => { - resolve(err ? filePath : realPath); - }); - }); + return this.pathUtils.getRealPath(filePath); } //================================= diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index 6980bde9e610..def6b6a7f00e 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as fs from 'fs-extra'; import * as nodepath from 'path'; import { getOSType, OSType } from '../utils/platform'; import { @@ -124,6 +125,15 @@ export class FileSystemPathUtils { return path1 === path2; } + public async getRealPath(filename: string): Promise { + try { + return await fs.realpath(filename); + } catch { + // We ignore the error. + return filename; + } + } + public getDisplayName(pathValue: string, cwd?: string): string { if (cwd && pathValue.startsWith(cwd)) { return `.${this.paths.sep}${this.raw.relative(cwd, pathValue)}`; diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index c351bc0be17b..2503924ffe4c 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -42,138 +42,7 @@ suite('FileSystem', () => { // arePathsSame() is tested in the FileSystemPathUtils tests. - suite('getRealPath', () => { - const prevCwd = process.cwd(); - let cwd: string; - setup(async function() { - if (OSX) { - // tslint:disable-next-line:no-suspicious-comment - // TODO(GH-8995) These tests are failing on Mac, so - // we are temporarily disabling it. - // tslint:disable-next-line:no-invalid-this - return this.skip(); - } - cwd = await fix.createDirectory('x/y/z'); - process.chdir(cwd); - }); - teardown(() => { - process.chdir(prevCwd); - }); - - test('basename-only', async () => { - const expected = await fix.createFile('x/y/z/spam.py'); - - const resolved = await fileSystem.getRealPath('spam.py'); - - expect(resolved).to.equal(expected); - }); - - test('absolute', async () => { - const filename = await fix.createFile('spam.py'); - const expected = filename; - - const resolved = await fileSystem.getRealPath(filename); - - expect(resolved).to.equal(expected); - }); - - test('relative', async () => { - const expected = await fix.createFile('x/y/z/w/spam.py'); - const relpath = fixPath('./w/spam.py'); - - const resolved = await fileSystem.getRealPath(relpath); - - expect(resolved).to.equal(expected); - }); - - test('parent', async () => { - const expected = await fix.resolve('x/y'); - - const resolved = await fileSystem.getRealPath('..'); - - expect(resolved).to.equal(expected); - }); - - test('cousin', async () => { - const expected = await fix.createFile('x/w/spam.py'); - const relpath = fixPath('../../w/spam.py'); - - const resolved = await fileSystem.getRealPath(relpath); - - expect(resolved).to.equal(expected); - }); - - test('does not exist', async () => { - const resolved = await fileSystem.getRealPath('spam.py'); - - // The original path was returned unchanged. - expect(resolved).to.equal('spam.py'); // instead of /x/y/z/spam.py - }); - - test('directory does not exist', async () => { - const relpath = fixPath('../../w/spam.py'); - - const resolved = await fileSystem.getRealPath(relpath); - - // The original path was returned unchanged. - expect(resolved).to.equal(relpath); // instead of /x/w/spam.py - }); - - test('symlink', async function() { - if (!SUPPORTS_SYMLINKS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - const expected = await fix.createFile('spam.py'); - await fix.createSymlink('x/y/z/eggs.py', expected); - - const resolved = await fileSystem.getRealPath('eggs.py'); - - expect(resolved).to.equal(expected); - }); - - test('symlink chain', async function() { - if (!SUPPORTS_SYMLINKS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - const expected = await fix.createFile('w/spam.py'); - const symlink1 = await fix.createSymlink('x/y/spam.py', expected); - await fix.createSymlink('x/y/z/eggs.py', symlink1); - - const resolved = await fileSystem.getRealPath('eggs.py'); - - expect(resolved).to.equal(expected); - }); - - test('symlink (target does not exist)', async function() { - if (!SUPPORTS_SYMLINKS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - const filename = await fix.resolve('spam.py'); - await fix.createSymlink('x/y/z/eggs.py', filename); - - const resolved = await fileSystem.getRealPath('eggs.py'); - - // The original path was returned unchanged. - expect(resolved).to.equal('eggs.py'); // instead of /spam.py - }); - - test('mixed', async function() { - if (!SUPPORTS_SYMLINKS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - const expected = await fix.createFile('x/y/w/eggs.py'); - await fix.createSymlink('x/w/spam.py', expected); - const relpath = fixPath('../../w/spam.py'); - - const resolved = await fileSystem.getRealPath(relpath); - - expect(resolved).to.equal(expected); - }); - }); + // getRealPath() is tested in the FileSystemPathUtils tests. }); suite('raw', () => { diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 7dcf5eb0fd78..65cdeb40de9a 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -13,8 +13,9 @@ import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; - -const IS_WINDOWS = /^win/.test(process.platform); +import { + fixPath, FSFixture, OSX, SUPPORTS_SYMLINKS, WINDOWS as IS_WINDOWS +} from './utils'; suite('FileSystem - Paths', () => { let paths: FileSystemPaths; @@ -182,8 +183,10 @@ suite('FileSystem - Executables', () => { suite('FileSystem - Path Utils', () => { let utils: FileSystemPathUtils; + let fix: FSFixture; setup(() => { utils = FileSystemPathUtils.withDefaults(); + fix = new FSFixture(); }); suite('arePathsSame', () => { @@ -225,6 +228,139 @@ suite('FileSystem - Path Utils', () => { }); }); + suite('getRealPath', () => { + const prevCwd = process.cwd(); + let cwd: string; + setup(async function() { + if (OSX) { + // tslint:disable-next-line:no-suspicious-comment + // TODO(GH-8995) These tests are failing on Mac, so + // we are temporarily disabling it. + // tslint:disable-next-line:no-invalid-this + return this.skip(); + } + cwd = await fix.createDirectory('x/y/z'); + process.chdir(cwd); + }); + teardown(() => { + process.chdir(prevCwd); + }); + + test('basename-only', async () => { + const expected = await fix.createFile('x/y/z/spam.py'); + + const resolved = await utils.getRealPath('spam.py'); + + expect(resolved).to.equal(expected); + }); + + test('absolute', async () => { + const filename = await fix.createFile('spam.py'); + const expected = filename; + + const resolved = await utils.getRealPath(filename); + + expect(resolved).to.equal(expected); + }); + + test('relative', async () => { + const expected = await fix.createFile('x/y/z/w/spam.py'); + const relpath = fixPath('./w/spam.py'); + + const resolved = await utils.getRealPath(relpath); + + expect(resolved).to.equal(expected); + }); + + test('parent', async () => { + const expected = await fix.resolve('x/y'); + + const resolved = await utils.getRealPath('..'); + + expect(resolved).to.equal(expected); + }); + + test('cousin', async () => { + const expected = await fix.createFile('x/w/spam.py'); + const relpath = fixPath('../../w/spam.py'); + + const resolved = await utils.getRealPath(relpath); + + expect(resolved).to.equal(expected); + }); + + test('does not exist', async () => { + const resolved = await utils.getRealPath('spam.py'); + + // The original path was returned unchanged. + expect(resolved).to.equal('spam.py'); // instead of /x/y/z/spam.py + }); + + test('directory does not exist', async () => { + const relpath = fixPath('../../w/spam.py'); + + const resolved = await utils.getRealPath(relpath); + + // The original path was returned unchanged. + expect(resolved).to.equal(relpath); // instead of /x/w/spam.py + }); + + test('symlink', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const expected = await fix.createFile('spam.py'); + await fix.createSymlink('x/y/z/eggs.py', expected); + + const resolved = await utils.getRealPath('eggs.py'); + + expect(resolved).to.equal(expected); + }); + + test('symlink chain', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const expected = await fix.createFile('w/spam.py'); + const symlink1 = await fix.createSymlink('x/y/spam.py', expected); + await fix.createSymlink('x/y/z/eggs.py', symlink1); + + const resolved = await utils.getRealPath('eggs.py'); + + expect(resolved).to.equal(expected); + }); + + test('symlink (target does not exist)', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const filename = await fix.resolve('spam.py'); + await fix.createSymlink('x/y/z/eggs.py', filename); + + const resolved = await utils.getRealPath('eggs.py'); + + // The original path was returned unchanged. + expect(resolved).to.equal('eggs.py'); // instead of /spam.py + }); + + test('mixed', async function() { + if (!SUPPORTS_SYMLINKS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + const expected = await fix.createFile('x/y/w/eggs.py'); + await fix.createSymlink('x/w/spam.py', expected); + const relpath = fixPath('../../w/spam.py'); + + const resolved = await utils.getRealPath(relpath); + + expect(resolved).to.equal(expected); + }); + }); + suite('getDisplayName', () => { const relname = path.join('spam', 'eggs', 'spam.py'); const cwd = path.resolve(path.sep, 'x', 'y', 'z'); diff --git a/src/test/common/platform/pathUtils.functional.test.ts b/src/test/common/platform/pathUtils.functional.test.ts index ad4f5d554ed9..76b70d9ff4c7 100644 --- a/src/test/common/platform/pathUtils.functional.test.ts +++ b/src/test/common/platform/pathUtils.functional.test.ts @@ -9,8 +9,7 @@ import { expect } from 'chai'; import * as os from 'os'; import * as path from 'path'; import { PathUtils } from '../../../client/common/platform/pathUtils'; - -const IS_WINDOWS = /^win/.test(process.platform); +import { WINDOWS as IS_WINDOWS } from './utils'; suite('FileSystem - PathUtils', () => { let utils: PathUtils; From e5fa475cd3a74977b2b47890023184aaf56f6a74 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 31 Dec 2019 13:37:21 -0700 Subject: [PATCH 10/15] Clean up pathutils tests. --- src/client/common/platform/pathUtils.ts | 2 + .../platform/pathUtils.functional.test.ts | 83 +++++-------------- src/test/common/platform/pathUtils.test.ts | 18 ---- 3 files changed, 22 insertions(+), 81 deletions(-) delete mode 100644 src/test/common/platform/pathUtils.test.ts diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index b92f72c577ff..8f7fcedd5784 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -19,6 +19,8 @@ export class PathUtils implements IPathUtils { constructor( @inject(IsWindows) isWindows: boolean ) { + // We cannot just use FileSystemPathUtils.withDefaults() because + // of the isWindows arg. this.utils = new FileSystemPathUtils( untildify('~'), FileSystemPaths.withDefaults(), diff --git a/src/test/common/platform/pathUtils.functional.test.ts b/src/test/common/platform/pathUtils.functional.test.ts index 76b70d9ff4c7..7a7cc9b88e92 100644 --- a/src/test/common/platform/pathUtils.functional.test.ts +++ b/src/test/common/platform/pathUtils.functional.test.ts @@ -6,41 +6,46 @@ // tslint:disable:max-func-body-length import { expect } from 'chai'; -import * as os from 'os'; -import * as path from 'path'; +import { FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; import { PathUtils } from '../../../client/common/platform/pathUtils'; import { WINDOWS as IS_WINDOWS } from './utils'; suite('FileSystem - PathUtils', () => { let utils: PathUtils; + let wrapped: FileSystemPathUtils; setup(() => { utils = new PathUtils(IS_WINDOWS); + wrapped = FileSystemPathUtils.withDefaults(); }); suite('home', () => { - const expected = os.homedir(); + test('matches wrapped object', () => { + const expected = wrapped.home; - test('matches node', () => { expect(utils.home).to.equal(expected); }); }); suite('delimiter', () => { - test('matches node', () => { - expect(utils.delimiter).to.be.equal(path.delimiter); + test('matches wrapped object', () => { + const expected = wrapped.executables.delimiter; + + expect(utils.delimiter).to.be.equal(expected); }); }); suite('separator', () => { - test('matches node', () => { - expect(utils.separator).to.be.equal(path.sep); + test('matches wrapped object', () => { + const expected = wrapped.paths.sep; + + expect(utils.separator).to.be.equal(expected); }); }); suite('getPathVariableName', () => { - const expected = IS_WINDOWS ? 'Path' : 'PATH'; + test('matches wrapped object', () => { + const expected = wrapped.executables.envVar; - test('matches platform', () => { const envVar = utils.getPathVariableName(); expect(envVar).to.equal(expected); @@ -48,48 +53,9 @@ suite('FileSystem - PathUtils', () => { }); suite('getDisplayName', () => { - const relname = path.join('spam', 'eggs', 'spam.py'); - const cwd = path.resolve(path.sep, 'x', 'y', 'z'); - - test('filename matches CWD', () => { - const filename = path.join(cwd, relname); - const expected = `.${path.sep}${relname}`; - - const display = utils.getDisplayName(filename, cwd); - - expect(display).to.equal(expected); - }); - - test('filename does not match CWD', () => { - const filename = path.resolve(cwd, '..', relname); - const expected = filename; - - const display = utils.getDisplayName(filename, cwd); - - expect(display).to.equal(expected); - }); - - test('filename matches home dir, not cwd', () => { - const filename = path.join(os.homedir(), relname); - const expected = path.join('~', relname); - - const display = utils.getDisplayName(filename, cwd); - - expect(display).to.equal(expected); - }); - - test('filename matches home dir', () => { - const filename = path.join(os.homedir(), relname); - const expected = path.join('~', relname); - - const display = utils.getDisplayName(filename); - - expect(display).to.equal(expected); - }); - - test('filename does not match home dir', () => { - const filename = relname; - const expected = filename; + test('matches wrapped object', () => { + const filename = 'spam.py'; + const expected = wrapped.getDisplayName(filename); const display = utils.getDisplayName(filename); @@ -98,18 +64,9 @@ suite('FileSystem - PathUtils', () => { }); suite('basename', () => { - test('with dirname', () => { - const filename = path.join('spam', 'eggs', 'spam.py'); - const expected = 'spam.py'; - - const basename = utils.basename(filename); - - expect(basename).to.equal(expected); - }); - - test('without dirname', () => { + test('matches wrapped object', () => { const filename = 'spam.py'; - const expected = filename; + const expected = wrapped.paths.basename(filename); const basename = utils.basename(filename); diff --git a/src/test/common/platform/pathUtils.test.ts b/src/test/common/platform/pathUtils.test.ts deleted file mode 100644 index f8f9d2d32597..000000000000 --- a/src/test/common/platform/pathUtils.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; -import { expect } from 'chai'; -import * as path from 'path'; -import { PathUtils } from '../../../client/common/platform/pathUtils'; -import { getOSType, OSType } from '../../common'; - -suite('PathUtils', () => { - let utils: PathUtils; - suiteSetup(() => { - utils = new PathUtils(getOSType() === OSType.Windows); - }); - test('Path Separator', () => { - expect(utils.separator).to.be.equal(path.sep); - }); -}); From 47b1ec0ada1db3314f980ddee66316d750d6e291 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 31 Dec 2019 14:22:09 -0700 Subject: [PATCH 11/15] Drop some dead code. --- src/test/common/platform/fs-paths.unit.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/common/platform/fs-paths.unit.test.ts b/src/test/common/platform/fs-paths.unit.test.ts index e64ee47ea6d5..ff092e2ae883 100644 --- a/src/test/common/platform/fs-paths.unit.test.ts +++ b/src/test/common/platform/fs-paths.unit.test.ts @@ -49,10 +49,6 @@ suite('FileSystem - Path Utils', () => { const osType = item.value; function setNormCase(filename: string, numCalls = 1): string { - //if (numCalls !== 1) { - // console.log(filename); - // return filename; - //} const norm = (osType === OSType.Windows) ? path.normalize(filename).toUpperCase() : filename; From eefc37111eae72eb2f989f32cac891b4823a602b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 31 Dec 2019 15:07:21 -0700 Subject: [PATCH 12/15] Ensure full coverage on FileSystem. --- .../platform/filesystem.functional.test.ts | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 2503924ffe4c..9197c7758d57 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -7,10 +7,15 @@ import { expect, use } from 'chai'; import * as fs from 'fs-extra'; import * as path from 'path'; import { convertStat, FileSystem } from '../../../client/common/platform/fileSystem'; +import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; import { PlatformService } from '../../../client/common/platform/platformService'; import { FileType, TemporaryFile } from '../../../client/common/platform/types'; import { sleep } from '../../../client/common/utils/async'; -import { assertDoesNotExist, assertExists, DOES_NOT_EXIST, fixPath, FSFixture, OSX, SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS, WINDOWS } from './utils'; +import { + assertDoesNotExist, assertExists, DOES_NOT_EXIST, + fixPath, FSFixture, + OSX, SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS, WINDOWS +} from './utils'; // tslint:disable:no-require-imports no-var-requires const assertArrays = require('chai-arrays'); @@ -32,17 +37,47 @@ suite('FileSystem', () => { }); suite('path-related', () => { + const paths = FileSystemPaths.withDefaults(); + const pathUtils = FileSystemPathUtils.withDefaults(paths); + suite('directorySeparatorChar', () => { - test('value', () => { + // tested fully in the FileSystemPaths tests. + + test('matches wrapped object', () => { + const expected = paths.sep; + const sep = fileSystem.directorySeparatorChar; - expect(sep).to.equal(path.sep); + expect(sep).to.equal(expected); }); }); - // arePathsSame() is tested in the FileSystemPathUtils tests. + suite('arePathsSame', () => { + // tested fully in the FileSystemPathUtils tests. + + test('matches wrapped object', () => { + const file1 = fixPath('a/b/c/spam.py'); + const file2 = fixPath('a/b/c/Spam.py'); + const expected = pathUtils.arePathsSame(file1, file2); - // getRealPath() is tested in the FileSystemPathUtils tests. + const areSame = fileSystem.arePathsSame(file1, file2); + + expect(areSame).to.equal(expected); + }); + }); + + suite('getRealPath', () => { + // tested fully in the FileSystemPathUtils tests. + + test('matches wrapped object', async () => { + const filename = fixPath('a/b/c/spam.py'); + const expected = await pathUtils.getRealPath(filename); + + const resolved = await fileSystem.getRealPath(filename); + + expect(resolved).to.equal(expected); + }); + }); }); suite('raw', () => { From 3e1d52fa4c57beb876df5d2d4a686e5161b3fdb6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 31 Dec 2019 15:15:43 -0700 Subject: [PATCH 13/15] Add missing doc comments. --- src/client/common/platform/fs-paths.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index def6b6a7f00e..f702bac4cecd 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -69,6 +69,10 @@ export class FileSystemPaths { } } +// Where to fine executables. +// +// In particular this class provides all the tools needed to find +// executables, including through an environment variable. export class Executables { constructor( public readonly delimiter: string, @@ -91,10 +95,12 @@ export class Executables { } } +// The dependencies FileSystemPathUtils has on node's path module. interface IRawPaths { relative(relpath: string, rootpath: string): string; } +// A collection of high-level utilities related to filesystem paths. export class FileSystemPathUtils { constructor( public readonly home: string, @@ -119,12 +125,16 @@ export class FileSystemPathUtils { ); } + // Return true if the two paths are equivalent on the current + // filesystem and false otherwise. On Windows this is significant. + // On non-Windows the filenames must always be exactly the same. public arePathsSame(path1: string, path2: string): boolean { path1 = this.paths.normCase(path1); path2 = this.paths.normCase(path2); return path1 === path2; } + // Return the canonicalized absolute filename. public async getRealPath(filename: string): Promise { try { return await fs.realpath(filename); @@ -134,13 +144,14 @@ export class FileSystemPathUtils { } } - public getDisplayName(pathValue: string, cwd?: string): string { - if (cwd && pathValue.startsWith(cwd)) { - return `.${this.paths.sep}${this.raw.relative(cwd, pathValue)}`; - } else if (pathValue.startsWith(this.home)) { - return `~${this.paths.sep}${this.raw.relative(this.home, pathValue)}`; + // Return the clean (displayable) form of the given filename. + public getDisplayName(filename: string, cwd?: string): string { + if (cwd && filename.startsWith(cwd)) { + return `.${this.paths.sep}${this.raw.relative(cwd, filename)}`; + } else if (filename.startsWith(this.home)) { + return `~${this.paths.sep}${this.raw.relative(this.home, filename)}`; } else { - return pathValue; + return filename; } } } From 231968b959bd8f35946e9d4cd4fb5dc4243fd8e3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 7 Jan 2020 13:41:31 -0700 Subject: [PATCH 14/15] Address/ignore prettier failures. --- src/client/common/platform/fileSystem.ts | 3 + src/client/common/platform/fs-paths.ts | 13 ++- src/client/common/platform/pathUtils.ts | 3 + .../platform/filesystem.functional.test.ts | 94 +++++++++++++++---- src/test/common/platform/filesystem.test.ts | 21 ++++- .../platform/fs-paths.functional.test.ts | 3 + .../common/platform/fs-paths.unit.test.ts | 3 + src/test/common/platform/utils.ts | 15 ++- 8 files changed, 128 insertions(+), 27 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index d050ccb3ea34..462c6f737fa1 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -12,6 +12,7 @@ import { promisify } from 'util'; import { createDeferred } from '../utils/async'; import { noop } from '../utils/misc'; import { FileSystemPaths, FileSystemPathUtils } from './fs-paths'; +// prettier-ignore import { FileStat, FileType, IFileSystem, IFileSystemPaths, IPlatformService, @@ -84,9 +85,11 @@ export function convertStat(old: fs.Stats, filetype: FileType): FileStat { export class FileSystem implements IFileSystem { private readonly paths: IFileSystemPaths; private readonly pathUtils: FileSystemPathUtils; + // prettier-ignore constructor( @inject(IPlatformService) platformService: IPlatformService ) { + // prettier-ignore this.paths = FileSystemPaths.withDefaults( platformService.isWindows ); diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts index f702bac4cecd..c27bd3e8418c 100644 --- a/src/client/common/platform/fs-paths.ts +++ b/src/client/common/platform/fs-paths.ts @@ -4,6 +4,7 @@ import * as fs from 'fs-extra'; import * as nodepath from 'path'; import { getOSType, OSType } from '../utils/platform'; +// prettier-ignore import { IExecutables, IFileSystemPaths @@ -22,6 +23,7 @@ interface INodePath { // The file path operations used by the extension. export class FileSystemPaths { + // prettier-ignore constructor( private readonly isCaseInsensitive: boolean, private readonly raw: INodePath @@ -29,12 +31,14 @@ export class FileSystemPaths { // Create a new object using common-case default values. // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. + // prettier-ignore public static withDefaults( isCaseInsensitive?: boolean ): FileSystemPaths { if (isCaseInsensitive === undefined) { - isCaseInsensitive = (getOSType() === OSType.Windows); + isCaseInsensitive = getOSType() === OSType.Windows; } + // prettier-ignore return new FileSystemPaths( isCaseInsensitive, nodepath @@ -63,6 +67,7 @@ export class FileSystemPaths { public normCase(filename: string): string { filename = this.raw.normalize(filename); + // prettier-ignore return this.isCaseInsensitive ? filename.toUpperCase() : filename; @@ -74,6 +79,7 @@ export class FileSystemPaths { // In particular this class provides all the tools needed to find // executables, including through an environment variable. export class Executables { + // prettier-ignore constructor( public readonly delimiter: string, private readonly osType: OSType @@ -82,6 +88,7 @@ export class Executables { // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. public static withDefaults(): Executables { + // prettier-ignore return new Executables( nodepath.delimiter, getOSType() @@ -89,6 +96,7 @@ export class Executables { } public get envVar(): string { + // prettier-ignore return this.osType === OSType.Windows ? 'Path' : 'PATH'; @@ -102,6 +110,7 @@ interface IRawPaths { // A collection of high-level utilities related to filesystem paths. export class FileSystemPathUtils { + // prettier-ignore constructor( public readonly home: string, public readonly paths: IFileSystemPaths, @@ -111,12 +120,14 @@ export class FileSystemPathUtils { // Create a new object using common-case default values. // We do not use an alternate constructor because defaults in the // constructor runs counter to our typical approach. + // prettier-ignore public static withDefaults( paths?: IFileSystemPaths ): FileSystemPathUtils { if (paths === undefined) { paths = FileSystemPaths.withDefaults(); } + // prettier-ignore return new FileSystemPathUtils( untildify('~'), paths, diff --git a/src/client/common/platform/pathUtils.ts b/src/client/common/platform/pathUtils.ts index 8f7fcedd5784..30c4db0968e8 100644 --- a/src/client/common/platform/pathUtils.ts +++ b/src/client/common/platform/pathUtils.ts @@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; import { IPathUtils, IsWindows } from '../types'; import { OSType } from '../utils/platform'; +// prettier-ignore import { Executables, FileSystemPaths, @@ -16,11 +17,13 @@ const untildify = require('untildify'); @injectable() export class PathUtils implements IPathUtils { private readonly utils: FileSystemPathUtils; + // prettier-ignore constructor( @inject(IsWindows) isWindows: boolean ) { // We cannot just use FileSystemPathUtils.withDefaults() because // of the isWindows arg. + // prettier-ignore this.utils = new FileSystemPathUtils( untildify('~'), FileSystemPaths.withDefaults(), diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 9197c7758d57..632b4286d087 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -11,6 +11,7 @@ import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/pla import { PlatformService } from '../../../client/common/platform/platformService'; import { FileType, TemporaryFile } from '../../../client/common/platform/types'; import { sleep } from '../../../client/common/utils/async'; +// prettier-ignore import { assertDoesNotExist, assertExists, DOES_NOT_EXIST, fixPath, FSFixture, @@ -26,7 +27,10 @@ suite('FileSystem', () => { let fileSystem: FileSystem; let fix: FSFixture; setup(async () => { - fileSystem = new FileSystem(new PlatformService()); + // prettier-ignore + fileSystem = new FileSystem( + new PlatformService() + ); fix = new FSFixture(); await assertDoesNotExist(DOES_NOT_EXIST); @@ -89,7 +93,11 @@ suite('FileSystem', () => { } const filename = await fix.createFile('x/y/z/spam.py', '...'); const symlink = await fix.createSymlink('x/y/z/eggs.py', filename); - const expected = convertStat(await fs.lstat(symlink), FileType.SymbolicLink); + // prettier-ignore + const expected = convertStat( + await fs.lstat(symlink), + FileType.SymbolicLink + ); const stat = await fileSystem.lstat(symlink); @@ -101,7 +109,11 @@ suite('FileSystem', () => { // Ideally we would compare to the result of // fileSystem.stat(). However, we do not have access // to the VS Code API here. - const expected = convertStat(await fs.lstat(filename), FileType.File); + // prettier-ignore + const expected = convertStat( + await fs.lstat(filename), + FileType.File + ); const stat = await fileSystem.lstat(filename); @@ -347,7 +359,9 @@ suite('FileSystem', () => { await fileSystem.writeFile(filename, data); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); @@ -357,7 +371,9 @@ suite('FileSystem', () => { await fileSystem.writeFile(filename, data); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); @@ -367,7 +383,9 @@ suite('FileSystem', () => { await fileSystem.writeFile(filename, data); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); }); @@ -421,9 +439,13 @@ suite('FileSystem', () => { await fileSystem.copyFile(src, dest); - const actual = await fs.readFile(dest).then(buffer => buffer.toString()); + // prettier-ignore + 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()); + // prettier-ignore + const original = await fs.readFile(src) + .then(buffer => buffer.toString()); expect(original).to.equal(data); }); @@ -435,9 +457,13 @@ suite('FileSystem', () => { await fileSystem.copyFile(src, dest); - const actual = await fs.readFile(dest).then(buffer => buffer.toString()); + // prettier-ignore + 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()); + // prettier-ignore + const original = await fs.readFile(src) + .then(buffer => buffer.toString()); expect(original).to.equal(data); }); @@ -708,7 +734,9 @@ suite('FileSystem', () => { }); test('throws an exception if file does not exist', () => { - expect(() => fileSystem.readFileSync(DOES_NOT_EXIST)).to.throw(Error); + expect(() => { + fileSystem.readFileSync(DOES_NOT_EXIST); + }).to.throw(Error); }); }); @@ -751,7 +779,9 @@ suite('FileSystem', () => { stream.write(data); stream.destroy(); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); @@ -763,7 +793,9 @@ suite('FileSystem', () => { stream.write(data); stream.destroy(); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); @@ -782,7 +814,9 @@ suite('FileSystem', () => { stream.write(data); stream.destroy(); - const actual = await fs.readFile(filename).then(buffer => buffer.toString()); + // prettier-ignore + const actual = await fs.readFile(filename) + .then(buffer => buffer.toString()); expect(actual).to.equal(data); }); }); @@ -899,7 +933,12 @@ suite('FileSystem', () => { const results = await fileSystem.getSubDirectories(dirname); - expect(results.sort()).to.deep.equal([symlink, subdir2, subdir1]); + // prettier-ignore + expect(results.sort()).to.deep.equal([ + symlink, + subdir2, + subdir1 + ]); }); } else { test('mixed types', async () => { @@ -915,7 +954,11 @@ suite('FileSystem', () => { const results = await fileSystem.getSubDirectories(dirname); - expect(results.sort()).to.deep.equal([subdir2, subdir1]); + // prettier-ignore + expect(results.sort()).to.deep.equal([ + subdir2, + subdir1 + ]); }); } @@ -952,7 +995,13 @@ suite('FileSystem', () => { const results = await fileSystem.getFiles(dirname); - expect(results.sort()).to.deep.equal([file3, file2, symlink, file1]); + // prettier-ignore + expect(results.sort()).to.deep.equal([ + file3, + file2, + symlink, + file1 + ]); }); } else { test('mixed types', async () => { @@ -968,7 +1017,12 @@ suite('FileSystem', () => { const results = await fileSystem.getFiles(dirname); - expect(results.sort()).to.deep.equal([file3, file2, file1]); + // prettier-ignore + expect(results.sort()).to.deep.equal([ + file3, + file2, + file1 + ]); }); } @@ -1123,7 +1177,9 @@ suite('FileSystem', () => { // 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; + const promise = fs.chmod(tempfile.filePath, '7777'); + + await expect(promise).to.not.eventually.be.rejected; }); }); diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts index 02008a49af20..98ef6a2af080 100644 --- a/src/test/common/platform/filesystem.test.ts +++ b/src/test/common/platform/filesystem.test.ts @@ -6,10 +6,20 @@ import { expect } from 'chai'; import * as fsextra from 'fs-extra'; -import { convertStat, FileSystem } from '../../../client/common/platform/fileSystem'; +// prettier-ignore +import { + convertStat, FileSystem +} from '../../../client/common/platform/fileSystem'; import { PlatformService } from '../../../client/common/platform/platformService'; -import { FileType, IFileSystem } from '../../../client/common/platform/types'; -import { assertDoesNotExist, DOES_NOT_EXIST, FSFixture, SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS } from './utils'; +// prettier-ignore +import { + FileType, IFileSystem +} from '../../../client/common/platform/types'; +// prettier-ignore +import { + assertDoesNotExist, DOES_NOT_EXIST, FSFixture, + SUPPORTS_SOCKETS, SUPPORTS_SYMLINKS +} from './utils'; // Note: all functional tests that do not trigger the VS Code "fs" API // are found in filesystem.functional.test.ts. @@ -18,7 +28,10 @@ suite('FileSystem', () => { let filesystem: IFileSystem; let fix: FSFixture; setup(async () => { - filesystem = new FileSystem(new PlatformService()); + // prettier-ignore + filesystem = new FileSystem( + new PlatformService() + ); fix = new FSFixture(); await assertDoesNotExist(DOES_NOT_EXIST); diff --git a/src/test/common/platform/fs-paths.functional.test.ts b/src/test/common/platform/fs-paths.functional.test.ts index 65cdeb40de9a..b90bf3faadc2 100644 --- a/src/test/common/platform/fs-paths.functional.test.ts +++ b/src/test/common/platform/fs-paths.functional.test.ts @@ -8,11 +8,13 @@ import { expect } from 'chai'; import * as os from 'os'; import * as path from 'path'; +// prettier-ignore import { Executables, FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths'; +// prettier-ignore import { fixPath, FSFixture, OSX, SUPPORTS_SYMLINKS, WINDOWS as IS_WINDOWS } from './utils'; @@ -111,6 +113,7 @@ suite('FileSystem - Paths', () => { test('parts get joined by path.sep', () => { const expected = path.join('x', 'y', 'z', 'spam.py'); + // prettier-ignore const result = paths.join( 'x', path.sep === '\\' ? 'y\\z' : 'y/z', diff --git a/src/test/common/platform/fs-paths.unit.test.ts b/src/test/common/platform/fs-paths.unit.test.ts index ff092e2ae883..a8aed6aa5a7c 100644 --- a/src/test/common/platform/fs-paths.unit.test.ts +++ b/src/test/common/platform/fs-paths.unit.test.ts @@ -30,6 +30,7 @@ suite('FileSystem - Path Utils', () => { let utils: FileSystemPathUtils; setup(() => { deps = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + // prettier-ignore utils = new FileSystemPathUtils( 'my-home', deps.object, @@ -49,6 +50,7 @@ suite('FileSystem - Path Utils', () => { const osType = item.value; function setNormCase(filename: string, numCalls = 1): string { + // prettier-ignore const norm = (osType === OSType.Windows) ? path.normalize(filename).toUpperCase() : filename; @@ -58,6 +60,7 @@ suite('FileSystem - Path Utils', () => { return filename; } + // prettier-ignore [ 'c:\\users\\Peter Smith\\my documents\\test.txt', diff --git a/src/test/common/platform/utils.ts b/src/test/common/platform/utils.ts index b4dfc2ada640..990fbe1e8598 100644 --- a/src/test/common/platform/utils.ts +++ b/src/test/common/platform/utils.ts @@ -35,11 +35,17 @@ export const SUPPORTS_SOCKETS = !WINDOWS; 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; + // prettier-ignore + 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; + // prettier-ignore + await expect( + fsextra.stat(filename) + ).to.not.eventually.be.rejected; } export function fixPath(filename: string): string { @@ -89,7 +95,10 @@ export class FSFixture extends CleanupFixture { relname = path.normalize(relname); const filename = path.join(tempDir, relname); if (mkdirs) { - await fsextra.mkdirp(path.dirname(filename)); + // prettier-ignore + await fsextra.mkdirp( + path.dirname(filename) + ); } return filename; } From f589160442ab7088890847ea3a80b4143713161e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 7 Jan 2020 14:59:41 -0700 Subject: [PATCH 15/15] Skip a test on Windows. --- src/test/common/platform/filesystem.functional.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 632b4286d087..bb8a07a141c7 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -1161,7 +1161,11 @@ suite('FileSystem', () => { expect(filename1).to.not.equal(filename2); }); - test('Ensure writing to a temp file is supported via file stream', async () => { + test('Ensure writing to a temp file is supported via file stream', async function() { + if (WINDOWS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } const tempfile = await createTemporaryFile('.tmp'); const stream = fileSystem.createWriteStream(tempfile.filePath); fix.addCleanup(() => stream.destroy());