diff --git a/lib/workers/repository/update/branch/commit.ts b/lib/workers/repository/update/branch/commit.ts index 78ccab0d79abce0916306354ba0a2b21bb31f9ce..827d03c5ee77b424679e0a00a881eeff23cd0c3b 100644 --- a/lib/workers/repository/update/branch/commit.ts +++ b/lib/workers/repository/update/branch/commit.ts @@ -55,7 +55,7 @@ export function commitFilesToBranch( branchName: config.branchName, files: updatedFiles, message: config.commitMessage!, - force: config.recreateMergedPr ?? !!config.forceCommit, + force: !!config.forceCommit, platformCommit: !!config.platformCommit, }); } diff --git a/lib/workers/repository/update/branch/handle-existing.ts b/lib/workers/repository/update/branch/handle-existing.ts index c031be82fa45137220e7196e689f8722dcc37ff8..735ce2b242a56a279c39872eca6018ca58c83ddc 100644 --- a/lib/workers/repository/update/branch/handle-existing.ts +++ b/lib/workers/repository/update/branch/handle-existing.ts @@ -41,5 +41,7 @@ export async function handlepr(config: BranchConfig, pr: Pr): Promise<void> { await deleteBranch(config.branchName); } } + } else if (pr.state === PrState.Merged) { + logger.debug({ pr: pr.number }, 'Merged PR is blocking this branch'); } } diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index e2b5dbc6c5c1af81efd845bd84b7543686558e65..4da4fa0c7ddd90b00d82d5df71c7f0c8204a41e1 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -281,81 +281,22 @@ describe('workers/repository/update/branch/index', () => { expect(reuse.shouldReuseExistingBranch).toHaveBeenCalledTimes(0); }); - it('recreates pr using old branch when it exists for a merged pr', async () => { - schedule.isScheduledNow.mockReturnValueOnce(true); + it('skips branch if merged PR found', async () => { + schedule.isScheduledNow.mockReturnValueOnce(false); git.branchExists.mockReturnValue(true); checkExisting.prAlreadyExisted.mockResolvedValueOnce({ number: 13, state: PrState.Merged, } as Pr); - git.getBranchCommit.mockReturnValue('target_branch_sha'); - getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ - updatedPackageFiles: [{}], - } as PackageFilesResult); - npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ - artifactErrors: [{}], - updatedArtifacts: [{}], - } as WriteExistingFilesResult); - platform.getBranchPr.mockResolvedValueOnce({ - state: PrState.Open, - } as Pr); - checkExisting.prAlreadyExisted.mockResolvedValueOnce({ - state: PrState.Merged, - } as Pr); - config.automerge = true; - const processBranchResult = await branchWorker.processBranch(config); - expect(processBranchResult).toEqual({ - branchExists: true, - commitSha: '123test', - prNo: undefined, - result: 'done', - }); - expect(logger.debug).not.toHaveBeenCalledWith( - { prTitle: config.prTitle }, - 'Merged PR already exists. Creating a fresh PR' - ); - expect(logger.debug).toHaveBeenCalledWith( - 'Disable automerge to prevent re-created PRs from being merged automatically.' - ); - }); - - it('recreates pr using new branch when merged pr exists but branch is deleted', async () => { - schedule.isScheduledNow.mockReturnValueOnce(true); - git.branchExists.mockReturnValue(false); - checkExisting.prAlreadyExisted.mockResolvedValueOnce({ - number: 13, - state: PrState.Merged, - } as Pr); - getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ - updatedPackageFiles: [{}], - } as PackageFilesResult); - npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ - artifactErrors: [{}], - updatedArtifacts: [{}], - } as WriteExistingFilesResult); - git.getBranchCommit.mockReturnValue('target_branch_sha'); - platform.getBranchPr.mockResolvedValueOnce(null); - checkExisting.prAlreadyExisted.mockResolvedValueOnce({ - state: PrState.Merged, - } as Pr); - const processBranchResult = await branchWorker.processBranch(config); - expect(processBranchResult).toEqual({ - branchExists: true, - commitSha: '123test', - prNo: undefined, - result: 'pr-created', - }); - expect(logger.debug).toHaveBeenCalledWith( - { prTitle: config.prTitle }, - 'Merged PR already exists. Creating a fresh PR' - ); + await branchWorker.processBranch(config); + expect(reuse.shouldReuseExistingBranch).toHaveBeenCalledTimes(0); }); it('throws error if closed PR found', async () => { schedule.isScheduledNow.mockReturnValueOnce(false); git.branchExists.mockReturnValue(true); platform.getBranchPr.mockResolvedValueOnce({ - state: PrState.Closed, + state: PrState.Merged, } as Pr); git.isBranchModified.mockResolvedValueOnce(true); await expect(branchWorker.processBranch(config)).rejects.toThrow( diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 57b09429534f2769cb45e7bc1e852ebfb092f487..a0692ecae05e96bfc5768009b0aa6a36115792da 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -83,18 +83,6 @@ export interface ProcessBranchResult { commitSha?: string | null; } -interface RecreateMergedPrConfig { - recreateMergedPr: boolean; - automerge: boolean; -} - -function recreateMergedPrConfig(branchPr: Pr | null): RecreateMergedPrConfig { - return { - recreateMergedPr: !branchPr, - automerge: false, - }; -} - export async function processBranch( branchConfig: BranchConfig ): Promise<ProcessBranchResult> { @@ -126,37 +114,19 @@ export async function processBranch( const artifactErrorTopic = emojify(':warning: Artifact update problem'); try { // Check if branch already existed - const existingPr = await prAlreadyExisted(config); + const existingPr = branchPr ? undefined : await prAlreadyExisted(config); if (existingPr && !dependencyDashboardCheck) { - // Recreates pr if merged pr already exists - if (existingPr.state === 'merged') { - if (config.automerge) { - logger.debug( - 'Disable automerge to prevent re-created PRs from being merged automatically.' - ); - } - - config = { ...config, ...recreateMergedPrConfig(branchPr) }; - if (!branchPr) { - logger.debug( - { prTitle: config.prTitle }, - 'Merged PR already exists. Creating a fresh PR' - ); - } - } else { - logger.debug( - { prTitle: config.prTitle }, - 'Closed PR already exists. Skipping branch.' - ); - await handlepr(config, existingPr); - return { - branchExists: false, - prNo: existingPr.number, - result: BranchResult.AlreadyExisted, - }; - } + logger.debug( + { prTitle: config.prTitle }, + 'Closed PR already exists. Skipping branch.' + ); + await handlepr(config, existingPr); + return { + branchExists: false, + prNo: existingPr.number, + result: BranchResult.AlreadyExisted, + }; } - // istanbul ignore if if (!branchExists && config.dependencyDashboardApproval) { if (dependencyDashboardCheck) { @@ -415,11 +385,6 @@ export async function processBranch( prNo: branchPr?.number, result: BranchResult.NoWork, }; - } else if (config.recreateMergedPr) { - logger.debug( - 'Rebase branch cause the commits from old branch have already been merged.' - ); - config.reuseExistingBranch = false; } else { config = { ...config, ...(await shouldReuseExistingBranch(config)) }; } diff --git a/lib/workers/types.ts b/lib/workers/types.ts index ade345c7c3e4b80753e57141fad14410fc18b28a..1da19fe992cae3caade42f0af9e1adeab3b150a1 100644 --- a/lib/workers/types.ts +++ b/lib/workers/types.ts @@ -126,7 +126,6 @@ export interface BranchConfig prNo?: number; stopUpdating?: boolean; isConflicted?: boolean; - recreateMergedPr?: boolean; branchFingerprint?: string; skipBranchUpdate?: boolean; }