From 5f8711da396049c206c9aa6b7b227ec7972580c3 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Thu, 20 Apr 2017 12:15:46 +0200
Subject: [PATCH] Refactor yarn helper and add tests (#174)
Refactor yarn helper and increase coverage
---
lib/helpers/yarn.js | 44 +++++++++++++++++++
lib/workers/branch.js | 48 +--------------------
test/helpers/yarn.spec.js | 85 +++++++++++++++++++++++++++++++++++--
test/workers/branch.spec.js | 55 ++++++++++++------------
4 files changed, 155 insertions(+), 77 deletions(-)
diff --git a/lib/helpers/yarn.js b/lib/helpers/yarn.js
index 2163a50ee0..43d3190616 100644
--- a/lib/helpers/yarn.js
+++ b/lib/helpers/yarn.js
@@ -6,6 +6,8 @@ const path = require('path');
module.exports = {
generateLockFile,
+ getLockFile,
+ maintainLockFile,
};
async function generateLockFile(newPackageJson, npmrcContent, yarnrcContent) {
@@ -26,7 +28,49 @@ async function generateLockFile(newPackageJson, npmrcContent, yarnrcContent) {
logger.debug(String(result.stderr));
yarnLock = fs.readFileSync(path.join(tmpDir.name, 'yarn.lock'));
} catch (error) {
+ /* istanbul ignore next */
throw error;
}
return yarnLock;
}
+
+async function getLockFile(packageFile, packageContent, api) {
+ // Detect if a yarn.lock file is in use
+ const yarnLockFileName = path.join(path.dirname(packageFile), 'yarn.lock');
+ if (!await api.getFileContent(yarnLockFileName)) {
+ return null;
+ }
+ // Copy over custom config commitFiles
+ const npmrcContent = await api.getFileContent('.npmrc');
+ const yarnrcContent = await api.getFileContent('.yarnrc');
+ // Generate yarn.lock using shell command
+ const newYarnLockContent =
+ await module.exports.generateLockFile(packageContent, npmrcContent, yarnrcContent);
+ // Return file object
+ return ({
+ name: yarnLockFileName,
+ contents: newYarnLockContent,
+ });
+}
+
+async function maintainLockFile(inputConfig) {
+ logger.debug(`maintainYarnLock(${JSON.stringify(inputConfig)})`);
+ const packageContent = await inputConfig.api.getFileContent(inputConfig.packageFile);
+ const yarnLockFileName = path.join(path.dirname(inputConfig.packageFile), 'yarn.lock');
+ logger.debug(`Checking for ${yarnLockFileName}`);
+ const existingYarnLock = await inputConfig.api.getFileContent(yarnLockFileName);
+ logger.silly(`existingYarnLock:\n${existingYarnLock}`);
+ if (!existingYarnLock) {
+ return null;
+ }
+ logger.debug('Found existing yarn.lock file');
+ const newYarnLock =
+ await module.exports.getLockFile(inputConfig.packageFile, packageContent, inputConfig.api);
+ logger.silly(`newYarnLock:\n${newYarnLock.contents}`);
+ if (existingYarnLock.toString() === newYarnLock.contents.toString()) {
+ logger.debug('Yarn lock file does not need updating');
+ return null;
+ }
+ logger.debug('Yarn lock needs updating');
+ return newYarnLock;
+}
diff --git a/lib/workers/branch.js b/lib/workers/branch.js
index a561fd0ff9..5838551e77 100644
--- a/lib/workers/branch.js
+++ b/lib/workers/branch.js
@@ -1,14 +1,11 @@
const logger = require('winston');
-const path = require('path');
const handlebars = require('handlebars');
const packageJsonHelper = require('../helpers/package-json');
const yarnHelper = require('../helpers/yarn');
module.exports = {
getParentBranch,
- getYarnLockFile,
ensureBranch,
- maintainYarnLock,
};
async function getParentBranch(branchName, config) {
@@ -52,47 +49,6 @@ async function getParentBranch(branchName, config) {
return branchName;
}
-async function getYarnLockFile(packageFile, packageContent, api) {
- // Detect if a yarn.lock file is in use
- const yarnLockFileName = path.join(path.dirname(packageFile), 'yarn.lock');
- if (!await api.getFileContent(yarnLockFileName)) {
- return null;
- }
- // Copy over custom config commitFiles
- const npmrcContent = await api.getFileContent('.npmrc');
- const yarnrcContent = await api.getFileContent('.yarnrc');
- // Generate yarn.lock using shell command
- const newYarnLockContent =
- await yarnHelper.generateLockFile(packageContent, npmrcContent, yarnrcContent);
- // Return file object
- return ({
- name: yarnLockFileName,
- contents: newYarnLockContent,
- });
-}
-
-async function maintainYarnLock(inputConfig) {
- logger.debug(`maintainYarnLock(${JSON.stringify(inputConfig)})`);
- const packageContent = await inputConfig.api.getFileContent(inputConfig.packageFile);
- const yarnLockFileName = path.join(path.dirname(inputConfig.packageFile), 'yarn.lock');
- logger.debug(`Checking for ${yarnLockFileName}`);
- const existingYarnLock = await inputConfig.api.getFileContent(yarnLockFileName);
- logger.silly(`existingYarnLock:\n${existingYarnLock}`);
- if (!existingYarnLock) {
- return null;
- }
- logger.debug('Found existing yarn.lock file');
- const newYarnLock =
- await getYarnLockFile(inputConfig.packageFile, packageContent, inputConfig.api);
- logger.silly(`newYarnLock:\n${newYarnLock.contents}`);
- if (existingYarnLock.toString() === newYarnLock.contents.toString()) {
- logger.debug('Yarn lock file does not need updating');
- return null;
- }
- logger.debug('Yarn lock needs updating');
- return newYarnLock;
-}
-
// Ensure branch exists with appropriate content
async function ensureBranch(upgrades) {
logger.debug(`ensureBranch(${JSON.stringify(upgrades)})`);
@@ -107,7 +63,7 @@ async function ensureBranch(upgrades) {
const commitFiles = [];
for (const upgrade of upgrades) {
if (upgrade.upgradeType === 'maintainYarnLock') {
- const newYarnLock = await maintainYarnLock(upgrade);
+ const newYarnLock = await yarnHelper.maintainLockFile(upgrade);
if (newYarnLock) {
commitFiles.push(newYarnLock);
}
@@ -141,7 +97,7 @@ async function ensureBranch(upgrades) {
contents: packageFiles[packageFile],
});
const yarnLockFile =
- await module.exports.getYarnLockFile(packageFile, packageFiles[packageFile], api);
+ await yarnHelper.getLockFile(packageFile, packageFiles[packageFile], api);
if (yarnLockFile) {
// Add new yarn.lock file too
logger.debug(`Adding ${yarnLockFile.name}`);
diff --git a/test/helpers/yarn.spec.js b/test/helpers/yarn.spec.js
index 6138d04d13..65152ea3eb 100644
--- a/test/helpers/yarn.spec.js
+++ b/test/helpers/yarn.spec.js
@@ -1,5 +1,84 @@
-require('../../lib/helpers/yarn');
+const yarnHelper = require('../../lib/helpers/yarn');
+const defaultConfig = require('../../lib/config/defaults').getConfig();
-it('placeholder', () => {
- // TODO: write tests for this module - this is here so the file shows up in coverage
+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, yarnrcContent)', () => {
+ tmp.dirSync = jest.fn(() => ({ name: 'somedir' }));
+ fs.writeFileSync = jest.fn();
+ fs.readFileSync = jest.fn(() => 'yarn-lock-contents');
+ cp.spawnSync = jest.fn(() => ({
+ stdout: '',
+ stderror: '',
+ }));
+ it('generates lock files', async () => {
+ const yarnLock =
+ await yarnHelper.generateLockFile('package-json-contents', 'npmrc-contents', 'yarnrc-contents');
+ expect(tmp.dirSync.mock.calls.length).toEqual(1);
+ expect(fs.writeFileSync.mock.calls.length).toEqual(3);
+ expect(fs.readFileSync.mock.calls.length).toEqual(1);
+ expect(yarnLock).toEqual('yarn-lock-contents');
+ });
+});
+describe('getLockFile(packageJson, config)', () => {
+ let api;
+ beforeEach(() => {
+ api = {
+ getFileContent: jest.fn(),
+ };
+ });
+ it('returns null if no existing yarn.lock', async () => {
+ api.getFileContent.mockReturnValueOnce(false);
+ expect(await yarnHelper.getLockFile('package.json', '', api)).toBe(null);
+ });
+ it('returns yarn.lock file', async () => {
+ api.getFileContent.mockReturnValueOnce('Existing yarn.lock');
+ api.getFileContent.mockReturnValueOnce(null); // npmrc
+ api.getFileContent.mockReturnValueOnce(null); // yarnrc
+ yarnHelper.generateLockFile = jest.fn();
+ yarnHelper.generateLockFile.mockReturnValueOnce('New yarn.lock');
+ const yarnLockFile = {
+ name: 'yarn.lock',
+ contents: 'New yarn.lock',
+ };
+ expect(await yarnHelper.getLockFile('package.json', '', api)).toMatchObject(yarnLockFile);
+ });
+});
+
+describe('maintainLockFile(inputConfig)', () => {
+ let config;
+ beforeEach(() => {
+ config = Object.assign({}, defaultConfig);
+ config.packageFile = 'package.json';
+ config.api = {
+ getFileContent: jest.fn(),
+ };
+ config.api.getFileContent.mockReturnValueOnce('oldPackageContent');
+ yarnHelper.getLockFile = jest.fn();
+ });
+ it('returns null if no file to maintain', async () => {
+ const yarnLock = await yarnHelper.maintainLockFile(config);
+ expect(config.api.getFileContent.mock.calls.length).toBe(2);
+ expect(yarnLock).toEqual(null);
+ });
+ it('returns null if contents match', async () => {
+ config.api.getFileContent.mockReturnValueOnce('oldYarnLockContent');
+ yarnHelper.getLockFile.mockReturnValueOnce({ contents: 'oldYarnLockContent' });
+ const yarnLock = await yarnHelper.maintainLockFile(config);
+ expect(config.api.getFileContent.mock.calls.length).toBe(2);
+ expect(yarnLock).toEqual(null);
+ });
+ it('returns new yarn lock if contents differ', async () => {
+ config.api.getFileContent.mockReturnValueOnce('oldYarnLockContent');
+ yarnHelper.getLockFile.mockReturnValueOnce({ contents: 'newYarnLockContent' });
+ const yarnLock = await yarnHelper.maintainLockFile(config);
+ expect(config.api.getFileContent.mock.calls.length).toBe(2);
+ expect(yarnLock).toEqual({ contents: 'newYarnLockContent' });
+ });
});
diff --git a/test/workers/branch.spec.js b/test/workers/branch.spec.js
index bb5d769a90..45d75729d5 100644
--- a/test/workers/branch.spec.js
+++ b/test/workers/branch.spec.js
@@ -74,35 +74,13 @@ describe('workers/branch', () => {
expect(await branchWorker.getParentBranch(branchName, config)).toBe(undefined);
});
});
- describe('getYarnLockFile(packageJson, config)', () => {
- let api;
- beforeEach(() => {
- api = {
- getFileContent: jest.fn(),
- };
- });
- it('returns null if no existing yarn.lock', async () => {
- api.getFileContent.mockReturnValueOnce(false);
- expect(await branchWorker.getYarnLockFile('package.json', '', api)).toBe(null);
- });
- it('returns yarn.lock file', async () => {
- api.getFileContent.mockReturnValueOnce('Existing yarn.lock');
- api.getFileContent.mockReturnValueOnce(null); // npmrc
- api.getFileContent.mockReturnValueOnce(null); // yarnrc
- yarnHelper.generateLockFile.mockReturnValueOnce('New yarn.lock');
- const yarnLockFile = {
- name: 'yarn.lock',
- contents: 'New yarn.lock',
- };
- expect(await branchWorker.getYarnLockFile('package.json', '', api)).toMatchObject(yarnLockFile);
- });
- });
describe('ensureBranch(config)', () => {
let config;
beforeEach(() => {
packageJsonHelper.setNewValue = jest.fn();
branchWorker.getParentBranch = jest.fn();
- branchWorker.getYarnLockFile = jest.fn();
+ yarnHelper.getLockFile = jest.fn();
+ yarnHelper.maintainLockFile = jest.fn();
config = Object.assign({}, defaultConfig);
config.api = {};
config.api.getFileContent = jest.fn();
@@ -120,7 +98,7 @@ describe('workers/branch', () => {
await branchWorker.ensureBranch([config]);
expect(branchWorker.getParentBranch.mock.calls.length).toBe(1);
expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1);
- expect(branchWorker.getYarnLockFile.mock.calls.length).toBe(0);
+ expect(yarnHelper.getLockFile.mock.calls.length).toBe(0);
});
it('commits one file if no yarn lock found', async () => {
branchWorker.getParentBranch.mockReturnValueOnce('dummy branch');
@@ -128,18 +106,39 @@ describe('workers/branch', () => {
await branchWorker.ensureBranch([config]);
expect(branchWorker.getParentBranch.mock.calls.length).toBe(1);
expect(packageJsonHelper.setNewValue.mock.calls.length).toBe(1);
- expect(branchWorker.getYarnLockFile.mock.calls.length).toBe(1);
+ expect(yarnHelper.getLockFile.mock.calls.length).toBe(1);
expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(1);
});
it('commits two files if yarn lock found', async () => {
branchWorker.getParentBranch.mockReturnValueOnce('dummy branch');
- branchWorker.getYarnLockFile.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(branchWorker.getYarnLockFile.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('maintains lock files if needing updates', async () => {
+ branchWorker.getParentBranch.mockReturnValueOnce('dummy branch');
+ yarnHelper.maintainLockFile.mockReturnValueOnce('non null response');
+ config.upgradeType = 'maintainYarnLock';
+ await branchWorker.ensureBranch([config]);
+ 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(yarnHelper.maintainLockFile.mock.calls.length).toBe(1);
+ expect(config.api.commitFilesToBranch.mock.calls[0][1].length).toBe(1);
+ });
+ it('skips maintaining lock files if no updates', async () => {
+ branchWorker.getParentBranch.mockReturnValueOnce('dummy branch');
+ config.upgradeType = 'maintainYarnLock';
+ await branchWorker.ensureBranch([config]);
+ 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(yarnHelper.maintainLockFile.mock.calls.length).toBe(1);
+ expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0);
+ });
});
});
--
GitLab