refactor(scm): use scm for mergeBranch, mergeToLocal (#23448)

Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
This commit is contained in:
Markus Schulz 2023-08-07 17:39:43 +02:00 committed by GitHub
parent 0d579fdcfc
commit 6c42022f78
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 101 additions and 34 deletions

View file

@ -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');
});
});

View file

@ -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);
}
}

View file

@ -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();
});
});

View file

@ -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();
}
}

View file

@ -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>;
}

View file

@ -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();
});
});

View file

@ -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));
}
incLimitedValue('Commits');
} catch (err) {
logger.debug(

View file

@ -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);
});
});

View file

@ -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(

View file

@ -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');
});

View file

@ -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