diff --git a/lib/workers/branch/npm.js b/lib/workers/branch/npm.js index 44c2c1cbefcd88adb9a319940752b70b5781930e..c289a2f2753f9ac5cb9d83279ce4bede1f9f7df7 100644 --- a/lib/workers/branch/npm.js +++ b/lib/workers/branch/npm.js @@ -1,7 +1,7 @@ const fs = require('fs-extra'); -const cp = require('child_process'); const path = require('path'); const getInstalledPath = require('get-installed-path'); +const { exec } = require('child-process-promise'); module.exports = { generateLockFile, @@ -10,58 +10,59 @@ module.exports = { async function generateLockFile(tmpDir, logger) { logger.debug(`Spawning npm install to create ${tmpDir}/package-lock.json`); let lockFile = null; - let result = {}; + let stdout; + let stderr; try { const startTime = process.hrtime(); - let cmd = 'node'; - const options = []; + let cmd; try { // See if renovate is installed locally - options.push( - path.join( - await getInstalledPath('npm', { - local: true, - }), - 'bin/npm-cli.js' - ) + const installedPath = path.join( + await getInstalledPath('npm', { + local: true, + }), + 'bin/npm-cli.js' ); + cmd = `node ${installedPath}`; } catch (localerr) { logger.debug('No locally installed npm found'); // Look inside globally installed renovate try { const renovateLocation = await getInstalledPath('renovate'); - options.push( - path.join( - await getInstalledPath('npm', { - local: true, - cwd: renovateLocation, - }), - 'bin/npm-cli.js' - ) + const installedPath = path.join( + await getInstalledPath('npm', { + local: true, + cwd: renovateLocation, + }), + 'bin/npm-cli.js' ); + cmd = `node ${installedPath}`; } catch (nestederr) { logger.debug('Could not find globally nested npm'); // look for global npm try { - options.push( - path.join(await getInstalledPath('npm'), 'bin/npm-cli.js') + const installedPath = path.join( + await getInstalledPath('npm'), + 'bin/npm-cli.js' ); + cmd = `node ${installedPath}`; } catch (globalerr) { logger.warn('Could not find globally installed npm'); cmd = 'npm'; } } } - logger.debug(`Using npm: ${options[0] || cmd}`); - options.push('install'); - options.push('--ignore-scripts'); - result = cp.spawnSync(cmd, options, { + logger.debug(`Using npm: ${cmd}`); + cmd += ' install'; + cmd += ' --ignore-scripts'; + // TODO: Switch to native util.promisify once using only node 8 + ({ stdout, stderr } = await exec(cmd, { cwd: tmpDir, shell: true, env: { NODE_ENV: 'dev', PATH: process.env.PATH }, - }); - logger.debug(`npm stdout:\n${String(result.stdout)}`); - logger.debug(`npm stderr:\n${String(result.stderr)}`); + })); + logger.debug(`npm stdout:\n${stdout}`); + logger.debug(`npm stderr:\n${stderr}`); const duration = process.hrtime(startTime); const durationSeconds = Math.round(duration[0] + duration[1] / 1e9); lockFile = fs.readFileSync(path.join(tmpDir, 'package-lock.json'), 'utf8'); @@ -73,8 +74,8 @@ async function generateLockFile(tmpDir, logger) { logger.warn( { err, - stdout: String(result.stdout), - stderr: String(result.stderr), + stdout, + stderr, }, 'npm install error' ); diff --git a/lib/workers/branch/yarn.js b/lib/workers/branch/yarn.js index ca76d727f2bb81dd795333e76bb48ad52bfdf19e..6d2d068d62c30eac68d1dd3069f4bd29633c0a94 100644 --- a/lib/workers/branch/yarn.js +++ b/lib/workers/branch/yarn.js @@ -1,7 +1,7 @@ const fs = require('fs-extra'); -const cp = require('child_process'); const path = require('path'); const getInstalledPath = require('get-installed-path'); +const { exec } = require('child-process-promise'); module.exports = { generateLockFile, @@ -10,58 +10,59 @@ module.exports = { async function generateLockFile(tmpDir, logger) { logger.debug(`Spawning yarn install to create ${tmpDir}/yarn.lock`); let lockFile = null; - let result = {}; + let stdout; + let stderr; try { const startTime = process.hrtime(); - let cmd = 'node'; - const options = []; + let cmd; try { // See if renovate is installed locally - options.push( - path.join( - await getInstalledPath('yarn', { - local: true, - }), - 'bin/yarn.js' - ) + const installedPath = path.join( + await getInstalledPath('yarn', { + local: true, + }), + 'bin/yarn.js' ); + cmd = `node ${installedPath}`; } catch (localerr) { logger.debug('No locally installed yarn found'); // Look inside globally installed renovate try { const renovateLocation = await getInstalledPath('renovate'); - options.push( - path.join( - await getInstalledPath('yarn', { - local: true, - cwd: renovateLocation, - }), - 'bin/yarn.js' - ) + const installedPath = path.join( + await getInstalledPath('yarn', { + local: true, + cwd: renovateLocation, + }), + 'bin/yarn.js' ); + cmd = `node ${installedPath}`; } catch (nestederr) { logger.debug('Could not find globally nested yarn'); // look for global yarn try { - options.push( - path.join(await getInstalledPath('yarn'), 'bin/yarn.js') + const installedPath = path.join( + await getInstalledPath('yarn'), + 'bin/yarn.js' ); + cmd = `node ${installedPath}`; } catch (globalerr) { logger.warn('Could not find globally installed yarn'); cmd = 'yarn'; } } } - logger.debug(`Using yarn: ${options[0] || cmd}`); - options.push('install'); - options.push('--ignore-scripts'); - result = cp.spawnSync(cmd, options, { + logger.debug(`Using yarn: ${cmd}`); + cmd += ' install'; + cmd += ' --ignore-scripts'; + // TODO: Switch to native util.promisify once using only node 8 + ({ stdout, stderr } = await exec(cmd, { cwd: tmpDir, shell: true, env: { NODE_ENV: 'dev', PATH: process.env.PATH }, - }); - logger.debug(`yarn stdout:\n${String(result.stdout)}`); - logger.debug(`yarn stderr:\n${String(result.stderr)}`); + })); + logger.debug(`yarn stdout:\n${stdout}`); + logger.debug(`yarn stderr:\n${stderr}`); const duration = process.hrtime(startTime); const durationSeconds = Math.round(duration[0] + duration[1] / 1e9); lockFile = fs.readFileSync(path.join(tmpDir, 'yarn.lock'), 'utf8'); @@ -73,8 +74,8 @@ async function generateLockFile(tmpDir, logger) { logger.warn( { err, - stdout: String(result.stdout), - stderr: String(result.stderr), + stdout, + stderr, }, 'yarn install error' ); diff --git a/package.json b/package.json index 36d5d135f668e297b39b7c62c2f61ee287439935..25358f7eeac221d6f09930e064a93fc271e2ab0f 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "bunyan": "1.8.12", "chalk": "2.1.0", "changelog": "1.4.0", + "child-process-promise": "2.2.1", "commander": "2.11.0", "conventional-commits-detector": "0.1.1", "fs-extra": "4.0.2", diff --git a/test/workers/branch/npm.spec.js b/test/workers/branch/npm.spec.js index 488c99eb8861acedf529930f4966fb4d66eb4776..7eec53be28d4da05291ec75f2c371d2af7c67fd1 100644 --- a/test/workers/branch/npm.spec.js +++ b/test/workers/branch/npm.spec.js @@ -3,30 +3,32 @@ const logger = require('../../_fixtures/logger'); const getInstalledPath = require('get-installed-path'); jest.mock('fs-extra'); -jest.mock('child_process'); +jest.mock('child-process-promise'); jest.mock('get-installed-path'); getInstalledPath.mockImplementation(() => null); const fs = require('fs-extra'); -const cp = require('child_process'); +const { exec } = require('child-process-promise'); describe('generateLockFile', () => { it('generates lock files', async () => { - cp.spawnSync = jest.fn(() => ({ + getInstalledPath.mockReturnValueOnce('node_modules/npm'); + exec.mockReturnValueOnce({ stdout: '', stderror: '', - })); + }); fs.readFileSync = jest.fn(() => 'package-lock-contents'); const lockFile = await npmHelper.generateLockFile('some-dir', logger); expect(fs.readFileSync.mock.calls.length).toEqual(1); expect(lockFile).toEqual('package-lock-contents'); }); it('catches errors', async () => { - cp.spawnSync = jest.fn(() => ({ + getInstalledPath.mockReturnValueOnce('node_modules/npm'); + exec.mockReturnValueOnce({ stdout: '', stderror: 'some-error', - })); + }); fs.readFileSync = jest.fn(() => { throw new Error('not found'); }); @@ -34,4 +36,59 @@ describe('generateLockFile', () => { expect(fs.readFileSync.mock.calls.length).toEqual(1); expect(lockFile).toBe(null); }); + it('finds npm embedded in renovate', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce( + () => '/node_modules/renovate/node_modules/npm' + ); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await npmHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); + it('finds npm globally', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/npm'); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await npmHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); + it('uses fallback npm', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await npmHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); }); diff --git a/test/workers/branch/yarn.spec.js b/test/workers/branch/yarn.spec.js index 9a16d7b42ee7ddd58d2975cc85ba049e0d272e0a..b40de5dd94f8beae90a550c28d1b28ace63e072a 100644 --- a/test/workers/branch/yarn.spec.js +++ b/test/workers/branch/yarn.spec.js @@ -3,30 +3,32 @@ const logger = require('../../_fixtures/logger'); const getInstalledPath = require('get-installed-path'); jest.mock('fs-extra'); -jest.mock('child_process'); +jest.mock('child-process-promise'); jest.mock('get-installed-path'); getInstalledPath.mockImplementation(() => null); const fs = require('fs-extra'); -const cp = require('child_process'); +const { exec } = require('child-process-promise'); describe('generateLockFile', () => { it('generates lock files', async () => { - cp.spawnSync = jest.fn(() => ({ + getInstalledPath.mockReturnValueOnce('node_modules/yarn'); + exec.mockReturnValueOnce({ stdout: '', stderror: '', - })); - fs.readFileSync = jest.fn(() => 'yarn-lock-contents'); - const yarnLock = await yarnHelper.generateLockFile('some-dir', logger); + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await yarnHelper.generateLockFile('some-dir', logger); expect(fs.readFileSync.mock.calls.length).toEqual(1); - expect(yarnLock).toEqual('yarn-lock-contents'); + expect(lockFile).toEqual('package-lock-contents'); }); - it('catches and throws errors', async () => { - cp.spawnSync = jest.fn(() => ({ + it('catches errors', async () => { + getInstalledPath.mockReturnValueOnce('node_modules/yarn'); + exec.mockReturnValueOnce({ stdout: '', stderror: 'some-error', - })); + }); fs.readFileSync = jest.fn(() => { throw new Error('not found'); }); @@ -34,4 +36,59 @@ describe('generateLockFile', () => { expect(fs.readFileSync.mock.calls.length).toEqual(1); expect(lockFile).toBe(null); }); + it('finds yarn embedded in renovate', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce( + () => '/node_modules/renovate/node_modules/yarn' + ); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await yarnHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); + it('finds yarn globally', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/yarn'); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await yarnHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); + it('uses fallback yarn', async () => { + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => '/node_modules/renovate'); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + getInstalledPath.mockImplementationOnce(() => { + throw new Error('not found'); + }); + exec.mockReturnValueOnce({ + stdout: '', + stderror: '', + }); + fs.readFileSync = jest.fn(() => 'package-lock-contents'); + const lockFile = await yarnHelper.generateLockFile('some-dir', logger); + expect(fs.readFileSync.mock.calls.length).toEqual(1); + expect(lockFile).toEqual('package-lock-contents'); + }); }); diff --git a/yarn.lock b/yarn.lock index edcb74ac5a16dbb3799ed1803f091fe691091a73..31cec12a53434aed829334ed25c937f126433590 100644 --- a/yarn.lock +++ b/yarn.lock @@ -751,6 +751,14 @@ check-error@^1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/check-error/-/check-error-1.0.2.tgz#574d312edd88bb5dd8912e9286dd6c0aed4aac82" +child-process-promise@2.2.1: + version "2.2.1" + resolved "https://registry.yarnpkg.com/child-process-promise/-/child-process-promise-2.2.1.tgz#4730a11ef610fad450b8f223c79d31d7bdad8074" + dependencies: + cross-spawn "^4.0.2" + node-version "^1.0.0" + promise-polyfill "^6.0.1" + chokidar@^1.6.1: version "1.7.0" resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-1.7.0.tgz#798e689778151c8076b4b360e5edd28cda2bb468" @@ -1096,6 +1104,13 @@ cross-spawn@4.0.0: lru-cache "^4.0.1" which "^1.2.9" +cross-spawn@^4.0.2: + version "4.0.2" + resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-4.0.2.tgz#7b9247621c23adfdd3856004a823cbe397424d41" + dependencies: + lru-cache "^4.0.1" + which "^1.2.9" + cross-spawn@^5.0.1, cross-spawn@^5.1.0: version "5.1.0" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-5.1.0.tgz#e8bd0efee58fcff6f8f94510a0a554bbfa235449" @@ -3612,6 +3627,10 @@ node-uuid@~1.4.7: version "1.4.8" resolved "https://registry.yarnpkg.com/node-uuid/-/node-uuid-1.4.8.tgz#b040eb0923968afabf8d32fb1f17f1167fdab907" +node-version@^1.0.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/node-version/-/node-version-1.1.0.tgz#f437d7ba407e65e2c4eaef8887b1718ba523d4f0" + "nopt@2 || 3", nopt@~3.0.1: version "3.0.6" resolved "https://registry.yarnpkg.com/nopt/-/nopt-3.0.6.tgz#c6465dbf08abcd4db359317f79ac68a646b28ff9" @@ -4203,6 +4222,10 @@ promise-inflight@^1.0.1, promise-inflight@~1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/promise-inflight/-/promise-inflight-1.0.1.tgz#98472870bf228132fcbdd868129bad12c3c029e3" +promise-polyfill@^6.0.1: + version "6.0.2" + resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-6.0.2.tgz#d9c86d3dc4dc2df9016e88946defd69b49b41162" + promise-retry@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/promise-retry/-/promise-retry-1.1.1.tgz#6739e968e3051da20ce6497fb2b50f6911df3d6d"