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

feat(automerge): merge failure comment #33337

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1997,6 +1997,13 @@ const options: RenovateOptions[] = [
type: 'string',
default: 'automergeComment',
},
{
name: 'automergeFailureComment',
description:
'If an error occurs while automerging, a comment will be created to signal the user. Only used if `automergeFailureComment=on-error`.',
type: 'string',
default: 'never',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set allowedValues

},
{
name: 'ignoreTests',
description: 'Set to `true` to enable automerging without tests.',
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export interface RenovateSharedConfig {
addLabels?: string[];
autoReplaceGlobalMatch?: boolean;
automerge?: boolean;
autoMergeFailureComment?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an enum type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
autoMergeFailureComment?: string;
autoMergeFailureComment?: 'on-error' | 'never';

like this

automergeSchedule?: string[];
automergeStrategy?: MergeStrategy;
branchName?: string;
Expand Down
23 changes: 23 additions & 0 deletions lib/modules/platform/github/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ describe('modules/platform/github/index', () => {
isArchived: false,
nameWithOwner: repository,
autoMergeAllowed: true,
automergeFailureComment: 'never',
hasIssuesEnabled: true,
mergeCommitAllowed: true,
rebaseMergeAllowed: true,
Expand Down Expand Up @@ -2798,6 +2799,18 @@ describe('modules/platform/github/index', () => {
platformPrOptions: { usePlatformAutomerge: true },
};

const prConfigComment: CreatePRConfig = {
sourceBranch: 'some-branch',
targetBranch: 'dev',
prTitle: 'The Title',
prBody: 'Hello world',
labels: ['deps', 'renovate'],
platformPrOptions: {
usePlatformAutomerge: true,
automergeFailureComment: 'on-error',
},
};

const mockScope = async (repoOpts: any = {}): Promise<httpMock.Scope> => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo', repoOpts);
Expand Down Expand Up @@ -2975,6 +2988,16 @@ describe('modules/platform/github/index', () => {
]);
});

it('should handle GraphQL errors with automergeFailureComment', async () => {
const scope = await mockScope({ automergeFailureComment: 'on-error' });
scope
.post('/repos/some/repo/issues/123/comments')
.reply(200)
.post('/graphql')
.reply(200, graphqlAutomergeErrorResp);
await expect(github.createPr(prConfigComment)).toResolve();
});

it('should handle REST API errors', async () => {
const scope = await mockScope();
scope.post('/graphql').reply(500);
Expand Down
10 changes: 9 additions & 1 deletion lib/modules/platform/github/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1694,13 +1694,21 @@ async function tryPrAutomerge(
{ prNumber, errors: res.errors },
'GitHub-native automerge: fail',
);
if (platformPrOptions.automergeFailureComment === 'on-error') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong file for implementation. This only works for GitHub, and even then only for GitHub- active automerge.

logger.warn('This automerge request failed');
await addComment(
prNumber,
'The Automerge Request for this PR has failed. Please attend.',
);
return;
}
return;
}

logger.debug(`GitHub-native automerge: success...PrNo: ${prNumber}`);
} catch (err) /* istanbul ignore next: missing test #22198 */ {
logger.warn({ prNumber, err }, 'GitHub-native automerge: REST API error');
}
return;
}

// Creates PR and returns PR number
Expand Down
1 change: 1 addition & 0 deletions lib/modules/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export interface Issue {
export type PlatformPrOptions = {
autoApprove?: boolean;
automergeStrategy?: MergeStrategy;
automergeFailureComment?: string;
azureWorkItemId?: number;
bbUseDefaultReviewers?: boolean;
bbAutoResolvePrTasks?: boolean;
Expand Down
1 change: 1 addition & 0 deletions lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function getPlatformPrOptions(
return {
autoApprove: !!config.autoApprove,
automergeStrategy: config.automergeStrategy,
automergeFailureComment: config.automergeFailureComment,
azureWorkItemId: config.azureWorkItemId ?? 0,
bbAutoResolvePrTasks: !!config.bbAutoResolvePrTasks,
bbUseDefaultReviewers: !!config.bbUseDefaultReviewers,
Expand Down
1 change: 1 addition & 0 deletions lib/workers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface BranchConfig
LegacyAdminConfig,
PlatformPrOptions {
automergeComment?: string;
automergeFailureComment?: string;
automergeType?: string;
automergedPreviously?: boolean;
baseBranch: string;
Expand Down
81 changes: 47 additions & 34 deletions test/documentation.spec.ts
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated changes

Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ describe('documentation', () => {
});

describe('website-documentation', () => {
function getConfigOptionSubHeaders(
async function getConfigOptionSubHeaders(
file: string,
configOption: string,
): string[] {
): Promise<string[]> {
const subHeadings = [];
const content = fs.readFileSync(`docs/usage/${file}`, 'utf8');
const content = await fs.readFile(`docs/usage/${file}`, 'utf8');
const reg = regEx(`##\\s${configOption}[\\s\\S]+?\n##\\s`);
const match = reg.exec(content);
const subHeadersMatch = match?.[0]?.matchAll(/\n###\s(?<child>\w+)\n/g);
Expand All @@ -39,8 +39,8 @@ describe('documentation', () => {
}

describe('docs/usage/configuration-options.md', () => {
function getConfigHeaders(file: string): string[] {
const content = fs.readFileSync(`docs/usage/${file}`, 'utf8');
async function getConfigHeaders(file: string): Promise<string[]> {
const content = await fs.readFile(`docs/usage/${file}`, 'utf8');
const matches = content.match(/\n## (.*?)\n/g) ?? [];
return matches.map((match) => match.substring(4, match.length - 1));
}
Expand All @@ -54,20 +54,20 @@ describe('documentation', () => {
.sort();
}

it('has doc headers sorted alphabetically', () => {
expect(getConfigHeaders('configuration-options.md')).toEqual(
getConfigHeaders('configuration-options.md').sort(),
it('has doc headers sorted alphabetically', async () => {
expect(await getConfigHeaders('configuration-options.md')).toEqual(
(await getConfigHeaders('configuration-options.md')).sort(),
);
});

it('has headers for every required option', () => {
expect(getConfigHeaders('configuration-options.md')).toEqual(
it('has headers for every required option', async () => {
expect(await getConfigHeaders('configuration-options.md')).toEqual(
getRequiredConfigOptions(),
);
});

function getConfigSubHeaders(file: string): string[] {
const content = fs.readFileSync(`docs/usage/${file}`, 'utf8');
async function getConfigSubHeaders(file: string): Promise<string[]> {
const content = await fs.readFile(`docs/usage/${file}`, 'utf8');
const matches = content.match(/\n### (.*?)\n/g) ?? [];
return matches
.map((match) => match.substring(5, match.length - 1))
Expand Down Expand Up @@ -100,30 +100,35 @@ describe('documentation', () => {
return parentNames;
}

it('has headers for every required sub-option', () => {
expect(getConfigSubHeaders('configuration-options.md')).toEqual(
it('has headers for every required sub-option', async () => {
expect(await getConfigSubHeaders('configuration-options.md')).toEqual(
getRequiredConfigSubOptions(),
);
});

it.each([...getParentNames()])(
test.each([...getParentNames()])(
'%s has sub-headers sorted alphabetically',
(parentName: string) => {
async (parentName: string) => {
expect(
getConfigOptionSubHeaders('configuration-options.md', parentName),
).toEqual(
getConfigOptionSubHeaders(
await getConfigOptionSubHeaders(
'configuration-options.md',
parentName,
),
).toEqual(
(
await getConfigOptionSubHeaders(
'configuration-options.md',
parentName,
)
).sort(),
);
},
);
});

describe('docs/usage/self-hosted-configuration.md', () => {
function getSelfHostedHeaders(file: string): string[] {
const content = fs.readFileSync(`docs/usage/${file}`, 'utf8');
async function getSelfHostedHeaders(file: string): Promise<string[]> {
const content = await fs.readFile(`docs/usage/${file}`, 'utf8');
const matches = content.match(/\n## (.*?)\n/g) ?? [];
return matches.map((match) => match.substring(4, match.length - 1));
}
Expand All @@ -135,32 +140,40 @@ describe('documentation', () => {
.sort();
}

it('has headers sorted alphabetically', () => {
expect(getSelfHostedHeaders('self-hosted-configuration.md')).toEqual(
getSelfHostedHeaders('self-hosted-configuration.md').sort(),
it('has headers sorted alphabetically', async () => {
expect(
await getSelfHostedHeaders('self-hosted-configuration.md'),
).toEqual(
(await getSelfHostedHeaders('self-hosted-configuration.md')).sort(),
);
});

it('has headers for every required option', () => {
expect(getSelfHostedHeaders('self-hosted-configuration.md')).toEqual(
getRequiredSelfHostedOptions(),
);
it('has headers for every required option', async () => {
expect(
await getSelfHostedHeaders('self-hosted-configuration.md'),
).toEqual(getRequiredSelfHostedOptions());
});
});

describe('docs/usage/self-hosted-experimental.md', () => {
function getSelfHostedExperimentalConfigHeaders(file: string): string[] {
const content = fs.readFileSync(`docs/usage/${file}`, 'utf8');
async function getSelfHostedExperimentalConfigHeaders(
file: string,
): Promise<string[]> {
const content = await fs.readFile(`docs/usage/${file}`, 'utf8');
const matches = content.match(/\n## (.*?)\n/g) ?? [];
return matches.map((match) => match.substring(4, match.length - 1));
}

it('has headers sorted alphabetically', () => {
it('has headers sorted alphabetically', async () => {
expect(
getSelfHostedExperimentalConfigHeaders('self-hosted-experimental.md'),
).toEqual(
getSelfHostedExperimentalConfigHeaders(
await getSelfHostedExperimentalConfigHeaders(
'self-hosted-experimental.md',
),
).toEqual(
(
await getSelfHostedExperimentalConfigHeaders(
'self-hosted-experimental.md',
)
).sort(),
);
});
Expand Down