Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DeviaVir merged 1 commit into from
Oct 27, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions lib/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand All @@ -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
}
})()

Expand Down Expand Up @@ -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`)
}

Expand Down Expand Up @@ -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')
Expand Down
94 changes: 85 additions & 9 deletions test/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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')

Expand All @@ -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')

Expand All @@ -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')
})
Expand All @@ -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')
})
Expand All @@ -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'
Expand All @@ -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')
})
Expand Down Expand Up @@ -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(
Expand Down