Use ci instead of install when installing packages (#502)

* Use `ci` instead of `install` when installing packages

* Update strings to say `npm ci` instead of `npm install`

* Only use `ci` for installing if `package-lock.json` exists

* Copy `package-lock.json` even if excluded by `excludeGlobs`

* Use `isFalse` instead of `isNotTrue`

* Break out `ci` check into `_shouldUseNpmCi` method

* Use `_shouldUseNpmCi` to log the expected command used to install w/ npm

* Adjust test names

* Use `function` instead of arrow function to ensure proper `this`

* Return in `beforeEach` for `when package-lock.json does not exist` block

* Hide `file` in `.bin` so that newer versions of `npm` don't remove it when testing

* Remove unneeded `fs.mkdirSync` call

bin/node-lambda

@@ -93,7 +93,7 @@ program
  .option('-R, --retentionInDays [AWS_LOGS_RETENTION_IN_DAYS]', 'CloudWatchLogs retentionInDays settings',
  AWS_LOGS_RETENTION_IN_DAYS)
  .option('-G, --sourceDirectory [SRC_DIRECTORY]', 'Path to lambda source Directory (e.g. "./some-lambda")', SRC_DIRECTORY)
- .option('-I, --dockerImage [DOCKER_IMAGE]', 'Docker image for npm install', DOCKER_IMAGE)
+ .option('-I, --dockerImage [DOCKER_IMAGE]', 'Docker image for npm ci', DOCKER_IMAGE)
  .option('-f, --configFile [CONFIG_FILE]',
  'Path to file holding secret environment variables (e.g. "deploy.env")', CONFIG_FILE)
  .option('-S, --eventSourceFile [EVENT_SOURCE_FILE]',
@@ -112,7 +112,7 @@ program
  .alias('zip')
  .description('Create zipped package for Amazon Lambda deployment')
  .option('-A, --packageDirectory [PACKAGE_DIRECTORY]', 'Local Package Directory', PACKAGE_DIRECTORY)
- .option('-I, --dockerImage [DOCKER_IMAGE]', 'Docker image for npm install', DOCKER_IMAGE)
+ .option('-I, --dockerImage [DOCKER_IMAGE]', 'Docker image for npm ci', DOCKER_IMAGE)
  .option('-n, --functionName [AWS_FUNCTION_NAME]', 'Lambda FunctionName', AWS_FUNCTION_NAME)
  .option('-H, --handler [AWS_HANDLER]', 'Lambda Handler {index.handler}', AWS_HANDLER)
  .option('-e, --environment [AWS_ENVIRONMENT]', 'Choose environment {dev, staging, production}',

lib/main.js

@@ -321,8 +321,8 @@ so you can easily test run multiple events.
  const options = {
  dereference: true, // same meaning as `-L` of `rsync` command
  filter: (src, dest) => {
- if (!program.prebuiltDirectory && src === 'package.json') {
- // include package.json unless prebuiltDirectory is set
+ if (!program.prebuiltDirectory && ['package.json', 'package-lock.json'].includes(src)) {
+ // include package.json & package-lock.json unless prebuiltDirectory is set
  return true
  }
 
@@ -344,7 +344,15 @@ so you can easily test run multiple events.
  })
  }
 
+ _shouldUseNpmCi (codeDirectory) {
+ return fs.existsSync(path.join(codeDirectory, 'package-lock.json'))
+ }
+
  _npmInstall (program, codeDirectory) {
+ const installCommand = this._shouldUseNpmCi(codeDirectory)
+ ? 'ci'
+ : 'install'
+
  const dockerBaseOptions = [
  'run', '--rm',
  '-v', `${fs.realpathSync(codeDirectory)}:/var/task`,
@@ -356,13 +364,13 @@ so you can easily test run multiple events.
  dockerVolumesOptions = dockerVolumesOptions.concat(['-v', volume])
  })
 
- const dockerCommand = [program.dockerImage, 'npm', '-s', 'install', '--production']
+ const dockerCommand = [program.dockerImage, 'npm', '-s', installCommand, '--production']
 
  const dockerOptions = dockerBaseOptions.concat(dockerVolumesOptions).concat(dockerCommand)
 
  const npmInstallBaseOptions = [
  '-s',
- 'install',
+ installCommand,
  '--production',
  '--prefix', codeDirectory
  ]
@@ -628,7 +636,7 @@ they may not work as expected in the Lambda environment.
  if (program.keepNodeModules) {
  return Promise.resolve()
  } else {
- console.log('=> Running npm install --production')
+ console.log(`=> Running npm ${this._shouldUseNpmCi(codeDirectory) ? 'ci' : 'install'} --production`)
  return this._npmInstall(program, codeDirectory)
  }
  }).then(() => {

test/main.js

@@ -485,6 +485,13 @@ describe('lib/main', function () {
  assert.include(contents, 'package.json')
  })
  })
+ it('_fileCopy should not exclude package-lock.json, even when excluded by excludeGlobs', () => {
+ program.excludeGlobs = '*.json'
+ return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
+ const contents = fs.readdirSync(codeDirectory)
+ assert.include(contents, 'package-lock.json')
+ })
+ })
 
  it('_fileCopy should not include package.json when --prebuiltDirectory is set', () => {
  const buildDir = '.build_' + Date.now()
@@ -505,19 +512,69 @@ describe('lib/main', function () {
  })
  })
 
+ describe('_shouldUseNpmCi', () => {
+ beforeEach(() => {
+ return lambda._cleanDirectory(codeDirectory)
+ })
+
+ describe('when package-lock.json exists', () => {
+ beforeEach(() => {
+ fs.writeFileSync(path.join(codeDirectory, 'package-lock.json'), JSON.stringify({}))
+ })
+
+ it('returns true', () => {
+ assert.isTrue(lambda._shouldUseNpmCi(codeDirectory))
+ })
+ })
+
+ describe('when package-lock.json does not exist', () => {
+ beforeEach(() => {
+ fs.removeSync(path.join(codeDirectory, 'package-lock.json'))
+ })
+
+ it('returns false', () => {
+ assert.isFalse(lambda._shouldUseNpmCi(codeDirectory))
+ })
+ })
+ })
+
  describe('_npmInstall', () => {
+ // npm treats files as packages when installing, and so removes them - hence hide in .bin
+ const nodeModulesFile = path.join(codeDirectory, 'node_modules', '.bin', 'file')
+
  beforeEach(() => {
  return lambda._cleanDirectory(codeDirectory).then(() => {
+ // hide our own file in node_modules to verify installs
+ fs.ensureFileSync(nodeModulesFile)
+
  return lambda._fileCopy(program, '.', codeDirectory, true)
  })
  })
 
- it('_npm adds node_modules', function () {
- _timeout({ this: this, sec: 30 }) // give it time to build the node modules
+ describe('when package-lock.json does exist', () => {
+ it('should use "npm ci"', function () {
+ _timeout({ this: this, sec: 30 }) // ci should be faster than install
 
- return lambda._npmInstall(program, codeDirectory).then(() => {
- const contents = fs.readdirSync(codeDirectory)
- assert.include(contents, 'node_modules')
+ return lambda._npmInstall(program, codeDirectory).then(() => {
+ const contents = fs.readdirSync(codeDirectory)
+ assert.include(contents, 'node_modules')
+ assert.isFalse(fs.existsSync(nodeModulesFile))
+ })
+ })
+ })
+ describe('when package-lock.json does not exist', () => {
+ beforeEach(() => {
+ return fs.removeSync(path.join(codeDirectory, 'package-lock.json'))
+ })
+
+ it('should use "npm install"', function () {
+ _timeout({ this: this, sec: 60 }) // install should be slower than ci
+
+ return lambda._npmInstall(program, codeDirectory).then(() => {
+ const contents = fs.readdirSync(codeDirectory)
+ assert.include(contents, 'node_modules')
+ assert.isTrue(fs.existsSync(nodeModulesFile))
+ })
  })
  })
  })