From a663ecddef84ed1d516efa803f30898eafcfd25a Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Fri, 2 Jun 2017 08:29:36 +0200
Subject: [PATCH] Add npm5 package-lock.json support (#224)
* Add npm5 package-lock.json support
Closes #222
* Fix comment typo
* Add package-lock.json failure test cases
---
lib/helpers/npm.js | 90 ++++++++++++++++++++++++++++++++++++
lib/workers/branch.js | 17 +++++++
test/helpers/npm.spec.js | 91 +++++++++++++++++++++++++++++++++++++
test/workers/branch.spec.js | 53 ++++++++++++++++++++-
4 files changed, 250 insertions(+), 1 deletion(-)
create mode 100644 lib/helpers/npm.js
create mode 100644 test/helpers/npm.spec.js
diff --git a/lib/helpers/npm.js b/lib/helpers/npm.js
new file mode 100644
index 0000000000..1677d454e3
--- /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 52a48d5778..902be0c4d2 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 0000000000..6ee4476ed3
--- /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 04179550d1..82e6ea49c5 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);
});
--
GitLab