From 0424518db20bb4f715ba34a99327d66d5abde5f0 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov <zharinov@users.noreply.github.com> Date: Fri, 28 Jan 2022 16:50:10 +0300 Subject: [PATCH] refactor(git): Delegate commit function to platform (#13823) --- lib/config/types.ts | 1 + lib/platform/commit.ts | 11 +++++++ lib/platform/types.ts | 2 ++ lib/util/git/types.ts | 1 + .../branch/__snapshots__/commit.spec.ts.snap | 21 ++++++++++++ lib/workers/branch/commit.spec.ts | 14 +++++++- lib/workers/branch/commit.ts | 6 ++-- .../onboarding/branch/create.spec.ts | 33 +++++++++++++++++-- .../repository/onboarding/branch/create.ts | 6 ++-- .../onboarding/branch/rebase.spec.ts | 14 +++++++- .../repository/onboarding/branch/rebase.ts | 12 +++---- 11 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 lib/platform/commit.ts diff --git a/lib/config/types.ts b/lib/config/types.ts index e102ae643c..e60ac6535b 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -67,6 +67,7 @@ export interface RenovateSharedConfig { timezone?: string; unicodeEmoji?: boolean; gitIgnoredAuthors?: string[]; + platformCommit?: boolean; } // Config options used only within the global worker diff --git a/lib/platform/commit.ts b/lib/platform/commit.ts new file mode 100644 index 0000000000..596e038b45 --- /dev/null +++ b/lib/platform/commit.ts @@ -0,0 +1,11 @@ +import { commitFiles } from '../util/git'; +import type { CommitFilesConfig, CommitSha } from '../util/git/types'; +import { platform } from '.'; + +export function commitAndPush( + commitConfig: CommitFilesConfig +): Promise<CommitSha | null> { + return commitConfig.platformCommit && platform.commitFiles + ? platform.commitFiles(commitConfig) + : commitFiles(commitConfig); +} diff --git a/lib/platform/types.ts b/lib/platform/types.ts index b1b3490dac..4c2be6278b 100644 --- a/lib/platform/types.ts +++ b/lib/platform/types.ts @@ -1,5 +1,6 @@ import type { MergeStrategy } from '../config/types'; import type { BranchStatus, PrState, VulnerabilityAlert } from '../types'; +import type { CommitFilesConfig, CommitSha } from '../util/git/types'; type VulnerabilityKey = string; type VulnerabilityRangeKey = string; @@ -195,4 +196,5 @@ export interface Platform { getBranchPr(branchName: string): Promise<Pr | null>; initPlatform(config: PlatformParams): Promise<PlatformResult>; filterUnavailableUsers?(users: string[]): Promise<string[]>; + commitFiles?(config: CommitFilesConfig): Promise<CommitSha | null>; } diff --git a/lib/util/git/types.ts b/lib/util/git/types.ts index c2e1519a66..42b29df65b 100644 --- a/lib/util/git/types.ts +++ b/lib/util/git/types.ts @@ -73,6 +73,7 @@ export interface CommitFilesConfig { files: FileChange[]; message: string; force?: boolean; + platformCommit?: boolean; } export type BranchName = string; diff --git a/lib/workers/branch/__snapshots__/commit.spec.ts.snap b/lib/workers/branch/__snapshots__/commit.spec.ts.snap index 1713c62d56..8925d1a4fa 100644 --- a/lib/workers/branch/__snapshots__/commit.spec.ts.snap +++ b/lib/workers/branch/__snapshots__/commit.spec.ts.snap @@ -14,6 +14,27 @@ Array [ ], "force": false, "message": "some commit message", + "platformCommit": false, + }, + ], +] +`; + +exports[`workers/branch/commit commitFilesToBranch commits via platform 1`] = ` +Array [ + Array [ + Object { + "branchName": "renovate/some-branch", + "files": Array [ + Object { + "contents": "some contents", + "path": "package.json", + "type": "addition", + }, + ], + "force": false, + "message": "some commit message", + "platformCommit": true, }, ], ] diff --git a/lib/workers/branch/commit.spec.ts b/lib/workers/branch/commit.spec.ts index b326976ded..346a883043 100644 --- a/lib/workers/branch/commit.spec.ts +++ b/lib/workers/branch/commit.spec.ts @@ -1,4 +1,4 @@ -import { defaultConfig, git, partial } from '../../../test/util'; +import { defaultConfig, git, partial, platform } from '../../../test/util'; import { GlobalConfig } from '../../config/global'; import type { BranchConfig } from '../types'; import { commitFilesToBranch } from './commit'; @@ -21,6 +21,7 @@ describe('workers/branch/commit', () => { }); jest.resetAllMocks(); git.commitFiles.mockResolvedValueOnce('123test'); + platform.commitFiles = jest.fn(); GlobalConfig.reset(); }); it('handles empty files', async () => { @@ -37,6 +38,17 @@ describe('workers/branch/commit', () => { expect(git.commitFiles).toHaveBeenCalledTimes(1); expect(git.commitFiles.mock.calls).toMatchSnapshot(); }); + it('commits via platform', async () => { + config.updatedPackageFiles.push({ + type: 'addition', + path: 'package.json', + contents: 'some contents', + }); + config.platformCommit = true; + await commitFilesToBranch(config); + expect(platform.commitFiles).toHaveBeenCalledTimes(1); + expect(platform.commitFiles.mock.calls).toMatchSnapshot(); + }); it('dry runs', async () => { GlobalConfig.set({ dryRun: true }); config.updatedPackageFiles.push({ diff --git a/lib/workers/branch/commit.ts b/lib/workers/branch/commit.ts index 4734ec057f..0295c72193 100644 --- a/lib/workers/branch/commit.ts +++ b/lib/workers/branch/commit.ts @@ -3,7 +3,7 @@ import minimatch from 'minimatch'; import { GlobalConfig } from '../../config/global'; import { CONFIG_SECRETS_EXPOSED } from '../../constants/error-messages'; import { logger } from '../../logger'; -import { commitFiles } from '../../util/git'; +import { commitAndPush } from '../../platform/commit'; import { sanitize } from '../../util/sanitize'; import type { BranchConfig } from '../types'; @@ -46,11 +46,13 @@ export function commitFilesToBranch( ); throw new Error(CONFIG_SECRETS_EXPOSED); } + // API will know whether to create new branch or not - return commitFiles({ + return commitAndPush({ branchName: config.branchName, files: updatedFiles, message: config.commitMessage, force: !!config.forceCommit, + platformCommit: !!config.platformCommit, }); } diff --git a/lib/workers/repository/onboarding/branch/create.spec.ts b/lib/workers/repository/onboarding/branch/create.spec.ts index 2fe405269a..a1467eb1db 100644 --- a/lib/workers/repository/onboarding/branch/create.spec.ts +++ b/lib/workers/repository/onboarding/branch/create.spec.ts @@ -1,4 +1,4 @@ -import { RenovateConfig, getConfig } from '../../../../../test/util'; +import { RenovateConfig, getConfig, platform } from '../../../../../test/util'; import { commitFiles } from '../../../../util/git'; import { CommitMessage } from '../../model/commit-message'; import { createOnboardingBranch } from './create'; @@ -20,7 +20,6 @@ describe('workers/repository/onboarding/branch/create', () => { describe('createOnboardingBranch', () => { it('applies the default commit message', async () => { await createOnboardingBranch(config); - expect(commitFiles).toHaveBeenCalledWith({ branchName: 'renovate/configure', files: [ @@ -31,6 +30,27 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message: 'Add renovate.json', + platformCommit: false, + }); + }); + it('commits via platform', async () => { + platform.commitFiles = jest.fn(); + + config.platformCommit = true; + + await createOnboardingBranch(config); + + expect(platform.commitFiles).toHaveBeenCalledWith({ + branchName: 'renovate/configure', + files: [ + { + type: 'addition', + path: 'renovate.json', + contents: '{"foo":"bar"}', + }, + ], + message: 'Add renovate.json', + platformCommit: true, }); }); it('applies supplied commit message', async () => { @@ -51,6 +71,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); describe('applies the commitMessagePrefix value', () => { @@ -72,6 +93,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); it('to the supplied commit message', async () => { @@ -95,6 +117,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); }); @@ -117,6 +140,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); it('to the supplied commit message', async () => { @@ -140,6 +164,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); }); @@ -163,6 +188,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); it('falls back to the default option if in list of allowed names', async () => { @@ -184,6 +210,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); it('uses the given name if valid', async () => { @@ -206,6 +233,7 @@ describe('workers/repository/onboarding/branch/create', () => { }, ], message, + platformCommit: false, }); }); it('applies to the default commit message', async () => { @@ -222,6 +250,7 @@ describe('workers/repository/onboarding/branch/create', () => { branchName: 'renovate/configure', files: [{ type: 'addition', path, contents: '{"foo":"bar"}' }], message, + platformCommit: false, }); }); }); diff --git a/lib/workers/repository/onboarding/branch/create.ts b/lib/workers/repository/onboarding/branch/create.ts index 73526db1e8..0b7ba32d79 100644 --- a/lib/workers/repository/onboarding/branch/create.ts +++ b/lib/workers/repository/onboarding/branch/create.ts @@ -2,7 +2,7 @@ import { configFileNames } from '../../../../config/app-strings'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; -import { commitFiles } from '../../../../util/git'; +import { commitAndPush } from '../../../../platform/commit'; import { OnboardingCommitMessageFactory } from './commit-message'; import { getOnboardingConfigContents } from './config'; @@ -30,7 +30,8 @@ export async function createOnboardingBranch( logger.info('DRY-RUN: Would commit files to onboarding branch'); return null; } - return commitFiles({ + + return commitAndPush({ branchName: config.onboardingBranch, files: [ { @@ -40,5 +41,6 @@ export async function createOnboardingBranch( }, ], message: commitMessage.toString(), + platformCommit: !!config.platformCommit, }); } diff --git a/lib/workers/repository/onboarding/branch/rebase.spec.ts b/lib/workers/repository/onboarding/branch/rebase.spec.ts index fb31bdbe91..d28b97185a 100644 --- a/lib/workers/repository/onboarding/branch/rebase.spec.ts +++ b/lib/workers/repository/onboarding/branch/rebase.spec.ts @@ -1,4 +1,9 @@ -import { RenovateConfig, defaultConfig, git } from '../../../../../test/util'; +import { + RenovateConfig, + defaultConfig, + git, + platform, +} from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; import { rebaseOnboardingBranch } from './rebase'; @@ -39,6 +44,13 @@ describe('workers/repository/onboarding/branch/rebase', () => { await rebaseOnboardingBranch(config); expect(git.commitFiles).toHaveBeenCalledTimes(1); }); + it('rebases via platform', async () => { + platform.commitFiles = jest.fn(); + config.platformCommit = true; + git.isBranchStale.mockResolvedValueOnce(true); + await rebaseOnboardingBranch(config); + expect(platform.commitFiles).toHaveBeenCalledTimes(1); + }); it('uses the onboardingConfigFileName if set', async () => { git.isBranchStale.mockResolvedValueOnce(true); await rebaseOnboardingBranch({ diff --git a/lib/workers/repository/onboarding/branch/rebase.ts b/lib/workers/repository/onboarding/branch/rebase.ts index 7a145e3f87..1a906389d7 100644 --- a/lib/workers/repository/onboarding/branch/rebase.ts +++ b/lib/workers/repository/onboarding/branch/rebase.ts @@ -2,12 +2,8 @@ import { configFileNames } from '../../../../config/app-strings'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; -import { - commitFiles, - getFile, - isBranchModified, - isBranchStale, -} from '../../../../util/git'; +import { commitAndPush } from '../../../../platform/commit'; +import { getFile, isBranchModified, isBranchStale } from '../../../../util/git'; import { OnboardingCommitMessageFactory } from './commit-message'; import { getOnboardingConfigContents } from './config'; @@ -47,7 +43,8 @@ export async function rebaseOnboardingBranch( logger.info('DRY-RUN: Would rebase files in onboarding branch'); return null; } - return commitFiles({ + + return commitAndPush({ branchName: config.onboardingBranch, files: [ { @@ -57,5 +54,6 @@ export async function rebaseOnboardingBranch( }, ], message: commitMessage.toString(), + platformCommit: !!config.platformCommit, }); } -- GitLab