Refactor yarn helper and add tests (#174)

Refactor yarn helper and increase coverage

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;
+}

lib/workers/branch.js

 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}`);

test/helpers/yarn.spec.js

-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' });
+  });
 });

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);
+    });
   });
 });