Skip to content

Commit

Permalink
fix: perform pr-update when requested regardless of pr-cache state (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
RahulGautamSingh authored Mar 25, 2023
1 parent cab3571 commit 7f9874c
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 16 deletions.
64 changes: 51 additions & 13 deletions lib/workers/repository/update/pr/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand All @@ -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();
Expand Down Expand Up @@ -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(),
Expand All @@ -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'
Expand All @@ -827,19 +824,60 @@ 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,
});
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 () => {
Expand Down
19 changes: 16 additions & 3 deletions lib/workers/repository/update/pr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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,
Expand All @@ -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') {
Expand Down

0 comments on commit 7f9874c

Please sign in to comment.