refactor(platform): mergePr args to object (#11155)

This commit is contained in:
Maksim 2021-08-08 05:35:35 +02:00 committed by GitHub
parent 1392955a04
commit 1bb004db4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 175 additions and 60 deletions

View file

@ -1080,7 +1080,10 @@ describe(getName(), () => {
.fn()
.mockReturnValue(GitPullRequestMergeStrategy.Squash);
const res = await azure.mergePr(pullRequestIdMock, branchNameMock);
const res = await azure.mergePr({
branchName: branchNameMock,
id: pullRequestIdMock,
});
expect(updatePullRequestMock).toHaveBeenCalledWith(
{
@ -1118,7 +1121,10 @@ describe(getName(), () => {
.fn()
.mockReturnValue(GitPullRequestMergeStrategy.Squash);
const res = await azure.mergePr(pullRequestIdMock, branchNameMock);
const res = await azure.mergePr({
branchName: branchNameMock,
id: pullRequestIdMock,
});
expect(res).toBe(false);
});
@ -1138,8 +1144,14 @@ describe(getName(), () => {
.fn()
.mockReturnValue(GitPullRequestMergeStrategy.Squash);
await azure.mergePr(1234, 'test-branch-1');
await azure.mergePr(5678, 'test-branch-2');
await azure.mergePr({
branchName: 'test-branch-1',
id: 1234,
});
await azure.mergePr({
branchName: 'test-branch-2',
id: 5678,
});
expect(azureHelper.getMergeMethod).toHaveBeenCalledTimes(1);
});
@ -1167,7 +1179,10 @@ describe(getName(), () => {
.fn()
.mockReturnValue(GitPullRequestMergeStrategy.Squash);
const res = await azure.mergePr(pullRequestIdMock, branchNameMock);
const res = await azure.mergePr({
branchName: branchNameMock,
id: pullRequestIdMock,
});
expect(getPullRequestByIdMock).toHaveBeenCalledTimes(2);
expect(res).toBe(true);
@ -1197,7 +1212,10 @@ describe(getName(), () => {
.fn()
.mockReturnValue(GitPullRequestMergeStrategy.Squash);
const res = await azure.mergePr(pullRequestIdMock, branchNameMock);
const res = await azure.mergePr({
branchName: branchNameMock,
id: pullRequestIdMock,
});
expect(getPullRequestByIdMock).toHaveBeenCalledTimes(
expectedNumRetries + 1

View file

@ -24,6 +24,7 @@ import type {
EnsureIssueResult,
FindPRConfig,
Issue,
MergePRConfig,
PlatformParams,
PlatformResult,
Pr,
@ -608,10 +609,10 @@ export async function setBranchStatus({
logger.trace(`Created commit status of ${state} on branch ${branchName}`);
}
export async function mergePr(
pullRequestId: number,
branchName: string
): Promise<boolean> {
export async function mergePr({
branchName,
id: pullRequestId,
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${pullRequestId}, ${branchName})`);
const azureApiGit = await azureApi.gitApi();

View file

@ -1598,13 +1598,21 @@ describe(getName(), () => {
)
.reply(200);
expect(await bitbucket.mergePr(5, 'branch')).toBe(true);
expect(
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
})
).toBe(true);
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('throws not-found 1', async () => {
await initRepo();
const res = bitbucket.mergePr(null as any, 'branch');
const res = bitbucket.mergePr({
branchName: 'branch',
id: null as any,
});
await expect(res).rejects.toThrow(REPOSITORY_NOT_FOUND);
expect(httpMock.getTrace()).toMatchSnapshot();
});
@ -1617,9 +1625,12 @@ describe(getName(), () => {
)
.reply(404);
await expect(bitbucket.mergePr(4, 'branch')).rejects.toThrow(
REPOSITORY_NOT_FOUND
);
await expect(
bitbucket.mergePr({
branchName: 'branch',
id: 4,
})
).rejects.toThrow(REPOSITORY_NOT_FOUND);
expect(httpMock.getTrace()).toMatchSnapshot();
});
@ -1639,9 +1650,12 @@ describe(getName(), () => {
)
.reply(404);
await expect(bitbucket.mergePr(5, 'branch')).rejects.toThrow(
REPOSITORY_NOT_FOUND
);
await expect(
bitbucket.mergePr({
branchName: 'branch',
id: 5,
})
).rejects.toThrow(REPOSITORY_NOT_FOUND);
expect(httpMock.getTrace()).toMatchSnapshot();
});
@ -1661,7 +1675,12 @@ describe(getName(), () => {
)
.reply(409);
expect(await bitbucket.mergePr(5, 'branch')).toBeFalsy();
expect(
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
})
).toBeFalsy();
expect(httpMock.getTrace()).toMatchSnapshot();
});
@ -1681,7 +1700,12 @@ describe(getName(), () => {
)
.reply(405);
await expect(bitbucket.mergePr(5, 'branch')).resolves.toBe(false);
await expect(
bitbucket.mergePr({
branchName: 'branch',
id: 5,
})
).resolves.toBe(false);
expect(httpMock.getTrace()).toMatchSnapshot();
});
});

View file

@ -31,6 +31,7 @@ import type {
EnsureIssueResult,
FindPRConfig,
Issue,
MergePRConfig,
PlatformParams,
PlatformResult,
Pr,
@ -962,10 +963,10 @@ export async function updatePr({
}
// https://docs.atlassian.com/bitbucket-server/rest/6.0.0/bitbucket-rest.html#idp261
export async function mergePr(
prNo: number,
branchName: string
): Promise<boolean> {
export async function mergePr({
branchName,
id: prNo,
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${prNo}, ${branchName})`);
// Used for "automerge" feature
try {

View file

@ -815,40 +815,63 @@ describe(getName(), () => {
it('posts Merge with optional merge strategy', async () => {
const scope = await initRepoMock();
scope.post('/2.0/repositories/some/repo/pullrequests/5/merge').reply(200);
await bitbucket.mergePr(5, 'branch');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('posts Merge with auto', async () => {
const scope = await initRepoMock();
scope.post('/2.0/repositories/some/repo/pullrequests/5/merge').reply(200);
await bitbucket.mergePr(5, 'branch', 'auto');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
strategy: 'auto',
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('posts Merge with merge-commit', async () => {
const scope = await initRepoMock();
scope.post('/2.0/repositories/some/repo/pullrequests/5/merge').reply(200);
await bitbucket.mergePr(5, 'branch', 'merge-commit');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
strategy: 'merge-commit',
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('posts Merge with squash', async () => {
const scope = await initRepoMock();
scope.post('/2.0/repositories/some/repo/pullrequests/5/merge').reply(200);
await bitbucket.mergePr(5, 'branch', 'squash');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
strategy: 'squash',
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('does not post Merge with rebase', async () => {
await bitbucket.mergePr(5, 'branch', 'rebase');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
strategy: 'rebase',
});
expect(httpMock.getTrace()).toEqual([]);
});
it('posts Merge with fast-forward', async () => {
const scope = await initRepoMock();
scope.post('/2.0/repositories/some/repo/pullrequests/5/merge').reply(200);
await bitbucket.mergePr(5, 'branch', 'fast-forward');
await bitbucket.mergePr({
branchName: 'branch',
id: 5,
strategy: 'fast-forward',
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
});

View file

@ -1,7 +1,6 @@
import URL from 'url';
import is from '@sindresorhus/is';
import parseDiff from 'parse-diff';
import type { MergeStrategy } from '../../config/types';
import { REPOSITORY_NOT_FOUND } from '../../constants/error-messages';
import { PLATFORM_TYPE_BITBUCKET } from '../../constants/platforms';
import { logger } from '../../logger';
@ -19,6 +18,7 @@ import type {
EnsureIssueResult,
FindPRConfig,
Issue,
MergePRConfig,
PlatformParams,
PlatformResult,
Pr,
@ -747,11 +747,11 @@ export async function updatePr({
}
}
export async function mergePr(
prNo: number,
branchName: string,
mergeStrategy: MergeStrategy
): Promise<boolean> {
export async function mergePr({
branchName,
id: prNo,
strategy: mergeStrategy,
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${prNo}, ${branchName}, ${mergeStrategy})`);
// Bitbucket Cloud does not support a rebase-alike; https://jira.atlassian.com/browse/BCLOUD-16610

View file

@ -899,7 +899,12 @@ describe(getName(), () => {
it('should return true when merging succeeds', async () => {
await initFakeRepo();
expect(await gitea.mergePr(1, 'some-branch')).toEqual(true);
expect(
await gitea.mergePr({
branchName: 'some-branch',
id: 1,
})
).toEqual(true);
expect(helper.mergePR).toHaveBeenCalledTimes(1);
expect(helper.mergePR).toHaveBeenCalledWith(
mockRepo.full_name,
@ -912,7 +917,12 @@ describe(getName(), () => {
helper.mergePR.mockRejectedValueOnce(new Error());
await initFakeRepo();
expect(await gitea.mergePr(1, 'some-branch')).toEqual(false);
expect(
await gitea.mergePr({
branchName: 'some-branch',
id: 1,
})
).toEqual(false);
});
});

View file

@ -25,6 +25,7 @@ import type {
EnsureIssueConfig,
FindPRConfig,
Issue,
MergePRConfig,
Platform,
PlatformParams,
PlatformResult,
@ -563,12 +564,12 @@ const platform: Platform = {
});
},
async mergePr(number: number, branchName: string): Promise<boolean> {
async mergePr({ id }: MergePRConfig): Promise<boolean> {
try {
await helper.mergePR(config.repository, number, config.mergeMethod);
await helper.mergePR(config.repository, id, config.mergeMethod);
return true;
} catch (err) {
logger.warn({ err, number }, 'Merging of PR failed');
logger.warn({ err, id }, 'Merging of PR failed');
return false;
}
},

View file

@ -2040,7 +2040,12 @@ describe(getName(), () => {
ref: 'someref',
},
};
expect(await github.mergePr(pr.number, '')).toBe(true);
expect(
await github.mergePr({
branchName: '',
id: pr.number,
})
).toBe(true);
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('should handle merge error', async () => {
@ -2056,7 +2061,12 @@ describe(getName(), () => {
ref: 'someref',
},
};
expect(await github.mergePr(pr.number, '')).toBe(false);
expect(
await github.mergePr({
branchName: '',
id: pr.number,
})
).toBe(false);
expect(httpMock.getTrace()).toMatchSnapshot();
});
});
@ -2104,7 +2114,12 @@ describe(getName(), () => {
ref: 'someref',
},
};
expect(await github.mergePr(pr.number, '')).toBe(true);
expect(
await github.mergePr({
branchName: '',
id: pr.number,
})
).toBe(true);
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('should try squash after rebase', async () => {
@ -2120,7 +2135,10 @@ describe(getName(), () => {
ref: 'someref',
},
};
await github.mergePr(pr.number, '');
await github.mergePr({
branchName: '',
id: pr.number,
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('should try merge after squash', async () => {
@ -2140,7 +2158,12 @@ describe(getName(), () => {
ref: 'someref',
},
};
expect(await github.mergePr(pr.number, '')).toBe(true);
expect(
await github.mergePr({
branchName: '',
id: pr.number,
})
).toBe(true);
expect(httpMock.getTrace()).toMatchSnapshot();
});
it('should give up', async () => {
@ -2162,7 +2185,12 @@ describe(getName(), () => {
ref: 'someref',
},
};
expect(await github.mergePr(pr.number, '')).toBe(false);
expect(
await github.mergePr({
branchName: '',
id: pr.number,
})
).toBe(false);
expect(httpMock.getTrace()).toMatchSnapshot();
});
});

View file

@ -34,6 +34,7 @@ import type {
EnsureIssueResult,
FindPRConfig,
Issue,
MergePRConfig,
PlatformResult,
Pr,
RepoParams,
@ -1477,10 +1478,10 @@ export async function updatePr({
}
}
export async function mergePr(
prNo: number,
branchName: string
): Promise<boolean> {
export async function mergePr({
branchName,
id: prNo,
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${prNo}, ${branchName})`);
// istanbul ignore if
if (config.prReviewsRequired) {

View file

@ -1796,7 +1796,9 @@ describe(getName(), () => {
.scope(gitlabApiHost)
.put('/api/v4/projects/undefined/merge_requests/1/merge')
.reply(200);
await gitlab.mergePr(1, undefined);
await gitlab.mergePr({
id: 1,
});
expect(httpMock.getTrace()).toMatchSnapshot();
});
});

View file

@ -31,6 +31,7 @@ import type {
EnsureIssueConfig,
FindPRConfig,
Issue,
MergePRConfig,
PlatformParams,
PlatformPrOptions,
PlatformResult,
@ -606,10 +607,10 @@ export async function updatePr({
await tryPrAutomerge(iid, platformOptions);
}
export async function mergePr(iid: number): Promise<boolean> {
export async function mergePr({ id }: MergePRConfig): Promise<boolean> {
try {
await gitlabApi.putJson(
`projects/${config.repository}/merge_requests/${iid}/merge`,
`projects/${config.repository}/merge_requests/${id}/merge`,
{
body: {
should_remove_source_branch: true,

View file

@ -117,6 +117,11 @@ export interface FindPRConfig {
state?: PrState.Open | PrState.Closed | PrState.NotOpen | PrState.All;
refreshCache?: boolean;
}
export interface MergePRConfig {
branchName?: string;
id: number;
strategy?: MergeStrategy;
}
export interface EnsureCommentConfig {
number: number;
topic: string;
@ -154,11 +159,7 @@ export interface Platform {
): Promise<EnsureIssueResult | null>;
massageMarkdown(prBody: string): string;
updatePr(prConfig: UpdatePrConfig): Promise<void>;
mergePr(
number: number,
branchName: string,
mergeStrategy?: MergeStrategy
): Promise<boolean>;
mergePr(config: MergePRConfig): Promise<boolean>;
addReviewers(number: number, reviewers: string[]): Promise<void>;
addAssignees(number: number, assignees: string[]): Promise<void>;
createPr(prConfig: CreatePRConfig): Promise<Pr>;

View file

@ -109,7 +109,11 @@ export async function checkAutoMerge(
};
}
logger.debug(`Automerging #${pr.number} with strategy ${automergeStrategy}`);
const res = await platform.mergePr(pr.number, branchName, automergeStrategy);
const res = await platform.mergePr({
branchName,
id: pr.number,
strategy: automergeStrategy,
});
if (res) {
logger.info({ pr: pr.number, prTitle: pr.title }, 'PR automerged');
let branchRemoved = false;