From d3c9fd583a8c56903b3ca1abe9e859c95dedd54c Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Fri, 17 Sep 2021 10:49:37 +0200
Subject: [PATCH] fix(worker/branch): optimize artifact error handling (#11771)
---
lib/workers/branch/artifacts.spec.ts | 47 ++++++++++++++++++++++++++++
lib/workers/branch/artifacts.ts | 37 ++++++++++++++++++++++
lib/workers/branch/index.ts | 27 +++-------------
3 files changed, 88 insertions(+), 23 deletions(-)
create mode 100644 lib/workers/branch/artifacts.spec.ts
create mode 100644 lib/workers/branch/artifacts.ts
diff --git a/lib/workers/branch/artifacts.spec.ts b/lib/workers/branch/artifacts.spec.ts
new file mode 100644
index 0000000000..d185d13c06
--- /dev/null
+++ b/lib/workers/branch/artifacts.spec.ts
@@ -0,0 +1,47 @@
+import { getConfig, platform } from '../../../test/util';
+import { setGlobalConfig } from '../../config/global';
+import { BranchStatus } from '../../types';
+import { BranchConfig } from '../types';
+import { setArtifactErrorStatus } from './artifacts';
+
+describe('workers/branch/artifacts', () => {
+ let config: BranchConfig;
+ beforeEach(() => {
+ setGlobalConfig({});
+ jest.resetAllMocks();
+ config = {
+ ...getConfig(),
+ branchName: 'renovate/pin',
+ upgrades: [],
+ artifactErrors: [{ lockFile: 'some' }],
+ };
+ });
+
+ describe('setArtifactsErrorStatus', () => {
+ it('adds status', async () => {
+ platform.getBranchStatusCheck.mockResolvedValueOnce(null);
+ await setArtifactErrorStatus(config);
+ expect(platform.setBranchStatus).toHaveBeenCalled();
+ });
+
+ it('skips status', async () => {
+ platform.getBranchStatusCheck.mockResolvedValueOnce(BranchStatus.red);
+ await setArtifactErrorStatus(config);
+ expect(platform.setBranchStatus).not.toHaveBeenCalled();
+ });
+
+ it('skips status (dry-run)', async () => {
+ setGlobalConfig({ dryRun: true });
+ platform.getBranchStatusCheck.mockResolvedValueOnce(null);
+ await setArtifactErrorStatus(config);
+ expect(platform.setBranchStatus).not.toHaveBeenCalled();
+ });
+
+ it('skips status (no errors)', async () => {
+ platform.getBranchStatusCheck.mockResolvedValueOnce(null);
+ config.artifactErrors.length = 0;
+ await setArtifactErrorStatus(config);
+ expect(platform.setBranchStatus).not.toHaveBeenCalled();
+ });
+ });
+});
diff --git a/lib/workers/branch/artifacts.ts b/lib/workers/branch/artifacts.ts
new file mode 100644
index 0000000000..6c982bb99c
--- /dev/null
+++ b/lib/workers/branch/artifacts.ts
@@ -0,0 +1,37 @@
+import { getGlobalConfig } from '../../config/global';
+import { logger } from '../../logger';
+import { platform } from '../../platform';
+import { BranchStatus } from '../../types';
+import type { BranchConfig } from '../types';
+
+export async function setArtifactErrorStatus(
+ config: BranchConfig
+): Promise<void> {
+ if (!config.artifactErrors?.length) {
+ // no errors
+ return;
+ }
+
+ const context = `renovate/artifacts`;
+ const description = 'Artifact file update failure';
+ const state = BranchStatus.red;
+ const existingState = await platform.getBranchStatusCheck(
+ config.branchName,
+ context
+ );
+
+ // Check if state needs setting
+ if (existingState !== state) {
+ logger.debug(`Updating status check state to failed`);
+ if (getGlobalConfig().dryRun) {
+ logger.info('DRY-RUN: Would set branch status in ' + config.branchName);
+ } else {
+ await platform.setBranchStatus({
+ branchName: config.branchName,
+ context,
+ description,
+ state,
+ });
+ }
+ }
+}
diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts
index 96b33b9493..da89344593 100644
--- a/lib/workers/branch/index.ts
+++ b/lib/workers/branch/index.ts
@@ -36,6 +36,7 @@ import { Limit, isLimitReached } from '../global/limits';
import { ensurePr, getPlatformPrOptions } from '../pr';
import { checkAutoMerge } from '../pr/automerge';
import { BranchConfig, BranchResult, PrBlockedBy } from '../types';
+import { setArtifactErrorStatus } from './artifacts';
import { tryBranchAutomerge } from './automerge';
import { prAlreadyExisted } from './check-existing';
import { commitFilesToBranch } from './commit';
@@ -456,11 +457,14 @@ export async function processBranch(
logger.info({ commitSha }, `Branch ${action}`);
}
// Set branch statuses
+ await setArtifactErrorStatus(config);
await setStability(config);
await setConfidence(config);
// break if we pushed a new commit because status check are pretty sure pending but maybe not reported yet
+ // but do not break when there are artifact errors
if (
+ !config.artifactErrors?.length &&
!dependencyDashboardCheck &&
!config.rebaseRequested &&
commitSha &&
@@ -669,29 +673,6 @@ export async function processBranch(
});
}
}
- const context = `renovate/artifacts`;
- const description = 'Artifact file update failure';
- const state = BranchStatus.red;
- const existingState = await platform.getBranchStatusCheck(
- config.branchName,
- context
- );
- // Check if state needs setting
- if (existingState !== state) {
- logger.debug(`Updating status check state to failed`);
- if (getGlobalConfig().dryRun) {
- logger.info(
- 'DRY-RUN: Would set branch status in ' + config.branchName
- );
- } else {
- await platform.setBranchStatus({
- branchName: config.branchName,
- context,
- description,
- state,
- });
- }
- }
} else if (config.automerge) {
logger.debug('PR is configured for automerge');
const prAutomergeResult = await checkAutoMerge(pr, config);
--
GitLab