From 8ddc944b9e76edcecc8f0f3f2c01667f000daf7c Mon Sep 17 00:00:00 2001 From: philipabed Date: Wed, 9 Nov 2022 11:58:00 +0200 Subject: [PATCH 01/22] if pr creation is not immediate, add another check that PR doesn't already exist for that branch --- lib/workers/repository/update/branch/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index ba1c42f328325f..4c92e1653469a5 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -559,13 +559,13 @@ export async function processBranch( !config.artifactErrors?.length && !userRebaseRequested && commitSha && + !branchPr && config.prCreation !== 'immediate' ) { logger.debug(`Branch status pending, current sha: ${commitSha}`); return { branchExists: true, updatesVerified, - prNo: branchPr?.number, result: BranchResult.Pending, commitSha, }; From 28ac082eef29ff33ee965fb55a87fd0aca3c7a72 Mon Sep 17 00:00:00 2001 From: philipabed Date: Thu, 10 Nov 2022 12:07:18 +0200 Subject: [PATCH 02/22] if pr creation is not immediate, add another check that PR doesn't already exist for that branch --- lib/workers/repository/update/branch/index.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 4c92e1653469a5..ef98d92a2ef288 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -553,15 +553,15 @@ export async function processBranch( await setStability(config); await setConfidence(config); - // break if we pushed a new commit because status check are pretty sure pending but maybe not reported yet - // but do not break when there are artifact errors - if ( + const skipCondition = !config.artifactErrors?.length && !userRebaseRequested && - commitSha && - !branchPr && - config.prCreation !== 'immediate' - ) { + config.prCreation !== 'immediate'; + + // new commit means status check are pretty sure pending but maybe not reported yet + // if PR has not been created + new commit + prCreation !== immediate skip + // but do not break when there are artifact errors + if (skipCondition && !branchPr && commitSha) { logger.debug(`Branch status pending, current sha: ${commitSha}`); return { branchExists: true, @@ -572,7 +572,9 @@ export async function processBranch( } // Try to automerge branch and finish if successful, but only if branch already existed before this run - if (branchExists || config.ignoreTests) { + // skip if we have a new commit while prCreation != immediate and there is an existing PR, + // we want to update the PR and skip the Auto merge since status checks aren't done yet + if (!(skipCondition && commitSha) && (branchExists || config.ignoreTests)) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { From 5a0ab45ac16692dbb534507294477c97e0f549a3 Mon Sep 17 00:00:00 2001 From: philipabed Date: Thu, 10 Nov 2022 12:35:21 +0200 Subject: [PATCH 03/22] if pr creation is not immediate, add another check that PR doesn't already exist for that branch --- lib/workers/repository/update/branch/index.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index ef98d92a2ef288..6b8e14545178fe 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -554,6 +554,7 @@ export async function processBranch( await setConfidence(config); const skipCondition = + commitSha && !config.artifactErrors?.length && !userRebaseRequested && config.prCreation !== 'immediate'; @@ -561,8 +562,8 @@ export async function processBranch( // new commit means status check are pretty sure pending but maybe not reported yet // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors - if (skipCondition && !branchPr && commitSha) { - logger.debug(`Branch status pending, current sha: ${commitSha}`); + if (skipCondition && !branchPr) { + logger.debug(`Branch status pending, current sha: ${commitSha!}`); return { branchExists: true, updatesVerified, @@ -574,7 +575,7 @@ export async function processBranch( // Try to automerge branch and finish if successful, but only if branch already existed before this run // skip if we have a new commit while prCreation != immediate and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet - if (!(skipCondition && commitSha) && (branchExists || config.ignoreTests)) { + if (!skipCondition && (branchExists || config.ignoreTests)) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { From f5a6da90fd8d901524dcf88b3b78c2c0b962ad9b Mon Sep 17 00:00:00 2001 From: philipabed Date: Thu, 10 Nov 2022 13:27:10 +0200 Subject: [PATCH 04/22] we should skip automerge on artifact errors and user rebase. --- lib/workers/repository/update/branch/index.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 6b8e14545178fe..b2a69731da22aa 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -553,16 +553,17 @@ export async function processBranch( await setStability(config); await setConfidence(config); - const skipCondition = - commitSha && - !config.artifactErrors?.length && - !userRebaseRequested && - config.prCreation !== 'immediate'; + const notImmediate = commitSha && config.prCreation !== 'immediate'; // new commit means status check are pretty sure pending but maybe not reported yet // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors - if (skipCondition && !branchPr) { + if ( + notImmediate && + !branchPr && + !config.artifactErrors?.length && + !userRebaseRequested + ) { logger.debug(`Branch status pending, current sha: ${commitSha!}`); return { branchExists: true, @@ -573,9 +574,14 @@ export async function processBranch( } // Try to automerge branch and finish if successful, but only if branch already existed before this run - // skip if we have a new commit while prCreation != immediate and there is an existing PR, + // skip if we have a non-immediate pr and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet - if (!skipCondition && (branchExists || config.ignoreTests)) { + if ( + !notImmediate && + !config.artifactErrors?.length && + !userRebaseRequested && + (branchExists || config.ignoreTests) + ) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { From 98b1b3008f9d4c41a7f20e4641ff429074042f2d Mon Sep 17 00:00:00 2001 From: philipabed Date: Thu, 10 Nov 2022 16:11:43 +0200 Subject: [PATCH 05/22] we should skip automerge on artifact errors and user rebase. --- lib/workers/repository/update/branch/index.ts | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index b2a69731da22aa..dae148ee8dd623 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -107,6 +107,10 @@ export async function processBranch( const dependencyDashboardCheck = config.dependencyDashboardChecks?.[config.branchName]; logger.debug(`dependencyDashboardCheck=${dependencyDashboardCheck!}`); + const userRebaseRequested = + dependencyDashboardCheck === 'rebase' || + !!config.dependencyDashboardRebaseAllOpen || + !!config.rebaseRequested; if (branchPr) { config.rebaseRequested = rebaseCheck(config, branchPr); logger.debug(`PR rebase requested=${config.rebaseRequested}`); @@ -381,10 +385,6 @@ export async function processBranch( } } - const userRebaseRequested = - dependencyDashboardCheck === 'rebase' || - !!config.dependencyDashboardRebaseAllOpen || - !!config.rebaseRequested; const userApproveAllPendingPR = !!config.dependencyDashboardAllPending; const userOpenAllRateLimtedPR = !!config.dependencyDashboardAllRateLimited; if (userRebaseRequested) { @@ -553,18 +553,17 @@ export async function processBranch( await setStability(config); await setConfidence(config); - const notImmediate = commitSha && config.prCreation !== 'immediate'; - // new commit means status check are pretty sure pending but maybe not reported yet // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors if ( - notImmediate && + commitSha && + config.prCreation !== 'immediate' && !branchPr && !config.artifactErrors?.length && !userRebaseRequested ) { - logger.debug(`Branch status pending, current sha: ${commitSha!}`); + logger.debug(`Branch status pending, current sha: ${commitSha}`); return { branchExists: true, updatesVerified, @@ -577,7 +576,7 @@ export async function processBranch( // skip if we have a non-immediate pr and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet if ( - !notImmediate && + !commitSha && !config.artifactErrors?.length && !userRebaseRequested && (branchExists || config.ignoreTests) @@ -820,13 +819,21 @@ export async function processBranch( } } else if (config.automerge) { logger.debug('PR is configured for automerge'); - const prAutomergeResult = await checkAutoMerge(pr, config); - if (prAutomergeResult?.automerged) { - return { - branchExists, - result: BranchResult.Automerged, - commitSha, - }; + if ( + !commitSha && + !config.artifactErrors?.length && + !userRebaseRequested + ) { + const prAutomergeResult = await checkAutoMerge(pr, config); + if (prAutomergeResult?.automerged) { + return { + branchExists, + result: BranchResult.Automerged, + commitSha, + }; + } + } else { + logger.debug('auto merge conditions are not met'); } } else { logger.debug('PR is not configured for automerge'); From 4b3cc448fdac5c23481a1d3f1f1f62f49d8248ab Mon Sep 17 00:00:00 2001 From: philipabed Date: Thu, 10 Nov 2022 22:14:33 +0200 Subject: [PATCH 06/22] we should skip automerge on artifact errors and user rebase. --- lib/workers/repository/update/branch/index.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index dae148ee8dd623..27328077e9604a 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -575,12 +575,7 @@ export async function processBranch( // Try to automerge branch and finish if successful, but only if branch already existed before this run // skip if we have a non-immediate pr and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet - if ( - !commitSha && - !config.artifactErrors?.length && - !userRebaseRequested && - (branchExists || config.ignoreTests) - ) { + if ((!commitSha && !config.artifactErrors?.length) || config.ignoreTests) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { @@ -820,9 +815,8 @@ export async function processBranch( } else if (config.automerge) { logger.debug('PR is configured for automerge'); if ( - !commitSha && - !config.artifactErrors?.length && - !userRebaseRequested + (!commitSha && !config.artifactErrors?.length) || + config.ignoreTests ) { const prAutomergeResult = await checkAutoMerge(pr, config); if (prAutomergeResult?.automerged) { From b0ff438f90c841c1b52f945eeceb1a2105fcb7f6 Mon Sep 17 00:00:00 2001 From: philipabed Date: Sun, 13 Nov 2022 12:26:55 +0200 Subject: [PATCH 07/22] we should skip automerge on artifact errors and if there's a new commit --- lib/workers/repository/update/branch/index.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 27328077e9604a..acdda6063bf0ae 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -575,7 +575,10 @@ export async function processBranch( // Try to automerge branch and finish if successful, but only if branch already existed before this run // skip if we have a non-immediate pr and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet - if ((!commitSha && !config.artifactErrors?.length) || config.ignoreTests) { + if ( + !config.artifactErrors?.length && + (!commitSha || (commitSha && config.ignoreTests)) + ) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { @@ -814,10 +817,9 @@ export async function processBranch( } } else if (config.automerge) { logger.debug('PR is configured for automerge'); - if ( - (!commitSha && !config.artifactErrors?.length) || - config.ignoreTests - ) { + if (commitSha) { + logger.debug('New commit detected, auto-merge PR skipped'); + } else { const prAutomergeResult = await checkAutoMerge(pr, config); if (prAutomergeResult?.automerged) { return { @@ -826,8 +828,6 @@ export async function processBranch( commitSha, }; } - } else { - logger.debug('auto merge conditions are not met'); } } else { logger.debug('PR is not configured for automerge'); From 47a049981355e08c0f256f3da777989d27e35826 Mon Sep 17 00:00:00 2001 From: philipabed Date: Sun, 13 Nov 2022 13:12:42 +0200 Subject: [PATCH 08/22] we should skip automerge on artifact errors and if there's a new commit --- lib/workers/repository/update/branch/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index acdda6063bf0ae..5309de21660ad7 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -107,10 +107,6 @@ export async function processBranch( const dependencyDashboardCheck = config.dependencyDashboardChecks?.[config.branchName]; logger.debug(`dependencyDashboardCheck=${dependencyDashboardCheck!}`); - const userRebaseRequested = - dependencyDashboardCheck === 'rebase' || - !!config.dependencyDashboardRebaseAllOpen || - !!config.rebaseRequested; if (branchPr) { config.rebaseRequested = rebaseCheck(config, branchPr); logger.debug(`PR rebase requested=${config.rebaseRequested}`); @@ -385,6 +381,10 @@ export async function processBranch( } } + const userRebaseRequested = + dependencyDashboardCheck === 'rebase' || + !!config.dependencyDashboardRebaseAllOpen || + !!config.rebaseRequested; const userApproveAllPendingPR = !!config.dependencyDashboardAllPending; const userOpenAllRateLimtedPR = !!config.dependencyDashboardAllRateLimited; if (userRebaseRequested) { From 7765635c1d9b8a13b04bfd51493a41b7818be823 Mon Sep 17 00:00:00 2001 From: philipabed Date: Sun, 13 Nov 2022 18:30:42 +0200 Subject: [PATCH 09/22] we should skip automerge on artifact errors and if there's a new commit --- lib/workers/repository/update/branch/index.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 5309de21660ad7..8811ac1fd39549 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -575,10 +575,7 @@ export async function processBranch( // Try to automerge branch and finish if successful, but only if branch already existed before this run // skip if we have a non-immediate pr and there is an existing PR, // we want to update the PR and skip the Auto merge since status checks aren't done yet - if ( - !config.artifactErrors?.length && - (!commitSha || (commitSha && config.ignoreTests)) - ) { + if (!config.artifactErrors?.length && (!commitSha || config.ignoreTests)) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { @@ -817,9 +814,8 @@ export async function processBranch( } } else if (config.automerge) { logger.debug('PR is configured for automerge'); - if (commitSha) { - logger.debug('New commit detected, auto-merge PR skipped'); - } else { + if (!commitSha || config.ignoreTests) { + logger.debug('checking auto-merge'); const prAutomergeResult = await checkAutoMerge(pr, config); if (prAutomergeResult?.automerged) { return { From 8c02a48b0ee27ba29e571b6c6bfe951ba29f9fbd Mon Sep 17 00:00:00 2001 From: philipabed Date: Mon, 14 Nov 2022 17:19:58 +0200 Subject: [PATCH 10/22] add unit tests --- .../repository/update/branch/index.spec.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 545e9a29e2bd56..fa67ecaeafc351 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1956,5 +1956,70 @@ describe('workers/repository/update/branch/index', () => { result: 'done', }); }); + + it('continues branch, skips automerge if there are artifact errors', async () => { + jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ + updatedPackageFiles: [{}], + artifactErrors: [{}], + } as PackageFilesResult); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + git.branchExists.mockReturnValue(true); + git.isBranchModified.mockResolvedValueOnce(true); + git.getBranchCommit.mockReturnValue('123test'); + platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); + const res = await branchWorker.processBranch(config); + expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); + expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled(); + expect(res).toEqual({ + branchExists: true, + commitSha: '123test', + prNo: undefined, + result: 'done', + updatesVerified: true, + }); + }); + + it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { + // schedule.isScheduledNow.mockReturnValueOnce(false); + git.branchExists.mockReturnValue(true); + git.isBranchModified.mockResolvedValueOnce(false); + git.getBranchCommit.mockReturnValue('123test'); + platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [partial({})], + } as WriteExistingFilesResult); + platform.getBranchPr.mockResolvedValueOnce({ + state: PrState.Open, + bodyStruct: { hash: hashBody(`- [ ] `) }, + } as Pr); + jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ + updatedPackageFiles: [{}], + } as PackageFilesResult); + jest.spyOn(prWorker, 'updatePrDebugData').mockReturnValueOnce({ + updatedInVer: '1.0.3', + createdInVer: '1.0.2', + }); + const inconfig = { + ...config, + ignoreTests: true, + prCreation: 'not-pending', + commitBody: '[skip-ci]', + fetchReleaseNotes: false, + } as BranchConfig; + expect(await branchWorker.processBranch(inconfig)).toEqual({ + branchExists: true, + updatesVerified: true, + prNo: undefined, + result: 'done', + commitSha: '123test', + }); + + expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0); + expect(prWorker.ensurePr).toHaveBeenCalledTimes(0); + }); }); }); From cf805b8bd141340d6c1103b94c07cb1e21dd5a26 Mon Sep 17 00:00:00 2001 From: philipabed Date: Mon, 14 Nov 2022 17:46:22 +0200 Subject: [PATCH 11/22] add unit tests --- lib/workers/repository/update/branch/index.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index fa67ecaeafc351..60ffcd11590504 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -2018,8 +2018,8 @@ describe('workers/repository/update/branch/index', () => { commitSha: '123test', }); - expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0); - expect(prWorker.ensurePr).toHaveBeenCalledTimes(0); + expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); + expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); }); }); }); From b7d6d8ce506c094d53d7fb2e1ebcc8d344d2467d Mon Sep 17 00:00:00 2001 From: philipabed Date: Mon, 14 Nov 2022 18:10:36 +0200 Subject: [PATCH 12/22] add unit tests --- lib/workers/repository/update/branch/index.spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 60ffcd11590504..8dabb6f04796c0 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -2005,10 +2005,7 @@ describe('workers/repository/update/branch/index', () => { }); const inconfig = { ...config, - ignoreTests: true, prCreation: 'not-pending', - commitBody: '[skip-ci]', - fetchReleaseNotes: false, } as BranchConfig; expect(await branchWorker.processBranch(inconfig)).toEqual({ branchExists: true, From 1964e356e7f86e41a9dbfda8b01cf83403a69283 Mon Sep 17 00:00:00 2001 From: philipabed Date: Mon, 14 Nov 2022 18:29:58 +0200 Subject: [PATCH 13/22] fix unit test --- lib/workers/repository/update/branch/index.spec.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 8dabb6f04796c0..02ab0cc6315e19 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1983,26 +1983,19 @@ describe('workers/repository/update/branch/index', () => { }); it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { - // schedule.isScheduledNow.mockReturnValueOnce(false); git.branchExists.mockReturnValue(true); git.isBranchModified.mockResolvedValueOnce(false); git.getBranchCommit.mockReturnValue('123test'); - platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], updatedArtifacts: [partial({})], } as WriteExistingFilesResult); platform.getBranchPr.mockResolvedValueOnce({ state: PrState.Open, - bodyStruct: { hash: hashBody(`- [ ] `) }, } as Pr); jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ updatedPackageFiles: [{}], } as PackageFilesResult); - jest.spyOn(prWorker, 'updatePrDebugData').mockReturnValueOnce({ - updatedInVer: '1.0.3', - createdInVer: '1.0.2', - }); const inconfig = { ...config, prCreation: 'not-pending', @@ -2014,7 +2007,6 @@ describe('workers/repository/update/branch/index', () => { result: 'done', commitSha: '123test', }); - expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); }); From ae8e04c556e709b95ca959c7cfdc0a532230272c Mon Sep 17 00:00:00 2001 From: philipabed Date: Tue, 15 Nov 2022 10:13:05 +0200 Subject: [PATCH 14/22] fix unit test --- lib/workers/repository/update/branch/index.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 02ab0cc6315e19..6072a15637d247 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1966,9 +1966,9 @@ describe('workers/repository/update/branch/index', () => { artifactErrors: [], updatedArtifacts: [], }); - git.branchExists.mockReturnValue(true); + git.branchExists.mockReturnValueOnce(true); git.isBranchModified.mockResolvedValueOnce(true); - git.getBranchCommit.mockReturnValue('123test'); + git.getBranchCommit.mockReturnValueOnce('123test'); platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); const res = await branchWorker.processBranch(config); expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); @@ -1983,9 +1983,9 @@ describe('workers/repository/update/branch/index', () => { }); it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { - git.branchExists.mockReturnValue(true); + git.branchExists.mockReturnValueOnce(true); git.isBranchModified.mockResolvedValueOnce(false); - git.getBranchCommit.mockReturnValue('123test'); + git.getBranchCommit.mockReturnValueOnce('123test'); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], updatedArtifacts: [partial({})], From af8a2aa44e313ec56337e9d769c5381ea92993e2 Mon Sep 17 00:00:00 2001 From: philipabed Date: Tue, 15 Nov 2022 11:35:48 +0200 Subject: [PATCH 15/22] fix unit test --- lib/workers/repository/update/branch/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 6072a15637d247..d699f93fc21347 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1983,7 +1983,7 @@ describe('workers/repository/update/branch/index', () => { }); it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { - git.branchExists.mockReturnValueOnce(true); + git.branchExists.mockReturnValue(true); git.isBranchModified.mockResolvedValueOnce(false); git.getBranchCommit.mockReturnValueOnce('123test'); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ From 20ac0aaa09390650753dc774d8d8f5e4a2c46026 Mon Sep 17 00:00:00 2001 From: philipabed Date: Tue, 15 Nov 2022 11:40:05 +0200 Subject: [PATCH 16/22] fix unit test --- .../repository/update/branch/index.spec.ts | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index d699f93fc21347..af385c707274e5 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1957,33 +1957,8 @@ describe('workers/repository/update/branch/index', () => { }); }); - it('continues branch, skips automerge if there are artifact errors', async () => { - jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ - updatedPackageFiles: [{}], - artifactErrors: [{}], - } as PackageFilesResult); - npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ - artifactErrors: [], - updatedArtifacts: [], - }); - git.branchExists.mockReturnValueOnce(true); - git.isBranchModified.mockResolvedValueOnce(true); - git.getBranchCommit.mockReturnValueOnce('123test'); - platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); - const res = await branchWorker.processBranch(config); - expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); - expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled(); - expect(res).toEqual({ - branchExists: true, - commitSha: '123test', - prNo: undefined, - result: 'done', - updatesVerified: true, - }); - }); - it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { - git.branchExists.mockReturnValue(true); + git.branchExists.mockReturnValueOnce(true); git.isBranchModified.mockResolvedValueOnce(false); git.getBranchCommit.mockReturnValueOnce('123test'); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ @@ -2010,5 +1985,30 @@ describe('workers/repository/update/branch/index', () => { expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); }); + + it('continues branch, skips automerge if there are artifact errors', async () => { + jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ + updatedPackageFiles: [{}], + artifactErrors: [{}], + } as PackageFilesResult); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + git.branchExists.mockReturnValueOnce(true); + git.isBranchModified.mockResolvedValueOnce(true); + git.getBranchCommit.mockReturnValueOnce('123test'); + platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); + const res = await branchWorker.processBranch(config); + expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); + expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled(); + expect(res).toEqual({ + branchExists: true, + commitSha: '123test', + prNo: undefined, + result: 'done', + updatesVerified: true, + }); + }); }); }); From d292fe9e599df72ad78bf435fe6f9676eca298fa Mon Sep 17 00:00:00 2001 From: philipabed Date: Tue, 15 Nov 2022 11:50:05 +0200 Subject: [PATCH 17/22] fix unit test --- .../repository/update/branch/index.spec.ts | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 6044012cf57092..4fec9d8003e16d 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1956,6 +1956,31 @@ describe('workers/repository/update/branch/index', () => { }); }); + it('continues branch, skips automerge if there are artifact errors', async () => { + jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ + updatedPackageFiles: [{}], + artifactErrors: [{}], + } as PackageFilesResult); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + git.branchExists.mockReturnValueOnce(true); + git.isBranchModified.mockResolvedValueOnce(true); + git.getBranchCommit.mockReturnValueOnce('123test'); + platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); + const res = await branchWorker.processBranch(config); + expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); + expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled(); + expect(res).toEqual({ + branchExists: true, + commitSha: '123test', + prNo: undefined, + result: 'done', + updatesVerified: true, + }); + }); + it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => { git.branchExists.mockReturnValueOnce(true); git.isBranchModified.mockResolvedValueOnce(false); @@ -1965,7 +1990,7 @@ describe('workers/repository/update/branch/index', () => { updatedArtifacts: [partial({})], } as WriteExistingFilesResult); platform.getBranchPr.mockResolvedValueOnce({ - state: PrState.Open, + state: 'open', } as Pr); jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ updatedPackageFiles: [{}], @@ -1984,30 +2009,5 @@ describe('workers/repository/update/branch/index', () => { expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); }); - - it('continues branch, skips automerge if there are artifact errors', async () => { - jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({ - updatedPackageFiles: [{}], - artifactErrors: [{}], - } as PackageFilesResult); - npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ - artifactErrors: [], - updatedArtifacts: [], - }); - git.branchExists.mockReturnValueOnce(true); - git.isBranchModified.mockResolvedValueOnce(true); - git.getBranchCommit.mockReturnValueOnce('123test'); - platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any); - const res = await branchWorker.processBranch(config); - expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled(); - expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled(); - expect(res).toEqual({ - branchExists: true, - commitSha: '123test', - prNo: undefined, - result: 'done', - updatesVerified: true, - }); - }); }); }); From 5f68f3eec238589c2fa02d6e6043adb01d35c8e8 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 16 Nov 2022 06:49:47 +0200 Subject: [PATCH 18/22] Restore ordering --- lib/workers/repository/update/branch/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 37c62b1d187111..f495e959958712 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -557,11 +557,11 @@ export async function processBranch( // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors if ( - commitSha && - config.prCreation !== 'immediate' && !branchPr && - !config.artifactErrors?.length && - !userRebaseRequested + !config.artifactErrors?.length && + !userRebaseRequested && + commitSha && + config.prCreation !== 'immediate' ) { logger.debug(`Branch status pending, current sha: ${commitSha}`); return { From 4ec4424e6da4c7b7474d578f5a336dcc42ad212b Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 16 Nov 2022 06:51:44 +0200 Subject: [PATCH 19/22] restore ordering --- lib/workers/repository/update/branch/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index f495e959958712..2401f16fd81c59 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -557,10 +557,10 @@ export async function processBranch( // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors if ( - !branchPr && !config.artifactErrors?.length && !userRebaseRequested && commitSha && + !branchPr && config.prCreation !== 'immediate' ) { logger.debug(`Branch status pending, current sha: ${commitSha}`); From 9c90e675e717e2c533ff5086758d386116bb38d2 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 16 Nov 2022 06:52:14 +0200 Subject: [PATCH 20/22] restore order --- lib/workers/repository/update/branch/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 2401f16fd81c59..29d7205f3688a6 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -557,10 +557,10 @@ export async function processBranch( // if PR has not been created + new commit + prCreation !== immediate skip // but do not break when there are artifact errors if ( - !config.artifactErrors?.length && + !branchPr && + !config.artifactErrors?.length && !userRebaseRequested && commitSha && - !branchPr && config.prCreation !== 'immediate' ) { logger.debug(`Branch status pending, current sha: ${commitSha}`); From 49731f782ca6336bc326619ef572fdb46ba4bef7 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Wed, 16 Nov 2022 07:39:41 +0200 Subject: [PATCH 21/22] Update lib/workers/repository/update/branch/index.ts Co-authored-by: Michael Kriese --- lib/workers/repository/update/branch/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 29d7205f3688a6..0d4ca5b4ef3356 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -814,7 +814,8 @@ export async function processBranch( } } else if (config.automerge) { logger.debug('PR is configured for automerge'); - if (!commitSha || config.ignoreTests) { + // skip automerge if there is a new commit since status checks aren't done yet + if (!config.artifactErrors?.length && (!commitSha || config.ignoreTests)) { logger.debug('checking auto-merge'); const prAutomergeResult = await checkAutoMerge(pr, config); if (prAutomergeResult?.automerged) { From 8ced9d6e516805e30314c61a2326d49f905a8a85 Mon Sep 17 00:00:00 2001 From: philipabed Date: Wed, 16 Nov 2022 09:30:59 +0200 Subject: [PATCH 22/22] minor fix --- lib/workers/repository/update/branch/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 0d4ca5b4ef3356..a82074da5ae30b 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -815,7 +815,7 @@ export async function processBranch( } else if (config.automerge) { logger.debug('PR is configured for automerge'); // skip automerge if there is a new commit since status checks aren't done yet - if (!config.artifactErrors?.length && (!commitSha || config.ignoreTests)) { + if (!commitSha || config.ignoreTests) { logger.debug('checking auto-merge'); const prAutomergeResult = await checkAutoMerge(pr, config); if (prAutomergeResult?.automerged) {