From a9a37ffa59b5f03979181904412d0d33d7d5c6e0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 21 Jan 2020 15:48:20 -0700 Subject: [PATCH 1/2] Make proper unit tests (and move the rest to "functional"). --- src/client/common/variables/environment.ts | 22 +- .../envVarsService.functional.test.ts | 85 ++ .../variables/envVarsService.unit.test.ts | 830 ++++++++++-------- 3 files changed, 584 insertions(+), 353 deletions(-) create mode 100644 src/test/common/variables/envVarsService.functional.test.ts diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index 29cef1926df6..d141848270b6 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -11,16 +11,20 @@ import { EnvironmentVariables, IEnvironmentVariablesService } from './types'; @injectable() export class EnvironmentVariablesService implements IEnvironmentVariablesService { - private readonly pathVariable: 'PATH' | 'Path'; - constructor(@inject(IPathUtils) pathUtils: IPathUtils, @inject(IFileSystem) private readonly fs: IFileSystem) { - this.pathVariable = pathUtils.getPathVariableName(); - } + private _pathVariable?: 'Path' | 'PATH'; + constructor( + // + @inject(IPathUtils) private readonly pathUtils: IPathUtils, + @inject(IFileSystem) private readonly fs: IFileSystem + ) {} + public async parseFile(filePath?: string, baseVars?: EnvironmentVariables): Promise { if (!filePath || !(await this.fs.fileExists(filePath))) { return; } return parseEnvFile(await this.fs.readFile(filePath), baseVars); } + public mergeVariables(source: EnvironmentVariables, target: EnvironmentVariables) { if (!target) { return; @@ -35,12 +39,22 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService } }); } + public appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) { return this.appendPaths(vars, 'PYTHONPATH', ...pythonPaths); } + public appendPath(vars: EnvironmentVariables, ...paths: string[]) { return this.appendPaths(vars, this.pathVariable, ...paths); } + + private get pathVariable(): 'Path' | 'PATH' { + if (!this._pathVariable) { + this._pathVariable = this.pathUtils.getPathVariableName(); + } + return this._pathVariable!; + } + private appendPaths(vars: EnvironmentVariables, variableName: 'PATH' | 'Path' | 'PYTHONPATH', ...pathsToAppend: string[]) { const valueToAppend = pathsToAppend .filter(item => typeof item === 'string' && item.trim().length > 0) diff --git a/src/test/common/variables/envVarsService.functional.test.ts b/src/test/common/variables/envVarsService.functional.test.ts new file mode 100644 index 000000000000..6dbb78fb49af --- /dev/null +++ b/src/test/common/variables/envVarsService.functional.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:max-func-body-length + +import { expect, use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; +import * as path from 'path'; +import { FileSystem } from '../../../client/common/platform/fileSystem'; +import { PathUtils } from '../../../client/common/platform/pathUtils'; +import { PlatformService } from '../../../client/common/platform/platformService'; +import { IPathUtils } from '../../../client/common/types'; +import { OSType } from '../../../client/common/utils/platform'; +import { EnvironmentVariablesService } from '../../../client/common/variables/environment'; +import { IEnvironmentVariablesService } from '../../../client/common/variables/types'; +import { getOSType } from '../../common'; + +use(chaiAsPromised); + +const envFilesFolderPath = path.join(__dirname, '..', '..', '..', '..', 'src', 'testMultiRootWkspc', 'workspace4'); + +suite('Environment Variables Service', () => { + let pathUtils: IPathUtils; + let variablesService: IEnvironmentVariablesService; + setup(() => { + pathUtils = new PathUtils(getOSType() === OSType.Windows); + const fs = new FileSystem(new PlatformService()); + variablesService = new EnvironmentVariablesService(pathUtils, fs); + }); + + suite('parseFile()', () => { + test('Custom variables should be undefined with no argument', async () => { + const vars = await variablesService.parseFile(undefined); + expect(vars).to.equal(undefined, 'Variables should be undefined'); + }); + + test('Custom variables should be undefined with non-existent files', async () => { + const vars = await variablesService.parseFile(path.join(envFilesFolderPath, 'abcd')); + expect(vars).to.equal(undefined, 'Variables should be undefined'); + }); + + test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { + const vars = await variablesService.parseFile(envFilesFolderPath); + expect(vars).to.equal(undefined, 'Variables should be undefined'); + }); + + test('Custom variables should be not undefined with a valid environment file', async () => { + const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); + expect(vars).to.not.equal(undefined, 'Variables should be undefined'); + }); + + test('Custom variables should be parsed from env file', async () => { + const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); + + expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('X1234PYEXTUNITTESTVAR', '1234', 'X1234PYEXTUNITTESTVAR value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '../workspace5', 'PYTHONPATH value is invalid'); + }); + + test('PATH and PYTHONPATH from env file should be returned as is', async () => { + const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env5')); + const expectedPythonPath = '/usr/one/three:/usr/one/four'; + const expectedPath = '/usr/x:/usr/y'; + expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); + expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); + expect(vars).to.have.property('X', '1', 'X value is invalid'); + expect(vars).to.have.property('Y', '2', 'Y value is invalid'); + expect(vars).to.have.property('PYTHONPATH', expectedPythonPath, 'PYTHONPATH value is invalid'); + expect(vars).to.have.property('PATH', expectedPath, 'PATH value is invalid'); + }); + + test('Simple variable substitution is supported', async () => { + const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env6'), { BINDIR: '/usr/bin' }); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); + expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); + expect(vars).to.have.property('PYTHON', '/usr/bin/python3', 'value is invalid'); + }); + }); +}); diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index d1c35b6f707f..afbd20129412 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -3,233 +3,363 @@ 'use strict'; +// tslint:disable:max-func-body-length + import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as path from 'path'; -import { FileSystem } from '../../../client/common/platform/fileSystem'; -import { PathUtils } from '../../../client/common/platform/pathUtils'; -import { PlatformService } from '../../../client/common/platform/platformService'; +import * as TypeMoq from 'typemoq'; +import { IFileSystem } from '../../../client/common/platform/types'; import { IPathUtils } from '../../../client/common/types'; -import { OSType } from '../../../client/common/utils/platform'; import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment'; -import { IEnvironmentVariablesService } from '../../../client/common/variables/types'; -import { getOSType } from '../../common'; use(chaiAsPromised); -const envFilesFolderPath = path.join(__dirname, '..', '..', '..', '..', 'src', 'testMultiRootWkspc', 'workspace4'); +type PathVar = 'Path' | 'PATH'; +const PATHS = [ + 'Path', // Windows + 'PATH' // non-Windows +]; -// tslint:disable-next-line:max-func-body-length suite('Environment Variables Service', () => { - let pathUtils: IPathUtils; - let variablesService: IEnvironmentVariablesService; + const filename = 'x/y/z/.env'; + let pathUtils: TypeMoq.IMock; + let fs: TypeMoq.IMock; + let variablesService: EnvironmentVariablesService; setup(() => { - pathUtils = new PathUtils(getOSType() === OSType.Windows); - const fs = new FileSystem(new PlatformService()); - variablesService = new EnvironmentVariablesService(pathUtils, fs); + pathUtils = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + fs = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + variablesService = new EnvironmentVariablesService( + // + pathUtils.object, + fs.object + ); }); + function verifyAll() { + pathUtils.verifyAll(); + fs.verifyAll(); + } + function setFile(fileName: string, text: string) { + fs + // + .setup(f => f.fileExists(fileName)) + .returns(() => Promise.resolve(true)); + fs + // + .setup(f => f.readFile(fileName)) + .returns(() => Promise.resolve(text)); + } + + suite('parseFile()', () => { + test('Custom variables should be undefined with no argument', async () => { + const vars = await variablesService.parseFile(undefined); + + expect(vars).to.equal(undefined, 'Variables should be undefined'); + verifyAll(); + }); - test('Custom variables should be undefined with no argument', async () => { - const vars = await variablesService.parseFile(undefined); - expect(vars).to.equal(undefined, 'Variables should be undefined'); - }); + test('Custom variables should be undefined with non-existent files', async () => { + fs + // + .setup(f => f.fileExists(filename)) + .returns(() => Promise.resolve(false)); - test('Custom variables should be undefined with non-existent files', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, 'abcd')); - expect(vars).to.equal(undefined, 'Variables should be undefined'); - }); + const vars = await variablesService.parseFile(filename); - test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { - const vars = await variablesService.parseFile(envFilesFolderPath); - expect(vars).to.equal(undefined, 'Variables should be undefined'); - }); + expect(vars).to.equal(undefined, 'Variables should be undefined'); + verifyAll(); + }); - test('Custom variables should be not undefined with a valid environment file', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); - expect(vars).to.not.equal(undefined, 'Variables should be undefined'); - }); + test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { + const dirname = 'x/y/z'; + fs + // + .setup(f => f.fileExists(dirname)) + .returns(() => Promise.resolve(false)); - test('Custom variables should be parsed from env file', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env')); + const vars = await variablesService.parseFile(dirname); - expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('X1234PYEXTUNITTESTVAR', '1234', 'X1234PYEXTUNITTESTVAR value is invalid'); - expect(vars).to.have.property('PYTHONPATH', '../workspace5', 'PYTHONPATH value is invalid'); - }); + expect(vars).to.equal(undefined, 'Variables should be undefined'); + verifyAll(); + }); - test('PATH and PYTHONPATH from env file should be returned as is', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env5')); - const expectedPythonPath = '/usr/one/three:/usr/one/four'; - const expectedPath = '/usr/x:/usr/y'; - expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); - expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); - expect(vars).to.have.property('X', '1', 'X value is invalid'); - expect(vars).to.have.property('Y', '2', 'Y value is invalid'); - expect(vars).to.have.property('PYTHONPATH', expectedPythonPath, 'PYTHONPATH value is invalid'); - expect(vars).to.have.property('PATH', expectedPath, 'PATH value is invalid'); - }); + test('Custom variables should be not undefined with a valid environment file', async () => { + setFile(filename, '...'); - test('Simple variable substitution is supported', async () => { - const vars = await variablesService.parseFile(path.join(envFilesFolderPath, '.env6'), { BINDIR: '/usr/bin' }); + const vars = await variablesService.parseFile(filename); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); - expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); - expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); - expect(vars).to.have.property('PYTHON', '/usr/bin/python3', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables should be undefined'); + verifyAll(); + }); + + test('Custom variables should be parsed from env file', async () => { + // src/testMultiRootWkspc/workspace4/.env + setFile( + filename, + ` +X1234PYEXTUNITTESTVAR=1234 +PYTHONPATH=../workspace5 + ` + ); + + const vars = await variablesService.parseFile(filename); + + expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('X1234PYEXTUNITTESTVAR', '1234', 'X1234PYEXTUNITTESTVAR value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '../workspace5', 'PYTHONPATH value is invalid'); + verifyAll(); + }); - test('Ensure variables are merged', async () => { - const vars1 = { ONE: '1', TWO: 'TWO' }; - const vars2 = { ONE: 'ONE', THREE: '3' }; - variablesService.mergeVariables(vars1, vars2); - expect(Object.keys(vars1)).lengthOf(2, 'Source variables modified'); - expect(Object.keys(vars2)).lengthOf(3, 'Variables not merged'); - expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); - expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); - expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); + test('PATH and PYTHONPATH from env file should be returned as is', async () => { + const expectedPythonPath = '/usr/one/three:/usr/one/four'; + const expectedPath = '/usr/x:/usr/y'; + // src/testMultiRootWkspc/workspace4/.env + setFile( + filename, + ` +X=1 +Y=2 +PYTHONPATH=/usr/one/three:/usr/one/four +# Unix PATH variable +PATH=/usr/x:/usr/y +# Windows Path variable +Path=/usr/x:/usr/y + ` + ); + + const vars = await variablesService.parseFile(filename); + + expect(vars).to.not.equal(undefined, 'Variables is is undefiend'); + expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); + expect(vars).to.have.property('X', '1', 'X value is invalid'); + expect(vars).to.have.property('Y', '2', 'Y value is invalid'); + expect(vars).to.have.property('PYTHONPATH', expectedPythonPath, 'PYTHONPATH value is invalid'); + expect(vars).to.have.property('PATH', expectedPath, 'PATH value is invalid'); + verifyAll(); + }); + + test('Simple variable substitution is supported', async () => { + // src/testMultiRootWkspc/workspace4/.env + setFile( + filename, + // tslint:disable:no-invalid-template-strings + '\ +REPO=/home/user/git/foobar\n\ +PYTHONPATH=${REPO}/foo:${REPO}/bar\n\ +PYTHON=${BINDIR}/python3\n\ + ' + // tslint:enable:no-invalid-template-strings + ); + + const vars = await variablesService.parseFile(filename, { BINDIR: '/usr/bin' }); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); + expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); + expect(vars).to.have.property('PYTHON', '/usr/bin/python3', 'value is invalid'); + verifyAll(); + }); }); - test('Ensure path variabnles variables are not merged into target', async () => { - const pathVariable = pathUtils.getPathVariableName(); - const vars1 = { ONE: '1', TWO: 'TWO', PYTHONPATH: 'PYTHONPATH' }; - // tslint:disable-next-line:no-any - (vars1 as any)[pathVariable] = 'PATH'; - const vars2 = { ONE: 'ONE', THREE: '3' }; - variablesService.mergeVariables(vars1, vars2); - expect(Object.keys(vars1)).lengthOf(4, 'Source variables modified'); - expect(Object.keys(vars2)).lengthOf(3, 'Variables not merged'); - expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); - expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); - expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); + PATHS.map(pathVariable => { + suite(`mergeVariables() (path var: ${pathVariable})`, () => { + setup(() => { + pathUtils + // + .setup(pu => pu.getPathVariableName()) + .returns(() => pathVariable as PathVar); + }); + + test('Ensure variables are merged', async () => { + const vars1 = { ONE: '1', TWO: 'TWO' }; + const vars2 = { ONE: 'ONE', THREE: '3' }; + + variablesService.mergeVariables(vars1, vars2); + + expect(Object.keys(vars1)).lengthOf(2, 'Source variables modified'); + expect(Object.keys(vars2)).lengthOf(3, 'Variables not merged'); + expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); + expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); + expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); + verifyAll(); + }); + + test('Ensure path variabnles variables are not merged into target', async () => { + const vars1 = { ONE: '1', TWO: 'TWO', PYTHONPATH: 'PYTHONPATH' }; + // tslint:disable-next-line:no-any + (vars1 as any)[pathVariable] = 'PATH'; + const vars2 = { ONE: 'ONE', THREE: '3' }; + + variablesService.mergeVariables(vars1, vars2); + + expect(Object.keys(vars1)).lengthOf(4, 'Source variables modified'); + expect(Object.keys(vars2)).lengthOf(3, 'Variables not merged'); + expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); + expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); + expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); + verifyAll(); + }); + + test('Ensure path variables variables in target are left untouched', async () => { + const vars1 = { ONE: '1', TWO: 'TWO' }; + const vars2 = { ONE: 'ONE', THREE: '3', PYTHONPATH: 'PYTHONPATH' }; + // tslint:disable-next-line:no-any + (vars2 as any)[pathVariable] = 'PATH'; + + variablesService.mergeVariables(vars1, vars2); + + expect(Object.keys(vars1)).lengthOf(2, 'Source variables modified'); + expect(Object.keys(vars2)).lengthOf(5, 'Variables not merged'); + expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); + expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); + expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); + expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); + expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + verifyAll(); + }); + }); }); - test('Ensure path variabnles variables in target are left untouched', async () => { - const pathVariable = pathUtils.getPathVariableName(); - const vars1 = { ONE: '1', TWO: 'TWO' }; - const vars2 = { ONE: 'ONE', THREE: '3', PYTHONPATH: 'PYTHONPATH' }; - // tslint:disable-next-line:no-any - (vars2 as any)[pathVariable] = 'PATH'; - variablesService.mergeVariables(vars1, vars2); - expect(Object.keys(vars1)).lengthOf(2, 'Source variables modified'); - expect(Object.keys(vars2)).lengthOf(5, 'Variables not merged'); - expect(vars2).to.have.property('ONE', 'ONE', 'Variable overwritten'); - expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value'); - expect(vars2).to.have.property('THREE', '3', 'Variable not merged'); - expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + PATHS.map(pathVariable => { + suite(`appendPath() (path var: ${pathVariable})`, () => { + setup(() => { + pathUtils + // + .setup(pu => pu.getPathVariableName()) + .returns(() => pathVariable as PathVar); + }); + + test('Ensure appending PATH has no effect if an undefined value or empty string is provided and PATH does not exist in vars object', async () => { + const vars = { ONE: '1' }; + + variablesService.appendPath(vars); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + + variablesService.appendPath(vars, ''); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + + variablesService.appendPath(vars, ' ', ''); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + + verifyAll(); + }); + + test(`Ensure appending PATH has no effect if an empty string is provided and path does not exist in vars object (${pathVariable})`, async () => { + const vars = { ONE: '1' }; + // tslint:disable-next-line:no-any + (vars as any)[pathVariable] = 'PATH'; + + variablesService.appendPath(vars); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + + variablesService.appendPath(vars, ''); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + + variablesService.appendPath(vars, ' ', ''); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); + + verifyAll(); + }); + + test(`Ensure PATH is appeneded (${pathVariable})`, async () => { + const vars = { ONE: '1' }; + // tslint:disable-next-line:no-any + (vars as any)[pathVariable] = 'PATH'; + const pathToAppend = `/usr/one${path.delimiter}/usr/three`; + + variablesService.appendPath(vars, pathToAppend); + + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value'); + verifyAll(); + }); + }); }); - test('Ensure appending PATH has no effect if an undefined value or empty string is provided and PATH does not exist in vars object', async () => { - const vars = { ONE: '1' }; - variablesService.appendPath(vars); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + suite('appendPythonPath()', () => { + test('Ensure appending PYTHONPATH has no effect if an undefined value or empty string is provided and PYTHONPATH does not exist in vars object', async () => { + const vars = { ONE: '1' }; - variablesService.appendPath(vars, ''); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + variablesService.appendPythonPath(vars); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - variablesService.appendPath(vars, ' ', ''); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - }); + variablesService.appendPythonPath(vars, ''); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - test('Ensure appending PYTHONPATH has no effect if an undefined value or empty string is provided and PYTHONPATH does not exist in vars object', async () => { - const vars = { ONE: '1' }; - variablesService.appendPythonPath(vars); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + variablesService.appendPythonPath(vars, ' ', ''); + expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - variablesService.appendPythonPath(vars, ''); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + verifyAll(); + }); - variablesService.appendPythonPath(vars, ' ', ''); - expect(Object.keys(vars)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - }); + test('Ensure appending PYTHONPATH has no effect if an empty string is provided and PYTHONPATH does not exist in vars object', async () => { + const vars = { ONE: '1', PYTHONPATH: 'PYTHONPATH' }; - test('Ensure appending PATH has no effect if an empty string is provided and path does not exist in vars object', async () => { - const pathVariable = pathUtils.getPathVariableName(); - const vars = { ONE: '1' }; - // tslint:disable-next-line:no-any - (vars as any)[pathVariable] = 'PATH'; - variablesService.appendPath(vars); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); - - variablesService.appendPath(vars, ''); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); - - variablesService.appendPath(vars, ' ', ''); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value'); - }); + variablesService.appendPythonPath(vars); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - test('Ensure appending PYTHONPATH has no effect if an empty string is provided and PYTHONPATH does not exist in vars object', async () => { - const vars = { ONE: '1', PYTHONPATH: 'PYTHONPATH' }; - variablesService.appendPythonPath(vars); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - - variablesService.appendPythonPath(vars, ''); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - - variablesService.appendPythonPath(vars, ' ', ''); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - }); + variablesService.appendPythonPath(vars, ''); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - test('Ensure PATH is appeneded', async () => { - const pathVariable = pathUtils.getPathVariableName(); - const vars = { ONE: '1' }; - // tslint:disable-next-line:no-any - (vars as any)[pathVariable] = 'PATH'; - const pathToAppend = `/usr/one${path.delimiter}/usr/three`; - variablesService.appendPath(vars, pathToAppend); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value'); - }); + variablesService.appendPythonPath(vars, ' ', ''); + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value'); - test('Ensure appending PYTHONPATH has no effect if an empty string is provided and PYTHONPATH does not exist in vars object', async () => { - const vars = { ONE: '1', PYTHONPATH: 'PYTHONPATH' }; - const pathToAppend = `/usr/one${path.delimiter}/usr/three`; - variablesService.appendPythonPath(vars, pathToAppend); - expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('ONE', '1', 'Incorrect value'); - expect(vars).to.have.property('PYTHONPATH', `PYTHONPATH${path.delimiter}${pathToAppend}`, 'Incorrect value'); + verifyAll(); + }); + + test('Ensure appending PYTHONPATH has no effect if an empty string is provided and PYTHONPATH does not exist in vars object', async () => { + const vars = { ONE: '1', PYTHONPATH: 'PYTHONPATH' }; + const pathToAppend = `/usr/one${path.delimiter}/usr/three`; + + variablesService.appendPythonPath(vars, pathToAppend); + + expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('ONE', '1', 'Incorrect value'); + expect(vars).to.have.property('PYTHONPATH', `PYTHONPATH${path.delimiter}${pathToAppend}`, 'Incorrect value'); + verifyAll(); + }); }); }); -// tslint:disable-next-line:max-func-body-length suite('Parsing Environment Variables Files', () => { - test('Custom variables should be parsed from env file', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + suite('parseEnvFile()', () => { + test('Custom variables should be parsed from env file', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` X1234PYEXTUNITTESTVAR=1234 PYTHONPATH=../workspace5 `); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('X1234PYEXTUNITTESTVAR', '1234', 'X1234PYEXTUNITTESTVAR value is invalid'); - expect(vars).to.have.property('PYTHONPATH', '../workspace5', 'PYTHONPATH value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('X1234PYEXTUNITTESTVAR', '1234', 'X1234PYEXTUNITTESTVAR value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '../workspace5', 'PYTHONPATH value is invalid'); + }); - test('PATH and PYTHONPATH from env file should be returned as is', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('PATH and PYTHONPATH from env file should be returned as is', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` X=1 Y=2 PYTHONPATH=/usr/one/three:/usr/one/four @@ -239,19 +369,19 @@ PATH=/usr/x:/usr/y Path=/usr/x:/usr/y `); - const expectedPythonPath = '/usr/one/three:/usr/one/four'; - const expectedPath = '/usr/x:/usr/y'; - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); - expect(vars).to.have.property('X', '1', 'X value is invalid'); - expect(vars).to.have.property('Y', '2', 'Y value is invalid'); - expect(vars).to.have.property('PYTHONPATH', expectedPythonPath, 'PYTHONPATH value is invalid'); - expect(vars).to.have.property('PATH', expectedPath, 'PATH value is invalid'); - }); + const expectedPythonPath = '/usr/one/three:/usr/one/four'; + const expectedPath = '/usr/x:/usr/y'; + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); + expect(vars).to.have.property('X', '1', 'X value is invalid'); + expect(vars).to.have.property('Y', '2', 'Y value is invalid'); + expect(vars).to.have.property('PYTHONPATH', expectedPythonPath, 'PYTHONPATH value is invalid'); + expect(vars).to.have.property('PATH', expectedPath, 'PATH value is invalid'); + }); - test('Variable names must be alpha + alnum/underscore', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Variable names must be alpha + alnum/underscore', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` SPAM=1234 ham=5678 Eggs=9012 @@ -265,29 +395,29 @@ VAR1=3456 VAR_2=7890 `); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('ham', '5678', 'value is invalid'); - expect(vars).to.have.property('Eggs', '9012', 'value is invalid'); - expect(vars).to.have.property('VAR1', '3456', 'value is invalid'); - expect(vars).to.have.property('VAR_2', '7890', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(5, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('ham', '5678', 'value is invalid'); + expect(vars).to.have.property('Eggs', '9012', 'value is invalid'); + expect(vars).to.have.property('VAR1', '3456', 'value is invalid'); + expect(vars).to.have.property('VAR_2', '7890', 'value is invalid'); + }); - test('Empty values become empty string', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Empty values become empty string', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` SPAM= `); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '', 'value is invalid'); + }); - test('Outer quotation marks are removed', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Outer quotation marks are removed', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` SPAM=1234 HAM='5678' EGGS="9012" @@ -300,26 +430,26 @@ VAR3='MN'OP' VAR4="QR"ST" `); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(10, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('HAM', '5678', 'value is invalid'); - expect(vars).to.have.property('EGGS', '9012', 'value is invalid'); - expect(vars).to.have.property('FOO', '"3456"', 'value is invalid'); - expect(vars).to.have.property('BAR', "'7890'", 'value is invalid'); - expect(vars).to.have.property('BAZ', '"ABCD', 'value is invalid'); - expect(vars).to.have.property('VAR1', '"EFGH', 'value is invalid'); - expect(vars).to.have.property('VAR2', 'IJKL"', 'value is invalid'); - // tslint:disable-next-line:no-suspicious-comment - // TODO: Should the outer marks be left? - expect(vars).to.have.property('VAR3', "MN'OP", 'value is invalid'); - expect(vars).to.have.property('VAR4', 'QR"ST', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(10, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('HAM', '5678', 'value is invalid'); + expect(vars).to.have.property('EGGS', '9012', 'value is invalid'); + expect(vars).to.have.property('FOO', '"3456"', 'value is invalid'); + expect(vars).to.have.property('BAR', "'7890'", 'value is invalid'); + expect(vars).to.have.property('BAZ', '"ABCD', 'value is invalid'); + expect(vars).to.have.property('VAR1', '"EFGH', 'value is invalid'); + expect(vars).to.have.property('VAR2', 'IJKL"', 'value is invalid'); + // tslint:disable-next-line:no-suspicious-comment + // TODO: Should the outer marks be left? + expect(vars).to.have.property('VAR3', "MN'OP", 'value is invalid'); + expect(vars).to.have.property('VAR4', 'QR"ST', 'value is invalid'); + }); - test('Whitespace is ignored', () => { - // tslint:disable:no-trailing-whitespace - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Whitespace is ignored', () => { + // tslint:disable:no-trailing-whitespace + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` SPAM=1234 HAM =5678 EGGS= 9012 @@ -330,25 +460,25 @@ VAR1=EFGH ... VAR2=IJKL VAR3=' MNOP ' `); - // tslint:enable:no-trailing-whitespace - - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(9, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('HAM', '5678', 'value is invalid'); - expect(vars).to.have.property('EGGS', '9012', 'value is invalid'); - expect(vars).to.have.property('FOO', '3456', 'value is invalid'); - expect(vars).to.have.property('BAR', '7890', 'value is invalid'); - expect(vars).to.have.property('BAZ', 'ABCD', 'value is invalid'); - expect(vars).to.have.property('VAR1', 'EFGH ...', 'value is invalid'); - expect(vars).to.have.property('VAR2', 'IJKL', 'value is invalid'); - expect(vars).to.have.property('VAR3', ' MNOP ', 'value is invalid'); - }); + // tslint:enable:no-trailing-whitespace + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(9, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('HAM', '5678', 'value is invalid'); + expect(vars).to.have.property('EGGS', '9012', 'value is invalid'); + expect(vars).to.have.property('FOO', '3456', 'value is invalid'); + expect(vars).to.have.property('BAR', '7890', 'value is invalid'); + expect(vars).to.have.property('BAZ', 'ABCD', 'value is invalid'); + expect(vars).to.have.property('VAR1', 'EFGH ...', 'value is invalid'); + expect(vars).to.have.property('VAR2', 'IJKL', 'value is invalid'); + expect(vars).to.have.property('VAR3', ' MNOP ', 'value is invalid'); + }); - test('Blank lines are ignored', () => { - // tslint:disable:no-trailing-whitespace - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Blank lines are ignored', () => { + // tslint:disable:no-trailing-whitespace + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` SPAM=1234 @@ -356,17 +486,17 @@ HAM=5678 `); - // tslint:enable:no-trailing-whitespace + // tslint:enable:no-trailing-whitespace - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('HAM', '5678', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('HAM', '5678', 'value is invalid'); + }); - test('Comments are ignored', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile(` + test('Comments are ignored', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile(` # step 1 SPAM=1234 # step 2 @@ -376,46 +506,46 @@ EGGS=9012 # ... # done `); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('HAM', '5678', 'value is invalid'); - expect(vars).to.have.property('EGGS', '9012 # ...', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('HAM', '5678', 'value is invalid'); + expect(vars).to.have.property('EGGS', '9012 # ...', 'value is invalid'); + }); - // Substitution - // tslint:disable:no-invalid-template-strings + suite('variable substitution', () => { + // tslint:disable:no-invalid-template-strings - test('Basic substitution syntax', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ + test('Basic substitution syntax', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ REPO=/home/user/git/foobar \n\ PYTHONPATH=${REPO}/foo:${REPO}/bar \n\ - '); + '); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); - expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); + }); - test('Curly braces are required for substitution', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ + test('Curly braces are required for substitution', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ SPAM=1234 \n\ EGGS=$SPAM \n\ - '); - - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('EGGS', '$SPAM', 'value is invalid'); - }); - - test('Nested substitution is not supported', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile( - '\ + '); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('EGGS', '$SPAM', 'value is invalid'); + }); + + test('Nested substitution is not supported', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile( + '\ SPAM=EGGS \n\ EGGS=??? \n\ HAM1="-- ${${SPAM}} --"\n\ @@ -423,82 +553,84 @@ abcEGGSxyz=!!! \n\ HAM2="-- ${abc${SPAM}xyz} --"\n\ HAM3="-- ${${SPAM} --"\n\ HAM4="-- ${${SPAM}} ${EGGS} --"\n\ - ' - ); - - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(7, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', 'EGGS', 'value is invalid'); - expect(vars).to.have.property('EGGS', '???', 'value is invalid'); - expect(vars).to.have.property('HAM1', '-- ${${SPAM}} --', 'value is invalid'); - expect(vars).to.have.property('abcEGGSxyz', '!!!', 'value is invalid'); - expect(vars).to.have.property('HAM2', '-- ${abc${SPAM}xyz} --', 'value is invalid'); - expect(vars).to.have.property('HAM3', '-- ${${SPAM} --', 'value is invalid'); - expect(vars).to.have.property('HAM4', '-- ${${SPAM}} ${EGGS} --', 'value is invalid'); - }); - - test('Other bad substitution syntax', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ + ' + ); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(7, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', 'EGGS', 'value is invalid'); + expect(vars).to.have.property('EGGS', '???', 'value is invalid'); + expect(vars).to.have.property('HAM1', '-- ${${SPAM}} --', 'value is invalid'); + expect(vars).to.have.property('abcEGGSxyz', '!!!', 'value is invalid'); + expect(vars).to.have.property('HAM2', '-- ${abc${SPAM}xyz} --', 'value is invalid'); + expect(vars).to.have.property('HAM3', '-- ${${SPAM} --', 'value is invalid'); + expect(vars).to.have.property('HAM4', '-- ${${SPAM}} ${EGGS} --', 'value is invalid'); + }); + + test('Other bad substitution syntax', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ SPAM=EGGS \n\ EGGS=??? \n\ HAM1=${} \n\ HAM2=${ \n\ HAM3=${SPAM+EGGS} \n\ HAM4=$SPAM \n\ - '); - - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(6, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', 'EGGS', 'value is invalid'); - expect(vars).to.have.property('EGGS', '???', 'value is invalid'); - expect(vars).to.have.property('HAM1', '${}', 'value is invalid'); - expect(vars).to.have.property('HAM2', '${', 'value is invalid'); - expect(vars).to.have.property('HAM3', '${SPAM+EGGS}', 'value is invalid'); - expect(vars).to.have.property('HAM4', '$SPAM', 'value is invalid'); - }); - - test('Recursive substitution is allowed', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ + '); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(6, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', 'EGGS', 'value is invalid'); + expect(vars).to.have.property('EGGS', '???', 'value is invalid'); + expect(vars).to.have.property('HAM1', '${}', 'value is invalid'); + expect(vars).to.have.property('HAM2', '${', 'value is invalid'); + expect(vars).to.have.property('HAM3', '${SPAM+EGGS}', 'value is invalid'); + expect(vars).to.have.property('HAM4', '$SPAM', 'value is invalid'); + }); + + test('Recursive substitution is allowed', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ REPO=/home/user/git/foobar \n\ PYTHONPATH=${REPO}/foo \n\ PYTHONPATH=${PYTHONPATH}:${REPO}/bar \n\ - '); + '); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); - expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); - expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(2, 'Incorrect number of variables'); + expect(vars).to.have.property('REPO', '/home/user/git/foobar', 'value is invalid'); + expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); + }); - test('Substitution may be escaped', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ + test('Substitution may be escaped', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ SPAM=1234 \n\ EGGS=\\${SPAM}/foo:\\${SPAM}/bar \n\ HAM=$ ... $$ \n\ - '); + '); + + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); + expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); + expect(vars).to.have.property('EGGS', '${SPAM}/foo:${SPAM}/bar', 'value is invalid'); + expect(vars).to.have.property('HAM', '$ ... $$', 'value is invalid'); + }); + + test('base substitution variables', () => { + // tslint:disable-next-line:no-multiline-string + const vars = parseEnvFile('\ +PYTHONPATH=${REPO}/foo:${REPO}/bar \n\ + ', { + REPO: '/home/user/git/foobar' + }); - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(3, 'Incorrect number of variables'); - expect(vars).to.have.property('SPAM', '1234', 'value is invalid'); - expect(vars).to.have.property('EGGS', '${SPAM}/foo:${SPAM}/bar', 'value is invalid'); - expect(vars).to.have.property('HAM', '$ ... $$', 'value is invalid'); - }); + expect(vars).to.not.equal(undefined, 'Variables is undefiend'); + expect(Object.keys(vars!)).lengthOf(1, 'Incorrect number of variables'); + expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); + }); - test('base substitution variables', () => { - // tslint:disable-next-line:no-multiline-string - const vars = parseEnvFile('\ -PYTHONPATH=${REPO}/foo:${REPO}/bar \n\ - ', { - REPO: '/home/user/git/foobar' + // tslint:enable:no-invalid-template-strings }); - - expect(vars).to.not.equal(undefined, 'Variables is undefiend'); - expect(Object.keys(vars!)).lengthOf(1, 'Incorrect number of variables'); - expect(vars).to.have.property('PYTHONPATH', '/home/user/git/foobar/foo:/home/user/git/foobar/bar', 'value is invalid'); }); - - // tslint:enable:no-invalid-template-strings }); From 986c9cfae5cb3c9abed3c683a571fc81732fe393 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 22 Jan 2020 11:34:33 -0700 Subject: [PATCH 2/2] Do not leave empty comments. --- src/client/common/variables/environment.ts | 2 +- .../variables/envVarsService.unit.test.ts | 36 +++++++------------ 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts index d141848270b6..4e3eac08de3a 100644 --- a/src/client/common/variables/environment.ts +++ b/src/client/common/variables/environment.ts @@ -13,7 +13,7 @@ import { EnvironmentVariables, IEnvironmentVariablesService } from './types'; export class EnvironmentVariablesService implements IEnvironmentVariablesService { private _pathVariable?: 'Path' | 'PATH'; constructor( - // + // We only use a small portion of either of these interfaces. @inject(IPathUtils) private readonly pathUtils: IPathUtils, @inject(IFileSystem) private readonly fs: IFileSystem ) {} diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts index afbd20129412..9bd6937eb95b 100644 --- a/src/test/common/variables/envVarsService.unit.test.ts +++ b/src/test/common/variables/envVarsService.unit.test.ts @@ -30,7 +30,7 @@ suite('Environment Variables Service', () => { pathUtils = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); fs = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); variablesService = new EnvironmentVariablesService( - // + // This is the only place that the mocks are used. pathUtils.object, fs.object ); @@ -40,14 +40,10 @@ suite('Environment Variables Service', () => { fs.verifyAll(); } function setFile(fileName: string, text: string) { - fs - // - .setup(f => f.fileExists(fileName)) - .returns(() => Promise.resolve(true)); - fs - // - .setup(f => f.readFile(fileName)) - .returns(() => Promise.resolve(text)); + fs.setup(f => f.fileExists(fileName)) // Handle the specific file. + .returns(() => Promise.resolve(true)); // The file exists. + fs.setup(f => f.readFile(fileName)) // Handle the specific file. + .returns(() => Promise.resolve(text)); // Pretend to read from the file. } suite('parseFile()', () => { @@ -59,10 +55,8 @@ suite('Environment Variables Service', () => { }); test('Custom variables should be undefined with non-existent files', async () => { - fs - // - .setup(f => f.fileExists(filename)) - .returns(() => Promise.resolve(false)); + fs.setup(f => f.fileExists(filename)) // Handle the specific file. + .returns(() => Promise.resolve(false)); // The file is missing. const vars = await variablesService.parseFile(filename); @@ -72,10 +66,8 @@ suite('Environment Variables Service', () => { test('Custom variables should be undefined when folder name is passed instead of a file name', async () => { const dirname = 'x/y/z'; - fs - // - .setup(f => f.fileExists(dirname)) - .returns(() => Promise.resolve(false)); + fs.setup(f => f.fileExists(dirname)) // Handle the specific "file". + .returns(() => Promise.resolve(false)); // It isn't a "regular" file. const vars = await variablesService.parseFile(dirname); @@ -167,9 +159,8 @@ PYTHON=${BINDIR}/python3\n\ suite(`mergeVariables() (path var: ${pathVariable})`, () => { setup(() => { pathUtils - // - .setup(pu => pu.getPathVariableName()) - .returns(() => pathVariable as PathVar); + .setup(pu => pu.getPathVariableName()) // This always gets called. + .returns(() => pathVariable as PathVar); // Pretend we're on a specific platform. }); test('Ensure variables are merged', async () => { @@ -226,9 +217,8 @@ PYTHON=${BINDIR}/python3\n\ suite(`appendPath() (path var: ${pathVariable})`, () => { setup(() => { pathUtils - // - .setup(pu => pu.getPathVariableName()) - .returns(() => pathVariable as PathVar); + .setup(pu => pu.getPathVariableName()) // This always gets called. + .returns(() => pathVariable as PathVar); // Pretend we're on a specific platform. }); test('Ensure appending PATH has no effect if an undefined value or empty string is provided and PATH does not exist in vars object', async () => {