From 43660d51c445162aa8fa381c582b6e6a70b4b8e6 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Mon, 14 Aug 2017 11:09:14 +0200
Subject: [PATCH] feat: validate schedule (#674)
Also enables config errors to be shown in Configure Renovate PR.
Closes #550
---
lib/config/validation.js | 9 ++++
lib/workers/branch/schedule.js | 39 ++++++++-------
lib/workers/repository/apis.js | 4 ++
.../__snapshots__/validation.spec.js.snap | 6 ++-
test/config/validation.spec.js | 11 +++--
test/workers/branch/schedule.spec.js | 47 +++++++++++--------
.../__snapshots__/apis.spec.js.snap | 2 +
test/workers/repository/apis.spec.js | 2 +
8 files changed, 80 insertions(+), 40 deletions(-)
diff --git a/lib/config/validation.js b/lib/config/validation.js
index d8055caeb1..6910e3e26c 100644
--- a/lib/config/validation.js
+++ b/lib/config/validation.js
@@ -1,4 +1,5 @@
const options = require('./definitions').getOptions();
+const { hasValidSchedule } = require('../workers/branch/schedule');
const optionTypes = {};
options.forEach(option => {
@@ -42,6 +43,14 @@ function validateConfig(config) {
depName: 'Configuration Error',
message: `Invalid configuration option: \`${key}\``,
});
+ } else if (key === 'schedule') {
+ const [validSchedule, errorMessage] = hasValidSchedule(val);
+ if (!validSchedule) {
+ errors.push({
+ depName: 'Configuration Error',
+ message: `Invalid schedule: \`${errorMessage}\``,
+ });
+ }
} else if (val != null) {
const type = optionTypes[key];
if (type === 'boolean') {
diff --git a/lib/workers/branch/schedule.js b/lib/workers/branch/schedule.js
index e5d2478227..a4e96ed2aa 100644
--- a/lib/workers/branch/schedule.js
+++ b/lib/workers/branch/schedule.js
@@ -10,29 +10,25 @@ function fixShortHours(input) {
return input.replace(/( \d?\d)((a|p)m)/g, '$1:00$2');
}
-function hasValidSchedule(schedule, logger) {
+function hasValidSchedule(schedule) {
if (!Array.isArray(schedule)) {
- logger.debug({ schedule }, `Invalid schedule`);
- return false;
- }
- if (schedule.length === 0) {
- logger.debug('No schedule defined');
- return false;
- }
- if (schedule.length === 1 && schedule[0] === '') {
- logger.debug('Empty schedule');
- return false;
+ return [false, 'Invalid schedule'];
}
+ let message;
// check if any of the schedules fail to parse
const hasFailedSchedules = schedule.some(scheduleText => {
const parsedSchedule = later.parse.text(fixShortHours(scheduleText));
if (parsedSchedule.error !== -1) {
- logger.debug(`Failed to parse schedule ${scheduleText}`);
+ message = `Failed to parse schedule ${scheduleText}`;
// It failed to parse
return true;
}
+ if (parsedSchedule.schedules.some(s => s.m)) {
+ message = 'Schedules should not specify minutes';
+ return true;
+ }
if (!parsedSchedule.schedules.some(s => s.d || s.t_a || s.t_b)) {
- logger.debug('Schedule has no days of week or time of day');
+ message = 'Schedule has no days of week or time of day';
return true;
}
// It must be OK
@@ -40,15 +36,24 @@ function hasValidSchedule(schedule, logger) {
});
if (hasFailedSchedules) {
// If any fail then we invalidate the whole thing
- return false;
+ return [false, message];
}
- return true;
+ return [true];
}
function isScheduledNow(config) {
config.logger.debug({ schedule: config.schedule }, `Checking schedule`);
- if (!module.exports.hasValidSchedule(config.schedule, config.logger)) {
- // Return true if the schedule is invalid
+ if (
+ !config.schedule ||
+ config.schedule.length === 0 ||
+ config.schedule[0] === ''
+ ) {
+ config.logger.debug('No schedule defined');
+ return true;
+ }
+ const [validSchedule, errorMessage] = hasValidSchedule(config.schedule);
+ if (!validSchedule) {
+ config.logger.error(errorMessage);
return true;
}
let now = moment();
diff --git a/lib/workers/repository/apis.js b/lib/workers/repository/apis.js
index a90bb92fc5..3976b7c63f 100644
--- a/lib/workers/repository/apis.js
+++ b/lib/workers/repository/apis.js
@@ -124,6 +124,10 @@ function migrateAndValidate(config, input) {
);
}); */
}
+ if (!config.repoIsOnboarded) {
+ migratedConfig.warnings = (migratedConfig.warnings || []).concat(warnings);
+ migratedConfig.errors = (migratedConfig.errors || []).concat(errors);
+ }
return migratedConfig;
}
diff --git a/test/config/__snapshots__/validation.spec.js.snap b/test/config/__snapshots__/validation.spec.js.snap
index 17be67ac69..0221af410d 100644
--- a/test/config/__snapshots__/validation.spec.js.snap
+++ b/test/config/__snapshots__/validation.spec.js.snap
@@ -8,7 +8,11 @@ Array [
},
Object {
"depName": "Configuration Error",
- "message": "Configuration option \`schedule\` should be a list (Array)",
+ "message": "Invalid schedule: \`Schedules should not specify minutes\`",
+ },
+ Object {
+ "depName": "Configuration Error",
+ "message": "Configuration option \`labels\` should be a list (Array)",
},
Object {
"depName": "Configuration Error",
diff --git a/test/config/validation.spec.js b/test/config/validation.spec.js
index 8b51001ae4..deef5300b4 100644
--- a/test/config/validation.spec.js
+++ b/test/config/validation.spec.js
@@ -1,4 +1,5 @@
const configValidation = require('../../lib/config/validation.js');
+const logger = require('../_fixtures/logger');
describe('config/validation', () => {
describe('validateConfig(config)', () => {
@@ -19,7 +20,8 @@ describe('config/validation', () => {
it('errors for all types', () => {
const config = {
enabled: 1,
- schedule: 5,
+ schedule: ['every 15 mins every weekday'],
+ labels: 5,
semanticPrefix: 7,
githubAppId: 'none',
lockFileMaintenance: false,
@@ -29,9 +31,12 @@ describe('config/validation', () => {
},
],
};
- const { warnings, errors } = configValidation.validateConfig(config);
+ const { warnings, errors } = configValidation.validateConfig(
+ config,
+ logger
+ );
expect(warnings).toHaveLength(0);
- expect(errors).toHaveLength(6);
+ expect(errors).toHaveLength(7);
expect(errors).toMatchSnapshot();
});
});
diff --git a/test/workers/branch/schedule.spec.js b/test/workers/branch/schedule.spec.js
index 98789f23c3..ad668d7fd9 100644
--- a/test/workers/branch/schedule.spec.js
+++ b/test/workers/branch/schedule.spec.js
@@ -8,44 +8,48 @@ describe('workers/branch/schedule', () => {
jest.resetAllMocks();
});
it('returns false if schedule is not an array', () => {
- expect(schedule.hasValidSchedule({ a: 1 }, logger)).toBe(false);
- });
- it('returns false if schedule array is empty', () => {
- expect(schedule.hasValidSchedule([], logger)).toBe(false);
- });
- it('returns false if only schedule is empty', () => {
- expect(schedule.hasValidSchedule([''], logger)).toBe(false);
+ expect(schedule.hasValidSchedule({ a: 1 }, logger)[0]).toBe(false);
});
it('returns false for invalid schedule', () => {
- expect(schedule.hasValidSchedule(['foo'], logger)).toBe(false);
+ expect(schedule.hasValidSchedule(['foo'], logger)[0]).toBe(false);
});
it('returns false if any schedule fails to parse', () => {
- expect(schedule.hasValidSchedule(['after 5:00pm', 'foo'], logger)).toBe(
- false
- );
+ expect(
+ schedule.hasValidSchedule(['after 5:00pm', 'foo'], logger)[0]
+ ).toBe(false);
+ });
+ it('returns false if using minutes', () => {
+ expect(
+ schedule.hasValidSchedule(['every 15 mins every weekday'], logger)[0]
+ ).toBe(false);
});
it('returns false if schedules have no days or time range', () => {
- expect(schedule.hasValidSchedule(['at 5:00pm'], logger)).toBe(false);
+ expect(schedule.hasValidSchedule(['at 5:00pm'], logger)[0]).toBe(false);
});
it('returns false if any schedule has no days or time range', () => {
expect(
- schedule.hasValidSchedule(['at 5:00pm', 'on saturday'], logger)
+ schedule.hasValidSchedule(['at 5:00pm', 'on saturday'], logger)[0]
).toBe(false);
});
it('returns true if schedule has days of week', () => {
expect(
- schedule.hasValidSchedule(['on friday and saturday'], logger)
+ schedule.hasValidSchedule(['on friday and saturday'], logger)[0]
).toBe(true);
});
it('returns true if schedule has a start time', () => {
- expect(schedule.hasValidSchedule(['after 8:00pm'], logger)).toBe(true);
+ expect(schedule.hasValidSchedule(['after 8:00pm'], logger)[0]).toBe(true);
});
it('returns true if schedule has an end time', () => {
- expect(schedule.hasValidSchedule(['before 6:00am'], logger)).toBe(true);
+ expect(schedule.hasValidSchedule(['before 6:00am'], logger)[0]).toBe(
+ true
+ );
});
it('returns true if schedule has a start and end time', () => {
expect(
- schedule.hasValidSchedule(['after 11:00pm and before 6:00am'], logger)
+ schedule.hasValidSchedule(
+ ['after 11:00pm and before 6:00am'],
+ logger
+ )[0]
).toBe(true);
});
it('returns true if schedule has days and a start and end time', () => {
@@ -53,11 +57,11 @@ describe('workers/branch/schedule', () => {
schedule.hasValidSchedule(
['after 11:00pm and before 6:00am every weekday'],
logger
- )
+ )[0]
).toBe(true);
});
it('supports hours shorthand', () => {
- const res = schedule.hasValidSchedule(
+ const [res] = schedule.hasValidSchedule(
[
'after 11pm and before 6am every weekend',
'after 11pm',
@@ -86,6 +90,11 @@ describe('workers/branch/schedule', () => {
const res = schedule.isScheduledNow(config);
expect(res).toBe(true);
});
+ it('returns true if invalid schedule', () => {
+ config.schedule = ['every 15 minutes'];
+ const res = schedule.isScheduledNow(config);
+ expect(res).toBe(true);
+ });
it('supports before hours true', () => {
config.schedule = ['before 4:00pm'];
const res = schedule.isScheduledNow(config);
diff --git a/test/workers/repository/__snapshots__/apis.spec.js.snap b/test/workers/repository/__snapshots__/apis.spec.js.snap
index 26408404c3..7f17464e1f 100644
--- a/test/workers/repository/__snapshots__/apis.spec.js.snap
+++ b/test/workers/repository/__snapshots__/apis.spec.js.snap
@@ -78,9 +78,11 @@ exports[`workers/repository/apis resolvePackageFiles includes files with content
Array [
Object {
"content": Object {},
+ "errors": Array [],
"hasPackageLock": true,
"hasYarnLock": true,
"packageFile": "package.json",
+ "warnings": Array [],
},
Object {
"content": Object {},
diff --git a/test/workers/repository/apis.spec.js b/test/workers/repository/apis.spec.js
index 48e42687de..adcbdae562 100644
--- a/test/workers/repository/apis.spec.js
+++ b/test/workers/repository/apis.spec.js
@@ -159,6 +159,7 @@ describe('workers/repository/apis', () => {
expect(returnConfig.errors).toHaveLength(0);
});
it('returns warning + error plus extended config if unknown keys', async () => {
+ config.repoIsOnboarded = true;
config.api.getFileContent.mockReturnValueOnce(
'{ "enabled": true, "foo": false, "maintainYarnLock": true, "schedule": "before 5am", "minor": {} }'
);
@@ -169,6 +170,7 @@ describe('workers/repository/apis', () => {
expect(returnConfig.errors).toMatchSnapshot();
});
it('returns error plus extended config if duplicate keys', async () => {
+ config.repoIsOnboarded = true;
config.api.getFileContent.mockReturnValueOnce(
'{ "enabled": true, "enabled": false }'
);
--
GitLab