From 03636915e4996971a31cd110d1c1bdd4f0de02e2 Mon Sep 17 00:00:00 2001
From: kroonprins <kroonprins@users.noreply.github.com>
Date: Fri, 25 Aug 2023 10:26:14 +0200
Subject: [PATCH] feat(platform/azure): check targetBranch when finding PRs
(#23941)
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Co-authored-by: Rhys Arkins <rhys@arkins.net>
---
.../azure/__snapshots__/index.spec.ts.snap | 72 --------
lib/modules/platform/azure/index.spec.ts | 170 +++++++++++++++++-
lib/modules/platform/azure/index.ts | 15 +-
lib/modules/platform/types.ts | 3 +-
lib/workers/repository/cache.ts | 2 +-
.../config-migration/branch/index.ts | 16 +-
.../repository/config-migration/pr/index.ts | 2 +-
lib/workers/repository/error-config.ts | 5 +-
lib/workers/repository/finalize/prune.ts | 1 +
.../repository/onboarding/branch/check.ts | 6 +-
lib/workers/repository/onboarding/pr/index.ts | 5 +-
lib/workers/repository/process/limits.ts | 2 +-
.../repository/update/branch/automerge.ts | 5 +-
.../update/branch/check-existing.ts | 2 +
lib/workers/repository/update/branch/index.ts | 6 +-
lib/workers/repository/update/pr/index.ts | 2 +-
16 files changed, 218 insertions(+), 96 deletions(-)
diff --git a/lib/modules/platform/azure/__snapshots__/index.spec.ts.snap b/lib/modules/platform/azure/__snapshots__/index.spec.ts.snap
index fc549c0707..ecd7882d4a 100644
--- a/lib/modules/platform/azure/__snapshots__/index.spec.ts.snap
+++ b/lib/modules/platform/azure/__snapshots__/index.spec.ts.snap
@@ -135,78 +135,6 @@ content",
]
`;
-exports[`modules/platform/azure/index findPr(branchName, prTitle, state) returns pr if found it all state 1`] = `
-{
- "bodyStruct": {
- "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
- },
- "createdAt": undefined,
- "number": 1,
- "pullRequestId": 1,
- "sourceBranch": "branch-a",
- "sourceRefName": "refs/heads/branch-a",
- "state": "closed",
- "status": 2,
- "targetBranch": "branch-b",
- "targetRefName": "refs/heads/branch-b",
- "title": "branch a pr",
-}
-`;
-
-exports[`modules/platform/azure/index findPr(branchName, prTitle, state) returns pr if found it close 1`] = `
-{
- "bodyStruct": {
- "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
- },
- "createdAt": undefined,
- "number": 1,
- "pullRequestId": 1,
- "sourceBranch": "branch-a",
- "sourceRefName": "refs/heads/branch-a",
- "state": "closed",
- "status": 2,
- "targetBranch": "branch-b",
- "targetRefName": "refs/heads/branch-b",
- "title": "branch a pr",
-}
-`;
-
-exports[`modules/platform/azure/index findPr(branchName, prTitle, state) returns pr if found it open 1`] = `
-{
- "bodyStruct": {
- "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
- },
- "createdAt": undefined,
- "number": 1,
- "pullRequestId": 1,
- "sourceBranch": "branch-a",
- "sourceRefName": "refs/heads/branch-a",
- "state": "open",
- "status": 1,
- "targetBranch": "branch-b",
- "targetRefName": "refs/heads/branch-b",
- "title": "branch a pr",
-}
-`;
-
-exports[`modules/platform/azure/index findPr(branchName, prTitle, state) returns pr if found not open 1`] = `
-{
- "bodyStruct": {
- "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
- },
- "createdAt": undefined,
- "number": 1,
- "pullRequestId": 1,
- "sourceBranch": "branch-a",
- "sourceRefName": "refs/heads/branch-a",
- "state": "merged",
- "status": 3,
- "targetBranch": "branch-b",
- "targetRefName": "refs/heads/branch-b",
- "title": "branch a pr",
-}
-`;
-
exports[`modules/platform/azure/index getJsonFile() supports fetch from another repo 1`] = `
[
[
diff --git a/lib/modules/platform/azure/index.spec.ts b/lib/modules/platform/azure/index.spec.ts
index 29405f4501..dbe82259e4 100644
--- a/lib/modules/platform/azure/index.spec.ts
+++ b/lib/modules/platform/azure/index.spec.ts
@@ -208,7 +208,7 @@ describe('modules/platform/azure/index', () => {
});
});
- describe('findPr(branchName, prTitle, state)', () => {
+ describe('findPr(branchName, prTitle, state, targetBranch)', () => {
it('returns pr if found it open', async () => {
azureApi.gitApi.mockImplementationOnce(
() =>
@@ -233,7 +233,21 @@ describe('modules/platform/azure/index', () => {
prTitle: 'branch a pr',
state: 'open',
});
- expect(res).toMatchSnapshot();
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 1,
+ pullRequestId: 1,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'open',
+ status: 1,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ });
});
it('returns pr if found not open', async () => {
@@ -260,7 +274,21 @@ describe('modules/platform/azure/index', () => {
prTitle: 'branch a pr',
state: '!open',
});
- expect(res).toMatchSnapshot();
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 1,
+ pullRequestId: 1,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'merged',
+ status: 3,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ });
});
it('returns pr if found it close', async () => {
@@ -287,7 +315,21 @@ describe('modules/platform/azure/index', () => {
prTitle: 'branch a pr',
state: 'closed',
});
- expect(res).toMatchSnapshot();
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 1,
+ pullRequestId: 1,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'closed',
+ status: 2,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ });
});
it('returns pr if found it all state', async () => {
@@ -313,7 +355,117 @@ describe('modules/platform/azure/index', () => {
branchName: 'branch-a',
prTitle: 'branch a pr',
});
- expect(res).toMatchSnapshot();
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 1,
+ pullRequestId: 1,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'closed',
+ status: 2,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ });
+ });
+
+ it('returns pr if found matches targetBranch', async () => {
+ azureApi.gitApi.mockImplementationOnce(() =>
+ partial<IGitApi>({
+ getPullRequests: jest
+ .fn()
+ .mockReturnValue([])
+ .mockReturnValueOnce([
+ {
+ pullRequestId: 1,
+ sourceRefName: 'refs/heads/branch-a',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ status: PullRequestStatus.Active,
+ },
+ {
+ pullRequestId: 2,
+ sourceRefName: 'refs/heads/branch-a',
+ targetRefName: 'refs/heads/branch-c',
+ title: 'branch a pr',
+ status: PullRequestStatus.Active,
+ },
+ ]),
+ getPullRequestCommits: jest.fn().mockReturnValue([]),
+ })
+ );
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'open',
+ targetBranch: 'branch-c',
+ });
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 2,
+ pullRequestId: 2,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'open',
+ status: 1,
+ targetBranch: 'branch-c',
+ targetRefName: 'refs/heads/branch-c',
+ title: 'branch a pr',
+ });
+ });
+
+ it('returns first pr if found does not match targetBranch', async () => {
+ azureApi.gitApi.mockImplementationOnce(() =>
+ partial<IGitApi>({
+ getPullRequests: jest
+ .fn()
+ .mockReturnValue([])
+ .mockReturnValueOnce([
+ {
+ pullRequestId: 1,
+ sourceRefName: 'refs/heads/branch-a',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ status: PullRequestStatus.Active,
+ },
+ {
+ pullRequestId: 2,
+ sourceRefName: 'refs/heads/branch-a',
+ targetRefName: 'refs/heads/branch-c',
+ title: 'branch a pr',
+ status: PullRequestStatus.Active,
+ },
+ ]),
+ getPullRequestCommits: jest.fn().mockReturnValue([]),
+ })
+ );
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'open',
+ targetBranch: 'branch-d',
+ });
+ expect(res).toEqual({
+ bodyStruct: {
+ hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
+ },
+ createdAt: undefined,
+ number: 1,
+ pullRequestId: 1,
+ sourceBranch: 'branch-a',
+ sourceRefName: 'refs/heads/branch-a',
+ state: 'open',
+ status: 1,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
+ title: 'branch a pr',
+ });
});
it('catches errors', async () => {
@@ -342,7 +494,7 @@ describe('modules/platform/azure/index', () => {
});
});
- describe('getBranchPr(branchName)', () => {
+ describe('getBranchPr(branchName, targetBranch)', () => {
it('should return null if no PR exists', async () => {
await initRepo({ repository: 'some/repo' });
azureApi.gitApi.mockResolvedValue(
@@ -364,6 +516,7 @@ describe('modules/platform/azure/index', () => {
{
pullRequestId: 1,
sourceRefName: 'refs/heads/branch-a',
+ targetRefName: 'refs/heads/branch-b',
title: 'branch a pr',
status: 1,
},
@@ -372,7 +525,7 @@ describe('modules/platform/azure/index', () => {
getPullRequestLabels: jest.fn().mockResolvedValue([]),
})
);
- const pr = await azure.getBranchPr('branch-a');
+ const pr = await azure.getBranchPr('branch-a', 'branch-b');
expect(pr).toEqual({
bodyStruct: {
hash: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855',
@@ -385,7 +538,8 @@ describe('modules/platform/azure/index', () => {
sourceRefName: 'refs/heads/branch-a',
state: 'open',
status: 1,
- targetBranch: undefined,
+ targetBranch: 'branch-b',
+ targetRefName: 'refs/heads/branch-b',
title: 'branch a pr',
});
});
diff --git a/lib/modules/platform/azure/index.ts b/lib/modules/platform/azure/index.ts
index 4f4ad1f3eb..e2f60bef7e 100644
--- a/lib/modules/platform/azure/index.ts
+++ b/lib/modules/platform/azure/index.ts
@@ -298,6 +298,7 @@ export async function findPr({
branchName,
prTitle,
state = 'all',
+ targetBranch,
}: FindPRConfig): Promise<Pr | null> {
let prsFiltered: Pr[] = [];
try {
@@ -330,14 +331,24 @@ export async function findPr({
if (prsFiltered.length === 0) {
return null;
}
+ if (targetBranch && prsFiltered.length > 1) {
+ const pr = prsFiltered.find((item) => item.targetBranch === targetBranch);
+ if (pr) {
+ return pr;
+ }
+ }
return prsFiltered[0];
}
-export async function getBranchPr(branchName: string): Promise<Pr | null> {
- logger.debug(`getBranchPr(${branchName})`);
+export async function getBranchPr(
+ branchName: string,
+ targetBranch?: string
+): Promise<Pr | null> {
+ logger.debug(`getBranchPr(${branchName}, ${targetBranch})`);
const existingPr = await findPr({
branchName,
state: 'open',
+ targetBranch,
});
return existingPr ? getPr(existingPr.number) : null;
}
diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts
index bab9f39a06..684005d9c9 100644
--- a/lib/modules/platform/types.ts
+++ b/lib/modules/platform/types.ts
@@ -137,6 +137,7 @@ export interface FindPRConfig {
prTitle?: string | null;
state?: 'open' | 'closed' | '!open' | 'all';
refreshCache?: boolean;
+ targetBranch?: string | null;
}
export interface MergePRConfig {
branchName?: string;
@@ -219,7 +220,7 @@ export interface Platform {
branchName: string,
internalChecksAsSuccess: boolean
): Promise<BranchStatus>;
- getBranchPr(branchName: string): Promise<Pr | null>;
+ getBranchPr(branchName: string, targetBranch?: string): Promise<Pr | null>;
initPlatform(config: PlatformParams): Promise<PlatformResult>;
filterUnavailableUsers?(users: string[]): Promise<string[]>;
commitFiles?(config: CommitFilesConfig): Promise<CommitSha | null>;
diff --git a/lib/workers/repository/cache.ts b/lib/workers/repository/cache.ts
index 300e4e874c..1f21b0c5d9 100644
--- a/lib/workers/repository/cache.ts
+++ b/lib/workers/repository/cache.ts
@@ -73,7 +73,7 @@ async function generateBranchCache(
let isBehindBase: boolean | undefined;
let isConflicted: boolean | undefined;
if (baseBranchSha && branchSha) {
- const branchPr = await platform.getBranchPr(branchName);
+ const branchPr = await platform.getBranchPr(branchName, baseBranch);
if (branchPr) {
prNo = branchPr.number;
}
diff --git a/lib/workers/repository/config-migration/branch/index.ts b/lib/workers/repository/config-migration/branch/index.ts
index 24651bf942..c5827006fc 100644
--- a/lib/workers/repository/config-migration/branch/index.ts
+++ b/lib/workers/repository/config-migration/branch/index.ts
@@ -21,7 +21,10 @@ export async function checkConfigMigrationBranch(
}
const configMigrationBranch = getMigrationBranchName(config);
- const branchPr = await migrationPrExists(configMigrationBranch); // handles open/autoClosed PRs
+ const branchPr = await migrationPrExists(
+ configMigrationBranch,
+ config.baseBranch
+ ); // handles open/autoClosed PRs
if (!branchPr) {
const commitMessageFactory = new ConfigMigrationCommitMessageFactory(
@@ -33,6 +36,7 @@ export async function checkConfigMigrationBranch(
branchName: configMigrationBranch,
prTitle,
state: 'closed',
+ targetBranch: config.baseBranch,
};
// handles closed PR
@@ -54,7 +58,8 @@ export async function checkConfigMigrationBranch(
await rebaseMigrationBranch(config, migratedConfigData);
if (platform.refreshPr) {
const configMigrationPr = await platform.getBranchPr(
- configMigrationBranch
+ configMigrationBranch,
+ config.baseBranch
);
if (configMigrationPr) {
await platform.refreshPr(configMigrationPr.number);
@@ -71,8 +76,11 @@ export async function checkConfigMigrationBranch(
return configMigrationBranch;
}
-export async function migrationPrExists(branchName: string): Promise<boolean> {
- return !!(await platform.getBranchPr(branchName));
+export async function migrationPrExists(
+ branchName: string,
+ targetBranch?: string
+): Promise<boolean> {
+ return !!(await platform.getBranchPr(branchName, targetBranch));
}
async function handlePr(config: RenovateConfig, pr: Pr): Promise<void> {
diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts
index 53fe6427a9..3d11d730af 100644
--- a/lib/workers/repository/config-migration/pr/index.ts
+++ b/lib/workers/repository/config-migration/pr/index.ts
@@ -31,7 +31,7 @@ export async function ensureConfigMigrationPr(
);
const prTitle = commitMessageFactory.getPrTitle();
- const existingPr = await platform.getBranchPr(branchName);
+ const existingPr = await platform.getBranchPr(branchName, config.baseBranch);
const filename = migratedConfigData.filename;
logger.debug('Filling in config migration PR template');
let prBody = `The Renovate config in this repository needs migrating. Typically this is because one or more configuration options you are using have been renamed.
diff --git a/lib/workers/repository/error-config.ts b/lib/workers/repository/error-config.ts
index 38ba2b17e4..f7b62dda01 100644
--- a/lib/workers/repository/error-config.ts
+++ b/lib/workers/repository/error-config.ts
@@ -48,7 +48,10 @@ async function raiseWarningIssue(
)}\`\n`;
}
- const pr = await platform.getBranchPr(config.onboardingBranch!);
+ const pr = await platform.getBranchPr(
+ config.onboardingBranch!,
+ config.baseBranch
+ );
if (pr?.state === 'open') {
await handleOnboardingPr(pr, body);
return;
diff --git a/lib/workers/repository/finalize/prune.ts b/lib/workers/repository/finalize/prune.ts
index 22cb17f7af..87cb4fb4d0 100644
--- a/lib/workers/repository/finalize/prune.ts
+++ b/lib/workers/repository/finalize/prune.ts
@@ -22,6 +22,7 @@ async function cleanUpBranches(
const pr = await platform.findPr({
branchName,
state: 'open',
+ targetBranch: config.baseBranch,
});
const branchIsModified = await scm.isBranchModified(branchName);
if (pr) {
diff --git a/lib/workers/repository/onboarding/branch/check.ts b/lib/workers/repository/onboarding/branch/check.ts
index 5f7c0e2f4c..540e848aac 100644
--- a/lib/workers/repository/onboarding/branch/check.ts
+++ b/lib/workers/repository/onboarding/branch/check.ts
@@ -46,6 +46,7 @@ function closedPrExists(config: RenovateConfig): Promise<Pr | null> {
branchName: config.onboardingBranch!,
prTitle: config.onboardingPrTitle,
state: '!open',
+ targetBranch: config.baseBranch,
});
}
@@ -150,5 +151,8 @@ export async function isOnboarded(config: RenovateConfig): Promise<boolean> {
export async function getOnboardingPr(
config: RenovateConfig
): Promise<Pr | null> {
- return await platform.getBranchPr(config.onboardingBranch!);
+ return await platform.getBranchPr(
+ config.onboardingBranch!,
+ config.baseBranch
+ );
}
diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts
index e82ace8812..d11dca54ca 100644
--- a/lib/workers/repository/onboarding/pr/index.ts
+++ b/lib/workers/repository/onboarding/pr/index.ts
@@ -41,7 +41,10 @@ export async function ensureOnboardingPr(
logger.debug('ensureOnboardingPr()');
logger.trace({ config });
// TODO #22198
- const existingPr = await platform.getBranchPr(config.onboardingBranch!);
+ const existingPr = await platform.getBranchPr(
+ config.onboardingBranch!,
+ config.defaultBranch
+ );
if (existingPr) {
// skip pr-update if branch is conflicted
if (
diff --git a/lib/workers/repository/process/limits.ts b/lib/workers/repository/process/limits.ts
index 2dcfe15a4a..24423a225f 100644
--- a/lib/workers/repository/process/limits.ts
+++ b/lib/workers/repository/process/limits.ts
@@ -49,7 +49,7 @@ export async function getConcurrentPrsRemaining(
const openPrs: Pr[] = [];
for (const { branchName } of branches) {
try {
- const pr = await platform.getBranchPr(branchName);
+ const pr = await platform.getBranchPr(branchName, config.baseBranch);
if (
pr &&
pr.sourceBranch !== config.onboardingBranch &&
diff --git a/lib/workers/repository/update/branch/automerge.ts b/lib/workers/repository/update/branch/automerge.ts
index e624283840..3ea24e8ee7 100644
--- a/lib/workers/repository/update/branch/automerge.ts
+++ b/lib/workers/repository/update/branch/automerge.ts
@@ -26,7 +26,10 @@ export async function tryBranchAutomerge(
if (!isScheduledNow(config, 'automergeSchedule')) {
return 'off schedule';
}
- const existingPr = await platform.getBranchPr(config.branchName!);
+ const existingPr = await platform.getBranchPr(
+ config.branchName!,
+ config.baseBranch
+ );
if (existingPr) {
return 'automerge aborted - PR exists';
}
diff --git a/lib/workers/repository/update/branch/check-existing.ts b/lib/workers/repository/update/branch/check-existing.ts
index 0892c60868..1e042068d5 100644
--- a/lib/workers/repository/update/branch/check-existing.ts
+++ b/lib/workers/repository/update/branch/check-existing.ts
@@ -20,6 +20,7 @@ export async function prAlreadyExisted(
branchName: config.branchName,
prTitle: config.prTitle,
state: '!open',
+ targetBranch: config.baseBranch,
});
if (!pr && config.branchPrefix !== config.branchPrefixOld) {
@@ -30,6 +31,7 @@ export async function prAlreadyExisted(
),
prTitle: config.prTitle,
state: '!open',
+ targetBranch: config.baseBranch,
});
if (pr) {
logger.debug('Found closed PR with branchPrefixOld');
diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts
index 24176316c9..54abd336fc 100644
--- a/lib/workers/repository/update/branch/index.ts
+++ b/lib/workers/repository/update/branch/index.ts
@@ -132,7 +132,10 @@ export async function processBranch(
}
}
- let branchPr = await platform.getBranchPr(config.branchName);
+ let branchPr = await platform.getBranchPr(
+ config.branchName,
+ config.baseBranch
+ );
logger.debug(`branchExists=${branchExists}`);
const dependencyDashboardCheck =
config.dependencyDashboardChecks?.[config.branchName];
@@ -262,6 +265,7 @@ export async function processBranch(
const oldPr = await platform.findPr({
branchName: config.branchName,
state: '!open',
+ targetBranch: config.baseBranch,
});
if (!oldPr) {
logger.debug('Branch has been edited but found no PR - skipping');
diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts
index 18ff29263b..705f6b0951 100644
--- a/lib/workers/repository/update/pr/index.ts
+++ b/lib/workers/repository/update/pr/index.ts
@@ -124,7 +124,7 @@ export async function ensurePr(
const dependencyDashboardCheck =
config.dependencyDashboardChecks?.[config.branchName];
// Check if PR already exists
- const existingPr = await platform.getBranchPr(branchName);
+ const existingPr = await platform.getBranchPr(branchName, config.baseBranch);
const prCache = getPrCache(branchName);
if (existingPr) {
logger.debug('Found existing PR');
--
GitLab