Skip to content

Commit

Permalink
Utils unit tests
Browse files Browse the repository at this point in the history
Enhance error message

Removed deprecated option from README

Convert all outstanding changes (yarn and package)

Adjusted NPM unit tests

Use spawn in any npm commands

Use spawn instead of exec

Fix unit test description

Do not swallow errors in case npm ls fails without a proper stdout
Frank Schmid committed Apr 25, 2018
1 parent 5cf2479 commit b379de9
Showing 12 changed files with 380 additions and 251 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -61,7 +61,6 @@ custom:
webpackConfig: 'webpack.config.js' # Name of webpack configuration file
includeModules: false # Node modules configuration for packaging
packager: 'npm' # Packager that will be used to package your external modules
packExternalModulesMaxBuffer: 200 * 1024 # Size of stdio buffers for spawned child processes
```

### Webpack configuration file
6 changes: 0 additions & 6 deletions lib/Configuration.js
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ const DefaultConfig = {
includeModules: false,
packager: 'npm',
packagerOptions: {},
packExternalModulesMaxBuffer: 200 * 1024,
config: null
};

@@ -32,7 +31,6 @@ class Configuration {
this._hasLegacyConfig = true;
}
if (custom.packExternalModulesMaxBuffer) {
this._config.packExternalModulesMaxBuffer = custom.packExternalModulesMaxBuffer;
this._hasLegacyConfig = true;
}
if (_.isString(custom.webpack)) {
@@ -55,10 +53,6 @@ class Configuration {
return this._config.includeModules;
}

get packExternalModulesMaxBuffer() {
return this._config.packExternalModulesMaxBuffer;
}

get packager() {
return this._config.packager;
}
10 changes: 0 additions & 10 deletions lib/Configuration.test.js
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ describe('Configuration', () => {
includeModules: false,
packager: 'npm',
packagerOptions: {},
packExternalModulesMaxBuffer: 200 * 1024,
config: null
};
});
@@ -43,12 +42,6 @@ describe('Configuration', () => {
expect(config).to.have.a.property('includeModules').that.deep.equals(testCustom.webpackIncludeModules);
});

it('should use custom.packExternalModulesMaxBuffer', () => {
const testCustom = { packExternalModulesMaxBuffer: 4711 };
const config = new Configuration(testCustom);
expect(config).to.have.a.property('packExternalModulesMaxBuffer').that.equals(4711);
});

it('should use custom.webpack as string', () => {
const testCustom = { webpack: 'myWebpackFile.js' };
const config = new Configuration(testCustom);
@@ -73,7 +66,6 @@ describe('Configuration', () => {
includeModules: { forceInclude: ['mod1'] },
packager: 'npm',
packagerOptions: {},
packExternalModulesMaxBuffer: 200 * 1024,
config: null
});
});
@@ -93,7 +85,6 @@ describe('Configuration', () => {
includeModules: { forceInclude: ['mod1'] },
packager: 'npm',
packagerOptions: {},
packExternalModulesMaxBuffer: 200 * 1024,
config: null
});
});
@@ -112,7 +103,6 @@ describe('Configuration', () => {
includeModules: { forceInclude: ['mod1'] },
packager: 'npm',
packagerOptions: {},
packExternalModulesMaxBuffer: 200 * 1024,
config: null
});
});
9 changes: 4 additions & 5 deletions lib/packExternalModules.js
Original file line number Diff line number Diff line change
@@ -195,9 +195,8 @@ module.exports = {
.then(packager => {
// Get first level dependency graph
this.options.verbose && this.serverless.cli.log(`Fetch dependency graph from ${packageJsonPath}`);
const maxExecBufferSize = this.configuration.packExternalModulesMaxBuffer;

return packager.getProdDependencies(path.dirname(packageJsonPath), 1, maxExecBufferSize)
return packager.getProdDependencies(path.dirname(packageJsonPath), 1)
.then(dependencyGraph => {
const problems = _.get(dependencyGraph, 'problems', []);
if (this.options.verbose && !_.isEmpty(problems)) {
@@ -271,7 +270,7 @@ module.exports = {
.then(() => {
const start = _.now();
this.serverless.cli.log('Packing external modules: ' + compositeModules.join(', '));
return packager.install(compositeModulePath, maxExecBufferSize, this.configuration.packagerOptions)
return packager.install(compositeModulePath, this.configuration.packagerOptions)
.then(() => this.options.verbose && this.serverless.cli.log(`Package took [${_.now() - start} ms]`))
.return(stats.stats);
})
@@ -320,13 +319,13 @@ module.exports = {
.then(() => {
// Prune extraneous packages - removes not needed ones
const startPrune = _.now();
return packager.prune(modulePath, maxExecBufferSize, this.configuration.packagerOptions)
return packager.prune(modulePath, this.configuration.packagerOptions)
.tap(() => this.options.verbose && this.serverless.cli.log(`Prune: ${modulePath} [${_.now() - startPrune} ms]`));
})
.then(() => {
// Prune extraneous packages - removes not needed ones
const startRunScripts = _.now();
return packager.runScripts(modulePath, maxExecBufferSize, _.keys(packageScripts))
return packager.runScripts(modulePath, _.keys(packageScripts))
.tap(() => this.options.verbose && this.serverless.cli.log(`Run scripts: ${modulePath} [${_.now() - startRunScripts} ms]`));
});
})
6 changes: 3 additions & 3 deletions lib/packagers/index.js
Original file line number Diff line number Diff line change
@@ -7,11 +7,11 @@
* interface Packager {
*
* static get lockfileName(): string;
* static getProdDependencies(cwd: string, depth: number = 1, maxExecBufferSize = undefined): BbPromise<Object>;
* static getProdDependencies(cwd: string, depth: number = 1): BbPromise<Object>;
* static rebaseLockfile(pathToPackageRoot: string, lockfile: Object): void;
* static install(cwd: string, maxExecBufferSize = undefined): BbPromise<void>;
* static install(cwd: string): BbPromise<void>;
* static prune(cwd: string): BbPromise<void>;
* static runScripts(cwd: string, maxExecBufferSize, scriptNames): BbPromise<void>;
* static runScripts(cwd: string, scriptNames): BbPromise<void>;
*
* }
*/
93 changes: 47 additions & 46 deletions lib/packagers/npm.js
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@

const _ = require('lodash');
const BbPromise = require('bluebird');
const childProcess = require('child_process');
const Utils = require('../utils');

class NPM {
static get lockfileName() { // eslint-disable-line lodash/prefer-constant
@@ -16,39 +16,44 @@ class NPM {
return true;
}

static getProdDependencies(cwd, depth, maxExecBufferSize) {
static getProdDependencies(cwd, depth) {
// Get first level dependency graph
const command = `npm ls -prod -json -depth=${depth || 1}`; // Only prod dependencies
const command = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
const args = [
'ls',
'-prod', // Only prod dependencies
'-json',
`-depth=${depth || 1}`
];

const ignoredNpmErrors = [
{ npmError: 'extraneous', log: false },
{ npmError: 'missing', log: false },
{ npmError: 'peer dep missing', log: true },
];

return BbPromise.fromCallback(cb => {
childProcess.exec(command, {
cwd: cwd,
maxBuffer: maxExecBufferSize,
encoding: 'utf8'
}, (err, stdout, stderr) => {
if (err) {
// Only exit with an error if we have critical npm errors for 2nd level inside
const errors = _.split(stderr, '\n');
const failed = _.reduce(errors, (failed, error) => {
if (failed) {
return true;
}
return !_.isEmpty(error) && !_.some(ignoredNpmErrors, ignoredError => _.startsWith(error, `npm ERR! ${ignoredError.npmError}`));
}, false);

return Utils.spawnProcess(command, args, {
cwd: cwd
})
.catch(err => {
if (err instanceof Utils.SpawnError) {
// Only exit with an error if we have critical npm errors for 2nd level inside
const errors = _.split(err.stderr, '\n');
const failed = _.reduce(errors, (failed, error) => {
if (failed) {
return cb(err);
return true;
}
return !_.isEmpty(error) && !_.some(ignoredNpmErrors, ignoredError => _.startsWith(error, `npm ERR! ${ignoredError.npmError}`));
}, false);

if (!failed && !_.isEmpty(err.stdout)) {
return BbPromise.resolve({ stdout: err.stdout });
}
return cb(null, stdout);
});
}

return BbPromise.reject(err);
})
.then(processOutput => processOutput.stdout)
.then(depJson => BbPromise.try(() => JSON.parse(depJson)));
}

@@ -73,36 +78,32 @@ class NPM {
}
}

static install(cwd, maxExecBufferSize) {
return BbPromise.fromCallback(cb => {
childProcess.exec('npm install', {
cwd: cwd,
maxBuffer: maxExecBufferSize,
encoding: 'utf8'
}, cb);
})
static install(cwd) {
const command = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
const args = ['install'];

return Utils.spawnProcess(command, args, { cwd })
.return();
}

static prune(cwd, maxExecBufferSize) {
return BbPromise.fromCallback(cb => {
childProcess.exec('npm prune', {
cwd: cwd,
maxBuffer: maxExecBufferSize,
encoding: 'utf8'
}, cb);
})
static prune(cwd) {
const command = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
const args = ['prune'];

return Utils.spawnProcess(command, args, { cwd })
.return();
}

static runScripts(cwd, maxExecBufferSize, scriptNames) {
return BbPromise.mapSeries(scriptNames, scriptName => BbPromise.fromCallback(cb => {
childProcess.exec(`npm run ${scriptName}`, {
cwd: cwd,
maxBuffer: maxExecBufferSize,
encoding: 'utf8'
}, cb);
}))
static runScripts(cwd, scriptNames) {
const command = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
return BbPromise.mapSeries(scriptNames, scriptName => {
const args = [
'run',
scriptName
];

return Utils.spawnProcess(command, args, { cwd });
})
.return();
}
}
Loading

0 comments on commit b379de9

Please sign in to comment.