From 1786438d33c8282de786862dd2a863bf67109e0f Mon Sep 17 00:00:00 2001 From: joegoldman2 <147369450+joegoldman2@users.noreply.github.com> Date: Wed, 27 Dec 2023 11:17:03 +0200 Subject: [PATCH] feat(platform/azure): implement automergeStrategy for Azure DevOps platform (#26429) --- lib/config/options/index.ts | 2 +- lib/modules/platform/azure/index.spec.ts | 182 +++++++++++++++++++++- lib/modules/platform/azure/index.ts | 12 +- lib/modules/platform/azure/readme.md | 4 - lib/modules/platform/azure/util.ts | 18 +++ lib/modules/platform/types.ts | 1 + lib/workers/repository/update/pr/index.ts | 1 + 7 files changed, 210 insertions(+), 10 deletions(-) diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 06319babafae1f..1a32eb51dc3e4c 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1828,7 +1828,7 @@ const options: RenovateOptions[] = [ type: 'string', allowedValues: ['auto', 'fast-forward', 'merge-commit', 'rebase', 'squash'], default: 'auto', - supportedPlatforms: ['bitbucket', 'gitea'], + supportedPlatforms: ['azure', 'bitbucket', 'gitea'], }, { name: 'automergeComment', diff --git a/lib/modules/platform/azure/index.spec.ts b/lib/modules/platform/azure/index.spec.ts index 3924120dd94b7e..50610286181d1d 100644 --- a/lib/modules/platform/azure/index.spec.ts +++ b/lib/modules/platform/azure/index.spec.ts @@ -2,6 +2,7 @@ import { Readable } from 'node:stream'; import is from '@sindresorhus/is'; import type { IGitApi } from 'azure-devops-node-api/GitApi'; import { + GitPullRequest, GitPullRequestMergeStrategy, GitStatusState, PullRequestStatus, @@ -936,7 +937,7 @@ describe('modules/platform/azure/index', () => { expect(pr).toMatchSnapshot(); }); - it('should only call getMergeMethod once per run', async () => { + it('should only call getMergeMethod once per run when automergeStrategy is auto', async () => { await initRepo({ repository: 'some/repo' }); const prResult = [ { @@ -1001,7 +1002,10 @@ describe('modules/platform/azure/index', () => { prTitle: 'The Title', prBody: 'Hello world', labels: ['deps', 'renovate'], - platformOptions: { usePlatformAutomerge: true }, + platformOptions: { + automergeStrategy: 'auto', + usePlatformAutomerge: true, + }, }); await azure.createPr({ @@ -1010,12 +1014,128 @@ describe('modules/platform/azure/index', () => { prTitle: 'The Second Title', prBody: 'Hello world', labels: ['deps', 'renovate'], - platformOptions: { usePlatformAutomerge: true }, + platformOptions: { + automergeStrategy: 'auto', + usePlatformAutomerge: true, + }, }); expect(updateFn).toHaveBeenCalledTimes(2); expect(azureHelper.getMergeMethod).toHaveBeenCalledTimes(1); }); + + it.each` + automergeStrategy + ${'fast-forward'} + ${'merge-commit'} + ${'rebase'} + ${'squash'} + `( + 'should not call getMergeMethod when automergeStrategy is $automergeStrategy', + async (automergeStrategy) => { + await initRepo({ repository: 'some/repo' }); + const prResult = { + pullRequestId: 123, + title: 'The Title', + createdBy: { + id: '123', + }, + }; + const prUpdateResults = { + ...prResult, + autoCompleteSetBy: { + id: prResult.createdBy.id, + }, + completionOptions: { + squashMerge: true, + deleteSourceBranch: true, + mergeCommitMessage: 'The Title', + }, + }; + const updateFn = jest.fn(() => Promise.resolve(prUpdateResults)); + + azureApi.gitApi.mockResolvedValue( + partial({ + createPullRequest: jest.fn(() => Promise.resolve(prResult)), + createPullRequestLabel: jest.fn().mockResolvedValue({}), + updatePullRequest: updateFn, + }), + ); + await azure.createPr({ + sourceBranch: 'some-branch', + targetBranch: 'dev', + prTitle: 'The Title', + prBody: 'Hello world', + labels: ['deps', 'renovate'], + platformOptions: { + automergeStrategy, + usePlatformAutomerge: true, + }, + }); + + expect(azureHelper.getMergeMethod).toHaveBeenCalledTimes(0); + }, + ); + + it.each` + automergeStrategy | prMergeStrategy + ${'fast-forward'} | ${GitPullRequestMergeStrategy.Rebase} + ${'merge-commit'} | ${GitPullRequestMergeStrategy.NoFastForward} + ${'rebase'} | ${GitPullRequestMergeStrategy.Rebase} + ${'squash'} | ${GitPullRequestMergeStrategy.Squash} + `( + 'should create PR with mergeStrategy $prMergeStrategy', + async ({ automergeStrategy, prMergeStrategy }) => { + await initRepo({ repository: 'some/repo' }); + const prResult = { + pullRequestId: 456, + title: 'The Title', + createdBy: { + id: '123', + }, + }; + const prUpdateResult = { + ...prResult, + autoCompleteSetBy: { + id: prResult.createdBy.id, + }, + completionOptions: { + mergeStrategy: prMergeStrategy, + squashMerge: false, + deleteSourceBranch: true, + mergeCommitMessage: 'The Title', + }, + }; + const updateFn = jest.fn().mockResolvedValue(prUpdateResult); + azureApi.gitApi.mockResolvedValueOnce( + partial({ + createPullRequest: jest.fn().mockResolvedValue(prResult), + createPullRequestLabel: jest.fn().mockResolvedValue({}), + updatePullRequest: updateFn, + }), + ); + const pr = await azure.createPr({ + sourceBranch: 'some-branch', + targetBranch: 'dev', + prTitle: 'The Title', + prBody: 'Hello world', + labels: ['deps', 'renovate'], + platformOptions: { + automergeStrategy, + usePlatformAutomerge: true, + }, + }); + + expect((pr as GitPullRequest).completionOptions?.mergeStrategy).toBe( + prMergeStrategy, + ); + expect(updateFn).toHaveBeenCalled(); + expect( + updateFn.mock.calls[0][0].completionOptions.mergeStrategy, + ).toBe(prMergeStrategy); + expect(azureHelper.getMergeMethod).toHaveBeenCalledTimes(0); + }, + ); }); it('should create and return an approved PR object', async () => { @@ -1528,6 +1648,7 @@ describe('modules/platform/azure/index', () => { const res = await azure.mergePr({ branchName: branchNameMock, id: pullRequestIdMock, + strategy: 'auto', }); expect(updatePullRequestMock).toHaveBeenCalledWith( @@ -1546,6 +1667,59 @@ describe('modules/platform/azure/index', () => { expect(res).toBeTrue(); }); + it.each` + automergeStrategy | prMergeStrategy + ${'fast-forward'} | ${GitPullRequestMergeStrategy.Rebase} + ${'merge-commit'} | ${GitPullRequestMergeStrategy.NoFastForward} + ${'rebase'} | ${GitPullRequestMergeStrategy.Rebase} + ${'squash'} | ${GitPullRequestMergeStrategy.Squash} + `( + 'should complete PR with mergeStrategy $prMergeStrategy', + async ({ automergeStrategy, prMergeStrategy }) => { + await initRepo({ repository: 'some/repo' }); + const pullRequestIdMock = 12345; + const branchNameMock = 'test'; + const lastMergeSourceCommitMock = { commitId: 'abcd1234' }; + const updatePullRequestMock = jest.fn(() => ({ + status: 3, + })); + azureApi.gitApi.mockImplementationOnce( + () => + ({ + getPullRequestById: jest.fn(() => ({ + lastMergeSourceCommit: lastMergeSourceCommitMock, + targetRefName: 'refs/heads/ding', + title: 'title', + })), + updatePullRequest: updatePullRequestMock, + }) as any, + ); + + azureHelper.getMergeMethod = jest.fn().mockReturnValue(prMergeStrategy); + + const res = await azure.mergePr({ + branchName: branchNameMock, + id: pullRequestIdMock, + strategy: automergeStrategy, + }); + + expect(updatePullRequestMock).toHaveBeenCalledWith( + { + status: PullRequestStatus.Completed, + lastMergeSourceCommit: lastMergeSourceCommitMock, + completionOptions: { + mergeStrategy: prMergeStrategy, + deleteSourceBranch: true, + mergeCommitMessage: 'title', + }, + }, + '1', + pullRequestIdMock, + ); + expect(res).toBeTrue(); + }, + ); + it('should return false if the PR does not update successfully', async () => { await initRepo({ repository: 'some/repo' }); const pullRequestIdMock = 12345; @@ -1593,10 +1767,12 @@ describe('modules/platform/azure/index', () => { await azure.mergePr({ branchName: 'test-branch-1', id: 1234, + strategy: 'auto', }); await azure.mergePr({ branchName: 'test-branch-2', id: 5678, + strategy: 'auto', }); expect(azureHelper.getMergeMethod).toHaveBeenCalledTimes(1); diff --git a/lib/modules/platform/azure/index.ts b/lib/modules/platform/azure/index.ts index c154d4cd6b8c7b..fe9fa26c19d833 100644 --- a/lib/modules/platform/azure/index.ts +++ b/lib/modules/platform/azure/index.ts @@ -52,6 +52,7 @@ import { getRenovatePRFormat, getRepoByName, getStorageExtraCloneOpts, + mapMergeStrategy, max4000Chars, } from './util'; @@ -491,7 +492,10 @@ export async function createPr({ config.repoId, ); if (platformOptions?.usePlatformAutomerge) { - const mergeStrategy = await getMergeStrategy(pr.targetRefName!); + const mergeStrategy = + platformOptions.automergeStrategy === 'auto' + ? await getMergeStrategy(pr.targetRefName!) + : mapMergeStrategy(platformOptions.automergeStrategy); pr = await azureApiGit.updatePullRequest( { autoCompleteSetBy: { @@ -736,13 +740,17 @@ export async function setBranchStatus({ export async function mergePr({ branchName, id: pullRequestId, + strategy, }: MergePRConfig): Promise { logger.debug(`mergePr(${pullRequestId}, ${branchName!})`); const azureApiGit = await azureApi.gitApi(); let pr = await azureApiGit.getPullRequestById(pullRequestId, config.project); - const mergeStrategy = await getMergeStrategy(pr.targetRefName!); + const mergeStrategy = + strategy === 'auto' + ? await getMergeStrategy(pr.targetRefName!) + : mapMergeStrategy(strategy); const objToUpdate: GitPullRequest = { status: PullRequestStatus.Completed, lastMergeSourceCommit: pr.lastMergeSourceCommit, diff --git a/lib/modules/platform/azure/readme.md b/lib/modules/platform/azure/readme.md index b65aaa7b87f3e3..cb06f4b7a66080 100644 --- a/lib/modules/platform/azure/readme.md +++ b/lib/modules/platform/azure/readme.md @@ -18,10 +18,6 @@ Permissions for your PAT should be at minimum: Remember to set `platform=azure` somewhere in your Renovate config file. -## Features awaiting implementation - -- The `automergeStrategy` configuration option has not been implemented for this platform, and all values behave as if the value `auto` was used. Renovate will use the merge strategy configured in the Azure Repos repository itself, and this cannot be overridden yet - ## Running Renovate in Azure Pipelines ### Setting up a new pipeline diff --git a/lib/modules/platform/azure/util.ts b/lib/modules/platform/azure/util.ts index adbb2560a6ff86..9ca3563961bcb5 100644 --- a/lib/modules/platform/azure/util.ts +++ b/lib/modules/platform/azure/util.ts @@ -1,9 +1,11 @@ import { GitPullRequest, + GitPullRequestMergeStrategy, GitRepository, GitStatusContext, PullRequestStatus, } from 'azure-devops-node-api/interfaces/GitInterfaces.js'; +import type { MergeStrategy } from '../../../config/types'; import { logger } from '../../../logger'; import type { HostRule, PrState } from '../../../types'; import type { GitOptions } from '../../../types/git'; @@ -181,3 +183,19 @@ export function getRepoByName( } return foundRepo ?? null; } + +export function mapMergeStrategy( + mergeStrategy?: MergeStrategy, +): GitPullRequestMergeStrategy { + switch (mergeStrategy) { + case 'rebase': + case 'fast-forward': + return GitPullRequestMergeStrategy.Rebase; + case 'merge-commit': + return GitPullRequestMergeStrategy.NoFastForward; + case 'squash': + return GitPullRequestMergeStrategy.Squash; + default: + return GitPullRequestMergeStrategy.NoFastForward; + } +} diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 4f9a980d566d81..78587912fe026f 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -96,6 +96,7 @@ export interface Issue { } export type PlatformPrOptions = { autoApprove?: boolean; + automergeStrategy?: MergeStrategy; azureWorkItemId?: number; bbUseDefaultReviewers?: boolean; gitLabIgnoreApprovals?: boolean; diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 6eb44a70eb04a6..e9dccd6b4a1846 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -55,6 +55,7 @@ export function getPlatformPrOptions( return { autoApprove: !!config.autoApprove, + automergeStrategy: config.automergeStrategy, azureWorkItemId: config.azureWorkItemId ?? 0, bbUseDefaultReviewers: !!config.bbUseDefaultReviewers, gitLabIgnoreApprovals: !!config.gitLabIgnoreApprovals,