From 6c42022f78c9ff73983e95c97099bb0cc4442b27 Mon Sep 17 00:00:00 2001
From: Markus Schulz <msc@onesty-tech.de>
Date: Mon, 7 Aug 2023 17:39:43 +0200
Subject: [PATCH] refactor(scm): use scm for mergeBranch, mergeToLocal (#23448)
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
---
lib/modules/platform/default-scm.spec.ts | 12 +++++
lib/modules/platform/default-scm.ts | 8 ++++
lib/modules/platform/local/scm.spec.ts | 8 ++++
lib/modules/platform/local/scm.ts | 8 ++++
lib/modules/platform/types.ts | 2 +
lib/util/git/index.spec.ts | 18 ++++++--
lib/util/git/index.ts | 45 ++++++++++++++-----
.../onboarding/branch/index.spec.ts | 18 ++++----
.../repository/onboarding/branch/index.ts | 9 ++--
.../update/branch/automerge.spec.ts | 4 +-
.../repository/update/branch/automerge.ts | 3 +-
11 files changed, 101 insertions(+), 34 deletions(-)
diff --git a/lib/modules/platform/default-scm.spec.ts b/lib/modules/platform/default-scm.spec.ts
index 7c9f7128d8..2f97a3c4ef 100644
--- a/lib/modules/platform/default-scm.spec.ts
+++ b/lib/modules/platform/default-scm.spec.ts
@@ -60,4 +60,16 @@ describe('modules/platform/default-scm', () => {
await defaultGitScm.checkoutBranch('branchName');
expect(git.checkoutBranch).toHaveBeenCalledTimes(1);
});
+
+ it('delegate mergeAndPush to util/git', async () => {
+ git.mergeBranch.mockResolvedValueOnce();
+ await defaultGitScm.mergeAndPush('branchName');
+ expect(git.mergeBranch).toHaveBeenCalledWith('branchName');
+ });
+
+ it('delegate mergeBranch to util/git', async () => {
+ git.mergeToLocal.mockResolvedValueOnce();
+ await defaultGitScm.mergeToLocal('branchName');
+ expect(git.mergeToLocal).toHaveBeenCalledWith('branchName');
+ });
});
diff --git a/lib/modules/platform/default-scm.ts b/lib/modules/platform/default-scm.ts
index f210500a9a..92ceef5421 100644
--- a/lib/modules/platform/default-scm.ts
+++ b/lib/modules/platform/default-scm.ts
@@ -38,4 +38,12 @@ export class DefaultGitScm implements PlatformScm {
checkoutBranch(branchName: string): Promise<CommitSha> {
return git.checkoutBranch(branchName);
}
+
+ mergeAndPush(branchName: string): Promise<void> {
+ return git.mergeBranch(branchName);
+ }
+
+ mergeToLocal(branchName: string): Promise<void> {
+ return git.mergeToLocal(branchName);
+ }
}
diff --git a/lib/modules/platform/local/scm.spec.ts b/lib/modules/platform/local/scm.spec.ts
index 6549475ad5..92efed3cf5 100644
--- a/lib/modules/platform/local/scm.spec.ts
+++ b/lib/modules/platform/local/scm.spec.ts
@@ -65,4 +65,12 @@ describe('modules/platform/local/scm', () => {
expect(await localFs.getFileList()).toHaveLength(2);
});
});
+
+ it('mergeAndPush', async () => {
+ await expect(localFs.mergeAndPush('branchName')).resolves.toBeUndefined();
+ });
+
+ it('mergeBranch', async () => {
+ await expect(localFs.mergeToLocal('branchName')).resolves.toBeUndefined();
+ });
});
diff --git a/lib/modules/platform/local/scm.ts b/lib/modules/platform/local/scm.ts
index 5b01391572..51ba447610 100644
--- a/lib/modules/platform/local/scm.ts
+++ b/lib/modules/platform/local/scm.ts
@@ -48,4 +48,12 @@ export class LocalFs implements PlatformScm {
checkoutBranch(branchName: string): Promise<CommitSha> {
return Promise.resolve('');
}
+
+ mergeAndPush(branchName: string): Promise<void> {
+ return Promise.resolve();
+ }
+
+ mergeToLocal(branchName: string): Promise<void> {
+ return Promise.resolve();
+ }
}
diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts
index 47deb04e5f..3962a08b8d 100644
--- a/lib/modules/platform/types.ts
+++ b/lib/modules/platform/types.ts
@@ -235,4 +235,6 @@ export interface PlatformScm {
commitAndPush(commitConfig: CommitFilesConfig): Promise<CommitSha | null>;
getFileList(): Promise<string[]>;
checkoutBranch(branchName: string): Promise<CommitSha>;
+ mergeToLocal(branchName: string): Promise<void>;
+ mergeAndPush(branchName: string): Promise<void>;
}
diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts
index 7fddad6143..8d8c38225c 100644
--- a/lib/util/git/index.spec.ts
+++ b/lib/util/git/index.spec.ts
@@ -344,14 +344,24 @@ describe('util/git/index', () => {
expect(merged.all).toContain('renovate/future_branch');
});
- it('does not push if localOnly=true', async () => {
+ it('should throw if branch merge throws', async () => {
+ await expect(git.mergeBranch('not_found')).rejects.toThrow();
+ });
+ });
+
+ describe('mergeToLocal(branchName)', () => {
+ it('should perform a branch merge without push', async () => {
+ expect(fs.existsSync(`${tmpDir.path}/future_file`)).toBeFalse();
const pushSpy = jest.spyOn(SimpleGit.prototype, 'push');
- await git.mergeBranch('renovate/future_branch', true);
+
+ await git.mergeToLocal('renovate/future_branch');
+
+ expect(fs.existsSync(`${tmpDir.path}/future_file`)).toBeTrue();
expect(pushSpy).toHaveBeenCalledTimes(0);
});
- it('should throw if branch merge throws', async () => {
- await expect(git.mergeBranch('not_found')).rejects.toThrow();
+ it('should throw', async () => {
+ await expect(git.mergeToLocal('not_found')).rejects.toThrow();
});
});
diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts
index 81811c059d..34c85c9b7c 100644
--- a/lib/util/git/index.ts
+++ b/lib/util/git/index.ts
@@ -779,10 +779,38 @@ export async function deleteBranch(branchName: string): Promise<void> {
delete config.branchCommits[branchName];
}
-export async function mergeBranch(
- branchName: string,
- localOnly = false
-): Promise<void> {
+export async function mergeToLocal(refSpecToMerge: string): Promise<void> {
+ let status: StatusResult | undefined;
+ try {
+ await syncGit();
+ await writeGitAuthor();
+ await git.reset(ResetMode.HARD);
+ await gitRetry(() =>
+ git.checkout([
+ '-B',
+ config.currentBranch,
+ 'origin/' + config.currentBranch,
+ ])
+ );
+ status = await git.status();
+ await fetchRevSpec(refSpecToMerge);
+ await gitRetry(() => git.merge(['FETCH_HEAD']));
+ } catch (err) {
+ logger.debug(
+ {
+ baseBranch: config.currentBranch,
+ baseSha: config.currentBranchSha,
+ refSpecToMerge,
+ status,
+ err,
+ },
+ 'mergeLocally error'
+ );
+ throw err;
+ }
+}
+
+export async function mergeBranch(branchName: string): Promise<void> {
let status: StatusResult | undefined;
try {
await syncGit();
@@ -799,13 +827,8 @@ export async function mergeBranch(
])
);
status = await git.status();
- if (localOnly) {
- // merge commit, don't push to origin
- await gitRetry(() => git.merge([branchName]));
- } else {
- await gitRetry(() => git.merge(['--ff-only', branchName]));
- await gitRetry(() => git.push('origin', config.currentBranch));
- }
+ await gitRetry(() => git.merge(['--ff-only', branchName]));
+ await gitRetry(() => git.push('origin', config.currentBranch));
incLimitedValue('Commits');
} catch (err) {
logger.debug(
diff --git a/lib/workers/repository/onboarding/branch/index.spec.ts b/lib/workers/repository/onboarding/branch/index.spec.ts
index 926ab508f7..c739f57c44 100644
--- a/lib/workers/repository/onboarding/branch/index.spec.ts
+++ b/lib/workers/repository/onboarding/branch/index.spec.ts
@@ -259,7 +259,7 @@ describe('workers/repository/onboarding/branch/index', () => {
const res = await checkOnboardingBranch(config);
expect(res.repoIsOnboarded).toBeFalse();
expect(res.branchList).toEqual(['renovate/configure']);
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
});
@@ -287,7 +287,7 @@ describe('workers/repository/onboarding/branch/index', () => {
.mockReturnValueOnce('onboarding-sha');
config.onboardingRebaseCheckbox = true;
await checkOnboardingBranch(config);
- expect(git.mergeBranch).not.toHaveBeenCalled();
+ expect(scm.mergeToLocal).not.toHaveBeenCalled();
});
it('processes modified onboarding branch and invalidates extract cache', async () => {
@@ -312,7 +312,7 @@ describe('workers/repository/onboarding/branch/index', () => {
onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(false);
config.baseBranch = 'master';
await checkOnboardingBranch(config);
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
'default-sha',
'new-onboarding-sha',
@@ -336,7 +336,7 @@ describe('workers/repository/onboarding/branch/index', () => {
onboardingCache.hasOnboardingBranchChanged.mockReturnValueOnce(true);
onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(true);
await checkOnboardingBranch(config);
- expect(git.mergeBranch).not.toHaveBeenCalled();
+ expect(scm.mergeToLocal).not.toHaveBeenCalled();
expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
'default-sha',
'onboarding-sha',
@@ -354,7 +354,7 @@ describe('workers/repository/onboarding/branch/index', () => {
.mockReturnValueOnce('onboarding-sha');
onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(false);
await checkOnboardingBranch(config);
- expect(git.mergeBranch).toHaveBeenCalled();
+ expect(scm.mergeToLocal).toHaveBeenCalled();
expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith(
'default-sha',
'onboarding-sha',
@@ -383,7 +383,7 @@ describe('workers/repository/onboarding/branch/index', () => {
`Platform '${pl}' does not support extended markdown`
);
expect(OnboardingState.prUpdateRequested).toBeTrue();
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
});
@@ -397,7 +397,7 @@ describe('workers/repository/onboarding/branch/index', () => {
`No rebase checkbox was found in the onboarding PR`
);
expect(OnboardingState.prUpdateRequested).toBeTrue();
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
});
@@ -411,7 +411,7 @@ describe('workers/repository/onboarding/branch/index', () => {
`Manual onboarding PR update requested`
);
expect(OnboardingState.prUpdateRequested).toBeTrue();
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
});
@@ -422,7 +422,7 @@ describe('workers/repository/onboarding/branch/index', () => {
await checkOnboardingBranch(config);
expect(OnboardingState.prUpdateRequested).toBeFalse();
- expect(git.mergeBranch).toHaveBeenCalledOnce();
+ expect(scm.mergeToLocal).toHaveBeenCalledOnce();
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
});
});
diff --git a/lib/workers/repository/onboarding/branch/index.ts b/lib/workers/repository/onboarding/branch/index.ts
index c2031ab1ef..af7c2cd71d 100644
--- a/lib/workers/repository/onboarding/branch/index.ts
+++ b/lib/workers/repository/onboarding/branch/index.ts
@@ -8,12 +8,9 @@ import {
} from '../../../../constants/error-messages';
import { logger } from '../../../../logger';
import type { Pr } from '../../../../modules/platform';
+import { scm } from '../../../../modules/platform/scm';
import { getCache } from '../../../../util/cache/repository';
-import {
- getBranchCommit,
- mergeBranch,
- setGitAuthor,
-} from '../../../../util/git';
+import { getBranchCommit, setGitAuthor } from '../../../../util/git';
import { extractAllDependencies } from '../../extract';
import { mergeRenovateConfig } from '../../init/merge';
import { OnboardingState } from '../common';
@@ -114,7 +111,7 @@ export async function checkOnboardingBranch(
// TODO #7154
if (!isConflicted) {
logger.debug('Merge onboarding branch in default branch');
- await mergeBranch(onboardingBranch!, true);
+ await scm.mergeToLocal(onboardingBranch!);
}
}
setOnboardingCache(
diff --git a/lib/workers/repository/update/branch/automerge.spec.ts b/lib/workers/repository/update/branch/automerge.spec.ts
index c675d1619d..3abe8d0dd5 100644
--- a/lib/workers/repository/update/branch/automerge.spec.ts
+++ b/lib/workers/repository/update/branch/automerge.spec.ts
@@ -1,4 +1,4 @@
-import { git, partial, platform, scm } from '../../../../../test/util';
+import { partial, platform, scm } from '../../../../../test/util';
import { GlobalConfig } from '../../../../config/global';
import type { RenovateConfig } from '../../../../config/types';
import type { Pr } from '../../../../modules/platform/types';
@@ -64,7 +64,7 @@ describe('workers/repository/update/branch/automerge', () => {
config.automergeType = 'branch';
config.baseBranch = 'test-branch';
platform.getBranchStatus.mockResolvedValueOnce('green');
- git.mergeBranch.mockImplementationOnce(() => {
+ scm.mergeAndPush.mockImplementationOnce(() => {
throw new Error('merge error');
});
diff --git a/lib/workers/repository/update/branch/automerge.ts b/lib/workers/repository/update/branch/automerge.ts
index b6595e2ec1..e355a4cf14 100644
--- a/lib/workers/repository/update/branch/automerge.ts
+++ b/lib/workers/repository/update/branch/automerge.ts
@@ -3,7 +3,6 @@ import type { RenovateConfig } from '../../../../config/types';
import { logger } from '../../../../logger';
import { platform } from '../../../../modules/platform';
import { scm } from '../../../../modules/platform/scm';
-import { mergeBranch } from '../../../../util/git';
import { isScheduledNow } from './schedule';
import { resolveBranchStatus } from './status-checks';
@@ -44,7 +43,7 @@ export async function tryBranchAutomerge(
logger.info(`DRY-RUN: Would automerge branch ${config.branchName!}`);
} else {
await scm.checkoutBranch(config.baseBranch!);
- await mergeBranch(config.branchName!);
+ await scm.mergeAndPush(config.branchName!);
}
logger.info({ branch: config.branchName }, 'Branch automerged');
return 'automerged'; // Branch no longer exists
--
GitLab