Skip to content

Commit

Permalink
feat(core/config): allow close to ignore for migration PRs (#16773)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gabriel-Ladzaretti committed Aug 21, 2022
1 parent 430a7a7 commit 01ceaea
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 4 deletions.
4 changes: 4 additions & 0 deletions docs/usage/configuration-options.md
Expand Up @@ -454,6 +454,10 @@ After we changed the [`baseBranches`](https://docs.renovatebot.com/configuration
This feature writes plain JSON for `.json` files, and JSON5 for `.json5` files.
JSON5 content can potentially be down leveled (`.json` files) and all comments will be removed.

<!-- prettier-ignore -->
!!! note
Closing the config migration PR will cause it to be ignored and not being reopend/recreated in the future.',

## configWarningReuseIssue

Renovate's default behavior is to reuse/reopen a single Config Warning issue in each repository so as to keep the "noise" down.
Expand Down
50 changes: 50 additions & 0 deletions lib/workers/repository/config-migration/branch/index.spec.ts
Expand Up @@ -5,11 +5,13 @@ import {
getConfig,
git,
mockedFunction,
partial,
platform,
} from '../../../../../test/util';
import { GlobalConfig } from '../../../../config/global';
import { logger } from '../../../../logger';
import type { Pr } from '../../../../modules/platform';
import { PrState } from '../../../../types';
import { createConfigMigrationBranch } from './create';
import type { MigratedData } from './migrated-data';
import { rebaseMigrationBranch } from './rebase';
Expand All @@ -19,6 +21,7 @@ jest.mock('./migrated-data');
jest.mock('./rebase');
jest.mock('./create');
jest.mock('../../../../util/git');
jest.mock('../../update/branch/handle-existing');

const migratedData = Fixtures.getJson<MigratedData>('./migrated-data.json');

Expand Down Expand Up @@ -98,5 +101,52 @@ describe('workers/repository/config-migration/branch/index', () => {
expect(git.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});

describe('handle closed PR', () => {
const title = 'PR title';
const pr = partial<Pr>({ title, state: PrState.Closed, number: 1 });

it('skips branch when there is a closed one delete it and add an ignore PR message', async () => {
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
git.branchExists.mockReturnValueOnce(true);
const res = await checkConfigMigrationBranch(config, migratedData);
expect(res).toBeNull();
expect(git.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(git.deleteBranch).toHaveBeenCalledTimes(1);
expect(logger.debug).toHaveBeenCalledWith(
{ prTitle: title },
'Closed PR already exists. Skipping branch.'
);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(platform.ensureComment).toHaveBeenCalledWith({
content:
'\n\nIf this PR was closed by mistake or you changed your mind, you can simply rename this PR and you will soon get a fresh replacement PR opened.',
topic: 'Renovate Ignore Notification',
number: 1,
});
});

it('dryrun: skips branch when there is a closed one and add an ignore PR message', async () => {
GlobalConfig.set({ dryRun: 'full' });
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
git.branchExists.mockReturnValueOnce(true);
const res = await checkConfigMigrationBranch(config, migratedData);
expect(res).toBeNull();
expect(logger.info).toHaveBeenCalledWith(
`DRY-RUN: Would ensure closed PR comment in PR #${pr.number}`
);
expect(logger.info).toHaveBeenCalledWith(
'DRY-RUN: Would delete branch ' + pr.sourceBranch
);
expect(logger.debug).toHaveBeenCalledWith(
{ prTitle: title },
'Closed PR already exists. Skipping branch.'
);
expect(platform.ensureComment).toHaveBeenCalledTimes(0);
});
});
});
});
71 changes: 67 additions & 4 deletions lib/workers/repository/config-migration/branch/index.ts
@@ -1,9 +1,16 @@
import { GlobalConfig } from '../../../../config/global';
import type { RenovateConfig } from '../../../../config/types';
import { logger } from '../../../../logger';
import { platform } from '../../../../modules/platform';
import { checkoutBranch } from '../../../../util/git';
import { FindPRConfig, Pr, platform } from '../../../../modules/platform';
import { ensureComment } from '../../../../modules/platform/comment';
import { PrState } from '../../../../types';
import {
branchExists,
checkoutBranch,
deleteBranch,
} from '../../../../util/git';
import { getMigrationBranchName } from '../common';
import { ConfigMigrationCommitMessageFactory } from './commit-message';
import { createConfigMigrationBranch } from './create';
import type { MigratedData } from './migrated-data';
import { rebaseMigrationBranch } from './rebase';
Expand All @@ -18,10 +25,38 @@ export async function checkConfigMigrationBranch(
return null;
}
const configMigrationBranch = getMigrationBranchName(config);
if (await migrationPrExists(configMigrationBranch)) {

const branchPr = await migrationPrExists(configMigrationBranch); // handles open/autoClosed PRs

if (!branchPr) {
const commitMessageFactory = new ConfigMigrationCommitMessageFactory(
config,
migratedConfigData.filename
);
const prTitle = commitMessageFactory.getPrTitle();
const closedPrConfig: FindPRConfig = {
branchName: configMigrationBranch,
prTitle,
state: PrState.Closed,
};

// handles closed PR
const closedPr = await platform.findPr(closedPrConfig);

// found closed migration PR
if (closedPr) {
logger.debug(
{ prTitle: closedPr.title },
'Closed PR already exists. Skipping branch.'
);
await handlepr(config, closedPr);
return null;
}
}

if (branchPr) {
logger.debug('Config Migration PR already exists');
await rebaseMigrationBranch(config, migratedConfigData);

if (platform.refreshPr) {
const configMigrationPr = await platform.getBranchPr(
configMigrationBranch
Expand All @@ -44,3 +79,31 @@ export async function checkConfigMigrationBranch(
export async function migrationPrExists(branchName: string): Promise<boolean> {
return !!(await platform.getBranchPr(branchName));
}

async function handlepr(config: RenovateConfig, pr: Pr): Promise<void> {
if (
pr.state === PrState.Closed &&
!config.suppressNotifications!.includes('prIgnoreNotification')
) {
if (GlobalConfig.get('dryRun')) {
logger.info(
`DRY-RUN: Would ensure closed PR comment in PR #${pr.number}`
);
} else {
const content =
'\n\nIf this PR was closed by mistake or you changed your mind, you can simply rename this PR and you will soon get a fresh replacement PR opened.';
await ensureComment({
number: pr.number,
topic: 'Renovate Ignore Notification',
content,
});
}
if (branchExists(pr.sourceBranch)) {
if (GlobalConfig.get('dryRun')) {
logger.info('DRY-RUN: Would delete branch ' + pr.sourceBranch);
} else {
await deleteBranch(pr.sourceBranch);
}
}
}
}
Expand Up @@ -9,6 +9,8 @@ exports[`workers/repository/config-migration/pr/index ensureConfigMigrationPr()
#### [PLEASE NOTE](https://docs.renovatebot.com/configuration-options#configmigration): JSON5 config file migrated! All comments & trailing commas were removed.
🔕 **Ignore**: Close this PR and you won't be reminded about config migration again, but one day your current config may no longer be valid.
❓ Got questions? Does something look wrong to you? Please don't hesitate to [request help here](https://github.com/renovatebot/renovate/discussions).
Expand All @@ -29,6 +31,8 @@ The Renovate config in this repository needs migrating. Typically this is becaus
🔕 **Ignore**: Close this PR and you won't be reminded about config migration again, but one day your current config may no longer be valid.
❓ Got questions? Does something look wrong to you? Please don't hesitate to [request help here](https://github.com/renovatebot/renovate/discussions).
Expand All @@ -49,6 +53,8 @@ The Renovate config in this repository needs migrating. Typically this is becaus
🔕 **Ignore**: Close this PR and you won't be reminded about config migration again, but one day your current config may no longer be valid.
❓ Got questions? Does something look wrong to you? Please don't hesitate to [request help here](https://github.com/renovatebot/renovate/discussions).
Expand All @@ -71,6 +77,8 @@ The Renovate config in this repository needs migrating. Typically this is becaus
🔕 **Ignore**: Close this PR and you won't be reminded about config migration again, but one day your current config may no longer be valid.
❓ Got questions? Does something look wrong to you? Please don't hesitate to [request help here](https://github.com/renovatebot/renovate/discussions).
Expand Down
2 changes: 2 additions & 0 deletions lib/workers/repository/config-migration/pr/index.ts
Expand Up @@ -47,6 +47,8 @@ ${
: ''
}
:no_bell: **Ignore**: Close this PR and you won't be reminded about config migration again, but one day your current config may no longer be valid.
:question: Got questions? Does something look wrong to you? Please don't hesitate to [request help here](${
// TODO: types (#7154)
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
Expand Down

0 comments on commit 01ceaea

Please sign in to comment.