From 2517c9f1d94f510741c1790bc82065e2a064d154 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Mon, 7 May 2018 12:59:32 +0200 Subject: [PATCH] refactor: renovateRepository split (#1928) --- lib/workers/repository/index.js | 62 +++++-------------- lib/workers/repository/onboarding/pr/index.js | 8 ++- .../repository/onboarding/pr/pr-list.js | 2 +- .../repository/process/extract-update.js | 21 +++++++ lib/workers/repository/process/index.js | 28 +++++++++ lib/workers/repository/{ => process}/write.js | 2 +- lib/workers/repository/updates/generate.js | 5 +- .../__snapshots__/index.spec.js.snap | 35 +---------- test/workers/repository/index.spec.js | 41 +----------- .../pr/__snapshots__/base-branch.spec.js.snap | 7 +++ .../pr/__snapshots__/pr-list.spec.js.snap | 8 +++ .../onboarding/pr/base-branch.spec.js | 26 ++++++++ .../repository/onboarding/pr/index.spec.js | 11 ++-- .../repository/onboarding/pr/pr-list.spec.js | 5 ++ .../process/__snapshots__/index.spec.js.snap | 17 +++++ .../process/__snapshots__/sort.spec.js.snap | 22 +++++++ .../repository/process/extract-update.spec.js | 18 ++++++ test/workers/repository/process/index.spec.js | 27 ++++++++ test/workers/repository/process/sort.spec.js | 30 +++++++++ .../repository/{ => process}/write.spec.js | 8 ++- 20 files changed, 248 insertions(+), 135 deletions(-) create mode 100644 lib/workers/repository/process/extract-update.js create mode 100644 lib/workers/repository/process/index.js rename lib/workers/repository/{ => process}/write.js (97%) create mode 100644 test/workers/repository/onboarding/pr/__snapshots__/base-branch.spec.js.snap create mode 100644 test/workers/repository/onboarding/pr/base-branch.spec.js create mode 100644 test/workers/repository/process/__snapshots__/index.spec.js.snap create mode 100644 test/workers/repository/process/__snapshots__/sort.spec.js.snap create mode 100644 test/workers/repository/process/extract-update.spec.js create mode 100644 test/workers/repository/process/index.spec.js create mode 100644 test/workers/repository/process/sort.spec.js rename test/workers/repository/{ => process}/write.spec.js (90%) diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js index 6f87d45d01..aa3035a0f9 100644 --- a/lib/workers/repository/index.js +++ b/lib/workers/repository/index.js @@ -1,12 +1,9 @@ const { initRepo } = require('./init'); -const { determineUpdates } = require('./updates'); const { ensureOnboardingPr } = require('./onboarding/pr'); -const { writeUpdates } = require('./write'); const { handleError } = require('./error'); -const { finaliseRepo } = require('./finalise'); const { processResult } = require('./result'); -const { resolvePackageFiles } = require('../../manager'); -const { sortBranches } = require('./process/sort'); +const { processRepo } = require('./process'); +const { finaliseRepo } = require('./finalise'); module.exports = { renovateRepository, @@ -17,52 +14,21 @@ async function renovateRepository(repoConfig) { logger.setMeta({ repository: config.repository }); logger.info('Renovating repository'); logger.trace({ config }, 'renovateRepository()'); - let commonConfig; - let res; try { config = await initRepo(config); - if (config.baseBranches && config.baseBranches.length) { - // At this point we know if we have multiple branches - // Do the following for every branch - commonConfig = JSON.parse(JSON.stringify(config)); - const configs = []; - logger.info({ baseBranches: config.baseBranches }, 'baseBranches'); - for (const [index, baseBranch] of commonConfig.baseBranches.entries()) { - config = JSON.parse(JSON.stringify(commonConfig)); - config.baseBranch = baseBranch; - config.branchPrefix += - config.baseBranches.length > 1 ? `${baseBranch}-` : ''; - platform.setBaseBranch(baseBranch); - config = await resolvePackageFiles(config); - config = await determineUpdates(config); - configs[index] = config; - } - // Combine all the results into one - for (const [index, entry] of configs.entries()) { - if (index === 0) { - config = entry; - } else { - config.branches = config.branches.concat(entry.branches); - } - } - // istanbul ignore next - config.branchList = config.branches.map(branch => branch.branchName); - } else { - config = await resolvePackageFiles(config); - config = await determineUpdates(config); + let res; + let branches; + let branchList; + let packageFiles; + ({ res, branches, branchList, packageFiles } = await processRepo(config)); // eslint-disable-line prefer-const + if (!config.repoIsOnboarded) { + res = await ensureOnboardingPr(config, packageFiles, branches); } - sortBranches(config.branches); - res = config.repoIsOnboarded - ? await writeUpdates(config) - : await ensureOnboardingPr(config, config.branches); - logger.setMeta({ repository: config.repository }); - config.branchPrefix = commonConfig - ? commonConfig.branchPrefix - : config.branchPrefix; - await finaliseRepo(commonConfig || config, config.branchList); + await finaliseRepo(config, branchList); + return processResult(config, res); } catch (err) /* istanbul ignore next */ { - res = await handleError(config, err); + return processResult(config, await handleError(config, err)); + } finally { + logger.info('Finished repository'); } - logger.info('Finished repository'); - return processResult(config, res); } diff --git a/lib/workers/repository/onboarding/pr/index.js b/lib/workers/repository/onboarding/pr/index.js index 5012b9f527..1832a368e4 100644 --- a/lib/workers/repository/onboarding/pr/index.js +++ b/lib/workers/repository/onboarding/pr/index.js @@ -3,7 +3,7 @@ const { getErrors, getWarnings } = require('./errors-warnings'); const { getBaseBranchDesc } = require('./base-branch'); const { getPrList } = require('./pr-list'); -async function ensureOnboardingPr(config, branches) { +async function ensureOnboardingPr(config, packageFiles, branches) { logger.debug('ensureOnboardingPr()'); logger.trace({ config }); const onboardingBranch = `renovate/configure`; @@ -27,15 +27,17 @@ You can post questions in [our Config Help repository](https://github.com/renova --- `; let prBody = prTemplate; - if (config.packageFiles.length) { + if (packageFiles && packageFiles.length) { prBody = prBody.replace( '{{PACKAGE FILES}}', '## Detected Package Files\n\n' + - config.packageFiles + packageFiles .map(packageFile => ` * \`${packageFile.packageFile}\``) .join('\n') ) + '\n'; + } else { + prBody = prBody.replace('{{PACKAGE FILES}}\n', ''); } prBody = prBody.replace('{{CONFIG}}\n', getConfigDesc(config)); prBody = prBody.replace('{{WARNINGS}}\n', getWarnings(config)); diff --git a/lib/workers/repository/onboarding/pr/pr-list.js b/lib/workers/repository/onboarding/pr/pr-list.js index 62cc391a48..9fd48e00e8 100644 --- a/lib/workers/repository/onboarding/pr/pr-list.js +++ b/lib/workers/repository/onboarding/pr/pr-list.js @@ -20,7 +20,7 @@ function getPrList(config, branches) { prDesc += ` - Schedule: ${JSON.stringify(branch.schedule)}\n`; } prDesc += ` - Branch name: \`${branch.branchName}\`\n`; - prDesc += config.baseBranch + prDesc += branch.baseBranch ? ` - Merge into: \`${branch.baseBranch}\`\n` : ''; for (const upgrade of branch.upgrades) { diff --git a/lib/workers/repository/process/extract-update.js b/lib/workers/repository/process/extract-update.js new file mode 100644 index 0000000000..c5d2d6e719 --- /dev/null +++ b/lib/workers/repository/process/extract-update.js @@ -0,0 +1,21 @@ +const { determineUpdates } = require('../updates'); +const { writeUpdates } = require('./write'); +const { sortBranches } = require('./sort'); +const { resolvePackageFiles } = require('../../../manager'); + +module.exports = { + extractAndUpdate, +}; + +async function extractAndUpdate(input) { + let config = await resolvePackageFiles(input); + config = await determineUpdates(config); + const { branches, branchList, packageFiles } = config; + sortBranches(branches); + let res; + if (config.repoIsOnboarded) { + res = await writeUpdates(config); + } + logger.setMeta({ repository: config.repository }); + return { res, branches, branchList, packageFiles }; +} diff --git a/lib/workers/repository/process/index.js b/lib/workers/repository/process/index.js new file mode 100644 index 0000000000..61de6660b0 --- /dev/null +++ b/lib/workers/repository/process/index.js @@ -0,0 +1,28 @@ +const { mergeChildConfig } = require('../../../config'); +const { extractAndUpdate } = require('./extract-update'); + +module.exports = { + processRepo, +}; + +async function processRepo(config) { + if (config.baseBranches && config.baseBranches.length) { + logger.info({ baseBranches: config.baseBranches }, 'baseBranches'); + let res; + let branches = []; + let branchList = []; + for (const baseBranch of config.baseBranches) { + logger.debug(`baseBranch: ${baseBranch}`); + const baseBranchConfig = mergeChildConfig(config, { baseBranch }); + baseBranchConfig.branchPrefix += `${baseBranch}-`; + baseBranchConfig.hasBaseBranches = true; + platform.setBaseBranch(baseBranch); + const baseBranchRes = await extractAndUpdate(baseBranchConfig); + ({ res } = baseBranchRes); + branches = branches.concat(baseBranchRes.branches); + branchList = branchList.concat(baseBranchRes.branchList); + } + return { res, branches, branchList }; + } + return extractAndUpdate(config); +} diff --git a/lib/workers/repository/write.js b/lib/workers/repository/process/write.js similarity index 97% rename from lib/workers/repository/write.js rename to lib/workers/repository/process/write.js index edd05a6d27..56b7c87b30 100644 --- a/lib/workers/repository/write.js +++ b/lib/workers/repository/process/write.js @@ -1,7 +1,7 @@ const moment = require('moment'); const tmp = require('tmp-promise'); -const branchWorker = require('../branch'); +const branchWorker = require('../../branch'); module.exports = { writeUpdates, diff --git a/lib/workers/repository/updates/generate.js b/lib/workers/repository/updates/generate.js index 82ebd1e234..8ed0b5b5c5 100644 --- a/lib/workers/repository/updates/generate.js +++ b/lib/workers/repository/updates/generate.js @@ -104,10 +104,7 @@ function generateBranchConfig(branchUpgrades) { } else { [upgrade.prTitle] = upgrade.commitMessage.split('\n'); } - upgrade.prTitle += - upgrade.baseBranches && upgrade.baseBranches.length > 1 - ? ' ({{baseBranch}})' - : ''; + upgrade.prTitle += upgrade.hasBaseBranches ? ' ({{baseBranch}})' : ''; logger.debug(`prTitle: ` + JSON.stringify(upgrade.prTitle)); // Compile again to allow for nested handlebars templates upgrade.prTitle = handlebars.compile(upgrade.prTitle)(upgrade); diff --git a/test/workers/repository/__snapshots__/index.spec.js.snap b/test/workers/repository/__snapshots__/index.spec.js.snap index 05596f40b3..559f51fc6d 100644 --- a/test/workers/repository/__snapshots__/index.spec.js.snap +++ b/test/workers/repository/__snapshots__/index.spec.js.snap @@ -1,39 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`workers/repository renovateRepository() ensures onboarding pr 1`] = ` -Object { - "res": "done", - "status": "onboarding", -} -`; - -exports[`workers/repository renovateRepository() ensures onboarding pr 2`] = ` -Array [ - Object { - "prTitle": "aaa", - "type": "pin", - }, - Object { - "prTitle": "bbb", - "type": "pin", - }, - Object { - "prTitle": "aaa", - "type": "minor", - }, -] -`; - -exports[`workers/repository renovateRepository() handles baseBranches 1`] = ` -Object { - "res": "done", - "status": "enabled", -} -`; - exports[`workers/repository renovateRepository() writes 1`] = ` Object { - "res": "done", - "status": "enabled", + "res": undefined, + "status": "onboarding", } `; diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js index acdf5f2fd5..894278d63f 100644 --- a/test/workers/repository/index.spec.js +++ b/test/workers/repository/index.spec.js @@ -1,16 +1,15 @@ const { initRepo } = require('../../../lib/workers/repository/init'); const { determineUpdates } = require('../../../lib/workers/repository/updates'); -const { writeUpdates } = require('../../../lib/workers/repository/write'); const { - ensureOnboardingPr, -} = require('../../../lib/workers/repository/onboarding/pr'); + writeUpdates, +} = require('../../../lib/workers/repository/process/write'); const { renovateRepository } = require('../../../lib/workers/repository/index'); jest.mock('../../../lib/workers/repository/init'); jest.mock('../../../lib/workers/repository/init/apis'); jest.mock('../../../lib/workers/repository/updates'); jest.mock('../../../lib/workers/repository/onboarding/pr'); -jest.mock('../../../lib/workers/repository/write'); +jest.mock('../../../lib/workers/repository/process/write'); jest.mock('../../../lib/workers/repository/finalise'); jest.mock('../../../lib/manager'); jest.mock('delay'); @@ -33,39 +32,5 @@ describe('workers/repository', () => { const res = await renovateRepository(config, 'some-token'); expect(res).toMatchSnapshot(); }); - it('ensures onboarding pr', async () => { - initRepo.mockReturnValue({}); - determineUpdates.mockReturnValue({ - repoIsOnboarded: false, - branches: [ - { - type: 'pin', - prTitle: 'bbb', - }, - { - type: 'pin', - prTitle: 'aaa', - }, - { - type: 'minor', - prTitle: 'aaa', - }, - ], - }); - ensureOnboardingPr.mockReturnValue('onboarding'); - const res = await renovateRepository(config, 'some-token'); - expect(res).toMatchSnapshot(); - expect(ensureOnboardingPr.mock.calls[0][0].branches).toMatchSnapshot(); - }); - it('handles baseBranches', async () => { - initRepo.mockReturnValue({ baseBranches: ['master', 'next'] }); - determineUpdates.mockReturnValue({ - repoIsOnboarded: true, - branches: [], - }); - writeUpdates.mockReturnValueOnce('done'); - const res = await renovateRepository(config, 'some-token'); - expect(res).toMatchSnapshot(); - }); }); }); diff --git a/test/workers/repository/onboarding/pr/__snapshots__/base-branch.spec.js.snap b/test/workers/repository/onboarding/pr/__snapshots__/base-branch.spec.js.snap new file mode 100644 index 0000000000..238f182681 --- /dev/null +++ b/test/workers/repository/onboarding/pr/__snapshots__/base-branch.spec.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`workers/repository/onboarding/pr/base-branch getBaseBranchDesc() describes baseBranch 1`] = ` +"You have configured renovate to use branch \`some-branch\` as base branch. + +" +`; diff --git a/test/workers/repository/onboarding/pr/__snapshots__/pr-list.spec.js.snap b/test/workers/repository/onboarding/pr/__snapshots__/pr-list.spec.js.snap index 470f56cd5c..8b500c27c8 100644 --- a/test/workers/repository/onboarding/pr/__snapshots__/pr-list.spec.js.snap +++ b/test/workers/repository/onboarding/pr/__snapshots__/pr-list.spec.js.snap @@ -1,5 +1,13 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`workers/repository/onboarding/pr/pr-list getPrList() handles emptyu 1`] = ` +" +### What to Expect + +It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. +" +`; + exports[`workers/repository/onboarding/pr/pr-list getPrList() handles multiple 1`] = ` " ### What to Expect diff --git a/test/workers/repository/onboarding/pr/base-branch.spec.js b/test/workers/repository/onboarding/pr/base-branch.spec.js new file mode 100644 index 0000000000..7b6edc92b5 --- /dev/null +++ b/test/workers/repository/onboarding/pr/base-branch.spec.js @@ -0,0 +1,26 @@ +const defaultConfig = require('../../../../../lib/config/defaults').getConfig(); + +const { + getBaseBranchDesc, +} = require('../../../../../lib/workers/repository/onboarding/pr/base-branch'); + +describe('workers/repository/onboarding/pr/base-branch', () => { + describe('getBaseBranchDesc()', () => { + let config; + beforeEach(() => { + jest.resetAllMocks(); + config = { + ...defaultConfig, + }; + }); + it('returns empty if no baseBranch', () => { + const res = getBaseBranchDesc(config); + expect(res).toEqual(''); + }); + it('describes baseBranch', () => { + config.baseBranch = 'some-branch'; + const res = getBaseBranchDesc(config); + expect(res).toMatchSnapshot(); + }); + }); +}); diff --git a/test/workers/repository/onboarding/pr/index.spec.js b/test/workers/repository/onboarding/pr/index.spec.js index 5230911877..32d2ca3b22 100644 --- a/test/workers/repository/onboarding/pr/index.spec.js +++ b/test/workers/repository/onboarding/pr/index.spec.js @@ -7,6 +7,8 @@ const { describe('workers/repository/onboarding/pr', () => { describe('ensureOnboardingPr()', () => { let config; + let packageFiles; + let branches; beforeEach(() => { jest.resetAllMocks(); config = { @@ -14,13 +16,14 @@ describe('workers/repository/onboarding/pr', () => { errors: [], warnings: [], description: [], - packageFiles: [{ packageFile: 'package.json' }], }; + packageFiles = [{ packageFile: 'package.json' }]; + branches = []; platform.createPr.mockReturnValue({}); }); let createPrBody; it('creates PR', async () => { - await ensureOnboardingPr(config, []); + await ensureOnboardingPr(config, packageFiles, branches); expect(platform.createPr.mock.calls).toHaveLength(1); createPrBody = platform.createPr.mock.calls[0][2]; }); @@ -29,7 +32,7 @@ describe('workers/repository/onboarding/pr', () => { title: 'Configure Renovate', body: createPrBody, }); - await ensureOnboardingPr(config, []); + await ensureOnboardingPr(config, packageFiles, branches); expect(platform.createPr.mock.calls).toHaveLength(0); expect(platform.updatePr.mock.calls).toHaveLength(0); }); @@ -39,7 +42,7 @@ describe('workers/repository/onboarding/pr', () => { title: 'Configure Renovate', body: createPrBody, }); - await ensureOnboardingPr(config, []); + await ensureOnboardingPr(config, [], branches); expect(platform.createPr.mock.calls).toHaveLength(0); expect(platform.updatePr.mock.calls).toHaveLength(1); }); diff --git a/test/workers/repository/onboarding/pr/pr-list.spec.js b/test/workers/repository/onboarding/pr/pr-list.spec.js index c214ecec4e..c6350a1ff4 100644 --- a/test/workers/repository/onboarding/pr/pr-list.spec.js +++ b/test/workers/repository/onboarding/pr/pr-list.spec.js @@ -13,6 +13,11 @@ describe('workers/repository/onboarding/pr/pr-list', () => { ...defaultConfig, }; }); + it('handles emptyu', () => { + const branches = []; + const res = getPrList(config, branches); + expect(res).toMatchSnapshot(); + }); it('has special lock file maintenance description', () => { const branches = [ { diff --git a/test/workers/repository/process/__snapshots__/index.spec.js.snap b/test/workers/repository/process/__snapshots__/index.spec.js.snap new file mode 100644 index 0000000000..b9bf68470c --- /dev/null +++ b/test/workers/repository/process/__snapshots__/index.spec.js.snap @@ -0,0 +1,17 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`workers/repository/process/index processRepo() processes baseBranches 1`] = ` +Object { + "branchList": Array [ + undefined, + undefined, + ], + "branches": Array [ + undefined, + undefined, + ], + "res": undefined, +} +`; + +exports[`workers/repository/process/index processRepo() processes single branches 1`] = `undefined`; diff --git a/test/workers/repository/process/__snapshots__/sort.spec.js.snap b/test/workers/repository/process/__snapshots__/sort.spec.js.snap new file mode 100644 index 0000000000..973edd26c1 --- /dev/null +++ b/test/workers/repository/process/__snapshots__/sort.spec.js.snap @@ -0,0 +1,22 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`workers/repository/process/sort sortBranches() sorts based on type and prTitle 1`] = ` +Array [ + Object { + "prTitle": "some other pin", + "type": "pin", + }, + Object { + "prTitle": "some pin", + "type": "pin", + }, + Object { + "prTitle": "a minor update", + "type": "minor", + }, + Object { + "prTitle": "some major update", + "type": "major", + }, +] +`; diff --git a/test/workers/repository/process/extract-update.spec.js b/test/workers/repository/process/extract-update.spec.js new file mode 100644 index 0000000000..f91a8c3bca --- /dev/null +++ b/test/workers/repository/process/extract-update.spec.js @@ -0,0 +1,18 @@ +const { + extractAndUpdate, +} = require('../../../../lib/workers/repository/process/extract-update'); +const updates = require('../../../../lib/workers/repository/updates'); + +jest.mock('../../../../lib/manager'); +jest.mock('../../../../lib/workers/repository/updates'); +jest.mock('../../../../lib/workers/repository/process/sort'); +jest.mock('../../../../lib/workers/repository/process/write'); + +describe('workers/repository/process/extract-update', () => { + describe('extractAndUpdate()', () => { + it('runs', async () => { + updates.determineUpdates.mockReturnValue({ repoIsOnboarded: true }); + await extractAndUpdate(); + }); + }); +}); diff --git a/test/workers/repository/process/index.spec.js b/test/workers/repository/process/index.spec.js new file mode 100644 index 0000000000..01dcc5ae57 --- /dev/null +++ b/test/workers/repository/process/index.spec.js @@ -0,0 +1,27 @@ +const { + processRepo, +} = require('../../../../lib/workers/repository/process/index'); +const extractUpdate = require('../../../../lib/workers/repository/process/extract-update'); + +jest.mock('../../../../lib/workers/repository/process/extract-update'); + +let config; +beforeEach(() => { + jest.resetAllMocks(); + config = require('../../../_fixtures/config'); +}); + +describe('workers/repository/process/index', () => { + describe('processRepo()', () => { + it('processes single branches', async () => { + const res = await processRepo(config); + expect(res).toMatchSnapshot(); + }); + it('processes baseBranches', async () => { + extractUpdate.extractAndUpdate.mockReturnValue({}); + config.baseBranches = ['branch1', 'branch2']; + const res = await processRepo(config); + expect(res).toMatchSnapshot(); + }); + }); +}); diff --git a/test/workers/repository/process/sort.spec.js b/test/workers/repository/process/sort.spec.js new file mode 100644 index 0000000000..71e9fa3ece --- /dev/null +++ b/test/workers/repository/process/sort.spec.js @@ -0,0 +1,30 @@ +const { + sortBranches, +} = require('../../../../lib/workers/repository/process/sort'); + +describe('workers/repository/process/sort', () => { + describe('sortBranches()', () => { + it('sorts based on type and prTitle', () => { + const branches = [ + { + type: 'major', + prTitle: 'some major update', + }, + { + type: 'pin', + prTitle: 'some pin', + }, + { + type: 'pin', + prTitle: 'some other pin', + }, + { + type: 'minor', + prTitle: 'a minor update', + }, + ]; + sortBranches(branches); + expect(branches).toMatchSnapshot(); + }); + }); +}); diff --git a/test/workers/repository/write.spec.js b/test/workers/repository/process/write.spec.js similarity index 90% rename from test/workers/repository/write.spec.js rename to test/workers/repository/process/write.spec.js index 00f8b604c0..48f1d22c1a 100644 --- a/test/workers/repository/write.spec.js +++ b/test/workers/repository/process/write.spec.js @@ -1,5 +1,7 @@ -const { writeUpdates } = require('../../../lib/workers/repository/write'); -const branchWorker = require('../../../lib/workers/branch'); +const { + writeUpdates, +} = require('../../../../lib/workers/repository/process/write'); +const branchWorker = require('../../../../lib/workers/branch'); const moment = require('moment'); branchWorker.processBranch = jest.fn(); @@ -7,7 +9,7 @@ branchWorker.processBranch = jest.fn(); let config; beforeEach(() => { jest.resetAllMocks(); - config = { ...require('../../_fixtures/config') }; + config = { ...require('../../../_fixtures/config') }; }); describe('workers/repository/write', () => { -- GitLab