Skip to content

Commit

Permalink
feat!: internalChecksAsSuccess (#20572)
Browse files Browse the repository at this point in the history
  • Loading branch information
rarkins committed Mar 8, 2023
1 parent aa1a8bb commit 3365726
Show file tree
Hide file tree
Showing 20 changed files with 281 additions and 48 deletions.
10 changes: 10 additions & 0 deletions docs/usage/configuration-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -1432,6 +1432,16 @@ If you wish for Renovate to process only select paths in the repository, use `in
Alternatively, if you need to just _exclude_ certain paths in the repository then consider `ignorePaths` instead.
If you are more interested in including only certain package managers (e.g. `npm`), then consider `enabledManagers` instead.

## internalChecksAsSuccess

By default, internal Renovate checks such as `renovate/stability-days` are not counted towards a branch being "green" or not.
This is primarily to prevent automerge when the only check is a passing Renovate check.

Internal checks will always be counted/considered if they are in pending or failed states.
If there are multiple passing checks for a branch, including non-Renovate ones, then this setting won't make any difference.

Change this setting to `true` if you want to use internal Renovate checks towards a passing branch result.

## internalChecksFilter

This setting determines whether Renovate controls when and how filtering of internal checks are performed, particularly when multiple versions of the same update type are available.
Expand Down
7 changes: 7 additions & 0 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,13 @@ const options: RenovateOptions[] = [
type: 'integer',
default: 0,
},
{
name: 'internalChecksAsSuccess',
description:
'Whether to consider passing internal checks such as stabilityDays when determining branch status.',
type: 'boolean',
default: false,
},
/*
* Undocumented experimental feature
{
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export interface RenovateSharedConfig {
ignoreDeps?: string[];
ignorePaths?: string[];
ignoreTests?: boolean;
internalChecksAsSuccess?: boolean;
labels?: string[];
addLabels?: string[];
dependencyDashboardApproval?: boolean;
Expand Down
26 changes: 22 additions & 4 deletions lib/modules/platform/azure/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,28 @@ describe('modules/platform/azure/index', () => {
]),
} as any)
);
const res = await azure.getBranchStatus('somebranch');
const res = await azure.getBranchStatus('somebranch', true);
expect(res).toBe('green');
});

it('should not treat internal checks as success', async () => {
await initRepo({ repository: 'some/repo' });
azureApi.gitApi.mockImplementationOnce(
() =>
({
getBranch: jest.fn(() => ({ commit: { commitId: 'abcd1234' } })),
getStatuses: jest.fn(() => [
{
state: GitStatusState.Succeeded,
context: { genre: 'renovate' },
},
]),
} as any)
);
const res = await azure.getBranchStatus('somebranch', false);
expect(res).toBe('yellow');
});

it('should pass through failed', async () => {
await initRepo({ repository: 'some/repo' });
azureApi.gitApi.mockImplementationOnce(
Expand All @@ -590,7 +608,7 @@ describe('modules/platform/azure/index', () => {
getStatuses: jest.fn(() => [{ state: GitStatusState.Error }]),
} as any)
);
const res = await azure.getBranchStatus('somebranch');
const res = await azure.getBranchStatus('somebranch', true);
expect(res).toBe('red');
});

Expand All @@ -603,7 +621,7 @@ describe('modules/platform/azure/index', () => {
getStatuses: jest.fn(() => [{ state: GitStatusState.Pending }]),
} as any)
);
const res = await azure.getBranchStatus('somebranch');
const res = await azure.getBranchStatus('somebranch', true);
expect(res).toBe('yellow');
});

Expand All @@ -616,7 +634,7 @@ describe('modules/platform/azure/index', () => {
getStatuses: jest.fn(() => []),
} as any)
);
const res = await azure.getBranchStatus('somebranch');
const res = await azure.getBranchStatus('somebranch', true);
expect(res).toBe('yellow');
});
});
Expand Down
16 changes: 15 additions & 1 deletion lib/modules/platform/azure/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ export async function getBranchStatusCheck(
}

export async function getBranchStatus(
branchName: string
branchName: string,
internalChecksAsSuccess: boolean
): Promise<BranchStatus> {
logger.debug(`getBranchStatus(${branchName})`);
const statuses = await getStatusCheck(branchName);
Expand All @@ -406,6 +407,19 @@ export async function getBranchStatus(
if (noOfPending) {
return 'yellow';
}
if (
!internalChecksAsSuccess &&
statuses.every(
(status) =>
status.state === GitStatusState.Succeeded &&
status.context?.genre === 'renovate'
)
) {
logger.debug(
'Successful checks are all internal renovate/ checks, so returning "pending" branch status'
);
return 'yellow';
}
return 'green';
}

Expand Down
26 changes: 18 additions & 8 deletions lib/modules/platform/bitbucket-server/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,9 @@ Followed by some information.
failed: 0,
});

expect(await bitbucket.getBranchStatus('somebranch')).toBe('green');
expect(await bitbucket.getBranchStatus('somebranch', true)).toBe(
'green'
);
});

it('should be pending', async () => {
Expand All @@ -1764,7 +1766,9 @@ Followed by some information.
failed: 0,
});

expect(await bitbucket.getBranchStatus('somebranch')).toBe('yellow');
expect(await bitbucket.getBranchStatus('somebranch', true)).toBe(
'yellow'
);

scope
.get(
Expand All @@ -1776,7 +1780,9 @@ Followed by some information.
failed: 0,
});

expect(await bitbucket.getBranchStatus('somebranch')).toBe('yellow');
expect(await bitbucket.getBranchStatus('somebranch', true)).toBe(
'yellow'
);
});

it('should be failed', async () => {
Expand All @@ -1791,23 +1797,27 @@ Followed by some information.
failed: 1,
});

expect(await bitbucket.getBranchStatus('somebranch')).toBe('red');
expect(await bitbucket.getBranchStatus('somebranch', true)).toBe(
'red'
);

scope
.get(
`${urlPath}/rest/build-status/1.0/commits/stats/0d9c7726c3d628b7e28af234595cfd20febdbf8e`
)
.replyWithError('requst-failed');

expect(await bitbucket.getBranchStatus('somebranch')).toBe('red');
expect(await bitbucket.getBranchStatus('somebranch', true)).toBe(
'red'
);
});

it('throws repository-changed', async () => {
git.branchExists.mockReturnValue(false);
await initRepo();
await expect(bitbucket.getBranchStatus('somebranch')).rejects.toThrow(
REPOSITORY_CHANGED
);
await expect(
bitbucket.getBranchStatus('somebranch', true)
).rejects.toThrow(REPOSITORY_CHANGED);
});
});

Expand Down
39 changes: 33 additions & 6 deletions lib/modules/platform/bitbucket/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ describe('modules/platform/bitbucket/index', () => {
},
],
});
expect(await bitbucket.getBranchStatus('master')).toBe('red');
expect(await bitbucket.getBranchStatus('master', true)).toBe('red');
});

it('getBranchStatus 4', async () => {
Expand All @@ -250,7 +250,7 @@ describe('modules/platform/bitbucket/index', () => {
},
],
});
expect(await bitbucket.getBranchStatus('branch')).toBe('green');
expect(await bitbucket.getBranchStatus('branch', true)).toBe('green');
});

it('getBranchStatus 5', async () => {
Expand All @@ -275,7 +275,9 @@ describe('modules/platform/bitbucket/index', () => {
},
],
});
expect(await bitbucket.getBranchStatus('pending/branch')).toBe('yellow');
expect(await bitbucket.getBranchStatus('pending/branch', true)).toBe(
'yellow'
);
});

it('getBranchStatus 6', async () => {
Expand All @@ -297,9 +299,34 @@ describe('modules/platform/bitbucket/index', () => {
.reply(200, {
values: [],
});
expect(await bitbucket.getBranchStatus('branch-with-empty-status')).toBe(
'yellow'
);
expect(
await bitbucket.getBranchStatus('branch-with-empty-status', true)
).toBe('yellow');
});

it('getBranchStatus 7', async () => {
const scope = await initRepoMock();
scope
.get('/2.0/repositories/some/repo/refs/branches/branch')
.reply(200, {
name: 'branch',
target: {
hash: 'branch_hash',
parents: [{ hash: 'master_hash' }],
},
})
.get(
'/2.0/repositories/some/repo/commit/branch_hash/statuses?pagelen=100'
)
.reply(200, {
values: [
{
key: 'renovate/stability-days',
state: 'SUCCESSFUL',
},
],
});
expect(await bitbucket.getBranchStatus('branch', false)).toBe('yellow');
});
});

Expand Down
15 changes: 14 additions & 1 deletion lib/modules/platform/bitbucket/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ async function getStatus(
}
// Returns the combined status for a branch.
export async function getBranchStatus(
branchName: string
branchName: string,
internalChecksAsSuccess: boolean
): Promise<BranchStatus> {
logger.debug(`getBranchStatus(${branchName})`);
const statuses = await getStatus(branchName);
Expand All @@ -377,6 +378,18 @@ export async function getBranchStatus(
if (noOfPending) {
return 'yellow';
}
if (
!internalChecksAsSuccess &&
statuses.every(
(status) =>
status.state === 'SUCCESSFUL' && status.key?.startsWith('renovate/')
)
) {
logger.debug(
'Successful checks are all internal renovate/ checks, so returning "pending" branch status'
);
return 'yellow';
}
return 'green';
}

Expand Down
43 changes: 40 additions & 3 deletions lib/modules/platform/gitea/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ describe('modules/platform/gitea/index', () => {
})
);

return gitea.getBranchStatus('some-branch');
return gitea.getBranchStatus('some-branch', true);
};

it('should return yellow for unknown result', async () => {
Expand All @@ -654,7 +654,7 @@ describe('modules/platform/gitea/index', () => {
it('should abort when branch status returns 404', async () => {
helper.getCombinedCommitStatus.mockRejectedValueOnce({ statusCode: 404 });

await expect(gitea.getBranchStatus('some-branch')).rejects.toThrow(
await expect(gitea.getBranchStatus('some-branch', true)).rejects.toThrow(
REPOSITORY_CHANGED
);
});
Expand All @@ -664,10 +664,47 @@ describe('modules/platform/gitea/index', () => {
new Error('getCombinedCommitStatus()')
);

await expect(gitea.getBranchStatus('some-branch')).rejects.toThrow(
await expect(gitea.getBranchStatus('some-branch', true)).rejects.toThrow(
'getCombinedCommitStatus()'
);
});

it('should treat internal checks as success', async () => {
helper.getCombinedCommitStatus.mockResolvedValueOnce({
worstStatus: 'success',
statuses: [
{
id: 1,
status: 'success',
context: 'renovate/stability-days',
description: 'internal check',
target_url: '',
created_at: '',
},
],
});
expect(await gitea.getBranchStatus('some-branch', true)).toBe('green');
});

it('should not treat internal checks as success', async () => {
await initFakeRepo();
helper.getCombinedCommitStatus.mockResolvedValueOnce(
partial<CombinedCommitStatus>({
worstStatus: 'success',
statuses: [
{
id: 1,
status: 'success',
context: 'renovate/stability-days',
description: 'internal check',
target_url: '',
created_at: '',
},
],
})
);
expect(await gitea.getBranchStatus('some-branch', false)).toBe('yellow');
});
});

describe('getBranchStatusCheck', () => {
Expand Down
16 changes: 15 additions & 1 deletion lib/modules/platform/gitea/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,10 @@ const platform: Platform = {
}
},

async getBranchStatus(branchName: string): Promise<BranchStatus> {
async getBranchStatus(
branchName: string,
internalChecksAsSuccess: boolean
): Promise<BranchStatus> {
let ccs: CombinedCommitStatus;
try {
ccs = await helper.getCombinedCommitStatus(config.repository, branchName);
Expand All @@ -404,6 +407,17 @@ const platform: Platform = {
}

logger.debug({ ccs }, 'Branch status check result');
if (
!internalChecksAsSuccess &&
ccs.worstStatus === 'success' &&
ccs.statuses.every((status) => status.context?.startsWith('renovate/'))
) {
logger.debug(
'Successful checks are all internal renovate/ checks, so returning "pending" branch status'
);
return 'yellow';
}

return helper.giteaToRenovateStatusMapping[ccs.worstStatus] ?? 'yellow';
},

Expand Down
Loading

0 comments on commit 3365726

Please sign in to comment.