feat(lib/main): separate _npmInstall method into _getNpmInstallCommand and _packageInstall (#580)

Separate `_npmInstall` method into two parts.
* Get the command string (`_getNpmInstallCommand`)
* Execute the command (`_packageInstall`)

(Prepare for "Support for yarn #579".)

Author: abetomo

lib/main.js

@@ -385,31 +385,41 @@ Emulate only the body of the API Gateway event.
     return fs.existsSync(path.join(codeDirectory, 'package-lock.json'))
   }
 
-  _npmInstall (program, codeDirectory) {
-    // Run on windows:
-    // https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows
-
-    const installCommand = this._shouldUseNpmCi(codeDirectory)
-      ? 'ci'
-      : 'install'
-    const npmInstallBaseOptions = [
+  _getNpmInstallCommand (program, codeDirectory) {
+    const installOptions = [
       '-s',
-      installCommand,
+      this._shouldUseNpmCi(codeDirectory) ? 'ci' : 'install',
       '--production',
       '--no-audit'
     ]
 
     if (program.optionalDependencies === false) {
-      npmInstallBaseOptions.push('--no-optional')
+      installOptions.push('--no-optional')
     }
 
+    if (!program.dockerImage) {
+      installOptions.push('--prefix', codeDirectory)
+    }
+
+    return {
+      packageManager: 'npm',
+      installOptions
+    }
+  }
+
+  _packageInstall (program, codeDirectory) {
+    // Run on windows:
+    // https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows
+
+    const { packageManager, installOptions } = this._getNpmInstallCommand(program, codeDirectory)
+
     const paramsOnContainer = (() => {
       // with docker
       let dockerVolumesOptions = []
       program.dockerVolumes && program.dockerVolumes.split(' ').forEach((volume) => {
         dockerVolumesOptions = dockerVolumesOptions.concat(['-v', volume])
       })
-      const dockerCommand = [program.dockerImage, 'npm'].concat(npmInstallBaseOptions)
+      const dockerCommand = [program.dockerImage, packageManager].concat(installOptions)
       const dockerBaseOptions = [
         'run', '--rm',
         '-v', `${fs.realpathSync(codeDirectory)}:/var/task`,
@@ -429,17 +439,16 @@ Emulate only the body of the API Gateway event.
     })()
 
     const paramsOnHost = (() => {
-      // simple npm install
-      const options = npmInstallBaseOptions.concat(['--prefix', codeDirectory])
+      // simple install
       if (process.platform === 'win32') {
         return {
           command: 'cmd.exe',
-          options: ['/c', 'npm'].concat(options)
+          options: ['/c', packageManager].concat(installOptions)
         }
       }
       return {
-        command: 'npm',
-        options
+        command: packageManager,
+        options: installOptions
       }
     })()
 
@@ -518,7 +527,7 @@ Emulate only the body of the API Gateway event.
 
   _codeDirectory () {
     // Why realpathSync?:
-    // If tmpdir is symbolic link and npm>=7, `this._npmInstall()` may not work properly.
+    // If tmpdir is symbolic link and npm>=7, `this._packageInstall()` may not work properly.
     return path.join(fs.realpathSync(os.tmpdir()), `${path.basename(path.resolve('.'))}-lambda`)
   }
 
@@ -691,8 +700,8 @@ they may not work as expected in the Lambda environment.
     console.log('=> Moving files to temporary directory')
     await this._fileCopy(program, lambdaSrcDirectory, codeDirectory, true)
     if (!program.keepNodeModules) {
-      console.log(`=> Running npm ${this._shouldUseNpmCi(codeDirectory) ? 'ci' : 'install'} --production`)
-      await this._npmInstall(program, codeDirectory)
+      console.log('=> Running package install')
+      await this._packageInstall(program, codeDirectory)
     }
     await this._postInstallScript(program, codeDirectory)
     console.log('=> Zipping deployment package')

test/main.js

@@ -590,7 +590,83 @@ describe('lib/main', function () {
     })
   })
 
-  describe('_npmInstall', function () {
+  describe('_getNpmInstallCommand', () => {
+    describe('when package-lock.json exists', () => {
+      const codeDirectory = '.'
+
+      it('npm ci', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(program, codeDirectory)
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(installOptions, ['-s', 'ci', '--production', '--no-audit', '--prefix', codeDirectory])
+      })
+
+      it('npm ci with "--no-optional"', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(
+          {
+            ...program,
+            optionalDependencies: false
+          },
+          codeDirectory
+        )
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(
+          installOptions,
+          ['-s', 'ci', '--production', '--no-audit', '--no-optional', '--prefix', codeDirectory]
+        )
+      })
+
+      it('npm ci on docker', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(
+          {
+            ...program,
+            dockerImage: 'test'
+          },
+          codeDirectory
+        )
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(installOptions, ['-s', 'ci', '--production', '--no-audit'])
+      })
+    })
+
+    describe('when package-lock.json does not exist', () => {
+      const codeDirectory = './test'
+
+      it('npm install', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(program, './test')
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(installOptions, ['-s', 'install', '--production', '--no-audit', '--prefix', './test'])
+      })
+
+      it('npm install with "--no-optional"', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(
+          {
+            ...program,
+            optionalDependencies: false
+          },
+          codeDirectory
+        )
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(
+          installOptions,
+          ['-s', 'install', '--production', '--no-audit', '--no-optional', '--prefix', codeDirectory]
+        )
+      })
+
+      it('npm install on docker', () => {
+        const { packageManager, installOptions } = lambda._getNpmInstallCommand(
+          {
+            ...program,
+            dockerImage: 'test'
+          },
+          codeDirectory
+        )
+        assert.equal(packageManager, 'npm')
+        assert.deepEqual(installOptions, ['-s', 'install', '--production', '--no-audit'])
+      })
+    })
+  })
+
+  describe('_packageInstall using "npm"', function () {
     _timeout({ this: this, sec: 60 }) // ci should be faster than install
 
     // npm treats files as packages when installing, and so removes them.
@@ -605,7 +681,7 @@ describe('lib/main', function () {
     describe('when package-lock.json does exist', () => {
       it('should use "npm ci"', () => {
         const beforeAwsSdkStat = fs.statSync(path.join(codeDirectory, 'node_modules', 'aws-sdk'))
-        return lambda._npmInstall(program, codeDirectory).then(() => {
+        return lambda._packageInstall(program, codeDirectory).then(() => {
           const contents = fs.readdirSync(path.join(codeDirectory, 'node_modules'))
           assert.include(contents, 'dotenv')
 
@@ -627,7 +703,7 @@ describe('lib/main', function () {
 
       it('should use "npm install"', () => {
         const beforeAwsSdkStat = fs.statSync(path.join(codeDirectory, 'node_modules', 'aws-sdk'))
-        return lambda._npmInstall(program, codeDirectory).then(() => {
+        return lambda._packageInstall(program, codeDirectory).then(() => {
           const contents = fs.readdirSync(path.join(codeDirectory, 'node_modules'))
           assert.include(contents, 'dotenv')
 
@@ -652,7 +728,7 @@ describe('lib/main', function () {
 
       describe('No `--no-optionalDependencies`', () => {
         it('optionalDependencies is installed', () => {
-          return lambda._npmInstall(program, codeDirectory).then(() => {
+          return lambda._packageInstall(program, codeDirectory).then(() => {
             const contents = fs.readdirSync(path.join(codeDirectory, 'node_modules'))
             assert.include(contents, 'npm')
           })
@@ -665,7 +741,7 @@ describe('lib/main', function () {
             ...program,
             optionalDependencies: false
           }
-          return lambda._npmInstall(params, codeDirectory).then(() => {
+          return lambda._packageInstall(params, codeDirectory).then(() => {
             const contents = fs.readdirSync(path.join(codeDirectory, 'node_modules'))
             assert.notInclude(contents, 'npm')
           })
@@ -674,7 +750,7 @@ describe('lib/main', function () {
     })
   })
 
-  describe('_npmInstall (When codeDirectory contains characters to be escaped)', () => {
+  describe('_packageInstall using "npm" (When codeDirectory contains characters to be escaped)', () => {
     beforeEach(() => {
       // Since '\' can not be included in the file or directory name in Windows
       const directoryName = process.platform === 'win32'
@@ -694,7 +770,7 @@ describe('lib/main', function () {
     it('_npm adds node_modules', function () {
       _timeout({ this: this, sec: 30 }) // give it time to build the node modules
 
-      return lambda._npmInstall(program, codeDirectory).then(() => {
+      return lambda._packageInstall(program, codeDirectory).then(() => {
         const contents = fs.readdirSync(codeDirectory)
         assert.include(contents, 'node_modules')
       })
@@ -768,15 +844,15 @@ describe('lib/main', function () {
     })
   })
 
-  describe('_zip', () => {
+  describe('_zip using "npm"', () => {
     beforeEach(function () {
       _timeout({ this: this, sec: 30 }) // give it time to build the node modules
       return Promise.resolve().then(() => {
         return lambda._cleanDirectory(codeDirectory)
       }).then(() => {
         return lambda._fileCopy(program, '.', codeDirectory, true)
       }).then(() => {
-        return lambda._npmInstall(program, codeDirectory)
+        return lambda._packageInstall(program, codeDirectory)
       }).then(() => {
         if (process.platform !== 'win32') {
           fs.symlinkSync(