From 2f987a4037c496a6cb1a3bd029690d92051e99cb Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Tue, 4 Jul 2017 12:39:28 +0200 Subject: [PATCH] feat: Show warnings and errors in Onboarding PR (#432) Warnings and Errors are bubbled up when renovating a repository, together with the existing upgrades. The Onboarding PR ("Configure Renovate") now displays them and encourages the user to fix before merging/closing the PR. Closes #414 --- lib/workers/package/index.js | 43 +++-- lib/workers/package/versions.js | 36 ++-- lib/workers/repository/index.js | 8 +- lib/workers/repository/onboarding.js | 29 ++- lib/workers/repository/upgrades.js | 54 ++++-- .../package/__snapshots__/index.spec.js.snap | 162 ++++++++++++++++ .../__snapshots__/versions.spec.js.snap | 44 +++++ test/workers/package/index.spec.js | 16 +- test/workers/package/versions.spec.js | 30 ++- .../__snapshots__/onboarding.spec.js.snap | 61 +++++- .../__snapshots__/upgrades.spec.js.snap | 175 ++++++++++++------ test/workers/repository/index.spec.js | 10 +- test/workers/repository/onboarding.spec.js | 57 +++++- test/workers/repository/upgrades.spec.js | 35 +++- 14 files changed, 610 insertions(+), 150 deletions(-) diff --git a/lib/workers/package/index.js b/lib/workers/package/index.js index 9b40ca2bf1..2fe7c3ac23 100644 --- a/lib/workers/package/index.js +++ b/lib/workers/package/index.js @@ -9,7 +9,7 @@ module.exports = { findUpgrades, }; -// Returns all upgrades for a given dependency config +// Returns all results for a given dependency config async function findUpgrades(config) { logger = config.logger || logger; if (config.enabled === false) { @@ -25,27 +25,32 @@ async function findUpgrades(config) { logger.debug('Skipping package as it is not scheduled'); return []; } + let results = []; const npmDep = await npmApi.getDependency(config.depName, logger); - // If dependency lookup fails then warn and return - if (!npmDep) { - logger.warn(`Failed to look up dependency ${config.depName}`); - return []; - } - const upgrades = await versions.determineUpgrades(npmDep, config); - if (upgrades.length > 0) { - logger.info( - { dependency: config.depName }, - `${upgrades.length} upgrade(s) available: ${upgrades.map( - upgrade => upgrade.newVersion - )}` - ); + if (npmDep) { + results = await versions.determineUpgrades(npmDep, config); + if (results.length > 0) { + logger.info( + { dependency: config.depName }, + `${results.length} result(s): ${results.map( + upgrade => upgrade.newVersion + )}` + ); + } } else { - logger.debug(`${config.depName}: No upgrades required`); + // If dependency lookup fails then warn and return + const result = { + upgradeType: 'error', + message: 'Failed to look up dependency', + }; + logger.warn(result.message); + results = [result]; } - // Flatten the upgrade on top of config, add repositoryUrl - return upgrades.map(upgrade => { - const upg = configParser.mergeChildConfig(config, upgrade); - upg.repositoryUrl = npmDep.repositoryUrl; + logger.debug(`${config.depName} results: ${JSON.stringify(results)}`); + // Flatten the result on top of config, add repositoryUrl + return results.map(result => { + const upg = configParser.mergeChildConfig(config, result); + upg.repositoryUrl = npmDep ? npmDep.repositoryUrl : ''; return configParser.filterConfig(upg, 'branch'); }); } diff --git a/lib/workers/package/versions.js b/lib/workers/package/versions.js index 731807f051..83e58f045f 100644 --- a/lib/workers/package/versions.js +++ b/lib/workers/package/versions.js @@ -1,4 +1,4 @@ -const logger = require('../../logger'); +let logger = require('../../logger'); const semver = require('semver'); const stable = require('semver-stable'); const _ = require('lodash'); @@ -13,20 +13,21 @@ module.exports = { }; function determineUpgrades(npmDep, config) { + logger = config.logger || logger; + const result = { + upgradeType: 'warning', + }; const currentVersion = config.currentVersion; if (!isValidVersion(currentVersion)) { - const knownTags = ['latest', 'next', 'future', 'alpha', 'beta']; - if (knownTags.indexOf(currentVersion) === -1) { - logger.warn(`${npmDep.name} currentVersion ${currentVersion} is invalid`); - } else { - logger.debug(`Skipping ${npmDep.name} with tag ${currentVersion}`); - } - return []; + result.message = `Dependency uses tag "${currentVersion}" as its version so that will never be changed`; + logger.warn(result.message); + return [result]; } const versions = npmDep.versions; if (!versions || Object.keys(versions).length === 0) { - logger.warn(`${npmDep.name} - no versions`); - return []; + result.message = `No versions returned from registry for this package`; + logger.warn(result.message); + return [result]; } const versionList = Object.keys(versions); const allUpgrades = {}; @@ -122,13 +123,14 @@ function determineUpgrades(npmDep, config) { const semverParsed = semverUtils.parseRange(currentVersion); if (semverParsed.length > 1) { // We don't know how to support complex semver ranges, so don't upgrade - logger.warn(`Can't support upgrading complex range ${currentVersion}`); - return []; + result.message = `Complex semver ranges such as "${currentVersion}" are not yet supported so won't ever be upgraded`; + logger.warn(result.message); + return [result]; } // We know we have a simple semver, now check which operator it is const currentSemver = semverParsed[0]; // Loop through all upgrades and convert to ranges - return _(upgrades) + const rangedUpgrades = _(upgrades) .reject(upgrade => upgrade.upgradeType === 'pin') .map(upgrade => Object.assign(upgrade, { isRange: true })) .map(upgrade => { @@ -171,11 +173,17 @@ function determineUpgrades(npmDep, config) { // Example: 1.2.x return Object.assign(upgrade, { newVersion: `${major}.${minor}.x` }); } - logger.warn(`Unsupported semver type: ${currentSemver}`); + result.message = `The current semver range "${currentVersion}" is not supported so won't ever be upgraded`; + logger.warn(result.message); return null; }) .compact() .value(); + if (result.message) { + // There must have been an error converting to ranges + return [result]; + } + return rangedUpgrades; } function isRange(input) { diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js index 736c74d148..f5854ce6a6 100644 --- a/lib/workers/repository/index.js +++ b/lib/workers/repository/index.js @@ -43,10 +43,8 @@ async function renovateRepository(packageFileConfig) { } } const allUpgrades = await upgrades.determineRepoUpgrades(config); - const branchUpgrades = await upgrades.branchifyUpgrades( - allUpgrades, - config.logger - ); + const res = await upgrades.branchifyUpgrades(allUpgrades, config.logger); + const branchUpgrades = res.branchUpgrades; config.logger.debug( `Updating ${Object.keys(branchUpgrades).length} branch(es)` ); @@ -55,7 +53,7 @@ async function renovateRepository(packageFileConfig) { await branchWorker.processBranchUpgrades(branchUpgrades[branchName]); } } else { - await onboarding.ensurePr(config, branchUpgrades); + await onboarding.ensurePr(config, res); config.logger.info('"Configure Renovate" PR needs to be closed first'); } } catch (error) { diff --git a/lib/workers/repository/onboarding.js b/lib/workers/repository/onboarding.js index 54b62d373c..95f93a0528 100644 --- a/lib/workers/repository/onboarding.js +++ b/lib/workers/repository/onboarding.js @@ -27,7 +27,10 @@ async function createBranch(config) { ); } -async function ensurePr(config, branchUpgrades) { +async function ensurePr(config, upgradeData) { + const branchUpgrades = upgradeData.branchUpgrades; + const warnings = upgradeData.warnings; + const errors = upgradeData.errors; const onboardBranchNames = Object.keys(branchUpgrades); let prBody = `Welcome to [Renovate](https://keylocation.sg/our-tech/renovate)! @@ -52,6 +55,30 @@ Alternatively, you can add the same configuration settings into a "renovate" sec 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. `; + if (warnings.length) { + let prWarnings = `---\n\n### Warnings (${warnings.length})\n\n`; + prWarnings += `Please correct - or verify that you can safely ignore - these warnings before you merge this PR. +`; + warnings.forEach(warning => { + prWarnings += `- \`${warning.depName}\`: ${warning.message}\n`; + }); + prWarnings += '\n---'; + prBody = prBody.replace('---', prWarnings); + } + if (errors.length) { + let prErrors = `---\n\n## Errors (${errors.length})\n\n`; + prErrors += `Renovate has raised errors when processing this repository that you should fix before merging or closing this PR. + +Please make any fixes in _this branch_. +`; + errors.forEach(error => { + prErrors += `- \`${error.depName}\`: ${error.message}\n`; + }); + prErrors += + '\nFeel free to raise create a [GitHub Issue](https:/github.com/singapore/renovate/issues) to ask any questions.'; + prErrors += '\n\n---'; + prBody = prBody.replace('---', prErrors); + } let prDesc = ` With your current configuration, renovate will initially create the following Pull Requests: diff --git a/lib/workers/repository/upgrades.js b/lib/workers/repository/upgrades.js index 41080208ba..520cd51a22 100644 --- a/lib/workers/repository/upgrades.js +++ b/lib/workers/repository/upgrades.js @@ -30,30 +30,46 @@ async function determineRepoUpgrades(config) { async function branchifyUpgrades(upgrades, logger) { logger.trace({ config: upgrades }, 'branchifyUpgrades'); logger.info(`Processing ${upgrades.length} dependency upgrade(s)`); - const branchUpgrades = {}; + const result = { + errors: [], + warnings: [], + branchUpgrades: {}, + }; for (const upg of upgrades) { const upgrade = Object.assign({}, upg); - // Check whether to use a group name - let branchName; - if (upgrade.groupName) { - // if groupName is defined then use group branchName template for combining - logger.debug( - { branchName }, - `Dependency ${upgrade.depName} is part of group '${upgrade.groupName}'` - ); - upgrade.groupSlug = - upgrade.groupSlug || - upgrade.groupName.toLowerCase().replace(/[^a-z0-9+]+/g, '-'); - branchName = handlebars.compile(upgrade.group.branchName)(upgrade); + // Split out errors and wrnings first + if (upgrade.upgradeType === 'error') { + result.errors.push(upgrade); + } else if (upgrade.upgradeType === 'warning') { + result.warnings.push(upgrade); } else { - // Use regular branchName template - branchName = handlebars.compile(upgrade.branchName)(upgrade); + // Check whether to use a group name + let branchName; + if (upgrade.groupName) { + // if groupName is defined then use group branchName template for combining + logger.debug( + { branchName }, + `Dependency ${upgrade.depName} is part of group '${upgrade.groupName}'` + ); + upgrade.groupSlug = + upgrade.groupSlug || + upgrade.groupName.toLowerCase().replace(/[^a-z0-9+]+/g, '-'); + branchName = handlebars.compile(upgrade.group.branchName)(upgrade); + } else { + // Use regular branchName template + branchName = handlebars.compile(upgrade.branchName)(upgrade); + } + result.branchUpgrades[branchName] = + result.branchUpgrades[branchName] || []; + result.branchUpgrades[branchName] = [upgrade].concat( + result.branchUpgrades[branchName] + ); } - branchUpgrades[branchName] = branchUpgrades[branchName] || []; - branchUpgrades[branchName] = [upgrade].concat(branchUpgrades[branchName]); } - logger.debug(`Returning ${Object.keys(branchUpgrades).length} branch(es)`); - return branchUpgrades; + logger.debug( + `Returning ${Object.keys(result.branchUpgrades).length} branch(es)` + ); + return result; } function getPackageFileConfig(repoConfig, index) { diff --git a/test/workers/package/__snapshots__/index.spec.js.snap b/test/workers/package/__snapshots__/index.spec.js.snap index 7c7e3242ee..28ae93077f 100644 --- a/test/workers/package/__snapshots__/index.spec.js.snap +++ b/test/workers/package/__snapshots__/index.spec.js.snap @@ -24,3 +24,165 @@ Array [ "repositoryUrl", ] `; + +exports[`lib/workers/package/index findUpgrades(config) returns error if no npm dep found 1`] = ` +Array [ + Object { + "assignees": Array [], + "automerge": "none", + "automergeType": "pr", + "branchName": "renovate/{{depName}}-{{newVersionMajor}}.x", + "commitMessage": "{{semanticPrefix}}Update dependency {{depName}} to version {{newVersion}}", + "depName": "foo", + "group": Object { + "branchName": "renovate/{{groupSlug}}", + "commitMessage": "{{semanticPrefix}}Renovate {{groupName}} packages", + "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request renovates the package group \\"{{groupName}}\\". + +{{#each upgrades as |upgrade|}} +- [{{upgrade.depName}}]({{upgrade.repositoryUrl}}): from \`{{upgrade.currentVersion}}\` to \`{{upgrade.newVersion}}\` +{{/each}} + +{{#unless isPin}} +### Commits + +{{#each upgrades as |upgrade|}} +{{#if upgrade.releases.length}} +<details> +<summary>{{upgrade.githubName}}</summary> +{{#each upgrade.releases as |release|}} + +#### {{release.version}} +{{#each release.commits as |commit|}} +- [\`{{commit.shortSha}}\`]({{commit.url}}){{commit.message}} +{{/each}} +{{/each}} + +</details> +{{/if}} +{{/each}} +{{/unless}} +<br /> + +This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://keylocation.sg/our-tech/renovate).", + "prTitle": "{{semanticPrefix}}Renovate {{groupName}} packages", + "recreateClosed": true, + }, + "groupName": null, + "groupSlug": null, + "labels": Array [], + "lazyGrouping": true, + "message": "Failed to look up dependency", + "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request updates dependency [{{depName}}]({{repositoryUrl}}) from version \`{{currentVersion}}\` to \`{{newVersion}}\` +{{#if releases.length}} + +### Commits + +<details> +<summary>{{githubName}}</summary> + +{{#each releases as |release|}} +#### {{release.version}} +{{#each release.commits as |commit|}} +- [\`{{commit.shortSha}}\`]({{commit.url}}) {{commit.message}} +{{/each}} +{{/each}} + +</details> +{{/if}} +<br /> + +This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://keylocation.sg/our-tech/renovate).", + "prCreation": "immediate", + "prTitle": "{{semanticPrefix}}{{#if isPin}}Pin{{else}}Update{{/if}} dependency {{depName}} to version {{#if isRange}}{{newVersion}}{{else}}{{#if isMajor}}{{newVersionMajor}}.x{{else}}{{newVersion}}{{/if}}{{/if}}", + "rebaseStalePrs": false, + "recreateClosed": false, + "repositoryUrl": "", + "reviewers": Array [], + "semanticCommits": false, + "semanticPrefix": "chore: ", + "upgradeType": "error", + }, +] +`; + +exports[`lib/workers/package/index findUpgrades(config) returns warning if warning found 1`] = ` +Array [ + Object { + "assignees": Array [], + "automerge": "none", + "automergeType": "pr", + "branchName": "renovate/{{depName}}-{{newVersionMajor}}.x", + "commitMessage": "{{semanticPrefix}}Update dependency {{depName}} to version {{newVersion}}", + "depName": "foo", + "group": Object { + "branchName": "renovate/{{groupSlug}}", + "commitMessage": "{{semanticPrefix}}Renovate {{groupName}} packages", + "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request renovates the package group \\"{{groupName}}\\". + +{{#each upgrades as |upgrade|}} +- [{{upgrade.depName}}]({{upgrade.repositoryUrl}}): from \`{{upgrade.currentVersion}}\` to \`{{upgrade.newVersion}}\` +{{/each}} + +{{#unless isPin}} +### Commits + +{{#each upgrades as |upgrade|}} +{{#if upgrade.releases.length}} +<details> +<summary>{{upgrade.githubName}}</summary> +{{#each upgrade.releases as |release|}} + +#### {{release.version}} +{{#each release.commits as |commit|}} +- [\`{{commit.shortSha}}\`]({{commit.url}}){{commit.message}} +{{/each}} +{{/each}} + +</details> +{{/if}} +{{/each}} +{{/unless}} +<br /> + +This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://keylocation.sg/our-tech/renovate).", + "prTitle": "{{semanticPrefix}}Renovate {{groupName}} packages", + "recreateClosed": true, + }, + "groupName": null, + "groupSlug": null, + "labels": Array [], + "lazyGrouping": true, + "message": "bad version", + "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request updates dependency [{{depName}}]({{repositoryUrl}}) from version \`{{currentVersion}}\` to \`{{newVersion}}\` +{{#if releases.length}} + +### Commits + +<details> +<summary>{{githubName}}</summary> + +{{#each releases as |release|}} +#### {{release.version}} +{{#each release.commits as |commit|}} +- [\`{{commit.shortSha}}\`]({{commit.url}}) {{commit.message}} +{{/each}} +{{/each}} + +</details> +{{/if}} +<br /> + +This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://keylocation.sg/our-tech/renovate).", + "prCreation": "immediate", + "prTitle": "{{semanticPrefix}}{{#if isPin}}Pin{{else}}Update{{/if}} dependency {{depName}} to version {{#if isRange}}{{newVersion}}{{else}}{{#if isMajor}}{{newVersionMajor}}.x{{else}}{{newVersion}}{{/if}}{{/if}}", + "rebaseStalePrs": false, + "recreateClosed": false, + "repositoryUrl": undefined, + "reviewers": Array [], + "semanticCommits": false, + "semanticPrefix": "chore: ", + "upgradeType": "warning", + }, +] +`; diff --git a/test/workers/package/__snapshots__/versions.spec.js.snap b/test/workers/package/__snapshots__/versions.spec.js.snap index 6fdbc4e39d..aa1cfc16bb 100644 --- a/test/workers/package/__snapshots__/versions.spec.js.snap +++ b/test/workers/package/__snapshots__/versions.spec.js.snap @@ -39,6 +39,13 @@ Array [ ] `; +exports[`workers/package/versions .determineUpgrades(npmDep, config) ignores complex ranges when not pinning 1`] = ` +Object { + "message": "Complex semver ranges such as \\"^0.7.0 || ^0.8.0\\" are not yet supported so won't ever be upgraded", + "upgradeType": "warning", +} +`; + exports[`workers/package/versions .determineUpgrades(npmDep, config) ignores pinning for ranges when other upgrade exists 1`] = ` Array [ Object { @@ -80,6 +87,43 @@ Array [ ] `; +exports[`workers/package/versions .determineUpgrades(npmDep, config) rejects less than ranges without pinning 1`] = ` +Array [ + Object { + "message": "The current semver range \\"< 0.7.2\\" is not supported so won't ever be upgraded", + "upgradeType": "warning", + }, +] +`; + +exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if empty versions 1`] = ` +Object { + "message": "No versions returned from registry for this package", + "upgradeType": "warning", +} +`; + +exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if invalid current version 1`] = ` +Object { + "message": "Dependency uses tag \\"invalid\\" as its version so that will never be changed", + "upgradeType": "warning", +} +`; + +exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if null versions 1`] = ` +Object { + "message": "No versions returned from registry for this package", + "upgradeType": "warning", +} +`; + +exports[`workers/package/versions .determineUpgrades(npmDep, config) return warning if using a known tag 1`] = ` +Object { + "message": "Dependency uses tag \\"next\\" as its version so that will never be changed", + "upgradeType": "warning", +} +`; + exports[`workers/package/versions .determineUpgrades(npmDep, config) returns both updates if automerging minor 1`] = ` Array [ Object { diff --git a/test/workers/package/index.spec.js b/test/workers/package/index.spec.js index 59ba394c04..fafbe18f4c 100644 --- a/test/workers/package/index.spec.js +++ b/test/workers/package/index.spec.js @@ -28,18 +28,24 @@ describe('lib/workers/package/index', () => { expect(res).toMatchObject([]); expect(npmApi.getDependency.mock.calls.length).toBe(0); }); - it('returns empty if no npm dep found', async () => { + it('returns error if no npm dep found', async () => { config.schedule = 'some schedule'; schedule.isScheduledNow.mockReturnValueOnce(true); const res = await pkgWorker.findUpgrades(config); - expect(res).toMatchObject([]); + expect(res[0].upgradeType).toEqual('error'); + expect(res).toMatchSnapshot(); expect(npmApi.getDependency.mock.calls.length).toBe(1); }); - it('returns empty if no upgrades found', async () => { + it('returns warning if warning found', async () => { npmApi.getDependency.mockReturnValueOnce({}); - versions.determineUpgrades.mockReturnValueOnce([]); + versions.determineUpgrades.mockReturnValueOnce([ + { + upgradeType: 'warning', + message: 'bad version', + }, + ]); const res = await pkgWorker.findUpgrades(config); - expect(res).toMatchObject([]); + expect(res).toMatchSnapshot(); }); it('returns array if upgrades found', async () => { npmApi.getDependency.mockReturnValueOnce({}); diff --git a/test/workers/package/versions.spec.js b/test/workers/package/versions.spec.js index ff698b4a51..b018b26980 100644 --- a/test/workers/package/versions.spec.js +++ b/test/workers/package/versions.spec.js @@ -10,28 +10,36 @@ describe('workers/package/versions', () => { }); describe('.determineUpgrades(npmDep, config)', () => { - it('return empty if invalid current version', () => { + it('return warning if invalid current version', () => { config.currentVersion = 'invalid'; - versions.determineUpgrades(qJson, config).should.have.length(0); + const res = versions.determineUpgrades(qJson, config); + expect(res).toHaveLength(1); + expect(res[0]).toMatchSnapshot(); }); - it('return empty if using a known tag', () => { + it('return warning if using a known tag', () => { config.currentVersion = 'next'; - versions.determineUpgrades(qJson, config).should.have.length(0); + const res = versions.determineUpgrades(qJson, config); + expect(res).toHaveLength(1); + expect(res[0]).toMatchSnapshot(); }); - it('return empty if null versions', () => { + it('return warning if null versions', () => { config.currentVersion = '1.0.0'; const testDep = { name: 'q', }; - versions.determineUpgrades(testDep, config).should.have.length(0); + const res = versions.determineUpgrades(testDep, config); + expect(res).toHaveLength(1); + expect(res[0]).toMatchSnapshot(); }); - it('return empty if empty versions', () => { + it('return warning if empty versions', () => { const testDep = { name: 'q', versions: [], }; config.currentVersion = '1.0.0'; - versions.determineUpgrades(testDep, config).should.have.length(0); + const res = versions.determineUpgrades(testDep, config); + expect(res).toHaveLength(1); + expect(res[0]).toMatchSnapshot(); }); it('supports minor and major upgrades for tilde ranges', () => { config.currentVersion = '^0.4.0'; @@ -129,7 +137,9 @@ describe('workers/package/versions', () => { it('ignores complex ranges when not pinning', () => { config.pinVersions = false; config.currentVersion = '^0.7.0 || ^0.8.0'; - expect(versions.determineUpgrades(qJson, config)).toHaveLength(0); + const res = versions.determineUpgrades(qJson, config); + expect(res).toHaveLength(1); + expect(res[0]).toMatchSnapshot(); }); it('returns nothing for greater than ranges', () => { config.pinVersions = false; @@ -144,7 +154,7 @@ describe('workers/package/versions', () => { it('rejects less than ranges without pinning', () => { config.pinVersions = false; config.currentVersion = '< 0.7.2'; - expect(versions.determineUpgrades(qJson, config)).toEqual([]); + expect(versions.determineUpgrades(qJson, config)).toMatchSnapshot(); }); it('supports > latest versions if configured', () => { config.respectLatest = false; diff --git a/test/workers/repository/__snapshots__/onboarding.spec.js.snap b/test/workers/repository/__snapshots__/onboarding.spec.js.snap index 59d42f5eba..c123a0ef26 100644 --- a/test/workers/repository/__snapshots__/onboarding.spec.js.snap +++ b/test/workers/repository/__snapshots__/onboarding.spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`lib/workers/repository/onboarding ensurePr(config, branchUpgrades) creates complex pr 1`] = ` +exports[`lib/workers/repository/onboarding ensurePr(config, upgradeData) creates complex pr 1`] = ` Array [ Array [ "renovate/configure", @@ -39,7 +39,7 @@ If the default settings are all suitable for you, simply close this Pull Request ] `; -exports[`lib/workers/repository/onboarding ensurePr(config, branchUpgrades) creates pr 1`] = ` +exports[`lib/workers/repository/onboarding ensurePr(config, upgradeData) creates pr 1`] = ` Array [ Array [ "renovate/configure", @@ -52,6 +52,63 @@ This is an onboarding PR to help you understand and configure Renovate before an 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. +", + ], +] +`; + +exports[`lib/workers/repository/onboarding ensurePr(config, upgradeData) creates shows warnings and errors 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. + +--- + +## Errors (1) + +Renovate has raised errors when processing this repository that you should fix before merging or closing this PR. + +Please make any fixes in _this branch_. +- \`a\`: uhoh a + +Feel free to raise create a [GitHub Issue](https:/github.com/singapore/renovate/issues) to ask any questions. + +--- + +### Warnings (1) + +Please correct - or verify that you can safely ignore - these warnings before you merge this PR. +- \`b\`: uhoh b + +--- + + +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. diff --git a/test/workers/repository/__snapshots__/upgrades.spec.js.snap b/test/workers/repository/__snapshots__/upgrades.spec.js.snap index 4c5aeb188c..946711bd74 100644 --- a/test/workers/repository/__snapshots__/upgrades.spec.js.snap +++ b/test/workers/repository/__snapshots__/upgrades.spec.js.snap @@ -2,86 +2,141 @@ exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) does not group if different compiled branch names 1`] = ` Object { - "bar-1.1.0": Array [ - Object { - "branchName": "bar-{{version}}", - "version": "1.1.0", - }, - ], - "foo-1.1.0": Array [ - Object { - "branchName": "foo-{{version}}", - "version": "1.1.0", - }, - ], - "foo-2.0.0": Array [ - Object { - "branchName": "foo-{{version}}", - "version": "2.0.0", - }, - ], + "branchUpgrades": Object { + "bar-1.1.0": Array [ + Object { + "branchName": "bar-{{version}}", + "version": "1.1.0", + }, + ], + "foo-1.1.0": Array [ + Object { + "branchName": "foo-{{version}}", + "version": "1.1.0", + }, + ], + "foo-2.0.0": Array [ + Object { + "branchName": "foo-{{version}}", + "version": "2.0.0", + }, + ], + }, + "errors": Array [], + "warnings": Array [], } `; exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) groups if same compiled branch names 1`] = ` Object { - "bar-1.1.0": Array [ - Object { - "branchName": "bar-{{version}}", - "version": "1.1.0", - }, - ], - "foo": Array [ - Object { - "branchName": "foo", - "version": "2.0.0", - }, - Object { - "branchName": "foo", - "version": "1.1.0", - }, - ], + "branchUpgrades": Object { + "bar-1.1.0": Array [ + Object { + "branchName": "bar-{{version}}", + "version": "1.1.0", + }, + ], + "foo": Array [ + Object { + "branchName": "foo", + "version": "2.0.0", + }, + Object { + "branchName": "foo", + "version": "1.1.0", + }, + ], + }, + "errors": Array [], + "warnings": Array [], } `; exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) groups if same compiled group name 1`] = ` Object { - "foo": Array [ + "branchUpgrades": Object { + "foo": Array [ + Object { + "branchName": "foo", + "version": "2.0.0", + }, + ], + "renovate/my-group": Array [ + Object { + "branchName": "bar-{{version}}", + "group": Object { + "branchName": "renovate/my-group", + }, + "groupName": "My Group", + "groupSlug": "my-group", + "version": "1.1.0", + }, + Object { + "branchName": "foo", + "group": Object { + "branchName": "renovate/{{groupSlug}}", + }, + "groupName": "My Group", + "groupSlug": "my-group", + "version": "1.1.0", + }, + ], + }, + "errors": Array [], + "warnings": Array [], +} +`; + +exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) mixes errors and warnings 1`] = ` +Object { + "branchUpgrades": Object { + "bar-1.1.0": Array [ + Object { + "branchName": "bar-{{version}}", + "version": "1.1.0", + }, + ], + "foo-1.1.0": Array [ + Object { + "branchName": "foo-{{version}}", + "version": "1.1.0", + }, + ], + }, + "errors": Array [ Object { - "branchName": "foo", - "version": "2.0.0", + "upgradeType": "error", }, ], - "renovate/my-group": Array [ - Object { - "branchName": "bar-{{version}}", - "group": Object { - "branchName": "renovate/my-group", - }, - "groupName": "My Group", - "groupSlug": "my-group", - "version": "1.1.0", - }, + "warnings": Array [ Object { - "branchName": "foo", - "group": Object { - "branchName": "renovate/{{groupSlug}}", - }, - "groupName": "My Group", - "groupSlug": "my-group", - "version": "1.1.0", + "branchName": "foo-{{version}}", + "upgradeType": "warning", + "version": "2.0.0", }, ], } `; +exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) returns empty object if no input array 1`] = ` +Object { + "branchUpgrades": Object {}, + "errors": Array [], + "warnings": Array [], +} +`; + exports[`workers/repository/upgrades branchifyUpgrades(upgrades, logger) returns one branch if one input 1`] = ` Object { - "foo-1.1.0": Array [ - Object { - "branchName": "foo-{{version}}", - "version": "1.1.0", - }, - ], + "branchUpgrades": Object { + "foo-1.1.0": Array [ + Object { + "branchName": "foo-{{version}}", + "version": "1.1.0", + }, + ], + }, + "errors": Array [], + "warnings": Array [], } `; diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js index ab6c20dc27..76ccad2ff4 100644 --- a/test/workers/repository/index.spec.js +++ b/test/workers/repository/index.spec.js @@ -18,7 +18,7 @@ describe('workers/repository', () => { onboarding.getOnboardingStatus = jest.fn(); onboarding.ensurePr = jest.fn(); upgrades.determineRepoUpgrades = jest.fn(() => []); - upgrades.branchifyUpgrades = jest.fn(() => ({})); + upgrades.branchifyUpgrades = jest.fn(() => ({ branchUpgrades: {} })); branchWorker.processBranchUpgrades = jest.fn(); config = { api: { @@ -63,9 +63,11 @@ describe('workers/repository', () => { config.hasRenovateJson = true; onboarding.getOnboardingStatus.mockReturnValueOnce(true); upgrades.branchifyUpgrades.mockReturnValueOnce({ - foo: {}, - bar: {}, - baz: {}, + branchUpgrades: { + foo: {}, + bar: {}, + baz: {}, + }, }); await repositoryWorker.renovateRepository(config); expect(branchWorker.processBranchUpgrades.mock.calls.length).toBe(3); diff --git a/test/workers/repository/onboarding.spec.js b/test/workers/repository/onboarding.spec.js index a3900b22d7..96d5afcee3 100644 --- a/test/workers/repository/onboarding.spec.js +++ b/test/workers/repository/onboarding.spec.js @@ -3,8 +3,9 @@ const logger = require('../../_fixtures/logger'); const defaultConfig = require('../../../lib/config/defaults').getConfig(); describe('lib/workers/repository/onboarding', () => { - describe('ensurePr(config, branchUpgrades)', () => { + describe('ensurePr(config, upgradeData)', () => { let config; + let upgradeData; beforeEach(() => { config = { api: { @@ -14,16 +15,21 @@ describe('lib/workers/repository/onboarding', () => { }, logger, }; + upgradeData = { + branchUpgrades: {}, + errors: [], + warnings: [], + }; }); it('creates pr', async () => { - await onboarding.ensurePr(config, {}); + await onboarding.ensurePr(config, upgradeData); 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(); }); it('updates pr', async () => { config.api.getBranchPr.mockReturnValueOnce({}); - await onboarding.ensurePr(config, {}); + await onboarding.ensurePr(config, upgradeData); expect(config.api.createPr.mock.calls.length).toBe(0); expect(config.api.updatePr.mock.calls.length).toBe(1); }); @@ -55,12 +61,12 @@ If the default settings are all suitable for you, simply close this Pull Request title: 'Configure Renovate', body: existingPrBody, }); - await onboarding.ensurePr(config, {}); + await onboarding.ensurePr(config, upgradeData); 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 = { + upgradeData.branchUpgrades = { 'branch-a': [ { prTitle: 'Pin a', @@ -81,7 +87,46 @@ If the default settings are all suitable for you, simply close this Pull Request }, ], }; - await onboarding.ensurePr(config, branchUpgrades); + await onboarding.ensurePr(config, upgradeData); + 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(); + }); + it('creates shows warnings and errors', async () => { + upgradeData.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', + }, + ], + }; + upgradeData.errors = [ + { + depName: 'a', + message: 'uhoh a', + }, + ]; + upgradeData.warnings = [ + { + depName: 'b', + message: 'uhoh b', + }, + ]; + await onboarding.ensurePr(config, upgradeData); 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(); diff --git a/test/workers/repository/upgrades.spec.js b/test/workers/repository/upgrades.spec.js index 3b86ece58e..7e178226e5 100644 --- a/test/workers/repository/upgrades.spec.js +++ b/test/workers/repository/upgrades.spec.js @@ -47,7 +47,7 @@ describe('workers/repository/upgrades', () => { describe('branchifyUpgrades(upgrades, logger)', () => { it('returns empty object if no input array', async () => { const res = await upgrades.branchifyUpgrades([], logger); - expect(res).toEqual({}); + expect(res).toMatchSnapshot(); }); it('returns one branch if one input', async () => { const input = [ @@ -57,7 +57,7 @@ describe('workers/repository/upgrades', () => { }, ]; const res = await upgrades.branchifyUpgrades(input, logger); - expect(Object.keys(res).length).toBe(1); + expect(Object.keys(res.branchUpgrades).length).toBe(1); expect(res).toMatchSnapshot(); }); it('does not group if different compiled branch names', async () => { @@ -76,7 +76,7 @@ describe('workers/repository/upgrades', () => { }, ]; const res = await upgrades.branchifyUpgrades(input, logger); - expect(Object.keys(res).length).toBe(3); + expect(Object.keys(res.branchUpgrades).length).toBe(3); expect(res).toMatchSnapshot(); }); it('groups if same compiled branch names', async () => { @@ -95,7 +95,7 @@ describe('workers/repository/upgrades', () => { }, ]; const res = await upgrades.branchifyUpgrades(input, logger); - expect(Object.keys(res).length).toBe(2); + expect(Object.keys(res.branchUpgrades).length).toBe(2); expect(res).toMatchSnapshot(); }); it('groups if same compiled group name', async () => { @@ -118,7 +118,32 @@ describe('workers/repository/upgrades', () => { }, ]; const res = await upgrades.branchifyUpgrades(input, logger); - expect(Object.keys(res).length).toBe(2); + expect(Object.keys(res.branchUpgrades).length).toBe(2); + expect(res).toMatchSnapshot(); + }); + it('mixes errors and warnings', async () => { + const input = [ + { + upgradeType: 'error', + }, + { + branchName: 'foo-{{version}}', + version: '1.1.0', + }, + { + upgradeType: 'warning', + branchName: 'foo-{{version}}', + version: '2.0.0', + }, + { + branchName: 'bar-{{version}}', + version: '1.1.0', + }, + ]; + const res = await upgrades.branchifyUpgrades(input, logger); + expect(Object.keys(res.branchUpgrades).length).toBe(2); + expect(res.errors).toHaveLength(1); + expect(res.warnings).toHaveLength(1); expect(res).toMatchSnapshot(); }); }); -- GitLab