Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dashboard): on demand config migration #31129

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
f5db234
init
RahulGautamSingh Aug 21, 2024
6cadf70
add checkbox code
RahulGautamSingh Aug 23, 2024
e7c182f
update dashboard text
RahulGautamSingh Aug 28, 2024
aca2d94
refactor
RahulGautamSingh Aug 28, 2024
cc9c35c
add tests
RahulGautamSingh Aug 29, 2024
5325b73
more tests
RahulGautamSingh Aug 30, 2024
a1bbfb9
cleanup
RahulGautamSingh Aug 30, 2024
e6aba4b
fix closed pr logic
RahulGautamSingh Aug 31, 2024
575216f
fix failing tests
RahulGautamSingh Aug 31, 2024
2616fc6
update docs
RahulGautamSingh Sep 3, 2024
c82eab0
update docs
RahulGautamSingh Sep 3, 2024
6a6b658
Apply suggestions from code review
RahulGautamSingh Sep 5, 2024
713684c
update docs
RahulGautamSingh Sep 5, 2024
21e75ff
apply suggestions
RahulGautamSingh Sep 5, 2024
3d49fd1
end with full-stop
RahulGautamSingh Sep 5, 2024
6d400d9
fix lint issues
RahulGautamSingh Sep 5, 2024
f2d2787
Apply suggestions from code review
RahulGautamSingh Sep 9, 2024
1cbeaff
Merge branch 'main' into feat/on-demand-config-migration
rarkins Sep 10, 2024
ba213f3
Update config-migration.md
rarkins Sep 10, 2024
1300605
Apply suggestions from code review
RahulGautamSingh Sep 17, 2024
ce1cf42
Merge branch 'main' into feat/on-demand-config-migration
RahulGautamSingh Sep 17, 2024
09b19d5
apply suggestions and refactor logic
RahulGautamSingh Sep 17, 2024
dfdf99d
add message for modified migration branch
RahulGautamSingh Sep 18, 2024
315a9fd
refactor: tidyting up
RahulGautamSingh Sep 20, 2024
2ffb2f5
Apply suggestions from code review
RahulGautamSingh Sep 23, 2024
edb48c3
apply suggestions
RahulGautamSingh Sep 23, 2024
f0e56c4
docs: apply suggestions
RahulGautamSingh Sep 23, 2024
8af5234
Update docs/usage/config-migration.md
RahulGautamSingh Sep 25, 2024
084513f
apply suggestions
RahulGautamSingh Sep 30, 2024
9cf67ca
Apply suggestions from code review
rarkins Oct 9, 2024
d2b3427
Apply suggestions from code review
RahulGautamSingh Oct 9, 2024
836a7a8
Merge branch 'main' into feat/on-demand-config-migration
rarkins Oct 11, 2024
7ac39a3
Update lib/workers/repository/config-migration/index.ts
rarkins Oct 11, 2024
a4cedd8
Merge branch 'main' into feat/on-demand-config-migration
RahulGautamSingh Oct 15, 2024
6ebbfe9
use comment as anchor
RahulGautamSingh Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
255 changes: 205 additions & 50 deletions lib/workers/repository/config-migration/branch/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,127 @@ describe('workers/repository/config-migration/branch/index', () => {
config.branchPrefix = 'some/';
});

it('Exits when Migration is not needed', async () => {
// exists when migration not needed
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
it('exits when migration is not needed', async () => {
await expect(
checkConfigMigrationBranch(config, null),
).resolves.toBeNull();
).resolves.toBeEmptyObject();
expect(logger.debug).toHaveBeenCalledWith(
'checkConfigMigrationBranch() Config does not need migration',
);
});

it('Updates migration branch & refresh PR', async () => {
it('returns add checkbox message when migration disabled and checkbox unchecked', async () => {
await expect(
checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: { configMigrationInfo: 'unchecked' },
},
migratedData,
),
).resolves.toMatchObject({ result: 'add-checkbox' });
expect(logger.debug).toHaveBeenCalledWith(
'Config migration needed but config migration is disabled and checkbox not ticked or not present.',
);
});

it('creates migration branch when migration disabled but checkbox checked', async () => {
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
);
await expect(
checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: { configMigrationInfo: 'checked' },
},
migratedData,
),
).resolves.toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR');
});

it('updates migration branch & refreshes pr when migration disabled but open pr exists', async () => {
platform.getBranchPr.mockResolvedValue(
mock<Pr>({
number: 1,
}),
);
platform.refreshPr = jest.fn().mockResolvedValueOnce(null);
mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed');
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: {
configMigrationInfo: 'migration-pr-exists',
},
},
migratedData,
);
// TODO: types (#22198)
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
prNumber: 1,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(platform.refreshPr).toHaveBeenCalledTimes(1);
expect(logger.debug).toHaveBeenCalledWith(
'Config Migration PR already exists',
);
});

it('creates migration branch when migration enabled but no pr exists', async () => {
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
);
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: true,
dependencyDashboardChecks: {
configMigrationInfo: 'no-checkbox',
},
},
migratedData,
);
// TODO: types (#22198)
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR');
});

it('updates migration branch & refresh PR when migration enabled and open pr exists', async () => {
platform.getBranchPr.mockResolvedValue(mock<Pr>());
// platform.refreshPr is undefined as it is an optional function
// declared as: refreshPr?(number: number): Promise<void>;
platform.refreshPr = jest.fn().mockResolvedValueOnce(null);
mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed');
const res = await checkConfigMigrationBranch(config, migratedData);
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: true,
dependencyDashboardChecks: {
configMigrationInfo: 'migration-pr-exists',
},
},
migratedData,
);
// TODO: types (#22198)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(logger.debug).toHaveBeenCalledWith(
Expand All @@ -68,23 +171,23 @@ describe('workers/repository/config-migration/branch/index', () => {
});
platform.getBranchPr.mockResolvedValueOnce(mock<Pr>());
mockedFunction(rebaseMigrationBranch).mockResolvedValueOnce('committed');
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#22198)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});

it('Creates migration PR', async () => {
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: true,
dependencyDashboardChecks: {
configMigrationInfo: 'migration-pr-exists',
},
},
migratedData,
);
const res = await checkConfigMigrationBranch(config, migratedData);
// TODO: types (#22198)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
expect(logger.debug).toHaveBeenCalledWith('Need to create migration PR');
});

it('Dry runs create migration PR', async () => {
Expand All @@ -94,9 +197,20 @@ describe('workers/repository/config-migration/branch/index', () => {
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
);
const res = await checkConfigMigrationBranch(config, migratedData);
const res = await checkConfigMigrationBranch(
{
...config,
dependencyDashboardChecks: {
configMigrationInfo: 'checked',
},
},
migratedData,
);
// TODO: types (#22198)
expect(res).toBe(`${config.branchPrefix!}migrate-config`);
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});
Expand All @@ -105,46 +219,87 @@ describe('workers/repository/config-migration/branch/index', () => {
const title = 'PR title';
const pr = partial<Pr>({ title, state: 'closed', number: 1 });

it('skips branch when there is a closed one delete it and add an ignore PR message', async () => {
it('adds a checkbox when migration is disabled but needed and a closed pr exists', async () => {
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
scm.branchExists.mockResolvedValueOnce(true);
const res = await checkConfigMigrationBranch(config, migratedData);
expect(res).toBeNull();
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
expect(scm.commitAndPush).toHaveBeenCalledTimes(0);
expect(scm.deleteBranch).toHaveBeenCalledTimes(1);
expect(logger.debug).toHaveBeenCalledWith(
{ prTitle: title },
'Closed PR already exists. Skipping branch.',
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: { configMigrationInfo: 'no-checkbox' },
},
migratedData,
);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(platform.ensureComment).toHaveBeenCalledWith({
content:
'\n\nIf you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR.',
topic: 'Renovate Ignore Notification',
number: 1,
expect(res).toMatchObject({
result: 'add-checkbox',
});
});

it('dryrun: skips branch when there is a closed one and add an ignore PR message', async () => {
GlobalConfig.set({ dryRun: 'full' });
it('deletes old branch and creates new migration branch when migration is disabled but needed, a closed pr exists and checkbox is checked', async () => {
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
scm.branchExists.mockResolvedValueOnce(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}`,
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
);
expect(logger.info).toHaveBeenCalledWith(
'DRY-RUN: Would delete branch ' + pr.sourceBranch,
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: { configMigrationInfo: 'checked' },
},
migratedData,
);
expect(logger.debug).toHaveBeenCalledWith(
{ prTitle: title },
'Closed PR already exists. Skipping branch.',
expect(scm.deleteBranch).toHaveBeenCalledTimes(1);
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(1);
});

it('adds a checkbox when migration is enabled and a closed pr exists', async () => {
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
scm.branchExists.mockResolvedValueOnce(true);
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: true,
dependencyDashboardChecks: { configMigrationInfo: 'no-checkbox' },
},
migratedData,
);
expect(res).toMatchObject({
result: 'add-checkbox',
});
});

it('dry run:deletes old branch and creates new migration branch when migration is disabled but needed, a closed pr exists and checkbox is checked', async () => {
GlobalConfig.set({
dryRun: 'full',
});
platform.findPr.mockResolvedValueOnce(pr);
platform.getBranchPr.mockResolvedValue(null);
scm.branchExists.mockResolvedValueOnce(true);
mockedFunction(createConfigMigrationBranch).mockResolvedValueOnce(
'committed',
);
const res = await checkConfigMigrationBranch(
{
...config,
configMigration: false,
dependencyDashboardChecks: { configMigrationInfo: 'checked' },
},
migratedData,
);
expect(platform.ensureComment).toHaveBeenCalledTimes(0);
expect(scm.deleteBranch).toHaveBeenCalledTimes(0);
expect(res).toMatchObject({
result: 'pr-exists',
migrationBranch: `${config.branchPrefix!}migrate-config`,
});
expect(scm.checkoutBranch).toHaveBeenCalledTimes(0);
});
});
});
Expand Down
Loading