From ffeb78415a5288264150701f03a0905bf28e2948 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Thu, 18 May 2023 11:26:12 +0530 Subject: [PATCH] feat(onboarding): merge onboardingBranch into baseBranch (#20893) Co-authored-by: Rhys Arkins Co-authored-by: Michael Kriese --- lib/util/cache/repository/types.ts | 3 +- lib/util/git/index.spec.ts | 6 + lib/util/git/index.ts | 14 +- .../onboarding/branch/check.spec.ts | 6 +- .../repository/onboarding/branch/check.ts | 19 +- .../onboarding/branch/index.spec.ts | 103 +++++-- .../repository/onboarding/branch/index.ts | 69 +++-- .../branch/onboarding-branch-cache.spec.ts | 258 +++++++++++++++--- .../branch/onboarding-branch-cache.ts | 88 +++++- .../onboarding/branch/rebase.spec.ts | 180 ------------ .../repository/onboarding/branch/rebase.ts | 89 ------ .../repository/onboarding/pr/index.spec.ts | 33 +-- lib/workers/repository/onboarding/pr/index.ts | 37 +-- 13 files changed, 489 insertions(+), 416 deletions(-) delete mode 100644 lib/workers/repository/onboarding/branch/rebase.spec.ts delete mode 100644 lib/workers/repository/onboarding/branch/rebase.ts diff --git a/lib/util/cache/repository/types.ts b/lib/util/cache/repository/types.ts index 6f5cbfe32601da..bbcb484864d822 100644 --- a/lib/util/cache/repository/types.ts +++ b/lib/util/cache/repository/types.ts @@ -27,9 +27,10 @@ export interface BranchUpgradeCache { } export interface OnboardingBranchCache { - onboardingBranch: string; defaultBranchSha: string; onboardingBranchSha: string; + isConflicted: boolean; + isModified: boolean; } export interface PrCache { diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts index 1551fdca230c63..297e3691d0232d 100644 --- a/lib/util/git/index.spec.ts +++ b/lib/util/git/index.spec.ts @@ -344,6 +344,12 @@ describe('util/git/index', () => { expect(merged.all).toContain('renovate/future_branch'); }); + it('does not push if localOnly=true', async () => { + const pushSpy = jest.spyOn(SimpleGit.prototype, 'push'); + await git.mergeBranch('renovate/future_branch', true); + expect(pushSpy).toHaveBeenCalledTimes(0); + }); + it('should throw if branch merge throws', async () => { await expect(git.mergeBranch('not_found')).rejects.toThrow(); }); diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts index 5d086bef8612ca..107c67decd1241 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -774,7 +774,10 @@ export async function deleteBranch(branchName: string): Promise { delete config.branchCommits[branchName]; } -export async function mergeBranch(branchName: string): Promise { +export async function mergeBranch( + branchName: string, + localOnly = false +): Promise { let status: StatusResult | undefined; try { await syncGit(); @@ -790,8 +793,13 @@ export async function mergeBranch(branchName: string): Promise { ]) ); status = await git.status(); - await gitRetry(() => git.merge(['--ff-only', branchName])); - await gitRetry(() => git.push('origin', config.currentBranch)); + 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( diff --git a/lib/workers/repository/onboarding/branch/check.spec.ts b/lib/workers/repository/onboarding/branch/check.spec.ts index 74684f8f809352..7fd1d200bee611 100644 --- a/lib/workers/repository/onboarding/branch/check.spec.ts +++ b/lib/workers/repository/onboarding/branch/check.spec.ts @@ -27,9 +27,10 @@ describe('workers/repository/onboarding/branch/check', () => { it('skips normal onboarding check if onboardingCache is valid', async () => { cache.getCache.mockReturnValueOnce({ onboardingBranchCache: { - onboardingBranch: 'configure/renovate', defaultBranchSha: 'default-sha', onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, }, }); git.getBranchCommit @@ -45,9 +46,10 @@ describe('workers/repository/onboarding/branch/check', () => { it('continues with normal logic if onboardingCache is invalid', async () => { cache.getCache.mockReturnValueOnce({ onboardingBranchCache: { - onboardingBranch: 'configure/renovate', defaultBranchSha: 'default-sha', onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, }, }); scm.getFileList.mockResolvedValue([]); diff --git a/lib/workers/repository/onboarding/branch/check.ts b/lib/workers/repository/onboarding/branch/check.ts index bf3abf09f1fe03..0f7fca6870bd7a 100644 --- a/lib/workers/repository/onboarding/branch/check.ts +++ b/lib/workers/repository/onboarding/branch/check.ts @@ -52,8 +52,13 @@ function closedPrExists(config: RenovateConfig): Promise { export async function isOnboarded(config: RenovateConfig): Promise { logger.debug('isOnboarded()'); const title = `Action required: Add a Renovate config`; + // Repo is onboarded if global config is bypassing onboarding and does not require a // configuration file. + // The repo is considered "not onboarded" if: + // - An onboarding cache is present, and + // - The current default branch SHA matches the default SHA found in the cache + // Also if there is a closed pr skip using cache as it is outdated if (config.requireConfig === 'optional' && config.onboarding === false) { // Return early and avoid checking for config files return true; @@ -63,19 +68,17 @@ export async function isOnboarded(config: RenovateConfig): Promise { return true; } - const pr = await closedPrExists(config); + const closedOnboardingPr = await closedPrExists(config); const cache = getCache(); const onboardingBranchCache = cache?.onboardingBranchCache; // if onboarding cache is present and base branch has not been updated branch is not onboarded // if closed pr exists then presence of onboarding cache doesn't matter as we need to skip onboarding if ( config.onboarding && - !pr && + !closedOnboardingPr && onboardingBranchCache && onboardingBranchCache.defaultBranchSha === - getBranchCommit(config.defaultBranch!) && - onboardingBranchCache.onboardingBranchSha === - getBranchCommit(config.onboardingBranch!) + getBranchCommit(config.defaultBranch!) ) { logger.debug('Onboarding cache is valid. Repo is not onboarded'); return false; @@ -123,7 +126,7 @@ export async function isOnboarded(config: RenovateConfig): Promise { throw new Error(REPOSITORY_NO_CONFIG); } - if (!pr) { + if (!closedOnboardingPr) { logger.debug('Found no closed onboarding PR'); return false; } @@ -136,9 +139,9 @@ export async function isOnboarded(config: RenovateConfig): Promise { if (!config.suppressNotifications!.includes('onboardingClose')) { // ensure PR comment await ensureComment({ - number: pr.number, + number: closedOnboardingPr.number, topic: `Renovate is disabled`, - content: `Renovate is disabled due to lack of config. If you wish to reenable it, you can either (a) commit a config file to your base branch, or (b) rename this closed PR to trigger a replacement onboarding PR.`, + content: `Renovate is disabled due to lack of config. If you wish to re-enable it, you can either (a) commit a config file to your base branch, or (b) rename this closed PR to trigger a replacement onboarding PR.`, }); } throw new Error(REPOSITORY_CLOSED_ONBOARDING); diff --git a/lib/workers/repository/onboarding/branch/index.spec.ts b/lib/workers/repository/onboarding/branch/index.spec.ts index d9821294b5880b..ca37252be7d2fe 100644 --- a/lib/workers/repository/onboarding/branch/index.spec.ts +++ b/lib/workers/repository/onboarding/branch/index.spec.ts @@ -3,6 +3,7 @@ import { RenovateConfig, fs, getConfig, + git, mocked, platform, scm, @@ -17,17 +18,15 @@ import { logger } from '../../../../logger'; import type { Pr } from '../../../../modules/platform'; import * as memCache from '../../../../util/cache/memory'; import * as _cache from '../../../../util/cache/repository'; +import type { RepoCacheData } from '../../../../util/cache/repository/types'; import type { FileAddition } from '../../../../util/git/types'; import { OnboardingState } from '../common'; import * as _config from './config'; import * as _onboardingCache from './onboarding-branch-cache'; -import * as _rebase from './rebase'; import { checkOnboardingBranch } from '.'; -const rebase: any = _rebase; const configModule: any = _config; -jest.mock('../../../repository/onboarding/branch/rebase'); jest.mock('../../../../util/cache/repository'); jest.mock('../../../../util/fs'); jest.mock('../../../../util/git'); @@ -40,13 +39,6 @@ const onboardingCache = mocked(_onboardingCache); describe('workers/repository/onboarding/branch/index', () => { describe('checkOnboardingBranch', () => { let config: RenovateConfig; - const dummyCache = { - onboardingBranchCache: { - onboardingBranch: 'configure/renovate', - defaultBranchSha: 'default-sha', - onboardingBranchSha: 'onboarding-sha', - }, - }; beforeEach(() => { memCache.init(); @@ -177,7 +169,6 @@ describe('workers/repository/onboarding/branch/index', () => { }); it('detects repo is onboarded via file', async () => { - cache.getCache.mockReturnValue(dummyCache); scm.getFileList.mockResolvedValueOnce(['renovate.json']); const res = await checkOnboardingBranch(config); expect(res.repoIsOnboarded).toBeTrue(); @@ -261,20 +252,90 @@ describe('workers/repository/onboarding/branch/index', () => { await expect(checkOnboardingBranch(config)).rejects.toThrow(); }); - it('updates onboarding branch', async () => { - cache.getCache.mockReturnValue(dummyCache); + it('processes onboarding branch', async () => { scm.getFileList.mockResolvedValue(['package.json']); platform.findPr.mockResolvedValue(null); platform.getBranchPr.mockResolvedValueOnce(mock()); - rebase.rebaseOnboardingBranch.mockResolvedValueOnce('123test'); const res = await checkOnboardingBranch(config); expect(res.repoIsOnboarded).toBeFalse(); expect(res.branchList).toEqual(['renovate/configure']); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); - expect(onboardingCache.setOnboardingCache).toHaveBeenCalledTimes(1); // update onboarding cache + expect(git.mergeBranch).toHaveBeenCalledOnce(); expect(scm.commitAndPush).toHaveBeenCalledTimes(0); }); + it('processes modified onboarding branch and invalidates extract cache', async () => { + const dummyCache = { + scan: { + master: { + sha: 'base_sha', + configHash: 'hash', + packageFiles: {}, + extractionFingerprints: {}, + }, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValue(dummyCache); + platform.findPr.mockResolvedValue(null); + platform.getBranchPr.mockResolvedValueOnce(mock()); + git.getBranchCommit + .mockReturnValueOnce('default-sha') + .mockReturnValueOnce('new-onboarding-sha'); + onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(true); + onboardingCache.hasOnboardingBranchChanged.mockReturnValueOnce(true); + onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(false); + config.baseBranch = 'master'; + await checkOnboardingBranch(config); + expect(git.mergeBranch).toHaveBeenCalledOnce(); + expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith( + 'default-sha', + 'new-onboarding-sha', + false, + true + ); + expect(dummyCache).toMatchObject({ + scan: {}, + }); + }); + + it('skips processing conflicted onboarding branch', async () => { + scm.getFileList.mockResolvedValue(['package.json']); + platform.findPr.mockResolvedValue(null); + platform.getBranchPr.mockResolvedValueOnce(mock()); + platform.getBranchPr.mockResolvedValueOnce(mock()); + git.getBranchCommit + .mockReturnValueOnce('default-sha') + .mockReturnValueOnce('onboarding-sha'); + onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(true); + onboardingCache.hasOnboardingBranchChanged.mockReturnValueOnce(true); + onboardingCache.isOnboardingBranchConflicted.mockResolvedValueOnce(true); + await checkOnboardingBranch(config); + expect(git.mergeBranch).not.toHaveBeenCalled(); + expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith( + 'default-sha', + 'onboarding-sha', + true, + true + ); + }); + + it('sets onboarding cache for existing onboarding branch', async () => { + scm.getFileList.mockResolvedValue(['package.json']); + platform.findPr.mockResolvedValue(null); + platform.getBranchPr.mockResolvedValueOnce(mock()); + git.getBranchCommit + .mockReturnValueOnce('default-sha') + .mockReturnValueOnce('onboarding-sha'); + onboardingCache.isOnboardingBranchModified.mockResolvedValueOnce(false); + await checkOnboardingBranch(config); + expect(git.mergeBranch).toHaveBeenCalled(); + expect(onboardingCache.setOnboardingCache).toHaveBeenCalledWith( + 'default-sha', + 'onboarding-sha', + false, + false + ); + }); + describe('tests onboarding rebase/retry checkbox handling', () => { beforeEach(() => { GlobalConfig.set({ platform: 'github' }); @@ -282,7 +343,6 @@ describe('workers/repository/onboarding/branch/index', () => { OnboardingState.prUpdateRequested = false; scm.getFileList.mockResolvedValueOnce(['package.json']); platform.findPr.mockResolvedValueOnce(null); - rebase.rebaseOnboardingBranch.mockResolvedValueOnce(null); }); it('detects unsupported platfom', async () => { @@ -296,7 +356,7 @@ describe('workers/repository/onboarding/branch/index', () => { `Platform '${pl}' does not support extended markdown` ); expect(OnboardingState.prUpdateRequested).toBeTrue(); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.mergeBranch).toHaveBeenCalledOnce(); expect(scm.commitAndPush).toHaveBeenCalledTimes(0); }); @@ -310,7 +370,7 @@ describe('workers/repository/onboarding/branch/index', () => { `No rebase checkbox was found in the onboarding PR` ); expect(OnboardingState.prUpdateRequested).toBeTrue(); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.mergeBranch).toHaveBeenCalledOnce(); expect(scm.commitAndPush).toHaveBeenCalledTimes(0); }); @@ -324,8 +384,7 @@ describe('workers/repository/onboarding/branch/index', () => { `Manual onboarding PR update requested` ); expect(OnboardingState.prUpdateRequested).toBeTrue(); - ``; - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.mergeBranch).toHaveBeenCalledOnce(); expect(scm.commitAndPush).toHaveBeenCalledTimes(0); }); @@ -336,7 +395,7 @@ describe('workers/repository/onboarding/branch/index', () => { await checkOnboardingBranch(config); expect(OnboardingState.prUpdateRequested).toBeFalse(); - expect(scm.checkoutBranch).toHaveBeenCalledTimes(1); + expect(git.mergeBranch).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 e131f8069a4a66..a4853d1fec8eca 100644 --- a/lib/workers/repository/onboarding/branch/index.ts +++ b/lib/workers/repository/onboarding/branch/index.ts @@ -7,9 +7,13 @@ import { REPOSITORY_NO_PACKAGE_FILES, } from '../../../../constants/error-messages'; import { logger } from '../../../../logger'; -import { Pr, platform } from '../../../../modules/platform'; -import { scm } from '../../../../modules/platform/scm'; -import { getBranchCommit, setGitAuthor } from '../../../../util/git'; +import type { Pr } from '../../../../modules/platform'; +import { getCache } from '../../../../util/cache/repository'; +import { + getBranchCommit, + mergeBranch, + setGitAuthor, +} from '../../../../util/git'; import { extractAllDependencies } from '../../extract'; import { mergeRenovateConfig } from '../../init/merge'; import { OnboardingState } from '../common'; @@ -18,9 +22,11 @@ import { getOnboardingConfig } from './config'; import { createOnboardingBranch } from './create'; import { deleteOnboardingCache, + hasOnboardingBranchChanged, + isOnboardingBranchConflicted, + isOnboardingBranchModified, setOnboardingCache, } from './onboarding-branch-cache'; -import { rebaseOnboardingBranch } from './rebase'; export async function checkOnboardingBranch( config: RenovateConfig @@ -28,6 +34,8 @@ export async function checkOnboardingBranch( logger.debug('checkOnboarding()'); logger.trace({ config }); let onboardingBranch = config.onboardingBranch; + let isConflicted = false; + let isModified = false; const repoIsOnboarded = await isOnboarded(config); if (repoIsOnboarded) { logger.debug('Repo is onboarded'); @@ -48,25 +56,17 @@ export async function checkOnboardingBranch( handleOnboardingManualRebase(onboardingPr); } logger.debug('Onboarding PR already exists'); - const { rawConfigHash } = onboardingPr.bodyStruct ?? {}; - const commit = await rebaseOnboardingBranch(config, rawConfigHash); - if (commit) { - logger.info( - { branch: config.onboardingBranch, commit, onboarding: true }, - 'Branch updated' - ); - // update onboarding cache - setOnboardingCache( - config.onboardingBranch!, - getBranchCommit(config.defaultBranch!)!, - commit + isModified = await isOnboardingBranchModified(config.onboardingBranch!); + if (isModified) { + if (hasOnboardingBranchChanged(config.onboardingBranch!)) { + invalidateExtractCache(config.baseBranch!); + } + isConflicted = await isOnboardingBranchConflicted( + config.baseBranch!, + config.onboardingBranch! ); } - // istanbul ignore if - if (platform.refreshPr) { - await platform.refreshPr(onboardingPr.number); - } } else { logger.debug('Onboarding PR does not exist'); const onboardingConfig = await getOnboardingConfig(config); @@ -93,20 +93,22 @@ export async function checkOnboardingBranch( { branch: onboardingBranch, commit, onboarding: true }, 'Branch created' ); - - // set onboarding branch cache - setOnboardingCache( - config.onboardingBranch!, - getBranchCommit(config.defaultBranch!)!, - commit - ); } } if (!GlobalConfig.get('dryRun')) { - logger.debug('Checkout onboarding branch.'); // TODO #7154 - await scm.checkoutBranch(onboardingBranch!); + if (!isConflicted) { + logger.debug('Merge onboarding branch in default branch'); + await mergeBranch(onboardingBranch!, true); + } } + setOnboardingCache( + getBranchCommit(config.defaultBranch!)!, + getBranchCommit(onboardingBranch!)!, + isConflicted, + isModified + ); + // TODO #7154 const branchList = [onboardingBranch!]; return { ...config, repoIsOnboarded, onboardingBranch, branchList }; @@ -126,3 +128,12 @@ function handleOnboardingManualRebase(onboardingPr: Pr): void { OnboardingState.prUpdateRequested = true; } } + +function invalidateExtractCache(baseBranch: string): void { + const cache = getCache(); + cache.scan ||= {}; + + if (cache.scan?.[baseBranch]) { + delete cache.scan[baseBranch]; + } +} diff --git a/lib/workers/repository/onboarding/branch/onboarding-branch-cache.spec.ts b/lib/workers/repository/onboarding/branch/onboarding-branch-cache.spec.ts index 10307e5a3ec374..40cb3017741454 100644 --- a/lib/workers/repository/onboarding/branch/onboarding-branch-cache.spec.ts +++ b/lib/workers/repository/onboarding/branch/onboarding-branch-cache.spec.ts @@ -1,61 +1,235 @@ -import { mocked } from '../../../../../test/util'; +import { git, mocked, scm } from '../../../../../test/util'; import * as _cache from '../../../../util/cache/repository'; import type { RepoCacheData } from '../../../../util/cache/repository/types'; import { deleteOnboardingCache, + hasOnboardingBranchChanged, + isOnboardingBranchConflicted, + isOnboardingBranchModified, setOnboardingCache, } from './onboarding-branch-cache'; jest.mock('../../../../util/cache/repository'); +jest.mock('../../../../util/git'); const cache = mocked(_cache); describe('workers/repository/onboarding/branch/onboarding-branch-cache', () => { - it('sets new cache', () => { - const dummyCache = {} satisfies RepoCacheData; - cache.getCache.mockReturnValueOnce(dummyCache); - setOnboardingCache('configure/renovate', 'default-sha', 'onboarding-sha'); - expect(dummyCache).toEqual({ - onboardingBranchCache: { - onboardingBranch: 'configure/renovate', - defaultBranchSha: 'default-sha', - onboardingBranchSha: 'onboarding-sha', - }, + describe('setOnboardingCache', () => { + it('does not create new cache', () => { + const dummyCache = {} satisfies RepoCacheData; + cache.getCache.mockReturnValue(dummyCache); + setOnboardingCache('default-sha', null as never, false, false); + expect(dummyCache).toEqual({}); + }); + + it('sets new cache', () => { + const dummyCache = {} satisfies RepoCacheData; + cache.getCache.mockReturnValue(dummyCache); + setOnboardingCache('default-sha', 'onboarding-sha', false, false); + expect(dummyCache).toEqual({ + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, + }, + }); + }); + + it('updates old cache', () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValue(dummyCache); + setOnboardingCache('default-sha-1', 'onboarding-sha-1', false, true); + expect(dummyCache).toEqual({ + onboardingBranchCache: { + defaultBranchSha: 'default-sha-1', + onboardingBranchSha: 'onboarding-sha-1', + isConflicted: false, + isModified: true, + }, + }); + }); + }); + + describe('deleteOnboardingCache', () => { + it('deletes cache', () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValue(dummyCache); + deleteOnboardingCache(); + expect(dummyCache.onboardingBranchCache).toBeUndefined(); }); }); - it('updates old cache', () => { - const dummyCache = { - onboardingBranchCache: { - onboardingBranch: 'configure/renovate', - defaultBranchSha: 'default-sha', - onboardingBranchSha: 'onboarding-sha', - }, - } satisfies RepoCacheData; - cache.getCache.mockReturnValueOnce(dummyCache); - setOnboardingCache( - 'configure/renovate', - 'default-sha-1', - 'onboarding-sha-1' - ); - expect(dummyCache).toEqual({ - onboardingBranchCache: { - onboardingBranch: 'configure/renovate', - defaultBranchSha: 'default-sha-1', - onboardingBranchSha: 'onboarding-sha-1', - }, + describe('hasOnboardingBranchChanged()', () => { + it('return true if cache is absent', () => { + cache.getCache.mockReturnValueOnce({}); + git.getBranchCommit.mockReturnValueOnce('onboarding-sha'); + expect(hasOnboardingBranchChanged('configure/renovate')).toBeTrue(); + }); + + it('returns true', () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'old-onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit.mockReturnValueOnce('new-onboarding-sha'); + expect(hasOnboardingBranchChanged('configure/renovate')).toBeTrue(); + }); + + it('returns false', () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit.mockReturnValueOnce('onboarding-sha'); + expect(hasOnboardingBranchChanged('configure/renovate')).toBeFalse(); + }); + + it('returns false when branch is modified but has not changed since last run', () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: true, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit.mockReturnValueOnce('onboarding-sha'); + expect(hasOnboardingBranchChanged('configure/renovate')).toBeFalse(); }); }); - it('deletes cache', () => { - const dummyCache = { - onboardingBranchCache: { - onboardingBranch: 'configure/renovate', - defaultBranchSha: 'default-sha', - onboardingBranchSha: 'onboarding-sha', - }, - } satisfies RepoCacheData; - cache.getCache.mockReturnValueOnce(dummyCache); - deleteOnboardingCache(); - expect(dummyCache.onboardingBranchCache).toBeUndefined(); + describe('isOnboardingBranchModified()', () => { + it('falls back to git if cache is absent', async () => { + cache.getCache.mockReturnValueOnce({}); + git.getBranchCommit.mockReturnValueOnce('onboarding-sha'); + scm.isBranchModified.mockResolvedValueOnce(false); + expect( + await isOnboardingBranchModified('configure/renovate') + ).toBeFalse(); + }); + + it('falls back to git if onboarding branch is updated', async () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'old-onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit.mockReturnValueOnce('new-onboarding-sha'); + scm.isBranchModified.mockResolvedValueOnce(true); + expect(await isOnboardingBranchModified('configure/renovate')).toBeTrue(); + }); + + it('returns cached value', async () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: true, + isModified: true, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit.mockReturnValueOnce('onboarding-sha'); + expect(await isOnboardingBranchModified('configure/renovate')).toBeTrue(); + }); + }); + + describe('isOnboardingBranchConflicted()', () => { + it('falls back to git if cache is absent', async () => { + cache.getCache.mockReturnValueOnce({}); + git.getBranchCommit + .mockReturnValueOnce('onboarding-sha') + .mockReturnValueOnce('default-sha'); + scm.isBranchConflicted.mockResolvedValueOnce(false); + expect( + await isOnboardingBranchConflicted('master', 'configure/renovate') + ).toBeFalse(); + }); + + it('falls back to git if default branch is updated', async () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'old-default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit + .mockReturnValueOnce('onboarding-sha') + .mockReturnValueOnce('new-default-sha'); + scm.isBranchConflicted.mockResolvedValueOnce(false); + expect( + await isOnboardingBranchConflicted('master', 'configure/renovate') + ).toBeFalse(); + }); + + it('falls back to git if onboarding branch is modified', async () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'old-onboarding-sha', + isConflicted: false, + isModified: false, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit + .mockReturnValueOnce('new-onboarding-sha') + .mockReturnValueOnce('default-sha'); + scm.isBranchConflicted.mockResolvedValueOnce(false); + expect( + await isOnboardingBranchConflicted('master', 'configure/renovate') + ).toBeFalse(); + }); + + it('returns cached value', async () => { + const dummyCache = { + onboardingBranchCache: { + defaultBranchSha: 'default-sha', + onboardingBranchSha: 'onboarding-sha', + isConflicted: true, + isModified: true, + }, + } satisfies RepoCacheData; + cache.getCache.mockReturnValueOnce(dummyCache); + git.getBranchCommit + .mockReturnValueOnce('onboarding-sha') + .mockReturnValueOnce('default-sha'); + expect( + await isOnboardingBranchConflicted('master', 'configure/renovate') + ).toBeTrue(); + }); }); }); diff --git a/lib/workers/repository/onboarding/branch/onboarding-branch-cache.ts b/lib/workers/repository/onboarding/branch/onboarding-branch-cache.ts index a6ecfcb29677b2..9f7ebd1b47cf1b 100644 --- a/lib/workers/repository/onboarding/branch/onboarding-branch-cache.ts +++ b/lib/workers/repository/onboarding/branch/onboarding-branch-cache.ts @@ -1,21 +1,37 @@ +import is from '@sindresorhus/is'; import { logger } from '../../../../logger'; +import { scm } from '../../../../modules/platform/scm'; import { getCache } from '../../../../util/cache/repository'; +import { getBranchCommit } from '../../../../util/git'; export function setOnboardingCache( - onboardingBranch: string, defaultBranchSha: string, - onboardingBranchSha: string + onboardingBranchSha: string, + isConflicted: boolean, + isModified: boolean ): void { + // do not update cache if commit is null/undefined + if ( + !( + is.nonEmptyString(defaultBranchSha) && + is.nonEmptyString(onboardingBranchSha) + ) + ) { + logger.debug('Onboarding cache not updated'); + return; + } + const cache = getCache(); const onboardingCache = { - onboardingBranch, defaultBranchSha, onboardingBranchSha, + isConflicted, + isModified, }; if (cache.onboardingBranchCache) { - logger.debug('Update Onboarding Cache'); + logger.debug({ onboardingCache }, 'Update Onboarding Cache'); } else { - logger.debug('Create Onboarding Cache'); + logger.debug({ onboardingCache }, 'Create Onboarding Cache'); } cache.onboardingBranchCache = onboardingCache; } @@ -28,3 +44,65 @@ export function deleteOnboardingCache(): void { delete cache.onboardingBranchCache; } } + +// checks if onboarding branch has been modified since last run +// return true if cache isn't present +export function hasOnboardingBranchChanged(onboardingBranch: string): boolean { + const cache = getCache(); + const onboardingSha = getBranchCommit(onboardingBranch); + + if (cache.onboardingBranchCache) { + return onboardingSha !== cache.onboardingBranchCache.onboardingBranchSha; + } + return true; +} + +// checks if onboarding branch has been modified by user +// once set to true it stays true as we do not rebase onboarding branches anymore (this feature will be added in future though) +export async function isOnboardingBranchModified( + onboardingBranch: string +): Promise { + const cache = getCache(); + const onboardingCache = cache.onboardingBranchCache; + const onboardingSha = getBranchCommit(onboardingBranch); + let isModified = false; + + if ( + onboardingCache && + onboardingSha === onboardingCache.onboardingBranchSha && + !is.undefined(onboardingCache.isModified) + ) { + return onboardingCache.isModified; + } else { + isModified = await scm.isBranchModified(onboardingBranch); + } + + return isModified; +} + +export async function isOnboardingBranchConflicted( + defaultBranch: string, + onboardingBranch: string +): Promise { + const cache = getCache(); + const onboardingCache = cache.onboardingBranchCache; + const onboardingSha = getBranchCommit(onboardingBranch); + const defaultBranchSha = getBranchCommit(defaultBranch); + let isConflicted = false; + + if ( + onboardingCache && + defaultBranchSha === onboardingCache.defaultBranchSha && + onboardingSha === onboardingCache.onboardingBranchSha && + !is.undefined(onboardingCache.isConflicted) + ) { + return onboardingCache.isConflicted; + } else { + isConflicted = await scm.isBranchConflicted( + defaultBranch, + onboardingBranch + ); + } + + return isConflicted; +} diff --git a/lib/workers/repository/onboarding/branch/rebase.spec.ts b/lib/workers/repository/onboarding/branch/rebase.spec.ts deleted file mode 100644 index 1bb98d5e05c17d..00000000000000 --- a/lib/workers/repository/onboarding/branch/rebase.spec.ts +++ /dev/null @@ -1,180 +0,0 @@ -import { RenovateConfig, getConfig, git, scm } from '../../../../../test/util'; -import { GlobalConfig } from '../../../../config/global'; -import * as memCache from '../../../../util/cache/memory'; -import { toSha256 } from '../../../../util/hasha'; -import { OnboardingState } from '../common'; -import { rebaseOnboardingBranch } from './rebase'; - -jest.mock('../../../../util/git'); - -describe('workers/repository/onboarding/branch/rebase', () => { - beforeAll(() => { - GlobalConfig.set({ - localDir: '', - }); - }); - - describe('rebaseOnboardingBranch()', () => { - let config: RenovateConfig; - const hash = ''; - - beforeEach(() => { - memCache.init(); - jest.resetAllMocks(); - OnboardingState.prUpdateRequested = false; - config = { - ...getConfig(), - repository: 'some/repo', - }; - }); - - it('does not rebase modified branch', async () => { - scm.isBranchModified.mockResolvedValueOnce(true); - await rebaseOnboardingBranch(config, hash); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - }); - - it.each` - checkboxEnabled - ${true} - ${false} - `( - 'does nothing if branch is up to date ' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled }) => { - config.onboardingRebaseCheckbox = checkboxEnabled; - const contents = - JSON.stringify(getConfig().onboardingConfig, null, 2) + '\n'; - git.getFile - .mockResolvedValueOnce(contents) // package.json - .mockResolvedValueOnce(contents); // renovate.json - await rebaseOnboardingBranch(config, toSha256(contents)); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - expect(OnboardingState.prUpdateRequested).toBeFalse(); - } - ); - - it.each` - checkboxEnabled | expected - ${true} | ${true} - ${false} | ${false} - `( - 'rebases onboarding branch ' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - config.onboardingRebaseCheckbox = checkboxEnabled; - scm.isBranchBehindBase.mockResolvedValueOnce(true); - await rebaseOnboardingBranch(config, hash); - expect(scm.commitAndPush).toHaveBeenCalledTimes(1); - expect(OnboardingState.prUpdateRequested).toBe(expected); - } - ); - - it.each` - checkboxEnabled | expected - ${true} | ${true} - ${false} | ${false} - `( - 'uses the onboardingConfigFileName if set ' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - scm.isBranchBehindBase.mockResolvedValueOnce(true); - await rebaseOnboardingBranch({ - ...config, - onboardingConfigFileName: '.github/renovate.json', - onboardingRebaseCheckbox: checkboxEnabled, - }); - expect(scm.commitAndPush).toHaveBeenCalledTimes(1); - expect(scm.commitAndPush.mock.calls[0][0].message).toContain( - '.github/renovate.json' - ); - expect(scm.commitAndPush.mock.calls[0][0].files[0].path).toBe( - '.github/renovate.json' - ); - expect(OnboardingState.prUpdateRequested).toBe(expected); - } - ); - - it.each` - checkboxEnabled | expected - ${true} | ${true} - ${false} | ${false} - `( - 'falls back to "renovate.json" if onboardingConfigFileName is not set ' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - scm.isBranchBehindBase.mockResolvedValueOnce(true); - await rebaseOnboardingBranch({ - ...config, - onboardingConfigFileName: undefined, - onboardingRebaseCheckbox: checkboxEnabled, - }); - expect(scm.commitAndPush).toHaveBeenCalledTimes(1); - expect(scm.commitAndPush.mock.calls[0][0].message).toContain( - 'renovate.json' - ); - expect(scm.commitAndPush.mock.calls[0][0].files[0].path).toBe( - 'renovate.json' - ); - expect(OnboardingState.prUpdateRequested).toBe(expected); - } - ); - - describe('handle onboarding config hashes', () => { - const contents = - JSON.stringify(getConfig().onboardingConfig, null, 2) + '\n'; - - beforeEach(() => { - scm.isBranchModified.mockResolvedValueOnce(true); - git.getFile.mockResolvedValueOnce(contents); - }); - - it.each` - checkboxEnabled | expected - ${true} | ${true} - ${false} | ${false} - `( - 'handles a missing previous config hash ' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - config.onboardingRebaseCheckbox = checkboxEnabled; - await rebaseOnboardingBranch(config, undefined); - - expect(OnboardingState.prUpdateRequested).toBe(expected); - } - ); - - it.each` - checkboxEnabled - ${true} - ${false} - `( - 'does nothing if config hashes match' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - git.getFile.mockResolvedValueOnce(contents); // package.json - config.onboardingRebaseCheckbox = checkboxEnabled; - await rebaseOnboardingBranch(config, toSha256(contents)); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - expect(OnboardingState.prUpdateRequested).toBeFalse(); - } - ); - - it.each` - checkboxEnabled | expected - ${true} | ${true} - ${false} | ${false} - `( - 'requests update if config hashes mismatch' + - '(config.onboardingRebaseCheckbox="$checkboxEnabled")', - async ({ checkboxEnabled, expected }) => { - git.getFile.mockResolvedValueOnce(contents); // package.json - config.onboardingRebaseCheckbox = checkboxEnabled; - await rebaseOnboardingBranch(config, hash); - expect(scm.commitAndPush).toHaveBeenCalledTimes(0); - expect(OnboardingState.prUpdateRequested).toBe(expected); - } - ); - }); - }); -}); diff --git a/lib/workers/repository/onboarding/branch/rebase.ts b/lib/workers/repository/onboarding/branch/rebase.ts deleted file mode 100644 index 4cb1d27345db4d..00000000000000 --- a/lib/workers/repository/onboarding/branch/rebase.ts +++ /dev/null @@ -1,89 +0,0 @@ -import is from '@sindresorhus/is'; -import { GlobalConfig } from '../../../../config/global'; -import type { RenovateConfig } from '../../../../config/types'; -import { logger } from '../../../../logger'; -import { scm } from '../../../../modules/platform/scm'; -import { getFile } from '../../../../util/git'; -import { toSha256 } from '../../../../util/hasha'; -import { OnboardingState, defaultConfigFile } from '../common'; -import { OnboardingCommitMessageFactory } from './commit-message'; -import { getOnboardingConfigContents } from './config'; - -export async function rebaseOnboardingBranch( - config: RenovateConfig, - previousConfigHash?: string -): Promise { - logger.debug('Checking if onboarding branch needs rebasing'); - const configFile = defaultConfigFile(config); - const existingContents = - (await getFile(configFile, config.onboardingBranch)) ?? ''; - const currentConfigHash = toSha256(existingContents); - const contents = await getOnboardingConfigContents(config, configFile); - - if (config.onboardingRebaseCheckbox) { - handleOnboardingManualRebase(previousConfigHash, currentConfigHash); - } - - // TODO #7154 - if (await scm.isBranchModified(config.onboardingBranch!)) { - logger.debug('Onboarding branch has been edited and cannot be rebased'); - return null; - } - - // TODO: fix types (#7154) - if ( - contents === existingContents && - !(await scm.isBranchBehindBase( - config.onboardingBranch!, - config.defaultBranch! - )) - ) { - logger.debug('Onboarding branch is up to date'); - return null; - } - - logger.debug('Rebasing onboarding branch'); - if (config.onboardingRebaseCheckbox) { - OnboardingState.prUpdateRequested = true; - } - // istanbul ignore next - const commitMessageFactory = new OnboardingCommitMessageFactory( - config, - configFile - ); - const commitMessage = commitMessageFactory.create(); - - // istanbul ignore if - if (GlobalConfig.get('dryRun')) { - logger.info('DRY-RUN: Would rebase files in onboarding branch'); - return null; - } - - // TODO #7154 - return scm.commitAndPush({ - baseBranch: config.baseBranch, - branchName: config.onboardingBranch!, - files: [ - { - type: 'addition', - path: configFile, - contents, - }, - ], - message: commitMessage.toString(), - platformCommit: !!config.platformCommit, - }); -} - -function handleOnboardingManualRebase( - previousConfigHash: string | undefined, - currentConfigHash: string -): void { - if (is.nullOrUndefined(previousConfigHash)) { - logger.debug('Missing previousConfigHash bodyStruct prop in onboarding PR'); - OnboardingState.prUpdateRequested = true; - } else if (previousConfigHash !== currentConfigHash) { - logger.debug('Onboarding config has been modified by the user'); - OnboardingState.prUpdateRequested = true; - } -} diff --git a/lib/workers/repository/onboarding/pr/index.spec.ts b/lib/workers/repository/onboarding/pr/index.spec.ts index 500a62d1e71ea0..0bc3ad94282deb 100644 --- a/lib/workers/repository/onboarding/pr/index.spec.ts +++ b/lib/workers/repository/onboarding/pr/index.spec.ts @@ -155,6 +155,7 @@ describe('workers/repository/onboarding/pr/index', () => { config.baseBranch = 'some-branch'; config.repository = 'test'; config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; + config.onboardingConfigFileName = undefined; // checks the case when fileName isn't available OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" await ensureOnboardingPr( { @@ -201,7 +202,7 @@ describe('workers/repository/onboarding/pr/index', () => { } ); - it('updates PR when conflicted', async () => { + it('ensures comment, when PR is conflicted', async () => { config.baseBranch = 'some-branch'; platform.getBranchPr.mockResolvedValueOnce( partial({ @@ -210,10 +211,10 @@ describe('workers/repository/onboarding/pr/index', () => { }) ); scm.isBranchConflicted.mockResolvedValueOnce(true); - scm.isBranchModified.mockResolvedValueOnce(true); await ensureOnboardingPr(config, {}, branches); + expect(platform.ensureComment).toHaveBeenCalledTimes(1); expect(platform.createPr).toHaveBeenCalledTimes(0); - expect(platform.updatePr).toHaveBeenCalledTimes(1); + expect(platform.updatePr).toHaveBeenCalledTimes(0); }); it('updates PR when modified', async () => { @@ -224,7 +225,6 @@ describe('workers/repository/onboarding/pr/index', () => { bodyStruct, }) ); - scm.isBranchModified.mockResolvedValueOnce(true); await ensureOnboardingPr(config, {}, branches); expect(platform.createPr).toHaveBeenCalledTimes(0); expect(platform.updatePr).toHaveBeenCalledTimes(1); @@ -242,34 +242,31 @@ describe('workers/repository/onboarding/pr/index', () => { expect(platform.createPr).toHaveBeenCalledTimes(1); }); - it('dryrun of updates PR when modified', async () => { + it('dryrun of creates PR', async () => { GlobalConfig.set({ dryRun: 'full' }); - config.baseBranch = 'some-branch'; - platform.getBranchPr.mockResolvedValueOnce( - partial({ - title: 'Configure Renovate', - bodyStruct, - }) - ); - scm.isBranchConflicted.mockResolvedValueOnce(true); - scm.isBranchModified.mockResolvedValueOnce(true); - await ensureOnboardingPr(config, {}, branches); + await ensureOnboardingPr(config, packageFiles, branches); expect(logger.info).toHaveBeenCalledWith( 'DRY-RUN: Would check branch renovate/configure' ); expect(logger.info).toHaveBeenLastCalledWith( - 'DRY-RUN: Would update onboarding PR' + 'DRY-RUN: Would create onboarding PR' ); }); - it('dryrun of creates PR', async () => { + it('dryrun of updates PR', async () => { GlobalConfig.set({ dryRun: 'full' }); + platform.getBranchPr.mockResolvedValueOnce( + partial({ + title: 'Configure Renovate', + bodyStruct, + }) + ); await ensureOnboardingPr(config, packageFiles, branches); expect(logger.info).toHaveBeenCalledWith( 'DRY-RUN: Would check branch renovate/configure' ); expect(logger.info).toHaveBeenLastCalledWith( - 'DRY-RUN: Would create onboarding PR' + 'DRY-RUN: Would update onboarding PR' ); }); diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 923dec654e8c2d..2bf76f3ee5526b 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -4,6 +4,7 @@ import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; import type { PackageFile } from '../../../../modules/manager/types'; import { platform } from '../../../../modules/platform'; +import { ensureComment } from '../../../../modules/platform/comment'; import { hashBody } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; import { emojify } from '../../../../util/emoji'; @@ -19,6 +20,7 @@ import { import { getPlatformPrOptions } from '../../update/pr'; import { prepareLabels } from '../../update/pr/labels'; import { addParticipants } from '../../update/pr/participants'; +import { isOnboardingBranchConflicted } from '../branch/onboarding-branch-cache'; import { OnboardingState, defaultConfigFile } from '../common'; import { getBaseBranchDesc } from './base-branch'; import { getConfigDesc } from './config-description'; @@ -39,6 +41,24 @@ export async function ensureOnboardingPr( logger.trace({ config }); // TODO #7154 const existingPr = await platform.getBranchPr(config.onboardingBranch!); + if (existingPr) { + // skip pr-update if branch is conflicted + if ( + await isOnboardingBranchConflicted( + config.defaultBranch!, + config.onboardingBranch! + ) + ) { + await ensureComment({ + number: existingPr.number, + topic: 'Branch Conflicted', + content: emojify( + `:warning: This PR has a merge conflict which Renovate is unable to automatically resolve, so updates to this PR description are now paused. Please resolve the merge conflict manually.\n\n` + ), + }); + return; + } + } const { rebaseCheckBox, renovateConfigHashComment } = await getRebaseCheckboxComponents(config); logger.debug('Filling in onboarding PR template'); @@ -96,23 +116,6 @@ If you need any further assistance then you can also [request help here](${ if (GlobalConfig.get('dryRun')) { // TODO: types (#7154) logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); - } else if (await scm.isBranchModified(config.onboardingBranch!)) { - configDesc = emojify( - `### Configuration\n\n:abcd: Renovate has detected a custom config for this PR. Feel free to ask for [help](${ - config.productLinks!.help - }) if you have any doubts and would like it reviewed.\n\n` - ); - const isConflicted = await scm.isBranchConflicted( - config.baseBranch!, - config.onboardingBranch! - ); - if (isConflicted) { - configDesc += emojify( - `:warning: This PR has a merge conflict. However, Renovate is unable to automatically fix that due to edits in this branch. Please resolve the merge conflict manually.\n\n` - ); - } else { - configDesc += `Important: Now that this branch is edited, Renovate can't rebase it from the base branch any more. If you make changes to the base branch that could impact this onboarding PR, please merge them manually.\n\n`; - } } else { configDesc = getConfigDesc(config, packageFiles!); }