From c472b7e6ccb5faebfacd23c1849e81ed842cfcbc Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Wed, 28 Jun 2017 11:23:40 +0200 Subject: [PATCH] Improve onboarding (#367) * Add branchName to mergeRenovateJson * Split onboarding into branch and PR * Update onboarding description * remove snapshot * Update tests * Update onboarding text --- lib/workers/repository/apis.js | 7 +- lib/workers/repository/index.js | 12 ++- lib/workers/repository/onboarding.js | 96 +++++++++++++++--- .../__snapshots__/onboarding.spec.js.snap | 85 +++++++++++----- test/workers/repository/index.spec.js | 19 +++- test/workers/repository/onboarding.spec.js | 97 +++++++++++++++---- 6 files changed, 249 insertions(+), 67 deletions(-) diff --git a/lib/workers/repository/apis.js b/lib/workers/repository/apis.js index ad4bb61b91..fbf5dad04a 100644 --- a/lib/workers/repository/apis.js +++ b/lib/workers/repository/apis.js @@ -50,8 +50,11 @@ async function initApis(inputConfig) { } // Check for config in `renovate.json` -async function mergeRenovateJson(config) { - const renovateJson = await config.api.getFileJson('renovate.json'); +async function mergeRenovateJson(config, branchName) { + const renovateJson = await config.api.getFileJson( + 'renovate.json', + branchName + ); if (!renovateJson) { config.logger.debug('No renovate.json found'); return config; diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js index 0d28c43ea1..82b8a3b84e 100644 --- a/lib/workers/repository/index.js +++ b/lib/workers/repository/index.js @@ -17,8 +17,7 @@ async function renovateRepository(packageFileConfig) { config = await apis.mergeRenovateJson(config); const repoIsOnboarded = await onboarding.getOnboardingStatus(config); if (!repoIsOnboarded) { - config.logger.info('"Configure Renovate" PR needs to be closed first'); - return; + config = await apis.mergeRenovateJson(config, 'renovate/configure'); } const hasConfiguredPackageFiles = config.packageFiles.length > 0; if (!hasConfiguredPackageFiles) { @@ -36,8 +35,13 @@ async function renovateRepository(packageFileConfig) { config.logger.debug( `Updating ${Object.keys(branchUpgrades).length} branch(es)` ); - for (const branchName of Object.keys(branchUpgrades)) { - await branchWorker.updateBranch(branchUpgrades[branchName]); + if (repoIsOnboarded) { + for (const branchName of Object.keys(branchUpgrades)) { + await branchWorker.updateBranch(branchUpgrades[branchName]); + } + } else { + await onboarding.ensurePr(config, branchUpgrades); + config.logger.info('"Configure Renovate" PR needs to be closed first'); } } catch (error) { // Swallow this error so that other repositories can be processed diff --git a/lib/workers/repository/onboarding.js b/lib/workers/repository/onboarding.js index 4ee7ad0c48..da6be62894 100644 --- a/lib/workers/repository/onboarding.js +++ b/lib/workers/repository/onboarding.js @@ -1,27 +1,22 @@ +const handlebars = require('handlebars'); const stringify = require('json-stringify-pretty-compact'); const defaultsParser = require('../../config/defaults'); +const onboardBranchName = 'renovate/configure'; +const onboardPrTitle = 'Configure Renovate'; + module.exports = { - onboardRepository, + createBranch, + ensurePr, getOnboardingStatus, }; -async function onboardRepository(config) { +async function createBranch(config) { const defaultConfig = defaultsParser.getOnboardingConfig(); - const prBody = `Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! Once you close this Pull Request, we will begin keeping your dependencies up-to-date via automated Pull Requests. - -The [Configuration](https://github.com/singapore/renovate/blob/master/docs/configuration.md) and [Configuration FAQ](https://github.com/singapore/renovate/blob/master/docs/faq.md) documents should be helpful. - -#### Important! - -You do not need to *merge* this Pull Request - renovate will begin even if it's closed *unmerged*. -In fact, you only need to add a \`renovate.json\` file to your repository if you wish to override any default settings. The file is included as part of this PR only in case you wish to change default settings before you start. -If the default settings are all suitable for you, simply close this Pull Request unmerged and your first renovation will begin the next time the program is run.`; - const defaultConfigString = `${stringify(defaultConfig)}\n`; await config.api.commitFilesToBranch( - 'renovate/configure', + onboardBranchName, [ { name: 'renovate.json', @@ -30,9 +25,78 @@ If the default settings are all suitable for you, simply close this Pull Request ], 'Add renovate.json' ); +} + +async function ensurePr(config, branchUpgrades) { + const onboardBranchNames = Object.keys(branchUpgrades); + let prBody = `Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! + +This is an onboarding PR to help you understand and configure Renovate before any changes are made to any \`package.json\` files. Once you close this Pull Request, we will begin keeping your dependencies up-to-date via automated Pull Requests. +--- + +{{PRDESCRIPTION}} + +Would you like to change this? Simply edit the \`renovate.json\` in this branch and Renovate will update this Pull Request description the next time it runs. + +The [Configuration](https://github.com/singapore/renovate/blob/master/docs/configuration.md) and [Configuration FAQ](https://github.com/singapore/renovate/blob/master/docs/faq.md) documents should be helpful if you wish to modify this behaviour. + +--- + +#### Important! + +You do not need to *merge* this Pull Request - renovate will begin even if it's closed *unmerged*. +In fact, you only need to add a \`renovate.json\` file to your repository if you wish to override any default settings. The file is included as part of this PR only in case you wish to change default settings before you start. + +Alternatively, you can add the same configuration settings into a "renovate" section of \`package.json\`, which might be more convenient if you have only one. + +If the default settings are all suitable for you, simply close this Pull Request unmerged and your first renovation will begin the next time the program is run. +`; + + let prDesc = ` +With your current configuration, renovate will initially create the following Pull Requests: + +| Pull Requests (${onboardBranchNames.length}) | +| ------ | +`; + onboardBranchNames.forEach(branchName => { + const upgrades = branchUpgrades[branchName]; + const upgrade0 = upgrades[0]; + const prTitle = handlebars.compile(upgrade0.prTitle)(upgrade0); + prDesc += `| **${prTitle}**<ul>`; + prDesc += `<li>Branch name: \`${branchName}\`</li>`; + upgrades.forEach(upgrade => { + if (upgrade.isPin) { + prDesc += '<li>Pins '; + } else { + prDesc += '<li>Upgrades '; + } + prDesc += `[${upgrade.depName}](${upgrade.repositoryUrl}) in \`${upgrade.depType}\` from \`${upgrade.currentVersion}\` to \`${upgrade.newVersion}\``; + prDesc += '</li>'; + }); + prDesc += '</ul> |\n'; + }); + if (onboardBranchNames.length === 0) { + // Overwrite empty content + prDesc = + 'It looks like your repository dependencies are already up-to-date and no initial Pull Requests will be necessary.'; + } + prBody = prBody.replace('{{PRDESCRIPTION}}', prDesc); + // Check if existing PR exists + const existingPr = await config.api.getBranchPr(onboardBranchName); + if (existingPr) { + // Check if existing PR needs updating + if (existingPr.title === onboardPrTitle && existingPr.body === prBody) { + config.logger.info(`${existingPr.displayNumber} does not need updating`); + return; + } + // PR must need updating + await config.api.updatePr(existingPr.number, onboardPrTitle, prBody); + config.logger.info(`Updated ${existingPr.displayNumber}`); + return; + } const pr = await config.api.createPr( - 'renovate/configure', - 'Configure Renovate', + onboardBranchName, + onboardPrTitle, prBody ); config.logger.debug(`Created ${pr.displayNumber} for configuration`); @@ -64,6 +128,6 @@ async function getOnboardingStatus(config) { ); return false; } - await module.exports.onboardRepository(config); + await module.exports.createBranch(config); return false; } diff --git a/test/workers/repository/__snapshots__/onboarding.spec.js.snap b/test/workers/repository/__snapshots__/onboarding.spec.js.snap index 680ece21f5..9e171f0f9b 100644 --- a/test/workers/repository/__snapshots__/onboarding.spec.js.snap +++ b/test/workers/repository/__snapshots__/onboarding.spec.js.snap @@ -1,33 +1,70 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`lib/workers/repository/onboarding onboardRepository(config) should commit files and create PR 1`] = ` +exports[`lib/workers/repository/onboarding ensurePr(config, branchUpgrades) creates complex pr 1`] = ` Array [ Array [ "renovate/configure", - Array [ - Object { - "contents": "{ - \\"enabled\\": true, - \\"packageFiles\\": [], - \\"depTypes\\": [\\"dependencies\\", \\"devDependencies\\", \\"optionalDependencies\\"], - \\"ignoreDeps\\": [], - \\"pinVersions\\": true, - \\"separateMajorReleases\\": true, - \\"rebaseStalePrs\\": false, - \\"prCreation\\": \\"immediate\\", - \\"automerge\\": \\"none\\", - \\"branchName\\": \\"renovate/{{depName}}-{{newVersionMajor}}.x\\", - \\"commitMessage\\": \\"Update dependency {{depName}} to version {{newVersion}}\\", - \\"maintainYarnLock\\": false, - \\"labels\\": [], - \\"assignees\\": [], - \\"reviewers\\": [] -} + "Configure Renovate", + "Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! + +This is an onboarding PR to help you understand and configure Renovate before any changes are made to any \`package.json\` files. Once you close this Pull Request, we will begin keeping your dependencies up-to-date via automated Pull Requests. +--- + + +With your current configuration, renovate will initially create the following Pull Requests: + +| Pull Requests (2) | +| ------ | +| **Pin a**<ul><li>Branch name: \`branch-a\`</li><li>Pins [a](https://a) in \`undefined\` from \`^1.0.0\` to \`1.1.0\`</li></ul> | +| **Upgrade b**<ul><li>Branch name: \`branch-b\`</li><li>Upgrades [b](https://b) in \`undefined\` from \`1.0.0\` to \`2.0.0\`</li></ul> | + + +Would you like to change this? Simply edit the \`renovate.json\` in this branch and Renovate will update this Pull Request description the next time it runs. + +The [Configuration](https://github.com/singapore/renovate/blob/master/docs/configuration.md) and [Configuration FAQ](https://github.com/singapore/renovate/blob/master/docs/faq.md) documents should be helpful if you wish to modify this behaviour. + +--- + +#### Important! + +You do not need to *merge* this Pull Request - renovate will begin even if it's closed *unmerged*. +In fact, you only need to add a \`renovate.json\` file to your repository if you wish to override any default settings. The file is included as part of this PR only in case you wish to change default settings before you start. + +Alternatively, you can add the same configuration settings into a \\"renovate\\" section of \`package.json\`, which might be more convenient if you have only one. + +If the default settings are all suitable for you, simply close this Pull Request unmerged and your first renovation will begin the next time the program is run. +", + ], +] +`; + +exports[`lib/workers/repository/onboarding ensurePr(config, branchUpgrades) creates pr 1`] = ` +Array [ + Array [ + "renovate/configure", + "Configure Renovate", + "Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! + +This is an onboarding PR to help you understand and configure Renovate before any changes are made to any \`package.json\` files. Once you close this Pull Request, we will begin keeping your dependencies up-to-date via automated Pull Requests. +--- + +It looks like your repository dependencies are already up-to-date and no initial Pull Requests will be necessary. + +Would you like to change this? Simply edit the \`renovate.json\` in this branch and Renovate will update this Pull Request description the next time it runs. + +The [Configuration](https://github.com/singapore/renovate/blob/master/docs/configuration.md) and [Configuration FAQ](https://github.com/singapore/renovate/blob/master/docs/faq.md) documents should be helpful if you wish to modify this behaviour. + +--- + +#### Important! + +You do not need to *merge* this Pull Request - renovate will begin even if it's closed *unmerged*. +In fact, you only need to add a \`renovate.json\` file to your repository if you wish to override any default settings. The file is included as part of this PR only in case you wish to change default settings before you start. + +Alternatively, you can add the same configuration settings into a \\"renovate\\" section of \`package.json\`, which might be more convenient if you have only one. + +If the default settings are all suitable for you, simply close this Pull Request unmerged and your first renovation will begin the next time the program is run. ", - "name": "renovate.json", - }, - ], - "Add renovate.json", ], ] `; diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js index 8d7103e6ff..130db81f97 100644 --- a/test/workers/repository/index.spec.js +++ b/test/workers/repository/index.spec.js @@ -7,10 +7,6 @@ const upgrades = require('../../../lib/workers/repository/upgrades'); const logger = require('../../_fixtures/logger'); -jest.mock('../../../lib/workers/repository/onboarding'); -jest.mock('../../../lib/workers/repository/upgrades'); -jest.mock('../../../lib/workers/branch'); - apis.initApis = jest.fn(input => input); apis.mergeRenovateJson = jest.fn(input => input); apis.detectPackageFiles = jest.fn(input => input); @@ -19,6 +15,10 @@ describe('workers/repository', () => { describe('renovateRepository', () => { let config; beforeEach(() => { + onboarding.getOnboardingStatus = jest.fn(); + onboarding.ensurePr = jest.fn(); + upgrades.groupUpgradesByBranch = jest.fn(); + branchWorker.updateBranch = jest.fn(); config = { logger, packageFiles: [], @@ -42,6 +42,17 @@ describe('workers/repository', () => { await repositoryWorker.renovateRepository(config); expect(branchWorker.updateBranch.mock.calls.length).toBe(3); }); + it('calls ensurePr', async () => { + onboarding.getOnboardingStatus.mockReturnValueOnce(false); + upgrades.groupUpgradesByBranch.mockReturnValueOnce({ + foo: {}, + bar: {}, + baz: {}, + }); + await repositoryWorker.renovateRepository(config); + expect(branchWorker.updateBranch.mock.calls.length).toBe(0); + expect(onboarding.ensurePr.mock.calls.length).toBe(1); + }); it('swallows errors', async () => { apis.initApis.mockImplementationOnce(() => { throw new Error('bad init'); diff --git a/test/workers/repository/onboarding.spec.js b/test/workers/repository/onboarding.spec.js index 24ca1ff910..ee0f2865cd 100644 --- a/test/workers/repository/onboarding.spec.js +++ b/test/workers/repository/onboarding.spec.js @@ -2,29 +2,87 @@ const onboarding = require('../../../lib/workers/repository/onboarding'); const logger = require('../../_fixtures/logger'); describe('lib/workers/repository/onboarding', () => { - describe('onboardRepository(config)', () => { + describe('ensurePr(config, branchUpgrades)', () => { let config; beforeEach(() => { config = { api: { - commitFilesToBranch: jest.fn(), createPr: jest.fn(() => ({ displayNumber: 1 })), + getBranchPr: jest.fn(), + updatePr: jest.fn(), }, logger, }; }); - it('should commit files and create PR', async () => { - config.platform = 'github'; - await onboarding.onboardRepository(config); - expect(config.api.commitFilesToBranch.mock.calls.length).toBe(1); + it('creates pr', async () => { + await onboarding.ensurePr(config, {}); expect(config.api.createPr.mock.calls.length).toBe(1); - expect( - config.api.createPr.mock.calls[0][2].indexOf('Pull Request') - ).not.toBe(-1); - expect( - config.api.createPr.mock.calls[0][2].indexOf('Merge Request') - ).toBe(-1); - expect(config.api.commitFilesToBranch.mock.calls).toMatchSnapshot(); + expect(config.api.updatePr.mock.calls.length).toBe(0); + expect(config.api.createPr.mock.calls).toMatchSnapshot(); + }); + it('updates pr', async () => { + config.api.getBranchPr.mockReturnValueOnce({}); + await onboarding.ensurePr(config, {}); + expect(config.api.createPr.mock.calls.length).toBe(0); + expect(config.api.updatePr.mock.calls.length).toBe(1); + }); + it('does not update pr', async () => { + const existingPrBody = `Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! + +This is an onboarding PR to help you understand and configure Renovate before any changes are made to any \`package.json\` files. Once you close this Pull Request, we will begin keeping your dependencies up-to-date via automated Pull Requests. +--- + +It looks like your repository dependencies are already up-to-date and no initial Pull Requests will be necessary. + +Would you like to change this? Simply edit the \`renovate.json\` in this branch and Renovate will update this Pull Request description the next time it runs. + +The [Configuration](https://github.com/singapore/renovate/blob/master/docs/configuration.md) and [Configuration FAQ](https://github.com/singapore/renovate/blob/master/docs/faq.md) documents should be helpful if you wish to modify this behaviour. + +--- + +#### Important! + +You do not need to *merge* this Pull Request - renovate will begin even if it's closed *unmerged*. +In fact, you only need to add a \`renovate.json\` file to your repository if you wish to override any default settings. The file is included as part of this PR only in case you wish to change default settings before you start. + +Alternatively, you can add the same configuration settings into a "renovate" section of \`package.json\`, which might be more convenient if you have only one. + +If the default settings are all suitable for you, simply close this Pull Request unmerged and your first renovation will begin the next time the program is run. +`; + config.api.getBranchPr.mockReturnValueOnce({ + title: 'Configure Renovate', + body: existingPrBody, + }); + await onboarding.ensurePr(config, {}); + expect(config.api.createPr.mock.calls.length).toBe(0); + expect(config.api.updatePr.mock.calls.length).toBe(0); + }); + it('creates complex pr', async () => { + const branchUpgrades = { + 'branch-a': [ + { + prTitle: 'Pin a', + isPin: true, + depName: 'a', + repositoryUrl: 'https://a', + currentVersion: '^1.0.0', + newVersion: '1.1.0', + }, + ], + 'branch-b': [ + { + prTitle: 'Upgrade b', + depName: 'b', + repositoryUrl: 'https://b', + currentVersion: '1.0.0', + newVersion: '2.0.0', + }, + ], + }; + await onboarding.ensurePr(config, branchUpgrades); + expect(config.api.createPr.mock.calls.length).toBe(1); + expect(config.api.updatePr.mock.calls.length).toBe(0); + expect(config.api.createPr.mock.calls).toMatchSnapshot(); }); }); describe('getOnboardingStatus(config)', () => { @@ -44,29 +102,34 @@ describe('lib/workers/repository/onboarding', () => { const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(true); expect(config.api.findPr.mock.calls.length).toBe(0); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); - it('returns complete if renovate onboarded', async () => { + it('returns true if renovate config present', async () => { config.renovateJsonPresent = true; const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(true); expect(config.api.findPr.mock.calls.length).toBe(0); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); - it('returns complete if pr and pr is closed', async () => { + it('returns true if pr and pr is closed', async () => { config.api.findPr.mockReturnValueOnce({ isClosed: true }); const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(true); expect(config.api.findPr.mock.calls.length).toBe(1); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); - it('returns in progres if pr and pr is not closed', async () => { + it('returns false if pr and pr is not closed', async () => { config.api.findPr.mockReturnValueOnce({}); const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(false); expect(config.api.findPr.mock.calls.length).toBe(1); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(0); }); - it('returns none if no pr', async () => { + it('returns false if no pr', async () => { const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(false); expect(config.api.findPr.mock.calls.length).toBe(1); + expect(config.api.commitFilesToBranch.mock.calls.length).toBe(1); }); }); }); -- GitLab