Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(worker/branch): optimize artifact error handling #11771

Merged
merged 6 commits into from Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions lib/workers/branch/artifacts.spec.ts
@@ -0,0 +1,39 @@
import { getConfig, platform } from '../../../test/util';
import { setGlobalConfig } from '../../config/global';
import { BranchStatus } from '../../types';
import { BranchConfig } from '../types';
import { setArtifactErrorStatus } from './artifacts';

const config: BranchConfig = {
...getConfig(),
branchName: 'renovate/pin',
upgrades: [],
};

describe('workers/branch/artifacts', () => {
beforeEach(() => {
setGlobalConfig({});
jest.resetAllMocks();
});

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();
});
});
});
31 changes: 31 additions & 0 deletions lib/workers/branch/artifacts.ts
@@ -0,0 +1,31 @@
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> {
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,
});
}
}
}
27 changes: 4 additions & 23 deletions lib/workers/branch/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -393,6 +394,7 @@ export async function processBranch(
removeMeta(['dep']);

if (config.artifactErrors?.length) {
await setArtifactErrorStatus(config);
viceice marked this conversation as resolved.
Show resolved Hide resolved
if (config.releaseTimestamp) {
logger.debug(`Branch timestamp: ` + config.releaseTimestamp);
const releaseTimestamp = DateTime.fromISO(config.releaseTimestamp);
Expand Down Expand Up @@ -460,7 +462,9 @@ export async function processBranch(
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 &&
Expand Down Expand Up @@ -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);
Expand Down