From c627c71c88bb77a3f351ba34c49043d3ba0f08a2 Mon Sep 17 00:00:00 2001 From: Rostislav Shtanko Date: Fri, 26 Aug 2016 14:52:10 +0300 Subject: [PATCH] fix up sets logic --- lib/config/options.js | 5 +- lib/test-reader/index.js | 17 +++- lib/test-reader/set-collection.js | 21 +++-- lib/test-reader/set.js | 27 ------- lib/test-reader/test-set.js | 35 +++++++++ test/unit/config-options/sets.test.js | 83 +++++++++++--------- test/unit/test-reader/index.test.js | 11 +++ test/unit/test-reader/set-collection.test.js | 36 ++++++--- test/unit/test-reader/set.test.js | 15 +++- 9 files changed, 159 insertions(+), 91 deletions(-) delete mode 100644 lib/test-reader/set.js create mode 100644 lib/test-reader/test-set.js diff --git a/lib/config/options.js b/lib/config/options.js index b126ef9ab..387f8a330 100644 --- a/lib/config/options.js +++ b/lib/config/options.js @@ -87,7 +87,6 @@ module.exports = root( sets: map(section({ files: option({ - defaultValue: 'gemini', validate: function(value) { if (!_.isArray(value) && !_.isString(value)) { throw new GeminiError('"sets.files" must be an array or string'); @@ -116,9 +115,7 @@ module.exports = root( } } }) - }), { - all: {} // Use `all` set with default values if no set specified in config - }), + })), browsers: map(section(browserOptions.getPerBrowser())) })) diff --git a/lib/test-reader/index.js b/lib/test-reader/index.js index 561c42e90..49b06ab22 100644 --- a/lib/test-reader/index.js +++ b/lib/test-reader/index.js @@ -1,8 +1,11 @@ 'use strict'; -const q = require('q'); +const path = require('path'); +const _ = require('lodash'); const globExtra = require('glob-extra'); +const q = require('q'); + const SetCollection = require('./set-collection'); const Suite = require('../suite'); const testsApi = require('../tests-api'); @@ -24,10 +27,20 @@ const loadSuites = (sets, emitter) => { return rootSuite; }; +const filesExist = (configSets, cliPaths) => { + return !_.isEmpty(configSets) || !_.isEmpty(cliPaths); +}; + +const getGeminiPath = (projectRoot) => path.resolve(projectRoot, 'gemini'); + module.exports = (cli, config, emitter) => { + const files = filesExist(config.sets, cli.paths) + ? cli.paths + : [getGeminiPath(config.system.projectRoot)]; + return q.all([ SetCollection.create(config, cli.sets), - globExtra.expandPaths(cli.paths, {formats: ['.js']}) + globExtra.expandPaths(files, {formats: ['.js']}) ]) .spread((sets, paths) => { sets.filterFiles(paths); diff --git a/lib/test-reader/set-collection.js b/lib/test-reader/set-collection.js index b3945da71..bbca1e02b 100644 --- a/lib/test-reader/set-collection.js +++ b/lib/test-reader/set-collection.js @@ -5,11 +5,15 @@ const q = require('q'); const GeminiError = require('../errors/gemini-error'); const globExtra = require('glob-extra'); -const Set = require('./set'); +const TestSet = require('./test-set'); module.exports = class SetCollection { static create(config, sets) { - const filteredSets = SetCollection._filter(config.sets, sets); + let filteredSets = SetCollection._filter(config.sets, sets); + + if (_.isEmpty(filteredSets)) { + filteredSets = [{files: {}, browsers: config.getBrowserIds()}]; + } return SetCollection._expand(filteredSets, config.system.projectRoot) .then((sets) => new SetCollection(sets)); @@ -29,10 +33,13 @@ module.exports = class SetCollection { const unknownSets = _.difference(setsToUse, _.keys(sets)); if (!_.isEmpty(unknownSets)) { - throw new GeminiError( - `No such sets: ${unknownSets.join(', ')}. Use one of the sets, specified ` + - `in config file: ${_.keys(sets).join(', ')}` - ); + let error = `No such sets: ${unknownSets.join(', ')}.`; + + if (!_.isEmpty(sets)) { + error += ` Use one of the sets, specified in config file: ${_.keys(sets).join(', ')}`; + } + + throw new GeminiError(error); } } @@ -47,7 +54,7 @@ module.exports = class SetCollection { } constructor(sets) { - this._sets = _.map(sets, (set) => Set.create(set)); + this._sets = _.map(sets, (set) => TestSet.create(set)); } filterFiles(files) { diff --git a/lib/test-reader/set.js b/lib/test-reader/set.js deleted file mode 100644 index 5f8b70446..000000000 --- a/lib/test-reader/set.js +++ /dev/null @@ -1,27 +0,0 @@ -'use strict'; - -const _ = require('lodash'); - -module.exports = class Set { - static create(set) { - return new Set(set); - } - - constructor(set) { - this._set = set; - } - - getFiles() { - return this._set.files; - } - - getBrowsers(path) { - return _.includes(this._set.files, path) ? this._set.browsers : []; - } - - filterFiles(files) { - if (!_.isEmpty(files)) { - this._set.files = _.intersection(files, this._set.files); - } - } -}; diff --git a/lib/test-reader/test-set.js b/lib/test-reader/test-set.js new file mode 100644 index 000000000..04ded4ddc --- /dev/null +++ b/lib/test-reader/test-set.js @@ -0,0 +1,35 @@ +'use strict'; + +const _ = require('lodash'); + +module.exports = class TestSet { + static create(set) { + return new TestSet(set); + } + + constructor(set) { + this._set = set; + } + + getFiles() { + return this._set.files; + } + + getBrowsers(path) { + return _.includes(this._set.files, path) ? this._set.browsers : []; + } + + filterFiles(files) { + this._set.files = this._getFilteredFiles(files); + } + + _getFilteredFiles(files) { + if (_.isEmpty(this._set.files)) { + return files; + } + + return _.isEmpty(files) + ? this._set.files + : _.intersection(files, this._set.files); + } +}; diff --git a/test/unit/config-options/sets.test.js b/test/unit/config-options/sets.test.js index 77d22993a..dd15245bd 100644 --- a/test/unit/config-options/sets.test.js +++ b/test/unit/config-options/sets.test.js @@ -1,12 +1,14 @@ 'use strict'; -var parser = require('lib/config/options'), - GeminiError = require('lib/errors/gemini-error'), - _ = require('lodash'); - -describe('config.sets', function() { - /// - function createConfig(opts) { - var REQUIRED_OPTIONS = { + +const MissingOptionError = require('gemini-configparser').MissingOptionError; +const _ = require('lodash'); + +const parser = require('lib/config/options'); +const GeminiError = require('lib/errors/gemini-error'); + +describe('config.sets', () => { + const createConfig = (opts) => { + const REQUIRED_OPTIONS = { system: { projectRoot: '/some/path' }, @@ -14,28 +16,28 @@ describe('config.sets', function() { desiredCapabilities: {} }; - var options = _.extend({}, REQUIRED_OPTIONS, opts); + const options = _.extend({}, REQUIRED_OPTIONS, opts); return parser({ options: options, env: {}, argv: [] }); - } + }; - describe('files', function() { - it('should be `gemini` by default', function() { - var config = createConfig({ - sets: { - someSet: {} - } - }); - - assert.deepEqual(config.sets.someSet.files, ['gemini']); + describe('files', () => { + it('should throw an error if files are not specified', () => { + assert.throws(() => { + createConfig({ + sets: { + someSet: {} + } + }); + }, MissingOptionError); }); - it('should convert string to array of strings', function() { - var config = createConfig({ + it('should convert string to array of strings', () => { + const config = createConfig({ sets: { someSet: { files: 'some/path' @@ -46,8 +48,8 @@ describe('config.sets', function() { assert.deepEqual(config.sets.someSet.files, ['some/path']); }); - it('should not accept non-string arrays', function() { - assert.throws(function() { + it('should not accept non-string arrays', () => { + assert.throws(() => { createConfig({ sets: { someSet: { @@ -58,8 +60,8 @@ describe('config.sets', function() { }, GeminiError); }); - it('should accept array with strings', function() { - var config = createConfig({ + it('should accept array with strings', () => { + const config = createConfig({ sets: { someSet: { files: [ @@ -77,26 +79,29 @@ describe('config.sets', function() { }); }); - describe('browsers', function() { - it('should contain all browsers by default', function() { + describe('browsers', () => { + it('should contain all browsers by default', () => { var config = createConfig({ browsers: { b1: {}, b2: {} }, sets: { - someSet: {} + someSet: { + files: ['some/path'] + } } }); assert.deepEqual(config.sets.someSet.browsers, ['b1', 'b2']); }); - it('should not accept non-arrays', function() { - assert.throws(function() { + it('should not accept non-arrays', () => { + assert.throws(() => { createConfig({ sets: { someSet: { + files: ['some/path'], browsers: 'something' } } @@ -104,8 +109,8 @@ describe('config.sets', function() { }, GeminiError); }); - it('should not accept unknown browsers', function() { - assert.throws(function() { + it('should not accept unknown browsers', () => { + assert.throws(() => { createConfig({ browsers: { b1: {}, @@ -113,6 +118,7 @@ describe('config.sets', function() { }, sets: { someSet: { + files: ['some/path'], browsers: ['b3'] } } @@ -120,17 +126,19 @@ describe('config.sets', function() { }, GeminiError); }); - it('should accept configured browsers', function() { - var config = createConfig({ + it('should accept configured browsers', () => { + const config = createConfig({ browsers: { b1: {}, b2: {} }, sets: { set1: { + files: ['some/path'], browsers: ['b1'] }, set2: { + files: ['other/path'], browsers: ['b2'] } } @@ -141,15 +149,14 @@ describe('config.sets', function() { }); }); - it('should have `all` set with default values if no set specified', function() { - var config = createConfig({ + it('should not have default set if sets are not specified', () => { + const config = createConfig({ browsers: { b1: {}, b2: {} } }); - assert.deepEqual(config.sets.all.files, ['gemini']); - assert.deepEqual(config.sets.all.browsers, ['b1', 'b2']); + assert.deepEqual(config.sets, {}); }); }); diff --git a/test/unit/test-reader/index.test.js b/test/unit/test-reader/index.test.js index 90089962c..0518a7ae8 100644 --- a/test/unit/test-reader/index.test.js +++ b/test/unit/test-reader/index.test.js @@ -100,6 +100,17 @@ describe('test-reader', () => { }); }); + it('should use gemini folder if sets are not specified in config and paths are not passed', () => { + const config = { + getBrowserIds: () => [] + }; + + globExtra.expandPaths.withArgs(['/root/gemini']).returns(q(['/root/gemini/file.js'])); + + return readTests_({config}) + .then(() => assert.calledWith(utils.requireWithNoCache, '/root/gemini/file.js')); + }); + it('should load suites related to sets from config', () => { const config = { sets: { diff --git a/test/unit/test-reader/set-collection.test.js b/test/unit/test-reader/set-collection.test.js index 9b0e1bc3a..3606370cd 100644 --- a/test/unit/test-reader/set-collection.test.js +++ b/test/unit/test-reader/set-collection.test.js @@ -4,7 +4,7 @@ const _ = require('lodash'); const q = require('q'); const globExtra = require('glob-extra'); const SetCollection = require('lib/test-reader/set-collection'); -const Set = require('lib/test-reader/set'); +const TestSet = require('lib/test-reader/test-set'); describe('set-collection', () => { const sandbox = sinon.sandbox.create(); @@ -20,6 +20,7 @@ describe('set-collection', () => { const mkConfigStub = (opts) => { return _.defaults(opts || {}, { sets: opts.sets || {}, + getBrowserIds: opts.getBrowserIds, system: { projectRoot: '/root' } @@ -32,6 +33,21 @@ describe('set-collection', () => { afterEach(() => sandbox.restore()); + describe('sets are not specified in config', () => { + it('should throw an error if an unknown set was passed', () => { + assert.throws(() => SetCollection.create(mkConfigStub(), ['unknown-set'], /unknown-set/)); + }); + + it('should create new set with empty files and browsers from config', () => { + sandbox.stub(TestSet, 'create').returns(mkSetStub()); + + const getBrowserIds = sandbox.stub().returns(['b1', 'b2']); + + return SetCollection.create(mkConfigStub({getBrowserIds})) + .then(() => assert.calledWith(TestSet.create, {files: [], browsers: ['b1', 'b2']})); + }); + }); + it('should create collection for specified sets', () => { const config = mkConfigStub({ sets: { @@ -40,14 +56,14 @@ describe('set-collection', () => { } }); - sandbox.stub(Set, 'create').returns(mkSetStub()); + sandbox.stub(TestSet, 'create').returns(mkSetStub()); globExtra.expandPaths.withArgs(['some/files']).returns(q(['some/files/file.js'])); return SetCollection.create(config, ['set1']) .then(() => { - assert.calledOnce(Set.create); - assert.calledWith(Set.create, {files: ['some/files/file.js']}); + assert.calledOnce(TestSet.create); + assert.calledWith(TestSet.create, {files: ['some/files/file.js']}); }); }); @@ -77,12 +93,12 @@ describe('set-collection', () => { .withArgs(['some/files']).returns(q(['some/files/file1.js'])) .withArgs(['other/files']).returns(q(['other/files/file2.js'])); - sandbox.stub(Set, 'create').returns(mkSetStub()); + sandbox.stub(TestSet, 'create').returns(mkSetStub()); return SetCollection.create(config) .then(() => { - assert.calledWith(Set.create, {files: ['some/files/file1.js']}); - assert.calledWith(Set.create, {files: ['other/files/file2.js']}); + assert.calledWith(TestSet.create, {files: ['some/files/file1.js']}); + assert.calledWith(TestSet.create, {files: ['other/files/file2.js']}); }); }); @@ -94,16 +110,16 @@ describe('set-collection', () => { } }); - assert.throws(() => SetCollection.create(config, ['set3']), /set3(.+) set1, set2/); + assert.throws(() => SetCollection.create(config, ['unknown-set']), /unknown-set(.+) set1, set2/); }); - it('should filter sets files by passed files', () => { + it('should filter passed files if sets are specified in config', () => { const sets = { set1: mkSetStub(), set2: mkSetStub() }; - sandbox.stub(Set, 'create') + sandbox.stub(TestSet, 'create') .onFirstCall().returns(sets.set1) .onSecondCall().returns(sets.set2); diff --git a/test/unit/test-reader/set.test.js b/test/unit/test-reader/set.test.js index 5471afc04..025201a81 100644 --- a/test/unit/test-reader/set.test.js +++ b/test/unit/test-reader/set.test.js @@ -1,12 +1,12 @@ 'use strict'; -const Set = require('lib/test-reader/set'); +const TestSet = require('lib/test-reader/test-set'); -describe('Set', () => { +describe('TestSet', () => { let set; beforeEach(() => { - set = new Set({ + set = new TestSet({ files: ['some/path/file.js'], browsers: ['bro1', 'bro2'] }); @@ -46,5 +46,14 @@ describe('Set', () => { assert.deepEqual(files, ['some/path/file.js']); }); + + it('should return passed files if set files are empty', () => { + set = new TestSet({files: []}); + set.filterFiles(['some/path/file.js']); + + const files = set.getFiles(); + + assert.deepEqual(files, ['some/path/file.js']); + }); }); });