diff --git a/lib/Configuration.js b/lib/Configuration.js index ca9a523eb..adb07b86d 100644 --- a/lib/Configuration.js +++ b/lib/Configuration.js @@ -15,7 +15,7 @@ const DefaultConfig = { packagerOptions: {}, keepOutputDirectory: false, config: null, - concurrency: undefined + concurrency: Infinity }; class Configuration { @@ -38,6 +38,21 @@ class Configuration { } } + // Concurrency may be passed via CLI, e.g. + // custom: + // webpack: + // concurrency: ${opt:compile-concurrency, 7} + // In this case it is typed as a string and we have to validate it + if (this._config.concurrency !== undefined) { + this._config.concurrency = Number(this._config.concurrency); + if (isNaN(this._config.concurrency) || this._config.concurrency < 1) { + throw new Error('concurrency option must be a positive number'); + } + } else if (this._config.serializedCompile === true) { + // Backwards compatibility with serializedCompile setting + this._config.concurrency = 1; + } + // Set defaults for all missing properties _.defaults(this._config, DefaultConfig); } @@ -79,11 +94,7 @@ class Configuration { } get concurrency() { - if (this._config.concurrency !== undefined) { - return this._config.concurrency; - } else if (this._config.serializedCompile === true) { - return 1; - } + return this._config.concurrency; } toJSON() { diff --git a/lib/Configuration.test.js b/lib/Configuration.test.js index 1258fc756..90410e58c 100644 --- a/lib/Configuration.test.js +++ b/lib/Configuration.test.js @@ -20,7 +20,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - concurrency: undefined + concurrency: Infinity }; }); @@ -80,7 +80,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - concurrency: undefined + concurrency: Infinity }); }); }); @@ -101,7 +101,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - concurrency: undefined + concurrency: Infinity }); }); @@ -121,8 +121,42 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - concurrency: undefined + concurrency: Infinity }); }); + + it('should accept a numeric string as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: '3' + } + }; + const config = new Configuration(testCustom); + expect(config._config.concurrency).to.equal(3); + }); + + it('should not accept an invalid string as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: '3abc' + } + }; + expect(() => new Configuration(testCustom)).throws(); + }); + + it('should not accept a non-positive number as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: 0 + } + }; + expect(() => new Configuration(testCustom)).throws(); + }); }); }); diff --git a/lib/compile.js b/lib/compile.js index 40e6a298f..73902ab56 100644 --- a/lib/compile.js +++ b/lib/compile.js @@ -51,7 +51,11 @@ module.exports = { const configs = ensureArray(this.webpackConfig); const logStats = getStatsLogger(configs[0].stats, this.serverless.cli.consoleLog); - const concurrency = this.concurrency === undefined ? Infinity : this.concurrency; + + if (!this.configuration) { + return BbPromise.reject('Missing plugin configuration'); + } + const concurrency = this.configuration.concurrency; return webpackConcurrentCompile(configs, logStats, concurrency) .then(stats => { diff --git a/lib/validate.js b/lib/validate.js index 2b3f29432..55afbbfbb 100644 --- a/lib/validate.js +++ b/lib/validate.js @@ -179,8 +179,9 @@ module.exports = { // In case of individual packaging we have to create a separate config for each function if (_.has(this.serverless, 'service.package') && this.serverless.service.package.individually) { this.multiCompile = true; - this.concurrency = this.configuration.concurrency; - this.options.verbose && this.serverless.cli.log(`Using ${this.concurrency ? 'concurrent' : 'multi'}-compile (individual packaging)`); + this.options.verbose && this.serverless.cli.log( + `Using ${this.configuration.concurrency !== Infinity ? 'concurrent' : 'multi'}-compile (individual packaging)` + ); if (this.webpackConfig.entry && !_.isEqual(this.webpackConfig.entry, entries)) { return BbPromise.reject( diff --git a/tests/compile.test.js b/tests/compile.test.js index 957583a85..b9104b5b2 100644 --- a/tests/compile.test.js +++ b/tests/compile.test.js @@ -65,6 +65,7 @@ describe('compile', () => { it('should compile with webpack from a context configuration', () => { const testWebpackConfig = 'testconfig'; module.webpackConfig = testWebpackConfig; + module.configuration = { concurrency: Infinity }; return expect(module.compile()).to.be.fulfilled.then(() => { expect(webpackMock).to.have.been.calledWith(testWebpackConfig); expect(webpackMock.compilerMock.run).to.have.been.calledOnce; @@ -72,8 +73,16 @@ describe('compile', () => { }); }); + it('should fail if configuration is missing', () => { + const testWebpackConfig = 'testconfig'; + module.webpackConfig = testWebpackConfig; + module.configuration = undefined; + return expect(module.compile()).to.be.rejectedWith('Missing plugin configuration'); + }); + it('should fail if there are compilation errors', () => { module.webpackConfig = 'testconfig'; + module.configuration = { concurrency: Infinity }; // We stub errors here. It will be reset again in afterEach() sandbox.stub(webpackMock.statsMock.compilation, 'errors').value(['error']); return expect(module.compile()).to.be.rejectedWith(/compilation error/); @@ -97,6 +106,7 @@ describe('compile', () => { }; module.webpackConfig = testWebpackConfig; module.multiCompile = true; + module.configuration = { concurrency: Infinity }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, multiStats); return expect(module.compile()).to.be.fulfilled.then(() => { @@ -124,7 +134,7 @@ describe('compile', () => { }; module.webpackConfig = testWebpackConfig; module.multiCompile = true; - module.concurrency = 1; + module.configuration = { concurrency: 1 }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, multiStats); return expect(module.compile()).to.be.fulfilled.then(() => { @@ -150,6 +160,7 @@ describe('compile', () => { }; module.webpackConfig = testWebpackConfig; + module.configuration = { concurrency: Infinity }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, mockStats); return expect(module.compile())