Skip to content

Commit 7cf85c2

Browse files
fix: don't reuse branch if results change (renovatebot#37083)
Co-authored-by: Rhys Arkins <rhys@arkins.net>
1 parent f5a374a commit 7cf85c2

File tree

2 files changed

+122
-1
lines changed

2 files changed

+122
-1
lines changed

lib/workers/repository/update/branch/index.spec.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,38 @@ describe('workers/repository/update/branch/index', () => {
933933
expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
934934
});
935935

936+
it('updates branch when forceRebase=true', async () => {
937+
expect.assertions(3);
938+
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
939+
partial<PackageFilesResult>({
940+
updatedPackageFiles: [partial<FileChange>()],
941+
}),
942+
);
943+
npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
944+
artifactErrors: [],
945+
updatedArtifacts: [partial<FileChange>()],
946+
});
947+
const inconfig = {
948+
...config,
949+
ignoreTests: true,
950+
prCreation: 'not-pending',
951+
commitBody: '[skip-ci]',
952+
fetchChangeLogs: 'branch',
953+
cacheFingerprintMatch: 'no-match',
954+
} satisfies BranchConfig;
955+
scm.getBranchCommit.mockResolvedValue('123test' as LongCommitSha); //TODO:not needed?
956+
expect(await branchWorker.processBranch(inconfig, true)).toEqual({
957+
branchExists: true,
958+
updatesVerified: true,
959+
prNo: undefined,
960+
result: 'pending',
961+
commitSha: '123test',
962+
});
963+
964+
expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0);
965+
expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
966+
});
967+
936968
it('ensures PR and comments notice', async () => {
937969
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
938970
partial<PackageFilesResult>({
@@ -1350,6 +1382,79 @@ describe('workers/repository/update/branch/index', () => {
13501382
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
13511383
});
13521384

1385+
it('rebases branch onto new basebranch if no fingerprint found', async () => {
1386+
const updatedPackageFile: FileChange = {
1387+
type: 'addition',
1388+
path: 'pom.xml',
1389+
contents: 'pom.xml file contents',
1390+
};
1391+
const pr = partial<Pr>({
1392+
state: 'open',
1393+
targetBranch: 'old_base',
1394+
bodyStruct: partial<PrBodyStruct>({
1395+
debugData: partial<PrDebugData>({ targetBranch: 'old_base' }),
1396+
}),
1397+
});
1398+
getUpdated.getUpdatedPackageFiles.mockResolvedValue(
1399+
partial<PackageFilesResult>({
1400+
updatedPackageFiles: [updatedPackageFile],
1401+
}),
1402+
);
1403+
npmPostExtract.getAdditionalFiles.mockResolvedValue({
1404+
artifactErrors: [partial<ArtifactError>()],
1405+
updatedArtifacts: [partial<FileChange>()],
1406+
});
1407+
scm.branchExists.mockResolvedValue(true);
1408+
commit.commitFilesToBranch.mockResolvedValueOnce(null);
1409+
platform.getBranchPr.mockResolvedValue(pr);
1410+
await branchWorker.processBranch({
1411+
...config,
1412+
baseBranch: 'old_base',
1413+
reuseExistingBranch: true,
1414+
cacheFingerprintMatch: 'no-fingerprint',
1415+
});
1416+
expect(logger.debug).toHaveBeenCalledWith(
1417+
'Existing branch needs updating. Restarting processBranch() with a clean branch',
1418+
);
1419+
expect(commit.commitFilesToBranch).toHaveBeenCalled();
1420+
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
1421+
});
1422+
1423+
// for coverage
1424+
it('rebases branch onto new basebranch if no fingerprint found - 2', async () => {
1425+
const pr = partial<Pr>({
1426+
state: 'open',
1427+
targetBranch: 'old_base',
1428+
bodyStruct: partial<PrBodyStruct>({
1429+
debugData: partial<PrDebugData>({ targetBranch: 'old_base' }),
1430+
}),
1431+
});
1432+
getUpdated.getUpdatedPackageFiles.mockResolvedValue(
1433+
partial<PackageFilesResult>({
1434+
updatedPackageFiles: [],
1435+
}),
1436+
);
1437+
npmPostExtract.getAdditionalFiles.mockResolvedValue({
1438+
artifactErrors: [partial<ArtifactError>()],
1439+
updatedArtifacts: [partial<FileChange>()],
1440+
});
1441+
scm.branchExists.mockResolvedValue(true);
1442+
commit.commitFilesToBranch.mockResolvedValueOnce(null);
1443+
platform.getBranchPr.mockResolvedValue(pr);
1444+
await branchWorker.processBranch({
1445+
...config,
1446+
baseBranch: 'old_base',
1447+
reuseExistingBranch: true,
1448+
updatedArtifacts: [{ type: 'deletion', path: 'dummy' }],
1449+
cacheFingerprintMatch: 'no-fingerprint',
1450+
});
1451+
expect(logger.debug).toHaveBeenCalledWith(
1452+
'Existing branch needs updating. Restarting processBranch() with a clean branch',
1453+
);
1454+
expect(commit.commitFilesToBranch).toHaveBeenCalled();
1455+
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
1456+
});
1457+
13531458
it('swallows pr errors', async () => {
13541459
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
13551460
partial<PackageFilesResult>({

lib/workers/repository/update/branch/index.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export interface ProcessBranchResult {
117117

118118
export async function processBranch(
119119
branchConfig: BranchConfig,
120+
forceRebase = false,
120121
): Promise<ProcessBranchResult> {
121122
let commitSha: string | null = null;
122123
let config: BranchConfig = { ...branchConfig };
@@ -437,7 +438,10 @@ export async function processBranch(
437438
!!config.rebaseRequested;
438439
const userApproveAllPendingPR = !!config.dependencyDashboardAllPending;
439440
const userOpenAllRateLimtedPR = !!config.dependencyDashboardAllRateLimited;
440-
if (userRebaseRequested) {
441+
if (forceRebase) {
442+
logger.debug('Force rebase because branch needs updating');
443+
config.reuseExistingBranch = false;
444+
} else if (userRebaseRequested) {
441445
logger.debug('User has requested rebase');
442446
config.reuseExistingBranch = false;
443447
} else if (dependencyDashboardCheck === 'global-config') {
@@ -501,6 +505,12 @@ export async function processBranch(
501505
logger.debug(
502506
`Updated ${config.updatedPackageFiles.length} package files`,
503507
);
508+
if (config.reuseExistingBranch && !forceRebase) {
509+
logger.debug(
510+
'Existing branch needs updating. Restarting processBranch() with a clean branch',
511+
);
512+
return processBranch(branchConfig, true);
513+
}
504514
} else {
505515
logger.debug('No package files need updating');
506516
}
@@ -523,6 +533,12 @@ export async function processBranch(
523533
},
524534
`Updated ${config.updatedArtifacts.length} lock files`,
525535
);
536+
if (config.reuseExistingBranch && !forceRebase) {
537+
logger.debug(
538+
'Existing branch needs updating. Restarting processBranch() with a clean branch',
539+
);
540+
return processBranch(branchConfig, true);
541+
}
526542
} else {
527543
logger.debug('No updated lock files in branch');
528544
}

0 commit comments

Comments
 (0)