diff --git a/lib/helpers/npm.js b/lib/helpers/npm.js new file mode 100644 index 0000000000000000000000000000000000000000..1677d454e3eb22c4c0b6c51e4505804343df6ad0 --- /dev/null +++ b/lib/helpers/npm.js @@ -0,0 +1,90 @@ +const logger = require('winston'); +const fs = require('fs'); +const cp = require('child_process'); +const tmp = require('tmp'); +const path = require('path'); + +module.exports = { + generateLockFile, + getLockFile, + maintainLockFile, +}; + +async function generateLockFile(newPackageJson, npmrcContent) { + logger.debug('Generating new package-lock.json file'); + const tmpDir = tmp.dirSync({ unsafeCleanup: true }); + let packageLock; + try { + fs.writeFileSync(path.join(tmpDir.name, 'package.json'), newPackageJson); + if (npmrcContent) { + fs.writeFileSync(path.join(tmpDir.name, '.npmrc'), npmrcContent); + } + logger.debug('Spawning npm install'); + const result = cp.spawnSync('npm', ['install'], { + cwd: tmpDir.name, + shell: true, + }); + logger.debug(String(result.stdout)); + logger.debug(String(result.stderr)); + packageLock = fs.readFileSync(path.join(tmpDir.name, 'package-lock.json')); + } catch (error) { + /* istanbul ignore next */ + throw error; + } + return packageLock; +} + +async function getLockFile(packageFile, packageContent, api) { + // Detect if a package-lock.json file is in use + const packageLockFileName = path.join( + path.dirname(packageFile), + 'package-lock.json' + ); + if (!await api.getFileContent(packageLockFileName)) { + return null; + } + // Copy over custom config commitFiles + const npmrcContent = await api.getFileContent('.npmrc'); + // Generate package-lock.json using shell command + const newPackageLockContent = await module.exports.generateLockFile( + packageContent, + npmrcContent + ); + // Return file object + return { + name: packageLockFileName, + contents: newPackageLockContent, + }; +} + +async function maintainLockFile(inputConfig) { + logger.debug(`maintainLockFile(${JSON.stringify(inputConfig)})`); + const packageContent = await inputConfig.api.getFileContent( + inputConfig.packageFile + ); + const packageLockFileName = path.join( + path.dirname(inputConfig.packageFile), + 'package-lock.json' + ); + logger.debug(`Checking for ${packageLockFileName}`); + const existingPackageLock = await inputConfig.api.getFileContent( + packageLockFileName + ); + logger.silly(`existingPackageLock:\n${existingPackageLock}`); + if (!existingPackageLock) { + return null; + } + logger.debug('Found existing package-lock.json file'); + const newPackageLock = await module.exports.getLockFile( + inputConfig.packageFile, + packageContent, + inputConfig.api + ); + logger.silly(`newPackageLock:\n${newPackageLock.contents}`); + if (existingPackageLock.toString() === newPackageLock.contents.toString()) { + logger.debug('npm lock file does not need updating'); + return null; + } + logger.debug('npm lock needs updating'); + return newPackageLock; +} diff --git a/lib/workers/branch.js b/lib/workers/branch.js index 52a48d5778be05e49cff67ef28e543b423e98129..902be0c4d2e1b2a888817c86152bb77f805356c4 100644 --- a/lib/workers/branch.js +++ b/lib/workers/branch.js @@ -1,6 +1,7 @@ const logger = require('winston'); const handlebars = require('handlebars'); const packageJsonHelper = require('../helpers/package-json'); +const npmHelper = require('../helpers/npm'); const yarnHelper = require('../helpers/yarn'); module.exports = { @@ -126,6 +127,22 @@ async function ensureBranch(upgrades) { logger.debug(err); throw new Error('Could not generate new yarn.lock file'); } + try { + const packageLockFile = await npmHelper.getLockFile( + packageFile, + packageFiles[packageFile], + api + ); + if (packageLockFile) { + // Add new package-lock.json file too + logger.debug(`Adding ${packageLockFile.name}`); + commitFiles.push(packageLockFile); + } + } catch (err) { + // This will include if npm < 5 + logger.verbose(err); + throw new Error('Could not generate new package-lock.json file'); + } } } if (commitFiles.length) { diff --git a/test/helpers/npm.spec.js b/test/helpers/npm.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..6ee4476ed3141a4afae86f0d57ddf2d335f9ec85 --- /dev/null +++ b/test/helpers/npm.spec.js @@ -0,0 +1,91 @@ +const npmHelper = require('../../lib/helpers/npm'); +const defaultConfig = require('../../lib/config/defaults').getConfig(); + +jest.mock('fs'); +jest.mock('child_process'); +jest.mock('tmp'); + +const fs = require('fs'); +const cp = require('child_process'); +const tmp = require('tmp'); + +describe('generateLockFile(newPackageJson, npmrcContent)', () => { + tmp.dirSync = jest.fn(() => ({ name: 'somedir' })); + fs.writeFileSync = jest.fn(); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + cp.spawnSync = jest.fn(() => ({ + stdout: '', + stderror: '', + })); + it('generates lock files', async () => { + const packageLock = await npmHelper.generateLockFile( + 'package-json-contents', + 'npmrc-contents' + ); + expect(tmp.dirSync.mock.calls.length).toEqual(1); + expect(fs.writeFileSync.mock.calls.length).toEqual(2); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(packageLock).toEqual('package-lock-contents'); + }); +}); +describe('getLockFile(packageJson, config)', () => { + let api; + beforeEach(() => { + api = { + getFileContent: jest.fn(), + }; + }); + it('returns null if no existing package-lock.json', async () => { + api.getFileContent.mockReturnValueOnce(false); + expect(await npmHelper.getLockFile('package.json', '', api)).toBe(null); + }); + it('returns package-lock.json file', async () => { + api.getFileContent.mockReturnValueOnce('Existing package-lock.json'); + api.getFileContent.mockReturnValueOnce(null); // npmrc + npmHelper.generateLockFile = jest.fn(); + npmHelper.generateLockFile.mockReturnValueOnce('New package-lock.json'); + const packageLockFile = { + name: 'package-lock.json', + contents: 'New package-lock.json', + }; + expect(await npmHelper.getLockFile('package.json', '', api)).toMatchObject( + packageLockFile + ); + }); +}); + +describe('maintainLockFile(inputConfig)', () => { + let config; + beforeEach(() => { + config = Object.assign({}, defaultConfig); + config.packageFile = 'package.json'; + config.api = { + getFileContent: jest.fn(), + }; + config.api.getFileContent.mockReturnValueOnce('oldPackageContent'); + npmHelper.getLockFile = jest.fn(); + }); + it('returns null if no file to maintain', async () => { + const packageLock = await npmHelper.maintainLockFile(config); + expect(config.api.getFileContent.mock.calls.length).toBe(2); + expect(packageLock).toEqual(null); + }); + it('returns null if contents match', async () => { + config.api.getFileContent.mockReturnValueOnce('oldPackageLockContent'); + npmHelper.getLockFile.mockReturnValueOnce({ + contents: 'oldPackageLockContent', + }); + const packageLock = await npmHelper.maintainLockFile(config); + expect(config.api.getFileContent.mock.calls.length).toBe(2); + expect(packageLock).toEqual(null); + }); + it('returns new package lock if contents differ', async () => { + config.api.getFileContent.mockReturnValueOnce('oldPackageLockContent'); + npmHelper.getLockFile.mockReturnValueOnce({ + contents: 'newPackageLockContent', + }); + const packageLock = await npmHelper.maintainLockFile(config); + expect(config.api.getFileContent.mock.calls.length).toBe(2); + expect(packageLock).toEqual({ contents: 'newPackageLockContent' }); + }); +}); diff --git a/test/workers/branch.spec.js b/test/workers/branch.spec.js index 04179550d1f895e9439ace729d8b1c9edd2f8d2b..82e6ea49c5c7e0c5ee88c1e56e0afb58d50102ad 100644 --- a/test/workers/branch.spec.js +++ b/test/workers/branch.spec.js @@ -1,4 +1,5 @@ const branchWorker = require('../../lib/workers/branch'); +const npmHelper = require('../../lib/helpers/npm'); const yarnHelper = require('../../lib/helpers/yarn'); const defaultConfig = require('../../lib/config/defaults').getConfig(); const packageJsonHelper = require('../../lib/helpers/package-json'); @@ -95,6 +96,7 @@ describe('workers/branch', () => { beforeEach(() => { packageJsonHelper.setNewValue = jest.fn(); branchWorker.getParentBranch = jest.fn(); + npmHelper.getLockFile = jest.fn(); yarnHelper.getLockFile = jest.fn(); yarnHelper.maintainLockFile = jest.fn(); config = Object.assign({}, defaultConfig); @@ -114,14 +116,16 @@ describe('workers/branch', () => { await branchWorker.ensureBranch([config]); expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(0); expect(yarnHelper.getLockFile.mock.calls.length).toBe(0); }); - it('commits one file if no yarn lock found', async () => { + it('commits one file if no yarn lock or package-lock.json found', async () => { branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); await branchWorker.ensureBranch([config]); expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(1); expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(1); }); @@ -132,9 +136,33 @@ describe('workers/branch', () => { await branchWorker.ensureBranch([config]); expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(1); expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(2); }); + it('commits two files if package lock found', async () => { + branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); + npmHelper.getLockFile.mockReturnValueOnce('non null response'); + packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); + await branchWorker.ensureBranch([config]); + expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); + expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(1); + expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); + expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(2); + }); + it('commits three files if yarn lock and package lock found', async () => { + branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); + npmHelper.getLockFile.mockReturnValueOnce('non null response'); + yarnHelper.getLockFile.mockReturnValueOnce('non null response'); + packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); + await branchWorker.ensureBranch([config]); + expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); + expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(1); + expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); + expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(3); + }); it('throws an error if no yarn lock generation possible', async () => { branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); yarnHelper.getLockFile.mockImplementationOnce(() => { @@ -151,6 +179,26 @@ describe('workers/branch', () => { expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(0); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); + }); + it('throws an error if no package lock generation possible', async () => { + branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); + npmHelper.getLockFile.mockImplementationOnce(() => { + throw new Error('no package lock generated'); + }); + packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); + let err; + try { + await branchWorker.ensureBranch([config]); + } catch (e) { + err = e; + } + expect(err.message).toBe('Could not generate new package-lock.json file'); + expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); + expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1); + expect(yarnHelper.getLockFile.mock.calls.length).toBe(1); + expect(npmHelper.getLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); it('maintains lock files if needing updates', async () => { @@ -161,6 +209,7 @@ describe('workers/branch', () => { expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(0); expect(yarnHelper.getLockFile.mock.calls.length).toBe(0); + expect(npmHelper.getLockFile.mock.calls.length).toBe(0); expect(yarnHelper.maintainLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(1); }); @@ -171,6 +220,7 @@ describe('workers/branch', () => { expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(0); expect(yarnHelper.getLockFile.mock.calls.length).toBe(0); + expect(npmHelper.getLockFile.mock.calls.length).toBe(0); expect(yarnHelper.maintainLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); @@ -190,6 +240,7 @@ describe('workers/branch', () => { expect(branchWorker.getParentBranch.mock.calls.length).toBe(1); expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(0); expect(yarnHelper.getLockFile.mock.calls.length).toBe(0); + expect(npmHelper.getLockFile.mock.calls.length).toBe(0); expect(yarnHelper.maintainLockFile.mock.calls.length).toBe(1); expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); });