From eeefc3c8f3f5793bcf8392e6e212ef7604ff9407 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Mon, 18 Dec 2017 09:39:52 +0100
Subject: [PATCH] feat: stop and raise error if repository is misconfigured
(#1302)
This PR updates Renovate to detect config validation problems and (1) stop processing, and (2) either raise an Issue if already onboarded, or (2) update the onboarding PR to reflect the error if still onboarding.
Closes #1300
---
lib/config/migrate-validate.js | 7 +-
lib/config/presets.js | 47 ++++++----
lib/manager/resolve.js | 12 +++
lib/platform/github/index.js | 54 +++++++++++
lib/platform/gitlab/index.js | 9 ++
lib/platform/vsts/index.js | 9 ++
lib/workers/repository/error-config.js | 31 +++++++
lib/workers/repository/error.js | 9 +-
lib/workers/repository/init/config.js | 35 ++++---
.../config/__snapshots__/presets.spec.js.snap | 58 ++++++++----
test/config/presets.spec.js | 91 ++++++++++++++++---
.../__snapshots__/resolve.spec.js.snap | 2 +-
test/manager/resolve.spec.js | 2 +-
.../platform/__snapshots__/index.spec.js.snap | 6 ++
test/platform/github/index.spec.js | 69 ++++++++++++++
test/workers/repository/error-config.spec.js | 32 +++++++
test/workers/repository/error.spec.js | 11 ++-
.../init/__snapshots__/config.spec.js.snap | 26 +++---
test/workers/repository/init/config.spec.js | 55 +++++++++--
19 files changed, 469 insertions(+), 96 deletions(-)
create mode 100644 lib/workers/repository/error-config.js
create mode 100644 test/workers/repository/error-config.spec.js
diff --git a/lib/config/migrate-validate.js b/lib/config/migrate-validate.js
index 54b6c7cc41..d29592e6e8 100644
--- a/lib/config/migrate-validate.js
+++ b/lib/config/migrate-validate.js
@@ -7,6 +7,7 @@ module.exports = {
};
function migrateAndValidate(config, input) {
+ logger.debug('migrateAndValidate()');
const { isMigrated, migratedConfig } = configMigration.migrateConfig(input);
if (isMigrated) {
logger.info(
@@ -18,15 +19,15 @@ function migrateAndValidate(config, input) {
const { warnings, errors } = configValidation.validateConfig(massagedConfig);
// istanbul ignore if
if (warnings.length) {
- logger.debug({ warnings }, 'Found renovate config warnings');
+ logger.info({ warnings }, 'Found renovate config warnings');
}
if (errors.length) {
- logger.warn({ errors }, 'Found renovate config errors');
+ logger.info({ errors }, 'Found renovate config errors');
}
+ massagedConfig.errors = (config.errors || []).concat(errors);
if (!config.repoIsOnboarded) {
// TODO #556 - enable warnings in real PRs
massagedConfig.warnings = (config.warnings || []).concat(warnings);
- massagedConfig.errors = (config.errors || []).concat(errors);
}
return massagedConfig;
}
diff --git a/lib/config/presets.js b/lib/config/presets.js
index 0ad3d850da..019350a9e0 100644
--- a/lib/config/presets.js
+++ b/lib/config/presets.js
@@ -27,8 +27,30 @@ async function resolveConfigPresets(inputConfig, existingPresets = []) {
logger.warn(`Already seen preset ${preset} in ${existingPresets}`);
} else {
logger.trace(`Resolving preset "${preset}"`);
+ let fetchedPreset;
+ try {
+ fetchedPreset = await getPreset(preset);
+ } catch (err) {
+ // istanbul ignore else
+ if (existingPresets.length === 0) {
+ const error = new Error('config-validation');
+ if (err.message === 'dep not found') {
+ error.validationError = `Cannot find preset's package (${preset})`;
+ } else if (err.message === 'preset renovate-config not found') {
+ // istanbul ignore next
+ error.validationError = `Preset package is missing a renovate-config entry (${preset})`;
+ } else if (err.message === 'preset not found') {
+ error.validationError = `Preset name not found within published preset config (${preset})`;
+ }
+ logger.info('Throwing preset error');
+ throw error;
+ } else {
+ logger.warn({ preset }, `Cannot find nested preset`);
+ fetchedPreset = {};
+ }
+ }
const presetConfig = await resolveConfigPresets(
- await getPreset(preset),
+ fetchedPreset,
existingPresets.concat([preset])
);
config = configParser.mergeChildConfig(config, presetConfig);
@@ -137,23 +159,16 @@ async function getPreset(preset) {
logger.trace(`getPreset(${preset})`);
const { packageName, presetName, params } = parsePreset(preset);
let presetConfig;
- try {
- const dep = await npm.getDependency(packageName);
- if (!dep) {
- logger.warn(`Failed to look up preset packageName ${packageName}`);
- return {};
- }
- if (!dep['renovate-config']) {
- logger.warn(`Package ${packageName} has no renovate-config`);
- return {};
- }
- presetConfig = dep['renovate-config'][presetName];
- } catch (err) {
- logger.warn({ err }, `Failed to look up package ${packageName}`);
+ const dep = await npm.getDependency(packageName);
+ if (!dep) {
+ throw Error('dep not found');
+ }
+ if (!dep['renovate-config']) {
+ throw Error('preset renovate-config not found');
}
+ presetConfig = dep['renovate-config'][presetName];
if (!presetConfig) {
- logger.warn(`Cannot find preset ${preset}`);
- return {};
+ throw Error('preset not found');
}
logger.debug(`Found preset ${preset}`);
logger.trace({ presetConfig });
diff --git a/lib/manager/resolve.js b/lib/manager/resolve.js
index 16baf370ba..45b3ffc4ef 100644
--- a/lib/manager/resolve.js
+++ b/lib/manager/resolve.js
@@ -87,6 +87,17 @@ async function resolvePackageFiles(config) {
{ config: migratedConfig },
'package.json migrated config'
);
+ // istanbul ignore if
+ if (migratedConfig.errors.length) {
+ const error = new Error('config-validation');
+ error.configFile = packageFile.packageFile;
+ error.validationError =
+ 'The `renovate` config inside `package.json` failed validation';
+ error.validationMessage = migratedConfig.errors
+ .map(e => e.message)
+ .join(', ');
+ throw error;
+ }
const resolvedConfig = await presets.resolveConfigPresets(
migratedConfig
);
@@ -144,5 +155,6 @@ async function resolvePackageFiles(config) {
logger.debug('queue');
const queue = allPackageFiles.map(p => resolvePackageFile(p));
const packageFiles = (await Promise.all(queue)).filter(p => p !== null);
+ platform.ensureIssueClosing('Action Required: Fix Renovate Configuration');
return checkMonorepos({ ...config, packageFiles });
}
diff --git a/lib/platform/github/index.js b/lib/platform/github/index.js
index 390d9ea09a..dc60e1b97d 100644
--- a/lib/platform/github/index.js
+++ b/lib/platform/github/index.js
@@ -27,6 +27,8 @@ module.exports = {
mergeBranch,
getBranchLastCommitTime,
// issue
+ ensureIssue,
+ ensureIssueClosing,
addAssignees,
addReviewers,
// Comments
@@ -106,6 +108,7 @@ async function initRepo(repoName, token, endpoint, forkMode, forkToken) {
logger.info({ err, res }, 'Unknown GitHub initRepo error');
throw err;
}
+ delete config.issueList;
delete config.prList;
delete config.fileList;
await Promise.all([getPrList(), getFileList()]);
@@ -422,6 +425,57 @@ async function getBranchLastCommitTime(branchName) {
// Issue
+async function getIssueList() {
+ if (!config.issueList) {
+ config.issueList = (await get(
+ `repos/${config.repoName}/issues?filter=created&state=open`
+ )).body.map(i => ({
+ number: i.number,
+ title: i.title,
+ }));
+ }
+ return config.issueList;
+}
+
+async function ensureIssue(title, body) {
+ logger.debug(`ensureIssue()`);
+ const issueList = await getIssueList();
+ const issue = issueList.find(i => i.title === title);
+ if (issue) {
+ const issueBody = (await get(
+ `repos/${config.repoName}/issues/${issue.number}`
+ )).body.body;
+ if (issueBody !== body) {
+ logger.debug('Updating issue body');
+ await get.patch(`repos/${config.repoName}/issues/${issue.number}`, {
+ body: { body },
+ });
+ return 'updated';
+ }
+ } else {
+ await get.post(`repos/${config.repoName}/issues`, {
+ body: {
+ title,
+ body,
+ },
+ });
+ return 'created';
+ }
+ return null;
+}
+
+async function ensureIssueClosing(title) {
+ logger.debug(`ensureIssueClosing()`);
+ const issueList = await getIssueList();
+ for (const issue of issueList) {
+ if (issue.title === title) {
+ await get.patch(`repos/${config.repoName}/issues/${issue.id}`, {
+ body: { state: 'closed' },
+ });
+ }
+ }
+}
+
async function addAssignees(issueNo, assignees) {
logger.debug(`Adding assignees ${assignees} to #${issueNo}`);
await get.post(`repos/${config.repoName}/issues/${issueNo}/assignees`, {
diff --git a/lib/platform/gitlab/index.js b/lib/platform/gitlab/index.js
index 29d26faead..732db8abea 100644
--- a/lib/platform/gitlab/index.js
+++ b/lib/platform/gitlab/index.js
@@ -23,6 +23,8 @@ module.exports = {
mergeBranch,
getBranchLastCommitTime,
// issue
+ ensureIssue,
+ ensureIssueClosing,
addAssignees,
addReviewers,
// Comments
@@ -304,6 +306,13 @@ async function getBranchLastCommitTime(branchName) {
// Issue
+function ensureIssue() {
+ // istanbul ignore next
+ logger.warn(`ensureIssue() is not implemented`);
+}
+
+function ensureIssueClosing() {}
+
async function addAssignees(iid, assignees) {
logger.debug(`Adding assignees ${assignees} to #${iid}`);
if (assignees.length > 1) {
diff --git a/lib/platform/vsts/index.js b/lib/platform/vsts/index.js
index f5ef87c8bb..3f5220efce 100644
--- a/lib/platform/vsts/index.js
+++ b/lib/platform/vsts/index.js
@@ -24,6 +24,8 @@ module.exports = {
mergeBranch,
getBranchLastCommitTime,
// issue
+ ensureIssue,
+ ensureIssueClosing,
addAssignees,
addReviewers,
// Comments
@@ -470,6 +472,13 @@ async function mergePr(pr) {
await null;
}
+function ensureIssue() {
+ // istanbul ignore next
+ logger.warn(`ensureIssue() is not implemented`);
+}
+
+function ensureIssueClosing() {}
+
/**
*
* @param {number} issueNo
diff --git a/lib/workers/repository/error-config.js b/lib/workers/repository/error-config.js
new file mode 100644
index 0000000000..ca65c6477a
--- /dev/null
+++ b/lib/workers/repository/error-config.js
@@ -0,0 +1,31 @@
+module.exports = {
+ raiseConfigWarningIssue,
+};
+
+async function raiseConfigWarningIssue(config, error) {
+ logger.debug('raiseConfigWarningIssue()');
+ let body = `There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop renovations until it is fixed.\n\n`;
+ if (error.configFile) {
+ body += `Configuration file: \`${error.configFile}\`\n`;
+ }
+ body += `Error type: ${error.validationError}\n`;
+ if (error.validationMessage) {
+ body += `Message: ${error.validationMessage}\n`;
+ }
+ if (config.repoIsOnboarded) {
+ const res = await platform.ensureIssue(
+ 'Action Required: Fix Renovate Configuration',
+ body
+ );
+ if (res) {
+ logger.warn({ configError: error, res }, 'Config Warning');
+ }
+ } else {
+ // update onboarding Pr
+ logger.info('Updating onboarding PR');
+ const pr = await platform.getBranchPr('renovate/configure');
+ body = `## Action Required: Fix Renovate Configuration\n\n${body}`;
+ body += `\n\nOnce you have resolved this problem (in this onboarding branch), Renovate will return to providing you with a preview of your repository's configuration.`;
+ await platform.updatePr(pr.number, 'Configure Renovate', body);
+ }
+}
diff --git a/lib/workers/repository/error.js b/lib/workers/repository/error.js
index d6abcbf91b..8a6a04d2f0 100644
--- a/lib/workers/repository/error.js
+++ b/lib/workers/repository/error.js
@@ -1,8 +1,10 @@
+const { raiseConfigWarningIssue } = require('./error-config');
+
module.exports = {
handleError,
};
-function handleError(config, err) {
+async function handleError(config, err) {
if (err.message === 'uninitiated') {
logger.info('Repository is uninitiated - skipping');
delete config.branchList; // eslint-disable-line no-param-reassign
@@ -19,6 +21,11 @@ function handleError(config, err) {
} else if (err.message === 'loops>5') {
logger.error('Repository has looped 5 times already');
return err.message;
+ } else if (err.message === 'config-validation') {
+ delete config.branchList; // eslint-disable-line no-param-reassign
+ logger.info({ error: err }, 'Repository has invalid config');
+ await raiseConfigWarningIssue(config, err);
+ return err.message;
}
// Swallow this error so that other repositories can be processed
logger.error({ err }, `Repository has unknown error`);
diff --git a/lib/workers/repository/init/config.js b/lib/workers/repository/init/config.js
index 6bdcd8f9dd..c6ac025c43 100644
--- a/lib/workers/repository/init/config.js
+++ b/lib/workers/repository/init/config.js
@@ -29,14 +29,11 @@ async function mergeRenovateConfig(config) {
allowDuplicateKeys
);
if (jsonValidationError) {
- const error = {
- depName: configFile,
- message: jsonValidationError,
- };
- logger.warn({ renovateConfig }, error.message);
- returnConfig.errors.push(error);
- // Return unless error can be ignored
- return returnConfig;
+ const error = new Error('config-validation');
+ error.configFile = configFile;
+ error.validationError = 'Invalid JSON (parsing failed)';
+ error.validationMessage = jsonValidationError;
+ throw error;
}
allowDuplicateKeys = false;
jsonValidationError = jsonValidator.validate(
@@ -44,17 +41,25 @@ async function mergeRenovateConfig(config) {
allowDuplicateKeys
);
if (jsonValidationError) {
- const error = {
- depName: configFile,
- message: jsonValidationError,
- };
- logger.warn({ renovateConfig }, error.message);
- returnConfig.errors.push(error);
- // Return unless error can be ignored
+ const error = new Error('config-validation');
+ error.configFile = configFile;
+ error.validationError = 'Duplicate keys in JSON';
+ error.validationMessage = JSON.stringify(jsonValidationError);
+ throw error;
}
const renovateJson = JSON.parse(renovateConfig);
logger.debug({ config: renovateJson }, 'renovate.json config');
const migratedConfig = migrateAndValidate(config, renovateJson);
+ if (migratedConfig.errors.length) {
+ const error = new Error('config-validation');
+ error.configFile = configFile;
+ error.validationError =
+ 'The renovate configuration file contains some invalid settings';
+ error.validationMessage = migratedConfig.errors
+ .map(e => e.message)
+ .join(', ');
+ throw error;
+ }
logger.debug({ config: migratedConfig }, 'renovate.json migrated config');
const decryptedConfig = decryptConfig(migratedConfig, config.privateKey);
const resolvedConfig = await presets.resolveConfigPresets(decryptedConfig);
diff --git a/test/config/__snapshots__/presets.spec.js.snap b/test/config/__snapshots__/presets.spec.js.snap
index 32a21dc924..4eb4714d24 100644
--- a/test/config/__snapshots__/presets.spec.js.snap
+++ b/test/config/__snapshots__/presets.spec.js.snap
@@ -31,7 +31,11 @@ Object {
}
`;
-exports[`config/presets getPreset handles 404 packages 1`] = `Object {}`;
+exports[`config/presets getPreset handles 404 packages 1`] = `undefined`;
+
+exports[`config/presets getPreset handles 404 packages 2`] = `undefined`;
+
+exports[`config/presets getPreset handles 404 packages 3`] = `undefined`;
exports[`config/presets getPreset handles missing params 1`] = `
Object {
@@ -49,11 +53,23 @@ Object {
}
`;
-exports[`config/presets getPreset handles no config 1`] = `Object {}`;
+exports[`config/presets getPreset handles no config 1`] = `undefined`;
+
+exports[`config/presets getPreset handles no config 2`] = `undefined`;
+
+exports[`config/presets getPreset handles no config 3`] = `undefined`;
+
+exports[`config/presets getPreset handles preset not found 1`] = `undefined`;
+
+exports[`config/presets getPreset handles preset not found 2`] = `undefined`;
+
+exports[`config/presets getPreset handles preset not found 3`] = `undefined`;
+
+exports[`config/presets getPreset handles throw errors 1`] = `undefined`;
-exports[`config/presets getPreset handles preset not found 1`] = `Object {}`;
+exports[`config/presets getPreset handles throw errors 2`] = `undefined`;
-exports[`config/presets getPreset handles throw errors 1`] = `Object {}`;
+exports[`config/presets getPreset handles throw errors 3`] = `undefined`;
exports[`config/presets getPreset ignores irrelevant params 1`] = `
Object {
@@ -688,29 +704,31 @@ Object {
}
`;
-exports[`config/presets resolvePreset returns same if invalid preset 1`] = `
-Object {
- "foo": 1,
-}
-`;
-
exports[`config/presets resolvePreset returns same if no presets 1`] = `
Object {
"foo": 1,
}
`;
-exports[`config/presets resolvePreset works with valid 1`] = `
-Object {
- "description": Array [
- "Use version pinning (maintain a single version only and not semver ranges)",
- ],
- "foo": 1,
- "pinVersions": true,
-}
-`;
+exports[`config/presets resolvePreset throws if invalid preset 1`] = `undefined`;
+
+exports[`config/presets resolvePreset throws if invalid preset 2`] = `"Preset name not found within published preset config (:invalid-preset)"`;
+
+exports[`config/presets resolvePreset throws if invalid preset 3`] = `undefined`;
-exports[`config/presets resolvePreset works with valid and invalid 1`] = `
+exports[`config/presets resolvePreset throws if invalid preset file 1`] = `undefined`;
+
+exports[`config/presets resolvePreset throws if invalid preset file 2`] = `"Cannot find preset's package (notfoundaaaaaaaa)"`;
+
+exports[`config/presets resolvePreset throws if invalid preset file 3`] = `undefined`;
+
+exports[`config/presets resolvePreset throws if valid and invalid 1`] = `undefined`;
+
+exports[`config/presets resolvePreset throws if valid and invalid 2`] = `"Preset name not found within published preset config (:invalid-preset)"`;
+
+exports[`config/presets resolvePreset throws if valid and invalid 3`] = `undefined`;
+
+exports[`config/presets resolvePreset works with valid 1`] = `
Object {
"description": Array [
"Use version pinning (maintain a single version only and not semver ranges)",
diff --git a/test/config/presets.spec.js b/test/config/presets.spec.js
index 7b4daa5bf6..6a910c3dfb 100644
--- a/test/config/presets.spec.js
+++ b/test/config/presets.spec.js
@@ -74,11 +74,33 @@ describe('config/presets', () => {
expect(config).toMatchObject(res);
expect(res).toMatchSnapshot();
});
- it('returns same if invalid preset', async () => {
+ it('throws if invalid preset file', async () => {
+ config.foo = 1;
+ config.extends = ['notfoundaaaaaaaa'];
+ let e;
+ try {
+ await presets.resolveConfigPresets(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
+ });
+ it('throws if invalid preset', async () => {
config.foo = 1;
config.extends = [':invalid-preset'];
- const res = await presets.resolveConfigPresets(config);
- expect(res).toMatchSnapshot();
+ let e;
+ try {
+ await presets.resolveConfigPresets(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('works with valid', async () => {
config.foo = 1;
@@ -87,12 +109,19 @@ describe('config/presets', () => {
expect(res).toMatchSnapshot();
expect(res.pinVersions).toBe(true);
});
- it('works with valid and invalid', async () => {
+ it('throws if valid and invalid', async () => {
config.foo = 1;
config.extends = [':invalid-preset', ':pinVersions'];
- const res = await presets.resolveConfigPresets(config);
- expect(res).toMatchSnapshot();
- expect(res.pinVersions).toBe(true);
+ let e;
+ try {
+ await presets.resolveConfigPresets(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('resolves app preset', async () => {
config.extends = [':app'];
@@ -281,20 +310,52 @@ describe('config/presets', () => {
expect(res).toMatchSnapshot();
});
it('handles 404 packages', async () => {
- const res = await presets.getPreset('notfound:foo');
- expect(res).toMatchSnapshot();
+ let e;
+ try {
+ await presets.getPreset('notfound:foo');
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('handles no config', async () => {
- const res = await presets.getPreset('noconfig:foo');
- expect(res).toMatchSnapshot();
+ let e;
+ try {
+ await presets.getPreset('noconfig:foo');
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('handles throw errors', async () => {
- const res = await presets.getPreset('throw:foo');
- expect(res).toMatchSnapshot();
+ let e;
+ try {
+ await presets.getPreset('throw:foo');
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('handles preset not found', async () => {
- const res = await presets.getPreset('wrongpreset:foo');
- expect(res).toMatchSnapshot();
+ let e;
+ try {
+ await presets.getPreset('wrongpreset:foo');
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
});
});
diff --git a/test/manager/__snapshots__/resolve.spec.js.snap b/test/manager/__snapshots__/resolve.spec.js.snap
index 565914a26e..a30be60196 100644
--- a/test/manager/__snapshots__/resolve.spec.js.snap
+++ b/test/manager/__snapshots__/resolve.spec.js.snap
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
-exports[`manager/resolve resolvePackageFiles() deetect package.json and warns if cannot parse 1`] = `
+exports[`manager/resolve resolvePackageFiles() detect package.json and warns if cannot parse 1`] = `
Object {
"assignees": Array [],
"autodiscover": false,
diff --git a/test/manager/resolve.spec.js b/test/manager/resolve.spec.js
index 2919825755..4198dea418 100644
--- a/test/manager/resolve.spec.js
+++ b/test/manager/resolve.spec.js
@@ -25,7 +25,7 @@ describe('manager/resolve', () => {
expect(res).toMatchSnapshot();
expect(res.errors).toHaveLength(2);
});
- it('deetect package.json and warns if cannot parse', async () => {
+ it('detect package.json and warns if cannot parse', async () => {
manager.detectPackageFiles = jest.fn(() => [
{ packageFile: 'package.json' },
]);
diff --git a/test/platform/__snapshots__/index.spec.js.snap b/test/platform/__snapshots__/index.spec.js.snap
index 789c8486ea..d9e87ca47a 100644
--- a/test/platform/__snapshots__/index.spec.js.snap
+++ b/test/platform/__snapshots__/index.spec.js.snap
@@ -17,6 +17,8 @@ Array [
"deleteBranch",
"mergeBranch",
"getBranchLastCommitTime",
+ "ensureIssue",
+ "ensureIssueClosing",
"addAssignees",
"addReviewers",
"ensureComment",
@@ -50,6 +52,8 @@ Array [
"deleteBranch",
"mergeBranch",
"getBranchLastCommitTime",
+ "ensureIssue",
+ "ensureIssueClosing",
"addAssignees",
"addReviewers",
"ensureComment",
@@ -83,6 +87,8 @@ Array [
"deleteBranch",
"mergeBranch",
"getBranchLastCommitTime",
+ "ensureIssue",
+ "ensureIssueClosing",
"addAssignees",
"addReviewers",
"ensureComment",
diff --git a/test/platform/github/index.spec.js b/test/platform/github/index.spec.js
index 0421e7dc88..f9fb1a97a0 100644
--- a/test/platform/github/index.spec.js
+++ b/test/platform/github/index.spec.js
@@ -821,6 +821,75 @@ describe('platform/github', () => {
expect(res).toBeDefined();
});
});
+ describe('ensureIssue()', () => {
+ it('creates issue', async () => {
+ get.mockImplementationOnce(() => ({
+ body: [
+ {
+ number: 1,
+ title: 'title-1',
+ },
+ {
+ number: 2,
+ title: 'title-2',
+ },
+ ],
+ }));
+ const res = await github.ensureIssue('new-title', 'new-content');
+ expect(res).toEqual('created');
+ });
+ it('updates issue', async () => {
+ get.mockReturnValueOnce({
+ body: [
+ {
+ number: 1,
+ title: 'title-1',
+ },
+ {
+ number: 2,
+ title: 'title-2',
+ },
+ ],
+ });
+ get.mockReturnValueOnce({ body: { body: 'new-content' } });
+ const res = await github.ensureIssue('title-2', 'newer-content');
+ expect(res).toEqual('updated');
+ });
+ it('skips update if unchanged', async () => {
+ get.mockReturnValueOnce({
+ body: [
+ {
+ number: 1,
+ title: 'title-1',
+ },
+ {
+ number: 2,
+ title: 'title-2',
+ },
+ ],
+ });
+ get.mockReturnValueOnce({ body: { body: 'newer-content' } });
+ const res = await github.ensureIssue('title-2', 'newer-content');
+ expect(res).toBe(null);
+ });
+ });
+ describe('ensureIssueClosing()', () => {
+ it('closes issue', async () => {
+ get.mockImplementationOnce(() => ({
+ body: [
+ {
+ number: 1,
+ title: 'title-1',
+ },
+ {
+ number: 2,
+ title: 'title-2',
+ },
+ ],
+ }));
+ await github.ensureIssueClosing('title-2');
+ });
+ });
describe('addAssignees(issueNo, assignees)', () => {
it('should add the given assignees to the issue', async () => {
await initRepo('some/repo', 'token');
diff --git a/test/workers/repository/error-config.spec.js b/test/workers/repository/error-config.spec.js
new file mode 100644
index 0000000000..b07b791d58
--- /dev/null
+++ b/test/workers/repository/error-config.spec.js
@@ -0,0 +1,32 @@
+const {
+ raiseConfigWarningIssue,
+} = require('../../../lib/workers/repository/error-config');
+
+let config;
+beforeEach(() => {
+ jest.resetAllMocks();
+ config = require('../../_fixtures/config');
+});
+
+describe('workers/repository/error-config', () => {
+ describe('raiseConfigWarningIssue()', () => {
+ it('creates issues', async () => {
+ const error = new Error('config-validation');
+ error.configFile = 'package.json';
+ error.validationMessage = 'some-message';
+ config.repoIsOnboarded = true;
+ platform.ensureIssue.mockReturnValue('created');
+ const res = await raiseConfigWarningIssue(config, error);
+ expect(res).toBeUndefined();
+ });
+ it('handles onboarding', async () => {
+ const error = new Error('config-validation');
+ error.configFile = 'package.json';
+ error.validationMessage = 'some-message';
+ config.repoIsOnboarded = false;
+ platform.getBranchPr.mockReturnValueOnce({ number: 1 });
+ const res = await raiseConfigWarningIssue(config, error);
+ expect(res).toBeUndefined();
+ });
+ });
+});
diff --git a/test/workers/repository/error.spec.js b/test/workers/repository/error.spec.js
index 1846e76a6f..7ded2443f2 100644
--- a/test/workers/repository/error.spec.js
+++ b/test/workers/repository/error.spec.js
@@ -1,5 +1,7 @@
const { handleError } = require('../../../lib/workers/repository/error');
+jest.mock('../../../lib/workers/repository/error-config');
+
let config;
beforeEach(() => {
jest.resetAllMocks();
@@ -14,15 +16,16 @@ describe('workers/repository/error', () => {
'fork',
'no-package-files',
'loops>5',
+ 'config-validation',
];
errors.forEach(err => {
- it(`errors ${err}`, () => {
- const res = handleError(config, new Error(err));
+ it(`errors ${err}`, async () => {
+ const res = await handleError(config, new Error(err));
expect(res).toEqual(err);
});
});
- it('handles unknown error', () => {
- const res = handleError(config, new Error('abcdefg'));
+ it('handles unknown error', async () => {
+ const res = await handleError(config, new Error('abcdefg'));
expect(res).toEqual('unknown-error');
});
});
diff --git a/test/workers/repository/init/__snapshots__/config.spec.js.snap b/test/workers/repository/init/__snapshots__/config.spec.js.snap
index 612e2d5005..8fc9ed4d12 100644
--- a/test/workers/repository/init/__snapshots__/config.spec.js.snap
+++ b/test/workers/repository/init/__snapshots__/config.spec.js.snap
@@ -1,15 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
-exports[`workers/repository/init/config mergeRenovateConfig() returns error if cannot parse 1`] = `
-Object {
- "depName": "renovate.json",
- "message": "Syntax error near cannot par",
-}
-`;
-
-exports[`workers/repository/init/config mergeRenovateConfig() returns error if duplicate keys 1`] = `
-Object {
- "depName": ".renovaterc",
- "message": "Syntax error: duplicated keys \\"enabled\\" near \\": false }",
-}
-`;
+exports[`workers/repository/init/config mergeRenovateConfig() returns error if cannot parse 1`] = `"renovate.json"`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() returns error if cannot parse 2`] = `"Invalid JSON (parsing failed)"`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() returns error if cannot parse 3`] = `"Syntax error near cannot par"`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() throws error if duplicate keys 1`] = `".renovaterc"`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() throws error if duplicate keys 2`] = `"Duplicate keys in JSON"`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() throws error if duplicate keys 3`] = `"\\"Syntax error: duplicated keys \\\\\\"enabled\\\\\\" near \\\\\\": false }\\""`;
+
+exports[`workers/repository/init/config mergeRenovateConfig() throws error if misconfigured 1`] = `[Error: config-validation]`;
diff --git a/test/workers/repository/init/config.spec.js b/test/workers/repository/init/config.spec.js
index 90d9f97917..593cd1f9bc 100644
--- a/test/workers/repository/init/config.spec.js
+++ b/test/workers/repository/init/config.spec.js
@@ -9,9 +9,18 @@ beforeEach(() => {
const {
mergeRenovateConfig,
} = require('../../../../lib/workers/repository/init/config');
+const migrateValidate = require('../../../../lib/config/migrate-validate');
+
+jest.mock('../../../../lib/config/migrate-validate');
describe('workers/repository/init/config', () => {
describe('mergeRenovateConfig()', () => {
+ beforeEach(() => {
+ migrateValidate.migrateAndValidate.mockReturnValue({
+ warnings: [],
+ errors: [],
+ });
+ });
it('returns config if not found', async () => {
platform.getFileList.mockReturnValue(['package.json']);
const res = await mergeRenovateConfig(config);
@@ -20,16 +29,30 @@ describe('workers/repository/init/config', () => {
it('returns error if cannot parse', async () => {
platform.getFileList.mockReturnValue(['package.json', 'renovate.json']);
platform.getFile.mockReturnValue('cannot parse');
- const res = await mergeRenovateConfig(config);
- expect(res.errors).toHaveLength(1);
- expect(res.errors[0]).toMatchSnapshot();
+ let e;
+ try {
+ await mergeRenovateConfig(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
- it('returns error if duplicate keys', async () => {
+ it('throws error if duplicate keys', async () => {
platform.getFileList.mockReturnValue(['package.json', '.renovaterc']);
platform.getFile.mockReturnValue('{ "enabled": true, "enabled": false }');
- const res = await mergeRenovateConfig(config);
- expect(res.errors).toHaveLength(1);
- expect(res.errors[0]).toMatchSnapshot();
+ let e;
+ try {
+ await mergeRenovateConfig(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e.configFile).toMatchSnapshot();
+ expect(e.validationError).toMatchSnapshot();
+ expect(e.validationMessage).toMatchSnapshot();
});
it('finds .renovaterc.json', async () => {
platform.getFileList.mockReturnValue([
@@ -39,5 +62,23 @@ describe('workers/repository/init/config', () => {
platform.getFile.mockReturnValue('{}');
await mergeRenovateConfig(config);
});
+ it('throws error if misconfigured', async () => {
+ platform.getFileList.mockReturnValue([
+ 'package.json',
+ '.renovaterc.json',
+ ]);
+ platform.getFile.mockReturnValue('{}');
+ migrateValidate.migrateAndValidate.mockReturnValueOnce({
+ errors: [{}],
+ });
+ let e;
+ try {
+ await mergeRenovateConfig(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
+ expect(e).toMatchSnapshot();
+ });
});
});
--
GitLab