From 1dd464cba5f3780509b13dab0e18a5cfacd68c86 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Mon, 26 Jun 2017 13:08:57 +0200 Subject: [PATCH] Refactor config definitions and onboarding values (#360) * Set levels in definitions * Rename global worker * getRepoConfig use options levels * Refactor packageFileConfig * Add onboarding configuration --- lib/config/defaults.js | 12 ++++- lib/config/definitions.js | 53 +++++++++++++++++-- lib/config/index.js | 48 +++++++++++++++-- lib/renovate.js | 4 +- lib/workers/{index.js => global.js} | 1 - lib/workers/package-file.js | 5 +- lib/workers/repository.js | 32 ++++------- test/_fixtures/logger/index.js | 9 +++- test/config/__snapshots__/index.spec.js.snap | 10 ++-- test/config/index.spec.js | 20 +++++-- test/renovate.spec.js | 2 +- ...index.spec.js.snap => global.spec.js.snap} | 2 +- .../repository-functions.spec.js.snap | 32 +++++++++++ .../workers/{index.spec.js => global.spec.js} | 12 +++-- test/workers/repository-functions.spec.js | 1 + 15 files changed, 186 insertions(+), 57 deletions(-) rename lib/workers/{index.js => global.js} (91%) rename test/workers/__snapshots__/{index.spec.js.snap => global.spec.js.snap} (83%) rename test/workers/{index.spec.js => global.spec.js} (80%) diff --git a/lib/config/defaults.js b/lib/config/defaults.js index 576644d9e1..9e58ceb894 100644 --- a/lib/config/defaults.js +++ b/lib/config/defaults.js @@ -3,6 +3,7 @@ const configDefinitions = require('./definitions'); module.exports = { getDefault, getConfig, + getOnboardingConfig, }; const defaultValues = { @@ -20,10 +21,19 @@ function getDefault(option) { function getConfig() { const options = configDefinitions.getOptions(); const config = {}; - options.forEach(option => { config[option.name] = getDefault(option); }); + return config; +} +function getOnboardingConfig() { + const options = configDefinitions.getOptions(); + const config = {}; + options.forEach(option => { + if (option.level !== 'global' && option.onboarding !== false) { + config[option.name] = getDefault(option); + } + }); return config; } diff --git a/lib/config/definitions.js b/lib/config/definitions.js index 7e69e6b2f3..74ae5c47b7 100644 --- a/lib/config/definitions.js +++ b/lib/config/definitions.js @@ -20,59 +20,75 @@ const options = [ { name: 'enabled', description: 'Enable or disable renovate', + level: 'packageFile', type: 'boolean', }, { name: 'logFile', description: 'Log file path', + level: 'global', type: 'string', }, { name: 'logFileLevel', description: 'Log file log level', + level: 'global', type: 'string', default: 'debug', }, { name: 'onboarding', description: 'Require a Configuration PR first', + level: 'repository', type: 'boolean', + onboarding: false, }, { name: 'platform', description: 'Platform type of repository', + level: 'repository', type: 'string', default: 'github', + onboarding: false, }, { name: 'endpoint', description: 'Custom endpoint to use', + level: 'repository', type: 'string', + onboarding: false, }, { name: 'token', description: 'Repository Auth Token', + level: 'repository', type: 'string', + onboarding: false, }, { name: 'autodiscover', description: 'Autodiscover all repositories', + level: 'repository', type: 'boolean', default: false, + onboarding: false, }, { name: 'githubAppId', description: 'GitHub App ID (enables GitHub App functionality if set)', + level: 'global', type: 'integer', }, { name: 'githubAppKey', description: 'GitHub App Private Key (.pem file contents)', + level: 'global', type: 'string', }, { name: 'repositories', description: 'List of Repositories', + level: 'global', type: 'list', cli: false, }, @@ -80,14 +96,21 @@ const options = [ name: 'packageFiles', description: 'Package file paths', type: 'list', + level: 'repository', }, { name: 'depTypes', description: 'Dependency types', + level: 'packageFile', type: 'list', default: ['dependencies', 'devDependencies', 'optionalDependencies'], }, // Version behaviour + { + name: 'pinVersions', + description: 'Convert ranged versions in package.json to pinned versions', + type: 'boolean', + }, { name: 'separateMajorReleases', description: @@ -103,16 +126,19 @@ const options = [ name: 'ignoreFuture', description: 'Ignore versions tagged as "future"', type: 'boolean', + onboarding: false, }, { name: 'ignoreUnstable', description: 'Ignore versions with unstable semver', type: 'boolean', + onboarding: false, }, { name: 'respectLatest', description: 'Ignore versions newer than npm "latest" version', type: 'boolean', + onboarding: false, }, // PR Behaviour { @@ -120,6 +146,7 @@ const options = [ description: 'Recreate PRs even if same ones were closed previously', type: 'boolean', default: false, + onboarding: false, }, { name: 'rebaseStalePrs', @@ -148,6 +175,7 @@ const options = [ 'How to automerge - "branch-merge-commit", "branch-push" or "pr". Branch support is GitHub-only', type: 'string', default: 'pr', + onboarding: false, }, // String templates { @@ -170,6 +198,7 @@ const options = [ type: 'string', default: template('prTitle'), cli: false, + onboarding: false, }, { name: 'prBody', @@ -177,49 +206,60 @@ const options = [ type: 'string', default: template('prBody'), cli: false, + onboarding: false, }, // Yarn Lock Maintenance { name: 'yarnCacheFolder', description: 'Location of yarn cache folder to use. Set to empty string to disable', + level: 'global', type: 'string', default: '/tmp/yarn-cache', }, { name: 'maintainYarnLock', description: 'Keep yarn.lock files updated in base branch', + level: 'packageFile', type: 'boolean', default: false, }, { name: 'yarnMaintenanceBranchName', description: 'Branch name template when maintaining yarn.lock', + level: 'packageFile', type: 'string', default: 'renovate/yarn-lock', cli: false, + onboarding: false, }, { name: 'yarnMaintenanceCommitMessage', description: 'Commit message template when maintaining yarn.lock', + level: 'packageFile', type: 'string', default: 'Renovate yarn.lock file', cli: false, + onboarding: false, }, { name: 'yarnMaintenancePrTitle', description: 'Pull Request title template when maintaining yarn.lock', + level: 'packageFile', type: 'string', default: 'Renovate yarn.lock file', cli: false, + onboarding: false, }, { name: 'yarnMaintenancePrBody', description: 'Pull Request body template when maintaining yarn.lock', + level: 'packageFile', type: 'string', default: 'This PR regenerates yarn.lock files based on the existing `package.json` files.', cli: false, + onboarding: false, }, // Dependency Groups { @@ -227,12 +267,14 @@ const options = [ description: 'Use group names only when multiple dependencies upgraded', type: 'boolean', default: true, + onboarding: false, }, { name: 'groupName', description: 'Human understandable name for the dependency group', type: 'string', default: null, + onboarding: false, }, { name: 'groupSlug', @@ -240,6 +282,7 @@ const options = [ 'Slug to use for group (e.g. in branch name). Will be calculated from groupName if null', type: 'string', default: null, + onboarding: false, }, { name: 'groupBranchName', @@ -247,6 +290,7 @@ const options = [ type: 'string', default: template('groupBranchName'), cli: false, + onboarding: false, }, { name: 'groupCommitMessage', @@ -254,6 +298,7 @@ const options = [ type: 'string', default: template('groupCommitMessage'), cli: false, + onboarding: false, }, { name: 'groupPrTitle', @@ -261,6 +306,7 @@ const options = [ type: 'string', default: template('groupPrTitle'), cli: false, + onboarding: false, }, { name: 'groupPrBody', @@ -268,6 +314,7 @@ const options = [ type: 'string', default: template('groupPrBody'), cli: false, + onboarding: false, }, // Pull Request options { @@ -285,15 +332,11 @@ const options = [ description: 'Requested reviewers for Pull Requests (GitHub only)', type: 'list', }, - { - name: 'pinVersions', - description: 'Convert ranged versions in package.json to pinned versions', - type: 'boolean', - }, // Debug options { name: 'logLevel', description: 'Logging level', + level: 'global', type: 'string', default: 'info', env: 'LOG_LEVEL', diff --git a/lib/config/index.js b/lib/config/index.js index 5dff1c6cc0..028897b2f0 100644 --- a/lib/config/index.js +++ b/lib/config/index.js @@ -2,6 +2,8 @@ const logger = require('../helpers/logger'); const githubApi = require('../api/github'); const gitlabApi = require('../api/gitlab'); +const definitions = require('./definitions'); + const defaultsParser = require('./defaults'); const fileParser = require('./file'); const cliParser = require('./cli'); @@ -12,6 +14,7 @@ const githubAppHelper = require('../helpers/github-app'); module.exports = { parseConfigs, getRepoConfig, + getPackageFileConfig, }; async function parseConfigs(env, argv) { @@ -114,12 +117,47 @@ async function parseConfigs(env, argv) { return config; } -function getRepoConfig(config, index) { - let repository = config.repositories[index]; +function getRepoConfig(globalConfig, index) { + let repository = globalConfig.repositories[index]; if (typeof repository === 'string') { repository = { repository }; } - const returnConfig = Object.assign({}, config, repository); - delete returnConfig.repositories; - return returnConfig; + const repoConfig = Object.assign({}, globalConfig, repository); + for (const option of definitions.getOptions()) { + if (option.level === 'global') { + delete repoConfig[option.name]; + } + } + repoConfig.logger = logger.child({ + repository: repoConfig.repository, + }); + return repoConfig; +} + +function getPackageFileConfig(repoConfig, index) { + let packageFile = repoConfig.packageFiles[index]; + if (typeof packageFile === 'string') { + packageFile = { packageFile }; + } else if (packageFile.fileName) { + // Retained deprecated 'fileName' for backwards compatibility + // TODO: Remove in renovate 9 + packageFile.packageFile = packageFile.fileName; + delete packageFile.fileName; + } + const packageFileConfig = Object.assign({}, repoConfig, packageFile); + for (const option of definitions.getOptions()) { + if (option.level === 'repository') { + delete packageFileConfig[option.name]; + } + } + repoConfig.logger.trace({ config: repoConfig }, 'repoConfig'); + packageFileConfig.logger = logger.child({ + repository: packageFileConfig.repository, + packageFile: packageFileConfig.packageFile, + }); + packageFileConfig.logger.trace( + { config: packageFileConfig }, + 'packageFileConfig' + ); + return packageFileConfig; } diff --git a/lib/renovate.js b/lib/renovate.js index 6fb57b2856..cdb96b2008 100644 --- a/lib/renovate.js +++ b/lib/renovate.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -const renovateWorker = require('./workers/index'); +const globalWorker = require('./workers/global'); -renovateWorker.start(); +globalWorker.start(); diff --git a/lib/workers/index.js b/lib/workers/global.js similarity index 91% rename from lib/workers/index.js rename to lib/workers/global.js index b1d7fe3929..63ff782035 100644 --- a/lib/workers/index.js +++ b/lib/workers/global.js @@ -13,7 +13,6 @@ async function start() { // Iterate through repositories sequentially for (let index = 0; index < config.repositories.length; index += 1) { const repoConfig = configParser.getRepoConfig(config, index); - repoConfig.logger = logger.child({ repository: repoConfig.repository }); repoConfig.logger.info('Renovating repository'); await repositoryWorker.processRepo(repoConfig); repoConfig.logger.info('Finished repository'); diff --git a/lib/workers/package-file.js b/lib/workers/package-file.js index 630e10c28f..774b20139c 100644 --- a/lib/workers/package-file.js +++ b/lib/workers/package-file.js @@ -16,10 +16,7 @@ module.exports = { // This function manages the queue per-package file async function processPackageFile(config) { // Initialize logger - logger = config.logger.child({ - repository: config.repository, - packageFile: config.packageFile, - }); + logger = config.logger; logger.info(`Processing package file`); diff --git a/lib/workers/repository.js b/lib/workers/repository.js index b60366567a..2f80b20644 100644 --- a/lib/workers/repository.js +++ b/lib/workers/repository.js @@ -3,6 +3,7 @@ const handlebars = require('handlebars'); const ini = require('ini'); const stringify = require('json-stringify-pretty-compact'); // Config +const configParser = require('../config'); const defaultsParser = require('../config/defaults'); // API const githubApi = require('../api/github'); @@ -75,16 +76,7 @@ async function mergeRenovateJson(config) { } async function onboardRepository(config) { - const defaultConfig = defaultsParser.getConfig(); - delete defaultConfig.onboarding; - delete defaultConfig.platform; - delete defaultConfig.endpoint; - delete defaultConfig.token; - delete defaultConfig.autodiscover; - delete defaultConfig.githubAppId; - delete defaultConfig.githubAppKey; - delete defaultConfig.repositories; - delete defaultConfig.logLevel; + const defaultConfig = defaultsParser.getOnboardingConfig(); let 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. @@ -161,20 +153,14 @@ async function determineRepoUpgrades(config) { config.logger.warn('No package files found'); } let upgrades = []; - for (let packageFile of config.packageFiles) { - if (typeof packageFile === 'string') { - packageFile = { packageFile }; - } else if (packageFile.fileName) { - // Retained deprecated 'fileName' for backwards compatibility - // TODO: Remove in renovate 9 - packageFile.packageFile = packageFile.fileName; - delete packageFile.fileName; - } - const packageFileConfig = Object.assign({}, config, packageFile); - delete packageFileConfig.packageFiles; + // Iterate through repositories sequentially + for (let index = 0; index < config.packageFiles.length; index += 1) { + const packageFileConfig = configParser.getPackageFileConfig(config, index); + packageFileConfig.logger.info('Renovating package file'); upgrades = upgrades.concat( await packageFileWorker.processPackageFile(packageFileConfig) ); + packageFileConfig.logger.info('Finished repository'); } return upgrades; } @@ -219,8 +205,8 @@ async function updateBranchesSequentially(branchUpgrades, logger) { } } -async function processRepo(repoConfig) { - let config = Object.assign({}, repoConfig); +async function processRepo(packageFileConfig) { + let config = Object.assign({}, packageFileConfig); config.logger.trace({ config }, 'processRepo'); try { config = await module.exports.initApis(config); diff --git a/test/_fixtures/logger/index.js b/test/_fixtures/logger/index.js index 94858060fb..80d9c5f0c2 100644 --- a/test/_fixtures/logger/index.js +++ b/test/_fixtures/logger/index.js @@ -5,5 +5,12 @@ module.exports = { info: jest.fn(), debug: jest.fn(), trace: jest.fn(), - child: jest.fn(() => module.exports), + child: jest.fn(() => ({ + fatal: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + trace: jest.fn(), + })), }; diff --git a/test/config/__snapshots__/index.spec.js.snap b/test/config/__snapshots__/index.spec.js.snap index 2f519246fb..5bc93770e5 100644 --- a/test/config/__snapshots__/index.spec.js.snap +++ b/test/config/__snapshots__/index.spec.js.snap @@ -1,16 +1,18 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`config/index .getRepoConfig(config, index) handles object repos 1`] = ` +exports[`config/index .(config, index) handles object repos 1`] = ` Object { - "global": "b", + "foo": "bar", + "maintainYarnLock": true, "repoField": "g", "repository": "e/f", } `; -exports[`config/index .getRepoConfig(config, index) massages string repos 1`] = ` +exports[`config/index .(config, index) massages string repos 1`] = ` Object { - "global": "b", + "foo": "bar", + "maintainYarnLock": true, "repository": "c/d", } `; diff --git a/test/config/index.spec.js b/test/config/index.spec.js index b96bd87116..d4e1859527 100644 --- a/test/config/index.spec.js +++ b/test/config/index.spec.js @@ -160,13 +160,15 @@ describe('config/index', () => { expect(glGot.mock.calls.length).toBe(0); }); }); - describe('.getRepoConfig(config, index)', () => { + describe('.(config, index)', () => { let configParser; beforeEach(() => { configParser = require('../../lib/config/index.js'); }); const config = { - global: 'b', + githubAppKey: 'foo', + maintainYarnLock: true, + foo: 'bar', repositories: [ 'c/d', { @@ -176,10 +178,20 @@ describe('config/index', () => { ], }; it('massages string repos', () => { - expect(configParser.getRepoConfig(config, 0)).toMatchSnapshot(); + const res = configParser.getRepoConfig(config, 0); + expect(res.githubAppKey).not.toBeDefined(); + expect(res.maintainYarnLock).toBeDefined(); + expect(res.logger).toBeDefined(); + delete res.logger; + expect(res).toMatchSnapshot(); }); it('handles object repos', () => { - expect(configParser.getRepoConfig(config, 1)).toMatchSnapshot(); + const res = configParser.getRepoConfig(config, 1); + expect(res.githubAppKey).not.toBeDefined(); + expect(res.maintainYarnLock).toBeDefined(); + expect(res.logger).toBeDefined(); + delete res.logger; + expect(res).toMatchSnapshot(); }); }); }); diff --git a/test/renovate.spec.js b/test/renovate.spec.js index 2d93861f8c..3366ac5e9a 100644 --- a/test/renovate.spec.js +++ b/test/renovate.spec.js @@ -1,4 +1,4 @@ -const renovateWorker = require('../lib/workers'); +const renovateWorker = require('../lib/workers/global'); renovateWorker.start = jest.fn(); diff --git a/test/workers/__snapshots__/index.spec.js.snap b/test/workers/__snapshots__/global.spec.js.snap similarity index 83% rename from test/workers/__snapshots__/index.spec.js.snap rename to test/workers/__snapshots__/global.spec.js.snap index c4d0b2a0df..0aaa44f801 100644 --- a/test/workers/__snapshots__/index.spec.js.snap +++ b/test/workers/__snapshots__/global.spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`lib/workers/index processes repositories 1`] = ` +exports[`lib/workers/global processes repositories 1`] = ` Array [ Array [ Object { diff --git a/test/workers/__snapshots__/repository-functions.spec.js.snap b/test/workers/__snapshots__/repository-functions.spec.js.snap index 1c9ad4be9d..4ac3130f12 100644 --- a/test/workers/__snapshots__/repository-functions.spec.js.snap +++ b/test/workers/__snapshots__/repository-functions.spec.js.snap @@ -93,3 +93,35 @@ Object { `; exports[`workers/repository initApis(config) throws if unknown platform 1`] = `"Unknown platform: foo"`; + +exports[`workers/repository onboardRepository(config) should commit files and create PR 1`] = ` +Array [ + Array [ + "renovate/configure", + Array [ + Object { + "contents": "{ + \\"enabled\\": true, + \\"packageFiles\\": [], + \\"depTypes\\": [\\"dependencies\\", \\"devDependencies\\", \\"optionalDependencies\\"], + \\"pinVersions\\": true, + \\"separateMajorReleases\\": true, + \\"ignoreDeps\\": [], + \\"rebaseStalePrs\\": false, + \\"prCreation\\": \\"immediate\\", + \\"automerge\\": \\"none\\", + \\"branchName\\": \\"renovate/{{depName}}-{{newVersionMajor}}.x\\", + \\"commitMessage\\": \\"Update dependency {{depName}} to version {{newVersion}}\\", + \\"maintainYarnLock\\": false, + \\"labels\\": [], + \\"assignees\\": [], + \\"reviewers\\": [] +} +", + "name": "renovate.json", + }, + ], + "Add renovate.json", + ], +] +`; diff --git a/test/workers/index.spec.js b/test/workers/global.spec.js similarity index 80% rename from test/workers/index.spec.js rename to test/workers/global.spec.js index e0a5512a3d..a4eb2fc8b1 100644 --- a/test/workers/index.spec.js +++ b/test/workers/global.spec.js @@ -1,8 +1,9 @@ -const indexWorker = require('../../lib/workers/index'); +const globalWorker = require('../../lib/workers/global'); const repositoryWorker = require('../../lib/workers/repository'); const configParser = require('../../lib/config'); +const logger = require('../_fixtures/logger'); -describe('lib/workers/index', () => { +describe('lib/workers/global', () => { beforeEach(() => { jest.resetAllMocks(); configParser.parseConfigs = jest.fn(); @@ -13,7 +14,7 @@ describe('lib/workers/index', () => { configParser.parseConfigs.mockReturnValueOnce({ repositories: [], }); - await indexWorker.start(); + await globalWorker.start(); }); it('processes repositories', async () => { configParser.parseConfigs.mockReturnValueOnce({ @@ -22,8 +23,9 @@ describe('lib/workers/index', () => { }); configParser.getRepoConfig.mockReturnValue({ repository: 'foo', + logger, }); - await indexWorker.start(); + await globalWorker.start(); expect(configParser.parseConfigs.mock.calls.length).toBe(1); expect(configParser.getRepoConfig.mock.calls).toMatchSnapshot(); expect(repositoryWorker.processRepo.mock.calls.length).toBe(2); @@ -32,6 +34,6 @@ describe('lib/workers/index', () => { configParser.parseConfigs.mockImplementationOnce(() => { throw new Error('a'); }); - await indexWorker.start(); + await globalWorker.start(); }); }); diff --git a/test/workers/repository-functions.spec.js b/test/workers/repository-functions.spec.js index a948846789..6bf7d95d98 100644 --- a/test/workers/repository-functions.spec.js +++ b/test/workers/repository-functions.spec.js @@ -120,6 +120,7 @@ describe('workers/repository', () => { expect( config.api.createPr.mock.calls[0][2].indexOf('Merge Request') ).toBe(-1); + expect(config.api.commitFilesToBranch.mock.calls).toMatchSnapshot(); }); it('should adapt for gitlab phrasing', async () => { config.platform = 'gitlab'; -- GitLab