From 023e520702b2ac486691bfe6e2233c87af18adc0 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Mon, 31 Jul 2017 14:50:44 +0200 Subject: [PATCH] fix: Do not log warning when deprecated config options found (#563) --- lib/config/definitions.js | 17 +++++++++++++++++ lib/config/validation.js | 19 ++++++++++++++++--- lib/workers/global/index.js | 9 ++++++--- lib/workers/package-file/index.js | 10 +++++++++- lib/workers/repository/apis.js | 9 ++++++--- test/config/validation.spec.js | 6 ++++-- test/workers/global/index.spec.js | 3 ++- .../__snapshots__/index.spec.js.snap | 2 +- test/workers/package-file/index.spec.js | 6 ++++-- .../__snapshots__/apis.spec.js.snap | 2 +- test/workers/repository/apis.spec.js | 4 ++-- test/workers/repository/index.spec.js | 1 + 12 files changed, 69 insertions(+), 19 deletions(-) diff --git a/lib/config/definitions.js b/lib/config/definitions.js index 024377e006..52ea17761f 100644 --- a/lib/config/definitions.js +++ b/lib/config/definitions.js @@ -14,6 +14,7 @@ function template(name, subdir = 'default/') { module.exports = { getOptions, + getDeprecatedOptions, }; const options = [ @@ -373,3 +374,19 @@ const options = [ function getOptions() { return options; } + +const deprecatedOptions = [ + 'maintainYarnLock', + 'yarnMaintenanceBranchName', + 'yarnMaintenanceCommitMessage', + 'yarnMaintenancePrTitle', + 'yarnMaintenancePrBody', + 'groupBranchName', + 'groupBranchName', + 'groupPrTitle', + 'groupPrBody', +]; + +function getDeprecatedOptions() { + return deprecatedOptions; +} diff --git a/lib/config/validation.js b/lib/config/validation.js index 993dc1ee91..4d4a38b635 100644 --- a/lib/config/validation.js +++ b/lib/config/validation.js @@ -1,4 +1,5 @@ const options = require('./definitions').getOptions(); +const deprecatedOptions = require('./definitions').getDeprecatedOptions(); const optionTypes = {}; options.forEach(option => { @@ -11,6 +12,7 @@ module.exports = { function validateConfig(config) { let errors = []; + let warnings = []; function isIgnored(key) { const ignoredNodes = ['api', 'depType']; @@ -26,9 +28,12 @@ function validateConfig(config) { return Object.prototype.toString.call(obj) === '[object Object]'; } + const foundDeprecated = []; for (const key of Object.keys(config)) { let val = config[key]; - if ( + if (deprecatedOptions.includes(key)) { + foundDeprecated.push(key); + } else if ( !isIgnored(key) && // We need to ignore some reserved keys !isAFunction(val) // Ignore all functions ) { @@ -72,7 +77,9 @@ function validateConfig(config) { } } else if (type === 'json') { if (isObject(val)) { - errors = errors.concat(module.exports.validateConfig(val)); + const subValidation = module.exports.validateConfig(val); + warnings = warnings.concat(subValidation.warnings); + errors = errors.concat(subValidation.errors); } else { errors.push({ depName: 'Configuration Error', @@ -83,5 +90,11 @@ function validateConfig(config) { } } } - return errors; + if (foundDeprecated.length) { + warnings.push({ + depName: 'Deprecation warning', + message: `The following configutation options are now unsupported and should be removed from renovate.json: ${foundDeprecated}`, + }); + } + return { errors, warnings }; } diff --git a/lib/workers/global/index.js b/lib/workers/global/index.js index 9118166c5b..1b74ca1456 100644 --- a/lib/workers/global/index.js +++ b/lib/workers/global/index.js @@ -12,9 +12,12 @@ module.exports = { async function start() { try { const config = await configParser.parseConfigs(process.env, process.argv); - const configErrors = configValidation.validateConfig(config); - if (configErrors.length) { - logger.error({ configErrors }, 'Found config errors'); + const { warnings, errors } = configValidation.validateConfig(config); + if (warnings.length) { + logger.warn({ warnings }, 'Found config warnings'); + } + if (errors.length) { + logger.error({ errors }, 'Found config errors'); } config.logger = logger; config.versions = versions.detectVersions(config); diff --git a/lib/workers/package-file/index.js b/lib/workers/package-file/index.js index 267d488a70..ac1d2a99e4 100644 --- a/lib/workers/package-file/index.js +++ b/lib/workers/package-file/index.js @@ -35,7 +35,15 @@ async function renovatePackageFile(packageFileConfig) { { config: packageContent.renovate }, 'package.json>renovate config' ); - const errors = configValidation.validateConfig(packageContent.renovate); + const { warnings, errors } = configValidation.validateConfig( + packageContent.renovate + ); + if (warnings.length) { + logger.debug( + { warnings }, + 'Found package.json>renovate configuration warnings' + ); + } if (errors.length) { logger.warn( { errors }, diff --git a/lib/workers/repository/apis.js b/lib/workers/repository/apis.js index 97afea74b5..d627768589 100644 --- a/lib/workers/repository/apis.js +++ b/lib/workers/repository/apis.js @@ -120,9 +120,12 @@ async function mergeRenovateJson(config, branchName) { } renovateJson = JSON.parse(renovateJsonContent); config.logger.debug({ config: renovateJson }, 'renovate.json config'); - const renovateJsonErrors = configValidation.validateConfig(renovateJson); - if (renovateJsonErrors.length) { - config.logger.warn({ renovateJsonErrors }, 'Found renovate.json errors'); + const { warnings, errors } = configValidation.validateConfig(renovateJson); + if (warnings.length) { + config.logger.debug({ warnings }, 'Found renovate.json warnings'); + } + if (errors.length) { + config.logger.warn({ errors }, 'Found renovate.json errors'); /* TODO #556 renovateJsonErrors.forEach(error => { config.errors.push( diff --git a/test/config/validation.spec.js b/test/config/validation.spec.js index bb5c210fec..1c0db1422c 100644 --- a/test/config/validation.spec.js +++ b/test/config/validation.spec.js @@ -11,7 +11,8 @@ describe('config/validation', () => { bar: 2, }, }; - const errors = configValidation.validateConfig(config); + const { warnings, errors } = configValidation.validateConfig(config); + expect(warnings).toHaveLength(0); expect(errors).toHaveLength(2); expect(errors).toMatchSnapshot(); }); @@ -23,7 +24,8 @@ describe('config/validation', () => { githubAppId: 'none', lockFileMaintenance: false, }; - const errors = configValidation.validateConfig(config); + const { warnings, errors } = configValidation.validateConfig(config); + expect(warnings).toHaveLength(0); expect(errors).toHaveLength(5); expect(errors).toMatchSnapshot(); }); diff --git a/test/workers/global/index.spec.js b/test/workers/global/index.spec.js index 551f8a41b9..c70c580efb 100644 --- a/test/workers/global/index.spec.js +++ b/test/workers/global/index.spec.js @@ -11,9 +11,10 @@ describe('lib/workers/global', () => { configParser.getRepositoryConfig = jest.fn(); repositoryWorker.renovateRepository = jest.fn(); }); - it('handles config errors', async () => { + it('handles config warnings and errors', async () => { configParser.parseConfigs.mockReturnValueOnce({ repositories: [], + maintainYarnLock: true, foo: 1, }); await globalWorker.start(); diff --git a/test/workers/package-file/__snapshots__/index.spec.js.snap b/test/workers/package-file/__snapshots__/index.spec.js.snap index c1631b6198..6652c791e1 100644 --- a/test/workers/package-file/__snapshots__/index.spec.js.snap +++ b/test/workers/package-file/__snapshots__/index.spec.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`packageFileWorker renovatePackageFile(config) handles renovate config errors 1`] = `Array []`; +exports[`packageFileWorker renovatePackageFile(config) handles renovate config warnings and errors 1`] = `Array []`; diff --git a/test/workers/package-file/index.spec.js b/test/workers/package-file/index.spec.js index 2141b83bf7..4223c38587 100644 --- a/test/workers/package-file/index.spec.js +++ b/test/workers/package-file/index.spec.js @@ -22,9 +22,11 @@ describe('packageFileWorker', () => { logger, }); }); - it('handles renovate config errors', async () => { + it('handles renovate config warnings and errors', async () => { config.enabled = false; - config.api.getFileJson.mockReturnValueOnce({ renovate: { foo: 1 } }); + config.api.getFileJson.mockReturnValueOnce({ + renovate: { foo: 1, maintainYarnLock: true }, + }); const res = await packageFileWorker.renovatePackageFile(config); expect(res).toMatchSnapshot(); }); diff --git a/test/workers/repository/__snapshots__/apis.spec.js.snap b/test/workers/repository/__snapshots__/apis.spec.js.snap index c9858fc3fe..143e1e3f2a 100644 --- a/test/workers/repository/__snapshots__/apis.spec.js.snap +++ b/test/workers/repository/__snapshots__/apis.spec.js.snap @@ -66,4 +66,4 @@ Array [ ] `; -exports[`workers/repository/apis mergeRenovateJson(config) returns error plus extended config if unknown keys 1`] = `Array []`; +exports[`workers/repository/apis mergeRenovateJson(config) returns warning + error plus extended config if unknown keys 1`] = `Array []`; diff --git a/test/workers/repository/apis.spec.js b/test/workers/repository/apis.spec.js index 4ece3f4b6d..39a1503ac7 100644 --- a/test/workers/repository/apis.spec.js +++ b/test/workers/repository/apis.spec.js @@ -135,9 +135,9 @@ describe('workers/repository/apis', () => { expect(returnConfig.renovateJsonPresent).toBe(true); expect(returnConfig.errors).toHaveLength(0); }); - it('returns error plus extended config if unknown keys', async () => { + it('returns warning + error plus extended config if unknown keys', async () => { config.api.getFileContent.mockReturnValueOnce( - '{ "enabled": true, "foo": false }' + '{ "enabled": true, "foo": false, "maintainYarnLock": true }' ); const returnConfig = await apis.mergeRenovateJson(config); expect(returnConfig.enabled).toBe(true); diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js index dffe0fe3d6..1c5d73be5c 100644 --- a/test/workers/repository/index.spec.js +++ b/test/workers/repository/index.spec.js @@ -21,6 +21,7 @@ describe('workers/repository', () => { upgrades.branchifyUpgrades = jest.fn(() => ({ branchUpgrades: {} })); branchWorker.processBranchUpgrades = jest.fn(() => 'some-branch'); config = { + lockFileMaintenance: true, api: { getFileJson: jest.fn(), setBaseBranch: jest.fn(), -- GitLab