From bae9ae0595ebf0e6b12f050e7f12c37f3a18c9a1 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Sat, 2 Sep 2017 08:51:49 +0200
Subject: [PATCH] feat: stop branch processing after lock file error or pin
dependencies (#768)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If a repository has a lock file error (e.g. can’t look up a private module) then it will no longer attempt to create every branch. Instead, it will error/exit after the first branch. Additionally, “Pin Dependencies†has been sorted to be first and further branches won’t be added or updated until Pin Dependencies has been merged.
---
lib/workers/branch/index.js | 8 +--
lib/workers/repository/index.js | 45 ++++++++++-----
.../__snapshots__/index.spec.js.snap | 51 +++++++++++++++++
test/workers/repository/index.spec.js | 57 +++++++++++++++++++
4 files changed, 141 insertions(+), 20 deletions(-)
create mode 100644 test/workers/repository/__snapshots__/index.spec.js.snap
diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js
index fce07cf9b4..ef803acd49 100644
--- a/lib/workers/branch/index.js
+++ b/lib/workers/branch/index.js
@@ -52,7 +52,7 @@ async function processBranch(branchConfig) {
}
Object.assign(config, await getUpdatedLockFiles(config));
if (config.lockFileError) {
- throw new Error('lockFileError');
+ return 'lockFileError';
}
if (config.updatedLockFiles.length) {
logger.debug(
@@ -81,11 +81,7 @@ async function processBranch(branchConfig) {
return 'automerged';
}
} catch (err) {
- if (err.message !== 'lockFileError') {
- logger.error({ err }, `Error updating branch: ${err.message}`);
- } else {
- logger.info('Error updating branch');
- }
+ logger.error({ err }, `Error updating branch: ${err.message}`);
// Don't throw here - we don't want to stop the other renovations
return 'error';
}
diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js
index 61f8c6899b..709a6ba7bd 100644
--- a/lib/workers/repository/index.js
+++ b/lib/workers/repository/index.js
@@ -10,9 +10,20 @@ const cleanup = require('./cleanup');
const { decryptConfig } = require('../../config/decrypt');
module.exports = {
+ pinDependenciesFirst,
renovateRepository,
};
+function pinDependenciesFirst(a, b) {
+ if (a.type === 'pin') {
+ return false;
+ }
+ if (b.type === 'pin') {
+ return true;
+ }
+ return a.branchName > b.branchName;
+}
+
async function renovateRepository(repoConfig, token) {
let config = { ...repoConfig };
const { logger } = config;
@@ -111,22 +122,28 @@ async function renovateRepository(repoConfig, token) {
logger.debug(`Updating ${branchUpgrades.length} branch(es)`);
logger.trace({ config: branchUpgrades }, 'branchUpgrades');
if (config.repoIsOnboarded) {
+ logger.info(`Processing ${branchUpgrades.length} branch(es)`);
+ // eslint-disable-next-line no-loop-function
+ branchUpgrades.sort(pinDependenciesFirst);
for (const branchUpgrade of branchUpgrades) {
- if (!baseBranchUpdated) {
- const branchResult = await branchWorker.processBranch(
- branchUpgrade,
- config.errors,
- config.warnings
- );
- if (branchResult === 'automerged') {
- // Stop procesing other branches because base branch has been changed by an automerge
- logger.info('Restarting repo renovation after automerge');
- baseBranchUpdated = true;
- }
- } else {
- logger.debug(
- `Skipping branchUpgrade as base branch has been modified`
+ const branchResult = await branchWorker.processBranch(
+ branchUpgrade,
+ config.errors,
+ config.warnings
+ );
+ if (branchResult === 'automerged') {
+ // Stop procesing other branches because base branch has been changed by an automerge
+ logger.info('Restarting repo renovation after automerge');
+ baseBranchUpdated = true;
+ break;
+ } else if (branchResult === 'lockFileError') {
+ logger.info('Lock file error - stopping branch updates');
+ break;
+ } else if (branchUpgrade.type === 'pin') {
+ logger.info(
+ 'Stopping branch processing until Pin Dependencies is merged'
);
+ break;
}
}
branchList = branchUpgrades.map(upgrade => upgrade.branchName);
diff --git a/test/workers/repository/__snapshots__/index.spec.js.snap b/test/workers/repository/__snapshots__/index.spec.js.snap
new file mode 100644
index 0000000000..d73bfc6c3d
--- /dev/null
+++ b/test/workers/repository/__snapshots__/index.spec.js.snap
@@ -0,0 +1,51 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`workers/repository pinDependenciesFirst returns pin first 1`] = `
+Array [
+ Object {
+ "branchName": "d",
+ "type": "pin",
+ },
+ Object {
+ "branchName": "a",
+ },
+ Object {
+ "branchName": "b",
+ },
+ Object {
+ "branchName": "c",
+ },
+]
+`;
+
+exports[`workers/repository pinDependenciesFirst returns pin first 2`] = `
+Array [
+ Object {
+ "branchName": "d",
+ "type": "pin",
+ },
+ Object {
+ "branchName": "a",
+ },
+ Object {
+ "branchName": "b",
+ },
+ Object {
+ "branchName": "c",
+ },
+]
+`;
+
+exports[`workers/repository pinDependenciesFirst returns sorted if no pin 1`] = `
+Array [
+ Object {
+ "branchName": "a",
+ },
+ Object {
+ "branchName": "b",
+ },
+ Object {
+ "branchName": "c",
+ },
+]
+`;
diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js
index eeeb26b130..f6b87e2af8 100644
--- a/test/workers/repository/index.spec.js
+++ b/test/workers/repository/index.spec.js
@@ -8,6 +8,37 @@ const upgrades = require('../../../lib/workers/repository/upgrades');
const logger = require('../../_fixtures/logger');
describe('workers/repository', () => {
+ describe('pinDependenciesFirst', () => {
+ it('returns sorted if no pin', () => {
+ const arr = [
+ { branchName: 'a' },
+ { branchName: 'c' },
+ { branchName: 'b' },
+ ];
+ arr.sort(repositoryWorker.pinDependenciesFirst);
+ expect(arr).toMatchSnapshot();
+ });
+ it('returns pin first', () => {
+ const arr = [
+ { branchName: 'a' },
+ { branchName: 'c' },
+ { branchName: 'd', type: 'pin' },
+ { branchName: 'b' },
+ ];
+ arr.sort(repositoryWorker.pinDependenciesFirst);
+ expect(arr).toMatchSnapshot();
+ });
+ it('returns pin first', () => {
+ const arr = [
+ { branchName: 'd', type: 'pin' },
+ { branchName: 'a' },
+ { branchName: 'c' },
+ { branchName: 'b' },
+ ];
+ arr.sort(repositoryWorker.pinDependenciesFirst);
+ expect(arr).toMatchSnapshot();
+ });
+ });
describe('renovateRepository', () => {
let config;
beforeEach(() => {
@@ -175,6 +206,32 @@ describe('workers/repository', () => {
expect(branchWorker.processBranch.mock.calls).toHaveLength(3);
expect(config.logger.error.mock.calls).toHaveLength(0);
});
+ it('stops branchWorker after lockFileError', async () => {
+ config.packageFiles = ['package.json'];
+ config.hasRenovateJson = true;
+ onboarding.getOnboardingStatus.mockReturnValue(true);
+ upgrades.branchifyUpgrades.mockReturnValueOnce({
+ upgrades: [{}, {}, {}],
+ });
+ branchWorker.processBranch.mockReturnValue('lockFileError');
+ await repositoryWorker.renovateRepository(config);
+ expect(upgrades.branchifyUpgrades.mock.calls).toHaveLength(1);
+ expect(branchWorker.processBranch.mock.calls).toHaveLength(1);
+ expect(config.logger.error.mock.calls).toHaveLength(0);
+ });
+ it('stops branchWorker after pin', async () => {
+ config.packageFiles = ['package.json'];
+ config.hasRenovateJson = true;
+ onboarding.getOnboardingStatus.mockReturnValue(true);
+ upgrades.branchifyUpgrades.mockReturnValueOnce({
+ upgrades: [{ type: 'pin' }, {}, {}],
+ });
+ branchWorker.processBranch.mockReturnValue('done');
+ await repositoryWorker.renovateRepository(config);
+ expect(upgrades.branchifyUpgrades.mock.calls).toHaveLength(1);
+ expect(branchWorker.processBranch.mock.calls).toHaveLength(1);
+ expect(config.logger.error.mock.calls).toHaveLength(0);
+ });
it('swallows errors', async () => {
apis.initApis.mockImplementationOnce(() => {
throw new Error('bad init');
--
GitLab