From 7f9874c8998e10c523fff1234c66ae4e6c327ab3 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh <rahultesnik@gmail.com> Date: Sat, 25 Mar 2023 16:50:21 +0530 Subject: [PATCH] fix: perform pr-update when requested regardless of pr-cache state (#21148) --- .../repository/update/pr/index.spec.ts | 64 +++++++++++++++---- lib/workers/repository/update/pr/index.ts | 19 +++++- 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/lib/workers/repository/update/pr/index.spec.ts b/lib/workers/repository/update/pr/index.spec.ts index 68c855903c..5bdc399e0a 100644 --- a/lib/workers/repository/update/pr/index.spec.ts +++ b/lib/workers/repository/update/pr/index.spec.ts @@ -21,7 +21,7 @@ import type { PrCache } from '../../../../util/cache/repository/types'; import { fingerprint } from '../../../../util/fingerprint'; import * as _limits from '../../../global/limits'; import type { BranchConfig, BranchUpgradeConfig } from '../../../types'; -import { embedChangelog } from '../../changelog'; +import { embedChangelogs } from '../../changelog'; import * as _statusChecks from '../branch/status-checks'; import * as _prBody from './body'; import type { ChangeLogChange, ChangeLogRelease } from './changelog/types'; @@ -270,12 +270,12 @@ describe('workers/repository/update/pr/index', () => { describe('Update', () => { it('updates PR due to title change', async () => { - const changedPr: Pr = { ...pr, title: 'Another title' }; + const changedPr: Pr = { ...pr, title: 'Another title' }; // user changed the prTitle platform.getBranchPr.mockResolvedValueOnce(changedPr); const res = await ensurePr(config); - expect(res).toEqual({ type: 'with-pr', pr: changedPr }); + expect(res).toEqual({ type: 'with-pr', pr }); // we redo the prTitle as per config expect(platform.updatePr).toHaveBeenCalled(); expect(platform.createPr).not.toHaveBeenCalled(); expect(logger.logger.info).toHaveBeenCalledWith( @@ -288,13 +288,13 @@ describe('workers/repository/update/pr/index', () => { it('updates PR due to body change', async () => { const changedPr: Pr = { ...pr, - bodyStruct: getPrBodyStruct(`${body} updated`), + bodyStruct: getPrBodyStruct(`${body} updated`), // user changed prBody }; platform.getBranchPr.mockResolvedValueOnce(changedPr); const res = await ensurePr(config); - expect(res).toEqual({ type: 'with-pr', pr: changedPr }); + expect(res).toEqual({ type: 'with-pr', pr }); // we redo the prBody as per config expect(platform.updatePr).toHaveBeenCalled(); expect(platform.createPr).not.toHaveBeenCalled(); expect(prCache.setPrCache).toHaveBeenCalled(); @@ -803,10 +803,7 @@ describe('workers/repository/update/pr/index', () => { }); it('updates pr-cache when pr fingerprint is different', async () => { - platform.getBranchPr.mockResolvedValue({ - ...existingPr, - title: 'Another title', - }); + platform.getBranchPr.mockResolvedValue(existingPr); cachedPr = { fingerprint: 'old', lastEdited: new Date('2020-01-20T00:00:00Z').toISOString(), @@ -815,7 +812,7 @@ describe('workers/repository/update/pr/index', () => { const res = await ensurePr(config); expect(res).toEqual({ type: 'with-pr', - pr: { ...existingPr, title: 'Another title' }, + pr: existingPr, }); expect(logger.logger.debug).toHaveBeenCalledWith( 'PR fingerprints mismatch, processing PR' @@ -827,11 +824,13 @@ describe('workers/repository/update/pr/index', () => { config.repositoryCache = 'enabled'; platform.getBranchPr.mockResolvedValue(existingPr); cachedPr = { - fingerprint: fingerprint(generatePrFingerprintConfig(config)), + fingerprint: fingerprint( + generatePrFingerprintConfig({ ...config, fetchReleaseNotes: true }) + ), lastEdited: new Date('2020-01-20T00:00:00Z').toISOString(), }; prCache.getPrCache.mockReturnValueOnce(cachedPr); - const res = await ensurePr(config); + const res = await ensurePr({ ...config, fetchReleaseNotes: true }); expect(res).toEqual({ type: 'with-pr', pr: existingPr, @@ -839,7 +838,46 @@ describe('workers/repository/update/pr/index', () => { expect(logger.logger.debug).toHaveBeenCalledWith( 'PR cache matches and no PR changes in last 24hrs, so skipping PR body check' ); - expect(embedChangelog).toHaveBeenCalledTimes(0); + expect(embedChangelogs).toHaveBeenCalledTimes(0); + }); + + it('updates PR when rebase requested by user regardless of pr-cache state', async () => { + config.repositoryCache = 'enabled'; + platform.getBranchPr.mockResolvedValue({ + number, + sourceBranch, + title: prTitle, + bodyStruct: { + hash: 'hash-with-checkbox-checked', + rebaseRequested: true, + }, + state: 'open', + }); + cachedPr = { + fingerprint: fingerprint( + generatePrFingerprintConfig({ ...config, fetchReleaseNotes: true }) + ), + lastEdited: new Date('2020-01-20T00:00:00Z').toISOString(), + }; + prCache.getPrCache.mockReturnValueOnce(cachedPr); + const res = await ensurePr({ ...config, fetchReleaseNotes: true }); + expect(res).toEqual({ + type: 'with-pr', + pr: { + number, + sourceBranch, + title: prTitle, + bodyStruct, + state: 'open', + }, + }); + expect(logger.logger.debug).toHaveBeenCalledWith( + 'PR rebase requested, so skipping cache check' + ); + expect(logger.logger.debug).not.toHaveBeenCalledWith( + `Pull Request #${number} does not need updating` + ); + expect(embedChangelogs).toHaveBeenCalledTimes(1); }); it('logs when cache is enabled but pr-cache is absent', async () => { diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index cc308d54ab..94292fce88 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -15,7 +15,10 @@ import { platform, } from '../../../../modules/platform'; import { ensureComment } from '../../../../modules/platform/comment'; -import { hashBody } from '../../../../modules/platform/pr-body'; +import { + getPrBodyStruct, + hashBody, +} from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; import { ExternalHostError } from '../../../../types/errors/external-host-error'; import { getElapsedHours } from '../../../../util/date'; @@ -119,7 +122,9 @@ export async function ensurePr( const prCache = getPrCache(branchName); if (existingPr) { logger.debug('Found existing PR'); - if (prCache) { + if (existingPr.bodyStruct?.rebaseRequested) { + logger.debug('PR rebase requested, so skipping cache check'); + } else if (prCache) { logger.trace({ prCache }, 'Found existing PR cache'); // return if pr cache is valid and pr was not changed in the past 24hrs if (validatePrCache(prCache, prFingerprint)) { @@ -354,6 +359,7 @@ export async function ensurePr( } if (GlobalConfig.get('dryRun')) { logger.info(`DRY-RUN: Would update PR #${existingPr.number}`); + return { type: 'with-pr', pr: existingPr }; } else { await platform.updatePr({ number: existingPr.number, @@ -364,7 +370,14 @@ export async function ensurePr( logger.info({ pr: existingPr.number, prTitle }, `PR updated`); setPrCache(branchName, prFingerprint, true); } - return { type: 'with-pr', pr: existingPr }; + return { + type: 'with-pr', + pr: { + ...existingPr, + bodyStruct: getPrBodyStruct(prBody), + title: prTitle, + }, + }; } logger.debug({ branch: branchName, prTitle }, `Creating PR`); if (config.updateType === 'rollback') { -- GitLab