Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(InputFileResolver): Don't exclude all files #210

Merged
merged 9 commits into from
Dec 31, 2016
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
"contributors": [
"Simon de Lang <simondelang@gmail.com>",
"Nico Jansen <jansennico@gmail.com>",
"global <jansennico@gmail.com>",
"Philipp Weissenbacher <philipp.weissenbacher@gmail.com>",
"Jasper Catthoor <jasper.catthoor@gmail.com>",
"Nico Stapelbroek <nstapelbroek@gmail.com>",
"Alex van Assem <avassem@gmail.com>",
"Jeremy Nagel <jeremy.nagel@learnosity.com>",
"Michael Williamson <mike@zwobble.org>"
"Michael Williamson <mike@zwobble.org>",
"Philipp Weissenbacher <philipp.weissenbacher@gmail.com>",
"Alex van Assem <avassem@gmail.com"
],
"license": "Apache-2.0",
"bin": {
Expand Down
5 changes: 3 additions & 2 deletions src/InputFileResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,15 @@ class PatternResolver {
// Start the globbing task for the current descriptor
const globbingTask = this.resolveGlobbingExpression(this.descriptor.pattern)
.then(filePaths => filePaths.map(filePath => this.createInputFile(filePath)));

// If there is a previous globbing expression, resolve that one as well
if (this.previous) {
// If there is a previous globbing expression, resolve that one as well
return Promise.all([this.previous.resolve(), globbingTask]).then(results => {
const previousFiles = results[0];
const currentFiles = results[1];
// If this expression started with a '!', exclude current files
if (this.ignore) {
return previousFiles.filter(previousFile => currentFiles.some(currentFile => previousFile.path !== currentFile.path));
return previousFiles.filter(previousFile => currentFiles.every(currentFile => previousFile.path !== currentFile.path));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does every work better than some. Can you explain that?

Copy link
Member Author

@nicojs nicojs Dec 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an expression starts with a ! (and needs to ignore all matching files), then every file resulting from the current globing expression should not match to any of the current files. every makes sure of that.

For example:

  • previousFiles: ['file1', 'file2', 'expectedFile']
  • currentFiles: ['file1', 'file2', 'somefile']
  • Expected result: 'expectedFile'

For an empty array, every returns true. More info: https://developer.mozilla.org/nl/docs/Web/JavaScript/Reference/Global_Objects/Array/every

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanx for the explanation. De rest of the code seems good.

} else {
// Only add files which were not already added
return previousFiles.concat(currentFiles.filter(currentFile => !previousFiles.some(file => file.path === currentFile.path)));
Expand Down
2 changes: 1 addition & 1 deletion src/utils/fileUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function normalize(files: string[]): void {
export function glob(expression: string): Promise<string[]> {
return new Promise<string[]>((resolve, reject) => {

nodeGlob(expression, (error, matches) => {
nodeGlob(expression, { nodir: true }, (error, matches) => {
if (error) {
reject(error);
} else {
Expand Down
26 changes: 17 additions & 9 deletions test/integration/utils/fileUtilsSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,34 @@ describe('fileUtils', () => {

afterEach(() => sandbox.restore());

describe('should be able to read a file', () => {

it('synchronously', () => {
describe('readFileSync()', () => {
it('should synchronously', () => {
const msg = 'hello 1 2';
sandbox.stub(fs, 'readFileSync', (filename: string, encoding: string) => msg);
const data = fileUtils.readFile('hello.js');
expect(data).to.equal(msg);
});
});

it('should indicate that an existing file exists', () => {
const exists = fileUtils.fileOrFolderExistsSync('src/Stryker.ts');
describe('fileOrFolderExistsSync()', () => {
it('should indicate that an existing file exists', () => {
const exists = fileUtils.fileOrFolderExistsSync('src/Stryker.ts');

expect(exists).to.equal(true);
});
it('should indicate that an non-existing file does not exists', () => {
const exists = fileUtils.fileOrFolderExistsSync('src/Strykerfaefeafe.js');

expect(exists).to.equal(true);
expect(exists).to.equal(false);
});
});

it('should indicate that an non-existing file does not exists', () => {
const exists = fileUtils.fileOrFolderExistsSync('src/Strykerfaefeafe.js');
describe('glob', () => {
it('should resolve files', () =>
expect(fileUtils.glob('testResources/sampleProject/**/*.js')).to.eventually.have.length(9));

expect(exists).to.equal(false);
it('should not resolve to directories', () =>
expect(fileUtils.glob('testResources/vendor/**/*.js')).to.eventually.have.length(1));
});

});
17 changes: 10 additions & 7 deletions test/unit/InputFileResolverSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,24 +161,27 @@ describe('InputFileResolver', () => {

it('should not exclude files added using an input file descriptor', () =>
expect(new InputFileResolver([], ['file2', { pattern: '!file2' }]).resolve()).to.eventually.deep.equal(fileDescriptors(['/file2.js'])));

it('should not exlude files when the globbing expression results in an empty array', () =>
expect(new InputFileResolver([], ['file2', '!does/not/exist']).resolve()).to.eventually.deep.equal(fileDescriptors(['/file2.js'])));
});

describe('when provided duplicate files', () => {

it('should deduplicate files that occur more than once', () =>
it('should deduplicate files that occur more than once', () =>
expect(new InputFileResolver([], ['file2', 'file2']).resolve()).to.eventually.deep.equal(fileDescriptors(['/file2.js'])));
it('should deduplicate files that previously occured in a wildcard expression', () =>

it('should deduplicate files that previously occured in a wildcard expression', () =>
expect(new InputFileResolver([], ['file*', 'file2']).resolve()).to.eventually.deep.equal(fileDescriptors(['/file1.js', '/file2.js', '/file3.js'])));
it('should order files by expression order', () =>

it('should order files by expression order', () =>
expect(new InputFileResolver([], ['file2', 'file*']).resolve()).to.eventually.deep.equal(fileDescriptors(['/file2.js', '/file1.js', '/file3.js'])));

});

describe('with url as file pattern', () => {
it('should pass through the web urls without globbing', () => {
return new InputFileResolver([], ['http://www', {pattern: 'https://ok'}])
return new InputFileResolver([], ['http://www', { pattern: 'https://ok' }])
.resolve()
.then(() => expect(fileUtils.glob).to.not.have.been.called);
});
Expand All @@ -188,7 +191,7 @@ describe('InputFileResolver', () => {
});

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".');
expect(() => new InputFileResolver([], [{ pattern: 'http://www', mutated: true }])).throws('Cannot mutate web url "http://www".');
});
});

Expand Down
1 change: 0 additions & 1 deletion test/unit/reporters/ProgressReporterSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ describe('ProgressReporter', () => {
`[:noCoverage :noCoverageLabel] ` +
`[:timeout :timeoutLabel] ` +
`[:error :errorLabel]`;
let progressBarOptions: any;

beforeEach(() => {
sut = new ProgressReporter();
Expand Down
Empty file.