Skip to content

Commit 4a5875b

Browse files
ericsnowcurrentlyericsnowcurrently
committed
Drop IPlatformService as a dependency of FileSystem.
1 parent c929e61 commit 4a5875b

File tree

15 files changed

+93
-90
lines changed

15 files changed

+93
-90
lines changed

‎src/client/common/envFileParser.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { IS_WINDOWS } from './platform/constants';
22
import { FileSystem } from './platform/fileSystem';
33
import { PathUtils } from './platform/pathUtils';
4-
import { IFileSystem, IPlatformService } from './platform/types';
4+
import { IFileSystem } from './platform/types';
55
import { EnvironmentVariablesService } from './variables/environment';
66
import {
77
EnvironmentVariables, IEnvironmentVariablesService
@@ -32,10 +32,7 @@ export function parseEnvFile(
3232
service?: IEnvironmentVariablesService,
3333
fs?: IFileSystem
3434
): EnvironmentVariables {
35-
if (!fs) {
36-
// tslint:disable-next-line:no-object-literal-type-assertion
37-
fs = new FileSystem({} as IPlatformService);
38-
}
35+
fs = fs ? fs : new FileSystem();
3936
const buffer = fs.readFileSync(envFile);
4037
const env = parseEnvironmentVariables(buffer)!;
4138
if (!service) {
@@ -65,8 +62,7 @@ export function mergeEnvVariables(
6562
if (!service) {
6663
service = new EnvironmentVariablesService(
6764
new PathUtils(IS_WINDOWS),
68-
// tslint:disable-next-line:no-object-literal-type-assertion
69-
new FileSystem({} as IPlatformService)
65+
new FileSystem()
7066
);
7167
}
7268
service.mergeVariables(sourceEnvVars, targetEnvVars);
@@ -96,8 +92,7 @@ export function mergePythonPath(
9692
if (!service) {
9793
service = new EnvironmentVariablesService(
9894
new PathUtils(IS_WINDOWS),
99-
// tslint:disable-next-line:no-object-literal-type-assertion
100-
new FileSystem({} as IPlatformService)
95+
new FileSystem()
10196
);
10297
}
10398
service.appendPythonPath(env, currentPythonPath);

‎src/client/common/platform/fileSystem.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@ import { createHash } from 'crypto';
66
import * as fs from 'fs';
77
import * as fsextra from 'fs-extra';
88
import * as glob from 'glob';
9-
import { inject, injectable } from 'inversify';
9+
import { injectable } from 'inversify';
1010
import * as path from 'path';
1111
import * as tmp from 'tmp';
1212
import { createDeferred } from '../utils/async';
1313
import { noop } from '../utils/misc';
14+
import { getOSType, OSType } from '../utils/platform';
1415
import {
1516
FileStat, FileType,
16-
IFileSystem, IPlatformService,
17+
IFileSystem,
1718
TemporaryFile, WriteStream
1819
} from './types';
1920

@@ -34,7 +35,7 @@ function getFileType(stat: FileStat): FileType {
3435
@injectable()
3536
export class FileSystem implements IFileSystem {
3637
constructor(
37-
@inject(IPlatformService) private platformService: IPlatformService
38+
private readonly isWindows = (getOSType() === OSType.Windows)
3839
) { }
3940

4041
//****************************
@@ -88,7 +89,7 @@ export class FileSystem implements IFileSystem {
8889
public arePathsSame(path1: string, path2: string): boolean {
8990
path1 = path.normalize(path1);
9091
path2 = path.normalize(path2);
91-
if (this.platformService.isWindows) {
92+
if (this.isWindows) {
9293
return path1.toUpperCase() === path2.toUpperCase();
9394
} else {
9495
return path1 === path2;

‎src/client/common/utils/localize.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as path from 'path';
77
import { EXTENSION_ROOT_DIR } from '../../constants';
88
import { FileSystem } from '../platform/fileSystem';
9-
import { IFileSystem, IPlatformService } from '../platform/types';
9+
import { IFileSystem } from '../platform/types';
1010

1111
// External callers of localize use these tables to retrieve localized values.
1212
export namespace Diagnostics {
@@ -483,10 +483,7 @@ function getString(key: string, defValue?: string) {
483483
}
484484

485485
function load(fs?: IFileSystem) {
486-
if (!fs) {
487-
// tslint:disable-next-line:no-object-literal-type-assertion
488-
fs = new FileSystem({} as IPlatformService);
489-
}
486+
fs = fs ? fs : new FileSystem();
490487
// Figure out our current locale.
491488
loadedLocale = parseLocale();
492489

‎src/client/interpreter/locators/helpers.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as path from 'path';
33
import { traceError } from '../../common/logger';
44
import { IS_WINDOWS } from '../../common/platform/constants';
55
import { FileSystem } from '../../common/platform/fileSystem';
6-
import { IFileSystem, IPlatformService } from '../../common/platform/types';
6+
import { IFileSystem } from '../../common/platform/types';
77
import { IInterpreterLocatorHelper, InterpreterType, PythonInterpreter } from '../contracts';
88
import { IPipEnvServiceHelper } from './types';
99

@@ -13,12 +13,9 @@ export async function lookForInterpretersInDirectory(
1313
pathToCheck: string,
1414
fs?: IFileSystem
1515
): Promise<string[]> {
16-
if (!fs) {
17-
// tslint:disable-next-line:no-object-literal-type-assertion
18-
fs = new FileSystem({} as IPlatformService);
19-
}
16+
fs = fs ? fs : new FileSystem();
2017
const files = await (
21-
fs.getFiles(pathToCheck)
18+
fs!.getFiles(pathToCheck)
2219
.catch(err => {
2320
traceError('Python Extension (lookForInterpretersInDirectory.fsReaddirAsync):', err);
2421
return [] as string[];

‎src/client/workspaceSymbols/parser.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as path from 'path';
22
import * as vscode from 'vscode';
33
import { FileSystem } from '../common/platform/fileSystem';
4-
import { IFileSystem, IPlatformService } from '../common/platform/types';
4+
import { IFileSystem } from '../common/platform/types';
55
import { ITag } from './contracts';
66

77
// tslint:disable:no-require-imports no-var-requires no-suspicious-comment
@@ -111,10 +111,7 @@ export function parseTags(
111111
token: vscode.CancellationToken,
112112
fs?: IFileSystem
113113
): Promise<ITag[]> {
114-
if (!fs) {
115-
// tslint:disable-next-line:no-object-literal-type-assertion
116-
fs = new FileSystem({} as IPlatformService);
117-
}
114+
fs = fs ? fs : new FileSystem();
118115
return fs.fileExists(tagFile).then(exists => {
119116
if (!exists) {
120117
return Promise.resolve([]);

‎src/test/common/crypto.unit.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,12 @@ import { assert, expect } from 'chai';
77
import * as path from 'path';
88
import { CryptoUtils } from '../../client/common/crypto';
99
import { FileSystem } from '../../client/common/platform/fileSystem';
10-
import { PlatformService } from '../../client/common/platform/platformService';
1110
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants';
1211

1312
// tslint:disable-next-line: max-func-body-length
1413
suite('Crypto Utils', async () => {
1514
let crypto: CryptoUtils;
16-
const fs = new FileSystem(new PlatformService());
15+
const fs = new FileSystem();
1716
const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'common', 'randomWords.txt');
1817
setup(() => {
1918
crypto = new CryptoUtils();

‎src/test/common/net/fileDownloader.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ suite('File Downloader', () => {
9595
httpClient = mock(HttpClient);
9696
appShell = mock(ApplicationShell);
9797
when(httpClient.downloadFile(anything())).thenCall(request);
98-
fs = new FileSystem(new PlatformService());
98+
fs = new FileSystem();
9999
});
100100
teardown(() => {
101101
rewiremock.disable();

‎src/test/common/platform/filesystem.unit.test.ts

Lines changed: 68 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,105 +4,127 @@
44
import { expect, use } from 'chai';
55
import * as fs from 'fs-extra';
66
import * as path from 'path';
7-
import * as TypeMoq from 'typemoq';
87
import { FileSystem } from '../../../client/common/platform/fileSystem';
9-
import { IFileSystem, IPlatformService, TemporaryFile } from '../../../client/common/platform/types';
8+
import { TemporaryFile } from '../../../client/common/platform/types';
109
// tslint:disable:no-require-imports no-var-requires
1110
const assertArrays = require('chai-arrays');
1211
use(require('chai-as-promised'));
1312
use(assertArrays);
1413

1514
// tslint:disable-next-line:max-func-body-length
1615
suite('FileSystem', () => {
17-
let platformService: TypeMoq.IMock<IPlatformService>;
18-
let fileSystem: IFileSystem;
1916
const fileToAppendTo = path.join(__dirname, 'created_for_testing_dummy.txt');
2017
setup(() => {
21-
platformService = TypeMoq.Mock.ofType<IPlatformService>();
22-
fileSystem = new FileSystem(platformService.object);
23-
cleanTestFiles();
18+
cleanTestFiles(); // This smells like functional testing...
2419
});
2520
teardown(cleanTestFiles);
2621
function cleanTestFiles() {
2722
if (fs.existsSync(fileToAppendTo)) {
2823
fs.unlinkSync(fileToAppendTo);
2924
}
3025
}
26+
3127
test('ReadFile returns contents of a file', async () => {
3228
const file = __filename;
29+
const filesystem = new FileSystem();
3330
const expectedContents = await fs.readFile(file).then(buffer => buffer.toString());
34-
const content = await fileSystem.readFile(file);
31+
32+
const content = await filesystem.readFile(file);
3533

3634
expect(content).to.be.equal(expectedContents);
3735
});
3836

3937
test('ReadFile throws an exception if file does not exist', async () => {
40-
const readPromise = fs.readFile('xyz', { encoding: 'utf8' });
38+
const filesystem = new FileSystem();
39+
40+
const readPromise = filesystem.readFile('xyz');
41+
4142
await expect(readPromise).to.be.rejectedWith();
4243
});
4344

44-
function caseSensitivityFileCheck(isWindows: boolean, isOsx: boolean, isLinux: boolean) {
45-
platformService.setup(p => p.isWindows).returns(() => isWindows);
46-
platformService.setup(p => p.isMac).returns(() => isOsx);
47-
platformService.setup(p => p.isLinux).returns(() => isLinux);
45+
suite('Case sensitivity', () => {
4846
const path1 = 'c:\\users\\Peter Smith\\my documents\\test.txt';
4947
const path2 = 'c:\\USERS\\Peter Smith\\my documents\\test.TXT';
5048
const path3 = 'c:\\USERS\\Peter Smith\\my documents\\test.exe';
5149

52-
if (isWindows) {
53-
expect(fileSystem.arePathsSame(path1, path2)).to.be.equal(true, 'file paths do not match (windows)');
54-
} else {
55-
expect(fileSystem.arePathsSame(path1, path2)).to.be.equal(false, 'file match (non windows)');
56-
}
50+
test('Case sensitivity is ignored when comparing file names on windows', () => {
51+
const isWindows = true;
52+
const filesystem = new FileSystem(isWindows);
5753

58-
expect(fileSystem.arePathsSame(path1, path1)).to.be.equal(true, '1. file paths do not match');
59-
expect(fileSystem.arePathsSame(path2, path2)).to.be.equal(true, '2. file paths do not match');
60-
expect(fileSystem.arePathsSame(path1, path3)).to.be.equal(false, '2. file paths do not match');
61-
}
54+
const same12 = filesystem.arePathsSame(path1, path2);
55+
const same11 = filesystem.arePathsSame(path1, path1);
56+
const same22 = filesystem.arePathsSame(path2, path2);
57+
const same13 = filesystem.arePathsSame(path1, path3);
6258

63-
test('Case sensitivity is ignored when comparing file names on windows', async () => {
64-
caseSensitivityFileCheck(true, false, false);
65-
});
59+
expect(same12).to.be.equal(true, 'file paths do not match (windows)');
60+
expect(same11).to.be.equal(true, '1. file paths do not match');
61+
expect(same22).to.be.equal(true, '2. file paths do not match');
62+
expect(same13).to.be.equal(false, '2. file paths do not match');
63+
});
6664

67-
test('Case sensitivity is not ignored when comparing file names on osx', async () => {
68-
caseSensitivityFileCheck(false, true, false);
69-
});
65+
test('Case sensitivity is not ignored when comparing file names on non-Windows', () => {
66+
const isWindows = false;
67+
const filesystem = new FileSystem(isWindows);
68+
69+
const same12 = filesystem.arePathsSame(path1, path2);
70+
const same11 = filesystem.arePathsSame(path1, path1);
71+
const same22 = filesystem.arePathsSame(path2, path2);
72+
const same13 = filesystem.arePathsSame(path1, path3);
7073

71-
test('Case sensitivity is not ignored when comparing file names on linux', async () => {
72-
caseSensitivityFileCheck(false, false, true);
74+
expect(same12).to.be.equal(false, 'file match (non windows)');
75+
expect(same11).to.be.equal(true, '1. file paths do not match');
76+
expect(same22).to.be.equal(true, '2. file paths do not match');
77+
expect(same13).to.be.equal(false, '2. file paths do not match');
78+
});
7379
});
80+
7481
test('Check existence of files synchronously', async () => {
75-
expect(fileSystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found');
82+
const filesystem = new FileSystem();
83+
84+
expect(filesystem.fileExistsSync(__filename)).to.be.equal(true, 'file not found');
7685
});
7786

7887
test('Test searching for files', async () => {
7988
const searchPattern = `${path.basename(__filename, __filename.substring(__filename.length - 3))}.*`;
80-
const files = await fileSystem.search(path.join(__dirname, searchPattern));
89+
const filesystem = new FileSystem();
90+
91+
const files = await filesystem.search(path.join(__dirname, searchPattern));
92+
8193
expect(files).to.be.array();
8294
expect(files.length).to.be.at.least(1);
8395
const expectedFileName = __filename.replace(/\\/g, '/');
8496
const fileName = files[0].replace(/\\/g, '/');
8597
expect(fileName).to.equal(expectedFileName);
8698
});
99+
87100
test('Ensure creating a temporary file results in a unique temp file path', async () => {
88-
const tempFile = await fileSystem.createTemporaryFile('.tmp');
89-
const tempFile2 = await fileSystem.createTemporaryFile('.tmp');
101+
const filesystem = new FileSystem();
102+
103+
const tempFile = await filesystem.createTemporaryFile('.tmp');
104+
const tempFile2 = await filesystem.createTemporaryFile('.tmp');
105+
90106
expect(tempFile.filePath).to.not.equal(tempFile2.filePath, 'Temp files must be unique, implementation of createTemporaryFile is off.');
91107
});
108+
92109
test('Ensure writing to a temp file is supported via file stream', async () => {
93-
await fileSystem.createTemporaryFile('.tmp').then((tf: TemporaryFile) => {
110+
const filesystem = new FileSystem();
111+
112+
await filesystem.createTemporaryFile('.tmp').then((tf: TemporaryFile) => {
94113
expect(tf).to.not.equal(undefined, 'Error trying to create a temporary file');
95-
const writeStream = fileSystem.createWriteStream(tf.filePath);
114+
const writeStream = filesystem.createWriteStream(tf.filePath);
96115
writeStream.write('hello', 'utf8', (err: Error) => {
97116
expect(err).to.equal(undefined, `Failed to write to a temp file, error is ${err}`);
98117
});
99118
}, (failReason) => {
100119
expect(failReason).to.equal('No errors occurred', `Failed to create a temporary file with error ${failReason}`);
101120
});
102121
});
122+
103123
test('Ensure chmod works against a temporary file', async () => {
104-
await fileSystem.createTemporaryFile('.tmp').then(async (fl: TemporaryFile) => {
105-
await fileSystem.chmod(fl.filePath, '7777').then(
124+
const filesystem = new FileSystem();
125+
126+
await filesystem.createTemporaryFile('.tmp').then(async (fl: TemporaryFile) => {
127+
await filesystem.chmod(fl.filePath, '7777').then(
106128
(_success: void) => {
107129
// cannot check for success other than we got here, chmod in Windows won't have any effect on the file itself.
108130
},
@@ -111,13 +133,19 @@ suite('FileSystem', () => {
111133
});
112134
});
113135
});
136+
114137
test('Getting hash for non existent file should throw error', async () => {
115-
const promise = fileSystem.getFileHash('some unknown file');
138+
const filesystem = new FileSystem();
139+
140+
const promise = filesystem.getFileHash('some unknown file');
116141

117142
await expect(promise).to.eventually.be.rejected;
118143
});
144+
119145
test('Getting hash for a file should return non-empty string', async () => {
120-
const hash = await fileSystem.getFileHash(__filename);
146+
const filesystem = new FileSystem();
147+
148+
const hash = await filesystem.getFileHash(__filename);
121149

122150
expect(hash).to.be.length.greaterThan(0);
123151
});

‎src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import * as fs from 'fs-extra';
88
import * as path from 'path';
99
import * as vscode from 'vscode';
1010
import { FileSystem } from '../../../../client/common/platform/fileSystem';
11-
import { PlatformService } from '../../../../client/common/platform/platformService';
1211
import { PYTHON_VIRTUAL_ENVS_LOCATION } from '../../../ciConstants';
1312
import { PYTHON_PATH, restorePythonPathInWorkspaceRoot, setPythonPathInWorkspaceRoot, updateSetting, waitForCondition } from '../../../common';
1413
import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../constants';
@@ -20,7 +19,7 @@ suite('Activation of Environments in Terminal', () => {
2019
const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', 'testExecInTerminal.py');
2120
let outputFile = '';
2221
let outputFileCounter = 0;
23-
const fileSystem = new FileSystem(new PlatformService());
22+
const fileSystem = new FileSystem();
2423
const outputFilesCreated: string[] = [];
2524
const envsLocation = PYTHON_VIRTUAL_ENVS_LOCATION !== undefined ?
2625
path.join(EXTENSION_ROOT_DIR_FOR_TESTS, PYTHON_VIRTUAL_ENVS_LOCATION) : path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'tmp', 'envPaths.json');

‎src/test/interpreters/condaService.unit.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ suite('Interpreters Conda Service', () => {
9494
config.setup(c => c.getSettings(TypeMoq.It.isValue(undefined))).returns(() => settings.object);
9595
settings.setup(p => p.condaPath).returns(() => condaPathSetting);
9696
fileSystem.setup(fs => fs.arePathsSame(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((p1, p2) => {
97-
return new FileSystem(platformService.object).arePathsSame(p1, p2);
97+
return new FileSystem(platformService.object.isWindows)
98+
.arePathsSame(p1, p2);
9899
});
99100

100101
condaService = new CondaService(

0 commit comments

Comments
 (0)