From a2f89b09bf4175aa7b96d9a8a84b701ebbc02ff1 Mon Sep 17 00:00:00 2001 From: Jasper Catthoor Date: Tue, 27 Dec 2016 11:59:49 +0100 Subject: [PATCH 1/5] Added IntelliJ project files to gitignore --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index ef8646b460..d7d7f0898b 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,6 @@ typings src/**/*.js test/**/*.js src/**/*.d.ts -test/**/*.d.ts \ No newline at end of file +test/**/*.d.ts +.idea +*.iml \ No newline at end of file From 869ee89d6fec2bf3ea98d36491e4efc7aaec8658 Mon Sep 17 00:00:00 2001 From: Jasper Catthoor Date: Tue, 27 Dec 2016 15:51:27 +0100 Subject: [PATCH 2/5] feat(InputfileResolverSpec): exclude online files from globbing --- src/InputFileResolver.ts | 44 +++++++++++++++++++++++------- test/unit/InputFileResolverSpec.ts | 16 +++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/InputFileResolver.ts b/src/InputFileResolver.ts index 42d3e9f901..f9d37839e5 100644 --- a/src/InputFileResolver.ts +++ b/src/InputFileResolver.ts @@ -7,6 +7,10 @@ const log = log4js.getLogger('InputFileResolver'); const DEFAULT_INPUT_FILE_PROPERTIES = { mutated: false, included: true }; +function isWebUrl(pattern: string): boolean { + return pattern.indexOf('http://') === 0 || pattern.indexOf('https://') === 0; +} + export default class InputFileResolver { private inputFileResolver: PatternResolver; @@ -14,6 +18,7 @@ export default class InputFileResolver { constructor(mutate: string[], allFileExpressions: Array) { this.validateFileDescriptor(allFileExpressions); + this.validateMutationArray(mutate); this.mutateResolver = PatternResolver.parse(mutate || []); this.inputFileResolver = PatternResolver.parse(allFileExpressions); } @@ -33,11 +38,26 @@ export default class InputFileResolver { if (_.isObject(maybeInputFileDescriptor)) { if (Object.keys(maybeInputFileDescriptor).indexOf('pattern') === -1) { throw Error(`File descriptor ${JSON.stringify(maybeInputFileDescriptor)} is missing mandatory property 'pattern'.`); + } else { + maybeInputFileDescriptor = maybeInputFileDescriptor as InputFileDescriptor; + if (isWebUrl(maybeInputFileDescriptor.pattern) && maybeInputFileDescriptor.mutated) { + throw new Error(`Cannot mutate web url "${maybeInputFileDescriptor.pattern}".`); + } } } }); } + private validateMutationArray(mutationArray: Array) { + if (mutationArray) { + mutationArray.forEach(mutation => { + if (isWebUrl(mutation)) { + throw new Error(`Cannot mutate web url "${mutation}".`) + } + }); + } + } + private markAdditionalFilesToMutate(allInputFiles: InputFile[], additionalMutateFiles: string[]) { let errors: string[] = []; additionalMutateFiles.forEach(mutateFile => { @@ -70,7 +90,7 @@ class PatternResolver { private descriptor: InputFileDescriptor; constructor(descriptor: InputFileDescriptor | string, private previous?: PatternResolver) { - if (typeof descriptor === 'string') { + if (typeof descriptor === 'string') { // mutator array is a string array this.descriptor = _.assign({ pattern: descriptor }, DEFAULT_INPUT_FILE_PROPERTIES); this.ignore = descriptor.indexOf('!') === 0; if (this.ignore) { @@ -87,7 +107,7 @@ class PatternResolver { return Promise.resolve([]); } else { // Start the globbing task for the current descriptor - const globbingTask = this.resolveFileGlob(this.descriptor.pattern) + const globbingTask = this.resolveGlobbingExpression(this.descriptor.pattern) .then(filePaths => filePaths.map(filePath => this.createInputFile(filePath))); if (this.previous) { // If there is a previous globbing expression, resolve that one as well @@ -123,14 +143,18 @@ class PatternResolver { return current; } - private resolveFileGlob(pattern: string): Promise { - return glob(pattern).then(files => { - if (files.length === 0) { - this.reportEmptyGlobbingExpression(pattern); - } - normalize(files); - return files; - }); + private resolveGlobbingExpression(pattern: string): Promise { + if (isWebUrl(pattern)) { + return Promise.resolve([pattern]); + } else { + return glob(pattern).then(files => { + if (files.length === 0) { + this.reportEmptyGlobbingExpression(pattern); + } + normalize(files); + return files; + }); + } } private reportEmptyGlobbingExpression(expression: string) { diff --git a/test/unit/InputFileResolverSpec.ts b/test/unit/InputFileResolverSpec.ts index 902833c240..d0a1a955b6 100644 --- a/test/unit/InputFileResolverSpec.ts +++ b/test/unit/InputFileResolverSpec.ts @@ -176,6 +176,22 @@ describe('InputFileResolver', () => { }); + describe('with url as file pattern', () => { + it('should pass through the web urls without globbing', () => { + return new InputFileResolver([], ['http://www', {pattern: 'https://ok'}]) + .resolve() + .then(() => expect(fileUtils.glob).to.not.have.been.called); + }); + + it('should fail when web url is in the mutated array', () => { + expect(() => new InputFileResolver(['http://www'], ['http://www'])).throws('Cannot mutate web url "http://www".'); + }); + + it('should fail when web url is to be mutated', () => { + expect(() => new InputFileResolver([], [ { pattern: 'http://www', mutated: true } ])).throws('Cannot mutate web url "http://www".'); + }); + }); + afterEach(() => { sandbox.restore(); }); From 6b09861fbc7f0be0b55f915144ef8b0995992f74 Mon Sep 17 00:00:00 2001 From: Simon de Lang Date: Wed, 28 Dec 2016 16:25:22 +0100 Subject: [PATCH 3/5] fix(InputFileResolver) Add missing semicolon --- src/InputFileResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/InputFileResolver.ts b/src/InputFileResolver.ts index f9d37839e5..968d10f66e 100644 --- a/src/InputFileResolver.ts +++ b/src/InputFileResolver.ts @@ -52,7 +52,7 @@ export default class InputFileResolver { if (mutationArray) { mutationArray.forEach(mutation => { if (isWebUrl(mutation)) { - throw new Error(`Cannot mutate web url "${mutation}".`) + throw new Error(`Cannot mutate web url "${mutation}".`); } }); } From 14c11de4202c5a5f5b2ab04417aba2dc861048cc Mon Sep 17 00:00:00 2001 From: Simon de Lang Date: Thu, 29 Dec 2016 16:10:45 +0100 Subject: [PATCH 4/5] fix(Sandbox): Don't copy online files to the Sandbox --- src/InputFileResolver.ts | 12 +++----- src/Sandbox.ts | 6 ++++ src/utils/fileUtils.ts | 5 +++- test/unit/SandboxSpec.ts | 17 +++++++++-- test/unit/utils/fileUtilsSpec.ts | 50 ++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 test/unit/utils/fileUtilsSpec.ts diff --git a/src/InputFileResolver.ts b/src/InputFileResolver.ts index 968d10f66e..993c7d3caa 100644 --- a/src/InputFileResolver.ts +++ b/src/InputFileResolver.ts @@ -1,5 +1,5 @@ import { InputFile, InputFileDescriptor } from 'stryker-api/core'; -import { glob, normalize } from './utils/fileUtils'; +import { glob, normalize, isOnlineFile } from './utils/fileUtils'; import * as _ from 'lodash'; import * as log4js from 'log4js'; @@ -7,10 +7,6 @@ const log = log4js.getLogger('InputFileResolver'); const DEFAULT_INPUT_FILE_PROPERTIES = { mutated: false, included: true }; -function isWebUrl(pattern: string): boolean { - return pattern.indexOf('http://') === 0 || pattern.indexOf('https://') === 0; -} - export default class InputFileResolver { private inputFileResolver: PatternResolver; @@ -40,7 +36,7 @@ export default class InputFileResolver { throw Error(`File descriptor ${JSON.stringify(maybeInputFileDescriptor)} is missing mandatory property 'pattern'.`); } else { maybeInputFileDescriptor = maybeInputFileDescriptor as InputFileDescriptor; - if (isWebUrl(maybeInputFileDescriptor.pattern) && maybeInputFileDescriptor.mutated) { + if (isOnlineFile(maybeInputFileDescriptor.pattern) && maybeInputFileDescriptor.mutated) { throw new Error(`Cannot mutate web url "${maybeInputFileDescriptor.pattern}".`); } } @@ -51,7 +47,7 @@ export default class InputFileResolver { private validateMutationArray(mutationArray: Array) { if (mutationArray) { mutationArray.forEach(mutation => { - if (isWebUrl(mutation)) { + if (isOnlineFile(mutation)) { throw new Error(`Cannot mutate web url "${mutation}".`); } }); @@ -144,7 +140,7 @@ class PatternResolver { } private resolveGlobbingExpression(pattern: string): Promise { - if (isWebUrl(pattern)) { + if (isOnlineFile(pattern)) { return Promise.resolve([pattern]); } else { return glob(pattern).then(files => { diff --git a/src/Sandbox.ts b/src/Sandbox.ts index 353d05bdec..3a2e1bbb74 100644 --- a/src/Sandbox.ts +++ b/src/Sandbox.ts @@ -5,6 +5,7 @@ import { RunResult, StatementMap } from 'stryker-api/test_runner'; import { InputFile, StrykerOptions } from 'stryker-api/core'; import { TestFramework } from 'stryker-api/test_framework'; import { wrapInClosure } from './utils/objectUtils'; +import { isOnlineFile } from './utils/fileUtils'; import IsolatedTestRunnerAdapterFactory from './isolated-runner/IsolatedTestRunnerAdapterFactory'; import IsolatedTestRunnerAdapter from './isolated-runner/IsolatedTestRunnerAdapter'; import IsolatedRunnerOptions from './isolated-runner/IsolatedRunnerOptions'; @@ -64,6 +65,11 @@ export default class Sandbox { } private copyFile(file: InputFile): Promise { + if (isOnlineFile(file.path)) { + this.fileMap[file.path] = file.path; + return Promise.resolve(); + } + const cwd = process.cwd(); const relativePath = file.path.substr(cwd.length); const folderName = StrykerTempFolder.ensureFolderExists(this.workingFolder + path.dirname(relativePath)); diff --git a/src/utils/fileUtils.ts b/src/utils/fileUtils.ts index c85d88d07d..95855a4284 100644 --- a/src/utils/fileUtils.ts +++ b/src/utils/fileUtils.ts @@ -65,7 +65,6 @@ export function glob(expression: string): Promise { }); } - export function readdir(path: string): Promise { return new Promise((resolve, reject) => { fs.readdir(path, (error, files) => { @@ -177,4 +176,8 @@ export function mkdirRecursive(folderName: string) { */ export function importModule(moduleName: string) { require(moduleName); +} + +export function isOnlineFile(path: string): boolean { + return path.indexOf('http://') === 0 || path.indexOf('https://') === 0; } \ No newline at end of file diff --git a/test/unit/SandboxSpec.ts b/test/unit/SandboxSpec.ts index d48f96f5ee..d39452a473 100644 --- a/test/unit/SandboxSpec.ts +++ b/test/unit/SandboxSpec.ts @@ -20,6 +20,7 @@ describe('Sandbox', () => { let testFramework: any; const expectedFileToMutate: InputFile = { path: path.resolve('file1'), mutated: true, included: true }; const notMutatedFile: InputFile = { path: path.resolve('file2'), mutated: false, included: false }; + const onlineFile = 'https://ajax.googleapis.com/ajax/libs/angularjs/1.3.12/angular.js'; const workingFolder = 'random-folder-3'; const expectedTargetFileToMutate = path.join(workingFolder, 'file1'); const expectedTestFrameworkHooksFile = path.join(workingFolder, '___testHooksForStryker.js'); @@ -33,7 +34,8 @@ describe('Sandbox', () => { }; files = [ expectedFileToMutate, - notMutatedFile + notMutatedFile, + { path: onlineFile, mutated: false, included: true } ]; sandbox.stub(StrykerTempFolder, 'createRandomFolder').returns(workingFolder); sandbox.stub(StrykerTempFolder, 'ensureFolderExists').returnsArg(0); @@ -71,6 +73,13 @@ describe('Sandbox', () => { expect(coverageInstrumenter.instrumenterStreamForFile).to.have.been.calledWith(expectedFileToMutate); expect(StrykerTempFolder.copyFile).to.have.been.calledWith(expectedFileToMutate.path, expectedTargetFileToMutate, expectedInstrumenterStream); }); + + it('should not have copied online files', () => { + let expectedBaseFolder = onlineFile.substr(workingFolder.length - 1); // The Sandbox expects all files to be absolute paths. An online file is not an absolute path. + + expect(StrykerTempFolder.ensureFolderExists).to.not.have.been.calledWith(workingFolder + path.dirname(expectedBaseFolder)); + expect(StrykerTempFolder.copyFile).to.not.have.been.calledWith(onlineFile, sinon.match.any, sinon.match.any); + }); }); }); @@ -92,7 +101,8 @@ describe('Sandbox', () => { files: [ { path: expectedTestFrameworkHooksFile, mutated: false, included: true }, { path: expectedTargetFileToMutate, mutated: true, included: true }, - { path: path.join(workingFolder, 'file2'), mutated: false, included: false } + { path: path.join(workingFolder, 'file2'), mutated: false, included: false }, + { path: onlineFile, mutated: false, included: true } ], port: 46, strykerOptions: options, @@ -155,7 +165,8 @@ describe('Sandbox', () => { files: [ { path: path.join(workingFolder, '___testHooksForStryker.js'), mutated: false, included: true }, { path: path.join(workingFolder, 'file1'), mutated: true, included: true }, - { path: path.join(workingFolder, 'file2'), mutated: false, included: false } + { path: path.join(workingFolder, 'file2'), mutated: false, included: false }, + { path: onlineFile, mutated: false, included: true } ], port: 46, strykerOptions: options, diff --git a/test/unit/utils/fileUtilsSpec.ts b/test/unit/utils/fileUtilsSpec.ts new file mode 100644 index 0000000000..6883ec5d26 --- /dev/null +++ b/test/unit/utils/fileUtilsSpec.ts @@ -0,0 +1,50 @@ +import { expect } from 'chai'; +import { isOnlineFile } from '../../../src/utils/fileUtils'; + +describe('fileUtils', () => { + + describe('isOnlineFile', () => { + + describe('returns true', () => { + + it('when provided with http://google.com', () => { + let url = 'http://google.com'; + + let result = isOnlineFile(url); + + expect(result).to.be.true; + }); + + it('when provided with https://google.com', () => { + let url = 'https://google.com'; + + let result = isOnlineFile(url); + + expect(result).to.be.true; + }); + + }); + + describe('returns true', () => { + + it('when provided with http:/google.com', () => { + let url = 'http:/google.com'; + + let result = isOnlineFile(url); + + expect(result).to.be.false; + }); + + it('when provided with google.com', () => { + let url = 'google.com'; + + let result = isOnlineFile(url); + + expect(result).to.be.false; + }); + + }); + + }); + +}); From 2eefab1f486413abcffe850a3e7a3ddd110d8a22 Mon Sep 17 00:00:00 2001 From: Simon de Lang Date: Fri, 30 Dec 2016 07:43:15 +0100 Subject: [PATCH 5/5] refactor(Sandbox): Use an if-else instead of if --- src/Sandbox.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Sandbox.ts b/src/Sandbox.ts index 3a2e1bbb74..dadc50df2b 100644 --- a/src/Sandbox.ts +++ b/src/Sandbox.ts @@ -68,16 +68,16 @@ export default class Sandbox { if (isOnlineFile(file.path)) { this.fileMap[file.path] = file.path; return Promise.resolve(); + } else { + const cwd = process.cwd(); + const relativePath = file.path.substr(cwd.length); + const folderName = StrykerTempFolder.ensureFolderExists(this.workingFolder + path.dirname(relativePath)); + const targetFile = path.join(folderName, path.basename(relativePath)); + this.fileMap[file.path] = targetFile; + const instrumentingStream = this.coverageInstrumenter ? + this.coverageInstrumenter.instrumenterStreamForFile(file) : null; + return StrykerTempFolder.copyFile(file.path, targetFile, instrumentingStream); } - - const cwd = process.cwd(); - const relativePath = file.path.substr(cwd.length); - const folderName = StrykerTempFolder.ensureFolderExists(this.workingFolder + path.dirname(relativePath)); - const targetFile = path.join(folderName, path.basename(relativePath)); - this.fileMap[file.path] = targetFile; - const instrumentingStream = this.coverageInstrumenter ? - this.coverageInstrumenter.instrumenterStreamForFile(file) : null; - return StrykerTempFolder.copyFile(file.path, targetFile, instrumentingStream); } private initializeTestRunner(): void | Promise {