From 547dcd84a726611eac929e8acdbe1abd36b6bdd4 Mon Sep 17 00:00:00 2001
From: Sourav Das <souravdasslg95@gmail.com>
Date: Tue, 14 Jan 2020 20:42:14 +0530
Subject: [PATCH] refactor(function): findPR function(#4996) (#5129)
---
lib/platform/azure/index.ts | 13 +++++-----
lib/platform/bitbucket-server/index.ts | 16 ++++++++-----
lib/platform/bitbucket/index.ts | 15 ++++++------
lib/platform/common.ts | 12 ++++++----
lib/platform/github/index.ts | 13 +++++-----
lib/platform/gitlab/index.ts | 11 +++++----
lib/workers/branch/check-existing.ts | 6 ++++-
lib/workers/repository/finalise/prune.ts | 5 +++-
lib/workers/repository/master-issue.ts | 10 ++++----
.../repository/onboarding/branch/check.ts | 6 ++++-
test/platform/azure/index.spec.ts | 23 ++++++++++++++----
test/platform/bitbucket-server/index.spec.ts | 14 ++++++++---
test/platform/bitbucket/index.spec.ts | 4 +++-
test/platform/github/index.spec.ts | 24 ++++++++++++++-----
test/platform/gitlab/index.spec.ts | 20 ++++++++++++----
test/workers/repository/master-issue.spec.ts | 12 +++++-----
16 files changed, 137 insertions(+), 67 deletions(-)
diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts
index 44ffff1b2b..4c4e11c45c 100644
--- a/lib/platform/azure/index.ts
+++ b/lib/platform/azure/index.ts
@@ -17,6 +17,7 @@ import {
VulnerabilityAlert,
CreatePRConfig,
BranchStatusConfig,
+ FindPRConfig,
EnsureCommentConfig,
} from '../common';
import { sanitize } from '../../util/sanitize';
@@ -275,11 +276,11 @@ export async function getPr(pullRequestId: number): Promise<Pr | null> {
return azurePr;
}
-export async function findPr(
- branchName: string,
- prTitle: string | null,
- state = 'all'
-): Promise<Pr | null> {
+export async function findPr({
+ branchName,
+ prTitle,
+ state = 'all',
+}: FindPRConfig): Promise<Pr | null> {
logger.debug(`findPr(${branchName}, ${prTitle}, ${state})`);
// TODO: fix typing
let prsFiltered: any[] = [];
@@ -316,7 +317,7 @@ export async function findPr(
export async function getBranchPr(branchName: string): Promise<Pr | null> {
logger.debug(`getBranchPr(${branchName})`);
- const existingPr = await findPr(branchName, null, 'open');
+ const existingPr = await findPr({ branchName, state: 'open' });
return existingPr ? getPr(existingPr.pullRequestId) : null;
}
diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts
index 9a1f57c452..3f46fa1149 100644
--- a/lib/platform/bitbucket-server/index.ts
+++ b/lib/platform/bitbucket-server/index.ts
@@ -16,6 +16,7 @@ import {
GotResponse,
CreatePRConfig,
BranchStatusConfig,
+ FindPRConfig,
EnsureCommentConfig,
} from '../common';
import { sanitize } from '../../util/sanitize';
@@ -382,12 +383,12 @@ export async function getPrList(_args?: any): Promise<Pr[]> {
// TODO: coverage
// istanbul ignore next
-export async function findPr(
- branchName: string,
- prTitle?: string,
+export async function findPr({
+ branchName,
+ prTitle,
state = 'all',
- refreshCache?: boolean
-): Promise<Pr | null> {
+ refreshCache,
+}: FindPRConfig): Promise<Pr | null> {
logger.debug(`findPr(${branchName}, "${prTitle}", "${state}")`);
const prList = await getPrList({ refreshCache });
const pr = prList.find(isRelevantPr(branchName, prTitle, state));
@@ -405,7 +406,10 @@ export async function getBranchPr(
refreshCache?: boolean
): Promise<Pr | null> {
logger.debug(`getBranchPr(${branchName})`);
- const existingPr = await findPr(branchName, undefined, 'open');
+ const existingPr = await findPr({
+ branchName,
+ state: 'open',
+ });
return existingPr ? getPr(existingPr.number, refreshCache) : null;
}
diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts
index 79bd2ce4fa..bc3551a59e 100644
--- a/lib/platform/bitbucket/index.ts
+++ b/lib/platform/bitbucket/index.ts
@@ -17,6 +17,7 @@ import {
CreatePRConfig,
EnsureIssueConfig,
BranchStatusConfig,
+ FindPRConfig,
EnsureCommentConfig,
} from '../common';
import { sanitize } from '../../util/sanitize';
@@ -224,11 +225,11 @@ export async function getPrList(): Promise<Pr[]> {
return config.prList;
}
-export async function findPr(
- branchName: string,
- prTitle?: string | null,
- state = 'all'
-): Promise<Pr | null> {
+export async function findPr({
+ branchName,
+ prTitle,
+ state = 'all',
+}: FindPRConfig): Promise<Pr | null> {
logger.debug(`findPr(${branchName}, ${prTitle}, ${state})`);
const prList = await getPrList();
const pr = prList.find(
@@ -248,7 +249,7 @@ export async function deleteBranch(
closePr?: boolean
): Promise<void> {
if (closePr) {
- const pr = await findPr(branchName, null, 'open');
+ const pr = await findPr({ branchName, state: 'open' });
if (pr) {
await api.post(
`/2.0/repositories/${config.repository}/pullrequests/${pr.number}/decline`
@@ -375,7 +376,7 @@ async function getBranchCommit(branchName: string): Promise<string | null> {
// Returns the Pull Request for a branch. Null if not exists.
export async function getBranchPr(branchName: string): Promise<Pr | null> {
logger.debug(`getBranchPr(${branchName})`);
- const existingPr = await findPr(branchName, null, 'open');
+ const existingPr = await findPr({ branchName, state: 'open' });
return existingPr ? getPr(existingPr.number) : null;
}
diff --git a/lib/platform/common.ts b/lib/platform/common.ts
index 93e8cc07e7..e70e17fdbe 100644
--- a/lib/platform/common.ts
+++ b/lib/platform/common.ts
@@ -129,6 +129,12 @@ export interface BranchStatusConfig {
state: string | null;
url?: string;
}
+export interface FindPRConfig {
+ branchName: string;
+ prTitle?: string | null;
+ state?: 'open' | 'closed' | '!open' | 'all';
+ refreshCache?: boolean;
+}
export interface EnsureCommentConfig {
number: number;
topic: string;
@@ -175,11 +181,7 @@ export interface Platform {
setBaseBranch(baseBranch: string): Promise<void>;
commitFilesToBranch(commitFile: CommitFilesConfig): Promise<void>;
getPr(number: number): Promise<Pr>;
- findPr(
- branchName: string,
- prTitle: string | null,
- state?: string
- ): Promise<Pr>;
+ findPr(findPRConfig: FindPRConfig): Promise<Pr>;
mergeBranch(branchName: string): Promise<void>;
getBranchStatus(
branchName: string,
diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts
index 1d4d9ece81..6de0bf12cf 100644
--- a/lib/platform/github/index.ts
+++ b/lib/platform/github/index.ts
@@ -16,6 +16,7 @@ import {
CreatePRConfig,
EnsureIssueConfig,
BranchStatusConfig,
+ FindPRConfig,
EnsureCommentConfig,
} from '../common';
@@ -981,11 +982,11 @@ export async function getPrList(): Promise<Pr[]> {
return config.prList!;
}
-export async function findPr(
- branchName: string,
- prTitle?: string | null,
- state = 'all'
-): Promise<Pr | null> {
+export async function findPr({
+ branchName,
+ prTitle,
+ state = 'all',
+}: FindPRConfig): Promise<Pr | null> {
logger.debug(`findPr(${branchName}, ${prTitle}, ${state})`);
const prList = await getPrList();
const pr = prList.find(
@@ -1003,7 +1004,7 @@ export async function findPr(
// Returns the Pull Request for a branch. Null if not exists.
export async function getBranchPr(branchName: string): Promise<Pr | null> {
logger.debug(`getBranchPr(${branchName})`);
- const existingPr = await findPr(branchName, null, 'open');
+ const existingPr = await findPr({ branchName, state: 'open' });
return existingPr ? getPr(existingPr.number) : null;
}
diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts
index b76614abb8..59c8c5c56a 100644
--- a/lib/platform/gitlab/index.ts
+++ b/lib/platform/gitlab/index.ts
@@ -15,6 +15,7 @@ import {
CreatePRConfig,
EnsureIssueConfig,
BranchStatusConfig,
+ FindPRConfig,
EnsureCommentConfig,
} from '../common';
import { configFileNames } from '../../config/app-strings';
@@ -959,11 +960,11 @@ function matchesState(state: string, desiredState: string): boolean {
return state === desiredState;
}
-export async function findPr(
- branchName: string,
- prTitle?: string | null,
- state = 'all'
-): Promise<Pr> {
+export async function findPr({
+ branchName,
+ prTitle,
+ state = 'all',
+}: FindPRConfig): Promise<Pr> {
logger.debug(`findPr(${branchName}, ${prTitle}, ${state})`);
const prList = await getPrList();
return prList.find(
diff --git a/lib/workers/branch/check-existing.ts b/lib/workers/branch/check-existing.ts
index 8badbb1e41..9f6236090d 100644
--- a/lib/workers/branch/check-existing.ts
+++ b/lib/workers/branch/check-existing.ts
@@ -14,7 +14,11 @@ export async function prAlreadyExisted(
}
logger.debug('recreateClosed is false');
// Return if same PR already existed
- const pr = await platform.findPr(config.branchName, config.prTitle, '!open');
+ const pr = await platform.findPr({
+ branchName: config.branchName,
+ prTitle: config.prTitle,
+ state: '!open',
+ });
if (pr) {
logger.debug('Found closed PR with current title');
const prDetails = await platform.getPr(pr.number);
diff --git a/lib/workers/repository/finalise/prune.ts b/lib/workers/repository/finalise/prune.ts
index d78e4022e8..8736fdfe08 100644
--- a/lib/workers/repository/finalise/prune.ts
+++ b/lib/workers/repository/finalise/prune.ts
@@ -8,7 +8,10 @@ async function cleanUpBranches(
): Promise<void> {
for (const branchName of remainingBranches) {
try {
- const pr = await platform.findPr(branchName, null, 'open');
+ const pr = await platform.findPr({
+ branchName,
+ state: 'open',
+ });
const branchPr = await platform.getBranchPr(branchName);
const skipAutoclose = branchPr && branchPr.isModified;
if (pr && !skipAutoclose) {
diff --git a/lib/workers/repository/master-issue.ts b/lib/workers/repository/master-issue.ts
index 82296b31f4..e299813e86 100644
--- a/lib/workers/repository/master-issue.ts
+++ b/lib/workers/repository/master-issue.ts
@@ -179,11 +179,11 @@ export async function ensureMasterIssue(
issueBody +=
'These updates were closed unmerged and will not be recreated unless you click a checkbox below.\n\n';
for (const branch of alreadyExisted) {
- const pr = await platform.findPr(
- branch.branchName,
- branch.prTitle,
- '!open'
- );
+ const pr = await platform.findPr({
+ branchName: branch.branchName,
+ prTitle: branch.prTitle,
+ state: '!open',
+ });
issueBody += getListItem(branch, 'recreate', pr);
}
issueBody += '\n';
diff --git a/lib/workers/repository/onboarding/branch/check.ts b/lib/workers/repository/onboarding/branch/check.ts
index 8b047417d8..ce6f8a3554 100644
--- a/lib/workers/repository/onboarding/branch/check.ts
+++ b/lib/workers/repository/onboarding/branch/check.ts
@@ -35,7 +35,11 @@ const packageJsonConfigExists = async (): Promise<boolean> => {
export type Pr = any;
const closedPrExists = (config: RenovateConfig): Promise<Pr> =>
- platform.findPr(config.onboardingBranch, config.onboardingPrTitle, '!open');
+ platform.findPr({
+ branchName: config.onboardingBranch,
+ prTitle: config.onboardingPrTitle,
+ state: '!open',
+ });
export const isOnboarded = async (config: RenovateConfig): Promise<boolean> => {
logger.debug('isOnboarded()');
diff --git a/test/platform/azure/index.spec.ts b/test/platform/azure/index.spec.ts
index c75c41c0de..2e54f5dd89 100644
--- a/test/platform/azure/index.spec.ts
+++ b/test/platform/azure/index.spec.ts
@@ -228,7 +228,11 @@ describe('platform/azure', () => {
state: 'open',
} as any)
);
- const res = await azure.findPr('branch-a', 'branch a pr', 'open');
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'open',
+ });
expect(res).toMatchSnapshot();
});
it('returns pr if found not open', async () => {
@@ -260,7 +264,11 @@ describe('platform/azure', () => {
state: 'closed',
} as any)
);
- const res = await azure.findPr('branch-a', 'branch a pr', '!open');
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: '!open',
+ });
expect(res).toMatchSnapshot();
});
it('returns pr if found it close', async () => {
@@ -292,7 +300,11 @@ describe('platform/azure', () => {
state: 'closed',
} as any)
);
- const res = await azure.findPr('branch-a', 'branch a pr', 'closed');
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'closed',
+ });
expect(res).toMatchSnapshot();
});
it('returns pr if found it all state', async () => {
@@ -324,7 +336,10 @@ describe('platform/azure', () => {
state: 'closed',
} as any)
);
- const res = await azure.findPr('branch-a', 'branch a pr');
+ const res = await azure.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ });
expect(res).toMatchSnapshot();
});
});
diff --git a/test/platform/bitbucket-server/index.spec.ts b/test/platform/bitbucket-server/index.spec.ts
index 454f0092ef..3d7aafa449 100644
--- a/test/platform/bitbucket-server/index.spec.ts
+++ b/test/platform/bitbucket-server/index.spec.ts
@@ -481,7 +481,7 @@ describe('platform/bitbucket-server', () => {
expect.assertions(2);
await initRepo();
expect(
- await bitbucket.findPr('userName1/pullRequest1')
+ await bitbucket.findPr({ branchName: 'userName1/pullRequest1' })
).toBeUndefined();
expect(api.get.mock.calls).toMatchSnapshot();
});
@@ -492,7 +492,11 @@ describe('platform/bitbucket-server', () => {
expect.assertions(2);
await initRepo();
expect(
- await bitbucket.findPr('userName1/pullRequest5', 'title', 'open')
+ await bitbucket.findPr({
+ branchName: 'userName1/pullRequest5',
+ prTitle: 'title',
+ state: 'open',
+ })
).toMatchSnapshot();
expect(api.get.mock.calls).toMatchSnapshot();
});
@@ -500,7 +504,11 @@ describe('platform/bitbucket-server', () => {
expect.assertions(2);
await initRepo();
expect(
- await bitbucket.findPr('userName1/pullRequest5', 'title', 'closed')
+ await bitbucket.findPr({
+ branchName: 'userName1/pullRequest5',
+ prTitle: 'title',
+ state: 'closed',
+ })
).toBeUndefined();
expect(api.get.mock.calls).toMatchSnapshot();
});
diff --git a/test/platform/bitbucket/index.spec.ts b/test/platform/bitbucket/index.spec.ts
index 029ff0f820..62cd49fe92 100644
--- a/test/platform/bitbucket/index.spec.ts
+++ b/test/platform/bitbucket/index.spec.ts
@@ -389,7 +389,9 @@ describe('platform/bitbucket', () => {
it('finds pr', async () => {
await initRepo();
await mocked(async () => {
- expect(await bitbucket.findPr('branch', 'title')).toMatchSnapshot();
+ expect(
+ await bitbucket.findPr({ branchName: 'branch', prTitle: 'title' })
+ ).toMatchSnapshot();
});
});
});
diff --git a/test/platform/github/index.spec.ts b/test/platform/github/index.spec.ts
index 0ccc9bf7fb..dab774cb20 100644
--- a/test/platform/github/index.spec.ts
+++ b/test/platform/github/index.spec.ts
@@ -1415,7 +1415,9 @@ describe('platform/github', () => {
},
],
} as any);
- const res = await github.findPr('branch-a', null);
+ const res = await github.findPr({
+ branchName: 'branch-a',
+ });
expect(res).toBeDefined();
});
it('returns true if not open', async () => {
@@ -1429,7 +1431,10 @@ describe('platform/github', () => {
},
],
} as any);
- const res = await github.findPr('branch-a', null, '!open');
+ const res = await github.findPr({
+ branchName: 'branch-a',
+ state: '!open',
+ });
expect(res).toBeDefined();
});
it('caches pr list', async () => {
@@ -1443,13 +1448,20 @@ describe('platform/github', () => {
},
],
} as any);
- let res = await github.findPr('branch-a', null);
+ let res = await github.findPr({ branchName: 'branch-a' });
expect(res).toBeDefined();
- res = await github.findPr('branch-a', 'branch a pr');
+ res = await github.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ });
expect(res).toBeDefined();
- res = await github.findPr('branch-a', 'branch a pr', 'open');
+ res = await github.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'open',
+ });
expect(res).toBeDefined();
- res = await github.findPr('branch-b');
+ res = await github.findPr({ branchName: 'branch-b' });
expect(res).not.toBeDefined();
});
});
diff --git a/test/platform/gitlab/index.spec.ts b/test/platform/gitlab/index.spec.ts
index d547309b4b..1209c40c90 100644
--- a/test/platform/gitlab/index.spec.ts
+++ b/test/platform/gitlab/index.spec.ts
@@ -778,7 +778,9 @@ describe('platform/gitlab', () => {
},
],
} as any);
- const res = await gitlab.findPr('branch-a', null);
+ const res = await gitlab.findPr({
+ branchName: 'branch-a',
+ });
expect(res).toBeDefined();
});
it('returns true if not open', async () => {
@@ -792,7 +794,10 @@ describe('platform/gitlab', () => {
},
],
} as any);
- const res = await gitlab.findPr('branch-a', null, '!open');
+ const res = await gitlab.findPr({
+ branchName: 'branch-a',
+ state: '!open',
+ });
expect(res).toBeDefined();
});
@@ -807,7 +812,11 @@ describe('platform/gitlab', () => {
},
],
} as any);
- const res = await gitlab.findPr('branch-a', 'branch a pr', 'open');
+ const res = await gitlab.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ state: 'open',
+ });
expect(res).toBeDefined();
});
@@ -822,7 +831,10 @@ describe('platform/gitlab', () => {
},
],
} as any);
- const res = await gitlab.findPr('branch-a', 'branch a pr');
+ const res = await gitlab.findPr({
+ branchName: 'branch-a',
+ prTitle: 'branch a pr',
+ });
expect(res).toBeDefined();
});
});
diff --git a/test/workers/repository/master-issue.spec.ts b/test/workers/repository/master-issue.spec.ts
index cde7b50edf..e40ac92f52 100644
--- a/test/workers/repository/master-issue.spec.ts
+++ b/test/workers/repository/master-issue.spec.ts
@@ -349,12 +349,12 @@ describe('workers/repository/master-issue', () => {
);
expect(platform.getBranchPr).toHaveBeenCalledTimes(0);
expect(platform.findPr).toHaveBeenCalledTimes(2);
- expect(platform.findPr.mock.calls[0][0]).toBe('branchName1');
- expect(platform.findPr.mock.calls[0][1]).toBe('pr1');
- expect(platform.findPr.mock.calls[0][2]).toBe('!open');
- expect(platform.findPr.mock.calls[1][0]).toBe('branchName2');
- expect(platform.findPr.mock.calls[1][1]).toBe('pr2');
- expect(platform.findPr.mock.calls[1][2]).toBe('!open');
+ expect(platform.findPr.mock.calls[0][0].branchName).toBe('branchName1');
+ expect(platform.findPr.mock.calls[0][0].prTitle).toBe('pr1');
+ expect(platform.findPr.mock.calls[0][0].state).toBe('!open');
+ expect(platform.findPr.mock.calls[1][0].branchName).toBe('branchName2');
+ expect(platform.findPr.mock.calls[1][0].prTitle).toBe('pr2');
+ expect(platform.findPr.mock.calls[1][0].state).toBe('!open');
// same with dry run
await dryRun(branches, platform, 0, 0, 0, 2);
--
GitLab