Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 26 additions & 1 deletion src/commands/app/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class Pack extends BaseCommand {
.map(([, extConfig]) => path.relative(process.cwd(), extConfig.app.dist))

try {
// 0. validate package.json and package-lock.json compatibility (skip if --no-lock-file)
if (flags['lock-file']) {
await this.validatePackageLockCompatibility()
}

// 1. create artifacts phase
this.spinner.start(`Creating package artifacts folder '${DEFAULTS.ARTIFACTS_FOLDER_PATH}'...`)
await fs.emptyDir(DEFAULTS.ARTIFACTS_FOLDER_PATH)
Expand Down Expand Up @@ -127,6 +132,26 @@ class Pack extends BaseCommand {
return this._spinner
}

/**
* Validates that package.json and package-lock.json are compatible
*/
async validatePackageLockCompatibility () {
const packageLockPath = path.join(process.cwd(), 'package-lock.json')
if (!(await fs.pathExists(packageLockPath))) {
return
}

this.spinner.start('Validating package.json and package-lock.json compatibility...')
try {
await execa('npm', ['ci', '--dry-run'], { cwd: process.cwd() })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! I guess npm versions should not be an issue since we can assume that they created their package-lock with whatever npm version they are using and its their issue to fix if there is a mismatch.

this.spinner.succeed('Validated package.json and package-lock.json compatibility')
} catch (error) {
this.spinner.fail('package.json and package-lock.json are incompatible')
const errorMessage = error.stderr || error.message || 'npm ci --dry-run failed'
throw new Error(`package.json and package-lock.json are incompatible. Run 'npm install' to update your package-lock.json.\n\nError: ${errorMessage}`)
}
}

/**
* Creates the deploy.yaml file
*
Expand Down Expand Up @@ -371,7 +396,7 @@ Pack.description = `This command will support packaging apps for redistribution.
Pack.flags = {
...BaseCommand.flags,
'lock-file': Flags.boolean({
description: 'Include the package-lock.json file in the packaged app',
description: 'Include the package-lock.json file in the packaged app. When disabled, compatibility validation is skipped since the provisioner will use npm install instead of npm ci.',
default: true,
allowNo: true
}),
Expand Down
228 changes: 228 additions & 0 deletions test/commands/app/pack.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,20 @@ describe('run', () => {
const command = new TheCommand()
command.argv = []

// mock package-lock.json exists and npm ci --dry-run succeeds
fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})
execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
return Promise.resolve({ stdout: '', stderr: '' })
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
Expand All @@ -534,6 +548,7 @@ describe('run', () => {
command.config = { runHook }
await command.run()

expect(execa).toHaveBeenCalledWith('npm', ['ci', '--dry-run'], { cwd: expect.any(String) })
expect(command.copyPackageFiles).toHaveBeenCalledTimes(1)
expect(command.filesToPack).toHaveBeenCalledTimes(1)
expect(command.filesToPack).toHaveBeenCalledWith({
Expand All @@ -557,6 +572,13 @@ describe('run', () => {
const command = new TheCommand()
command.argv = ['--no-lock-file']

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
Expand All @@ -567,6 +589,8 @@ describe('run', () => {
command.config = { runHook }
await command.run()

const npmCiCalls = execa.mock.calls.filter(call => call[0] === 'npm' && call[1] && call[1][0] === 'ci')
expect(npmCiCalls.length).toBe(0)
expect(command.copyPackageFiles).toHaveBeenCalledTimes(1)
expect(command.filesToPack).toHaveBeenCalledTimes(1)
expect(command.filesToPack).toHaveBeenCalledWith({
Expand All @@ -590,6 +614,19 @@ describe('run', () => {
const command = new TheCommand()
command.argv = ['--verbose']

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})
execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
return Promise.resolve({ stdout: '', stderr: '' })
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

const errorObject = new Error('zip error')

// since we already unit test the methods above, we mock it here
Expand Down Expand Up @@ -626,6 +663,19 @@ describe('run', () => {
const command = new TheCommand()
command.argv = []

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})
execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
return Promise.resolve({ stdout: '', stderr: '' })
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

const errorMessage = 'zip error'

// since we already unit test the methods above, we mock it here
Expand Down Expand Up @@ -662,6 +712,20 @@ describe('run', () => {
const command = new TheCommand()
command.argv = ['new_folder', '--output', 'app-2.zip']

// mock package-lock.json exists and npm ci --dry-run succeeds
fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})
execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
return Promise.resolve({ stdout: '', stderr: '' })
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => ([]))
Expand Down Expand Up @@ -689,6 +753,18 @@ describe('run', () => {

test('outputs error if events hook throws', async () => {
mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json'))
fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})
execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
return Promise.resolve({ stdout: '', stderr: '' })
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})
const runHook = jest.fn()
.mockResolvedValue({
successes: [],
Expand Down Expand Up @@ -721,4 +797,156 @@ describe('run', () => {
await expect(command.run()).rejects.toThrow('invalid fake config error')
expect(mockGetFullConfig).toHaveBeenCalledWith({ validateAppConfig: true })
})

test('skips npm ci check when package-lock.json does not exist', async () => {
mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json'))

const command = new TheCommand()
command.argv = []

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return false
}
return false
})

command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
command.createDeployYamlFile = jest.fn()
command.addCodeDownloadAnnotation = jest.fn()
command.zipHelper = jest.fn()
const runHook = jest.fn()
command.config = { runHook }
await command.run()

const npmCiCalls = execa.mock.calls.filter(call => call[0] === 'npm' && call[1] && call[1][0] === 'ci')
expect(npmCiCalls.length).toBe(0)
expect(command.copyPackageFiles).toHaveBeenCalledTimes(1)
})

test('errors when npm ci --dry-run fails', async () => {
mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json'))

const command = new TheCommand()
command.argv = []

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})

execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
const error = new Error('npm ci failed')
error.stderr = 'npm ERR! code ERESOLVE\nnpm ERR! ERESOLVE unable to resolve dependency tree'
return Promise.reject(error)
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
command.createDeployYamlFile = jest.fn()
command.addCodeDownloadAnnotation = jest.fn()
command.zipHelper = jest.fn()
command.error = jest.fn()
const runHook = jest.fn()
command.config = { runHook }

await command.run()

expect(execa).toHaveBeenCalledWith('npm', ['ci', '--dry-run'], { cwd: expect.any(String) })
expect(command.error).toHaveBeenCalledTimes(1)
const errorMessage = command.error.mock.calls[0][0]
expect(errorMessage).toContain('package.json and package-lock.json are incompatible')
expect(errorMessage).toContain('Run \'npm install\' to update your package-lock.json')
expect(command.copyPackageFiles).not.toHaveBeenCalled()
})

test('errors when npm ci --dry-run fails without stderr', async () => {
mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json'))

const command = new TheCommand()
command.argv = []

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})

execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
const error = new Error('npm ci failed')
return Promise.reject(error)
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
command.createDeployYamlFile = jest.fn()
command.addCodeDownloadAnnotation = jest.fn()
command.zipHelper = jest.fn()
command.error = jest.fn()
const runHook = jest.fn()
command.config = { runHook }

await command.run()

expect(execa).toHaveBeenCalledWith('npm', ['ci', '--dry-run'], { cwd: expect.any(String) })
expect(command.error).toHaveBeenCalledTimes(1)
const errorMessage = command.error.mock.calls[0][0]
expect(errorMessage).toContain('package.json and package-lock.json are incompatible')
expect(errorMessage).toContain('Run \'npm install\' to update your package-lock.json')
expect(command.copyPackageFiles).not.toHaveBeenCalled()
})

test('errors when npm ci --dry-run fails without stderr or message', async () => {
mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json'))

const command = new TheCommand()
command.argv = []

fs.pathExists.mockImplementation(async (filePath) => {
if (filePath.includes('package-lock.json')) {
return true
}
return false
})

execa.mockImplementationOnce((cmd, args) => {
if (cmd === 'npm' && args[0] === 'ci' && args[1] === '--dry-run') {
const error = {}
return Promise.reject(error)
}
return Promise.resolve({ stdout: JSON.stringify([{ files: [] }]) })
})

// since we already unit test the methods above, we mock it here
command.copyPackageFiles = jest.fn()
command.filesToPack = jest.fn(() => (['some-file']))
command.createDeployYamlFile = jest.fn()
command.addCodeDownloadAnnotation = jest.fn()
command.zipHelper = jest.fn()
command.error = jest.fn()
const runHook = jest.fn()
command.config = { runHook }

await command.run()

expect(execa).toHaveBeenCalledWith('npm', ['ci', '--dry-run'], { cwd: expect.any(String) })
expect(command.error).toHaveBeenCalledTimes(1)
const errorMessage = command.error.mock.calls[0][0]
expect(errorMessage).toContain('package.json and package-lock.json are incompatible')
expect(errorMessage).toContain('Run \'npm install\' to update your package-lock.json')
expect(errorMessage).toContain('Error: npm ci --dry-run failed')
expect(command.copyPackageFiles).not.toHaveBeenCalled()
})
})