diff --git a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts index 49978519f210aacda3bfe339916aaf8cd24c6723..b43ca2b6f801e9aef14ba3ead1be816890f5f6ab 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts @@ -3,6 +3,7 @@ import { Fixtures } from '../../../../../test/fixtures'; import { mockedFunction } from '../../../../../test/util'; import { migrateConfig } from '../../../../config/migration'; +import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; import { getFileList } from '../../../../util/git'; import { detectRepoFileConfig } from '../../init/merge'; @@ -34,8 +35,8 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { }); mockedFunction(detectRepoFileConfig).mockResolvedValue({ configFileName: 'renovate.json', + configFileRaw: rawNonMigrated, }); - mockedFunction(readLocalFile).mockResolvedValue(rawNonMigrated); mockedFunction(migrateConfig).mockReturnValue({ isMigrated: true, migratedConfig: migratedConfigObj, @@ -100,28 +101,29 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('Migrate a JSON5 config file', async () => { mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json5', + configFileRaw: rawNonMigratedJson5, }); - mockedFunction(readLocalFile).mockResolvedValueOnce(rawNonMigratedJson5); MigratedDataFactory.reset(); await expect(MigratedDataFactory.getAsync()).resolves.toEqual( migratedDataJson5 ); }); - it('Returns nothing due to fs error', async () => { - mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ - configFileName: undefined, - }); - mockedFunction(readLocalFile).mockRejectedValueOnce(null); + it('Returns nothing due to detectRepoFileConfig throwing', async () => { + const err = new Error('error-message'); + mockedFunction(detectRepoFileConfig).mockRejectedValueOnce(err); MigratedDataFactory.reset(); await expect(MigratedDataFactory.getAsync()).resolves.toBeNull(); + expect(logger.debug).toHaveBeenCalledWith( + { err }, + 'MigratedDataFactory.getAsync() Error initializing renovate MigratedData' + ); }); it('format and migrate a JSON config file', async () => { mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json', }); - mockedFunction(readLocalFile).mockResolvedValueOnce(rawNonMigrated); mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); MigratedDataFactory.reset(); await expect(MigratedDataFactory.getAsync()).resolves.toEqual( @@ -132,9 +134,21 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('should not stop run for invalid package.json', async () => { mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json', + configFileRaw: 'abci', }); mockedFunction(readLocalFile).mockResolvedValueOnce(rawNonMigrated); - mockedFunction(readLocalFile).mockResolvedValue('abci'); + MigratedDataFactory.reset(); + await expect(MigratedDataFactory.getAsync()).resolves.toEqual( + migratedData + ); + }); + + it('should not stop run for readLocalFile error', async () => { + mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ + configFileName: 'renovate.json', + configFileRaw: 'abci', + }); + mockedFunction(readLocalFile).mockRejectedValueOnce(null); MigratedDataFactory.reset(); await expect(MigratedDataFactory.getAsync()).resolves.toEqual( migratedData diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index da22bb5f4b0daf5beacf343ee2ff83dd26d20a23..a6062a49a38f04d37152556879e647bd8a2244d3 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -45,7 +45,9 @@ export async function applyPrettierFormatting( prettierExists = packageJsonContent && JSON.parse(packageJsonContent).prettier; } catch { - logger.warn('Invalid JSON found in package.json'); + logger.warn( + 'applyPrettierFormatting() - Error processing package.json file' + ); } } @@ -86,8 +88,11 @@ export class MigratedDataFactory { private static async build(): Promise<MigratedData | null> { let res: MigratedData | null = null; try { - const rc = await detectRepoFileConfig(); - const configFileParsed = rc?.configFileParsed || {}; + const { + configFileName, + configFileRaw: raw, + configFileParsed = {}, + } = await detectRepoFileConfig(); // get migrated config const { isMigrated, migratedConfig } = migrateConfig(configFileParsed); @@ -98,13 +103,11 @@ export class MigratedDataFactory { delete migratedConfig.errors; delete migratedConfig.warnings; - const filename = rc.configFileName ?? ''; - const raw = await readLocalFile(filename, 'utf8'); - // indent defaults to 2 spaces // TODO #7154 const indent = detectIndent(raw!); const indentSpace = indent.indent ?? ' '; + const filename = configFileName!; let content: string; if (filename.endsWith('.json5')) { diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index fc3817f2b4d42a268e43cd8c5a58938016cc2eef..1da3459f0a4c85d45cfedd4b1e298fae41db50ad 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -2,11 +2,7 @@ import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; import { commitAndPush } from '../../../../modules/platform/commit'; -import { - getFile, - isBranchBehindBase, - isBranchModified, -} from '../../../../util/git'; +import { getFile, isBranchModified } from '../../../../util/git'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; import type { MigratedData } from './migrated-data'; @@ -24,10 +20,7 @@ export async function rebaseMigrationBranch( const configFileName = migratedConfigData.filename; const contents = migratedConfigData.content; const existingContents = await getFile(configFileName, branchName); - if ( - contents === existingContents && - !(await isBranchBehindBase(branchName)) - ) { + if (contents === existingContents) { logger.debug('Migration branch is up to date'); return null; } diff --git a/lib/workers/repository/config-migration/index.spec.ts b/lib/workers/repository/config-migration/index.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..0a4380e34c88583a0cff4302c26f7db3e15bef9a --- /dev/null +++ b/lib/workers/repository/config-migration/index.spec.ts @@ -0,0 +1,51 @@ +import { Fixtures } from '../../../../test/fixtures'; +import { defaultConfig, mockedFunction } from '../../../../test/util'; +import { checkConfigMigrationBranch } from './branch'; +import { MigratedDataFactory } from './branch/migrated-data'; +import { ensureConfigMigrationPr } from './pr'; +import { configMigration } from './index'; + +jest.mock('./pr'); +jest.mock('./branch'); +jest.mock('./branch/migrated-data'); + +const content = Fixtures.getJson('./migrated-data.json', './branch'); +const filename = 'renovate.json'; +const branchName = 'renovate/config-migration'; +const config = { + ...defaultConfig, + configMigration: true, +}; + +describe('workers/repository/config-migration/index', () => { + beforeEach(() => { + jest.resetAllMocks(); + mockedFunction(MigratedDataFactory.getAsync).mockResolvedValue({ + filename, + content, + }); + }); + + it('does nothing when config migration is disabled', async () => { + await configMigration({ ...config, configMigration: false }, []); + expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); + expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); + expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); + }); + + it('ensures config migration PR when migrated', async () => { + const branchList: string[] = []; + mockedFunction(checkConfigMigrationBranch).mockResolvedValue(branchName); + await configMigration(config, branchList); + expect(branchList).toContainEqual(branchName); + expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(1); + }); + + it('skips pr creation when migration is not needed', async () => { + const branchList: string[] = []; + mockedFunction(checkConfigMigrationBranch).mockResolvedValue(null); + await configMigration(config, branchList); + expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(1); + expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); + }); +}); diff --git a/lib/workers/repository/config-migration/index.ts b/lib/workers/repository/config-migration/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..33a401d063575070fa2edf29664032076b2e0341 --- /dev/null +++ b/lib/workers/repository/config-migration/index.ts @@ -0,0 +1,22 @@ +import type { RenovateConfig } from '../../../config/types'; +import { checkConfigMigrationBranch } from './branch'; +import { MigratedDataFactory } from './branch/migrated-data'; +import { ensureConfigMigrationPr } from './pr'; + +export async function configMigration( + config: RenovateConfig, + branchList: string[] +): Promise<void> { + if (config.configMigration) { + const migratedConfigData = await MigratedDataFactory.getAsync(); + const migrationBranch = await checkConfigMigrationBranch( + config, + migratedConfigData + ); // null if migration not needed + if (migrationBranch) { + branchList.push(migrationBranch); + await ensureConfigMigrationPr(config, migratedConfigData!); + } + MigratedDataFactory.reset(); + } +} diff --git a/lib/workers/repository/finalise/index.ts b/lib/workers/repository/finalise/index.ts index 5a588bda292e523da925651c27edd63738674836..0c8f93dcf72191bd7eb0a132fff716a426d39498 100644 --- a/lib/workers/repository/finalise/index.ts +++ b/lib/workers/repository/finalise/index.ts @@ -1,12 +1,9 @@ -/* eslint-disable @typescript-eslint/no-unnecessary-type-assertion */ import type { RenovateConfig } from '../../../config/types'; import { logger } from '../../../logger'; import { platform } from '../../../modules/platform'; import * as repositoryCache from '../../../util/cache/repository'; import { clearRenovateRefs } from '../../../util/git'; -import { checkConfigMigrationBranch } from '../config-migration/branch'; -import { MigratedDataFactory } from '../config-migration/branch/migrated-data'; -import { ensureConfigMigrationPr } from '../config-migration/pr'; +import { configMigration } from '../config-migration'; import { PackageFiles } from '../package-files'; import { pruneStaleBranches } from './prune'; import { runRenovateRepoStats } from './repository-statistics'; @@ -16,18 +13,7 @@ export async function finaliseRepo( config: RenovateConfig, branchList: string[] ): Promise<void> { - if (config.configMigration) { - const migratedConfigData = await MigratedDataFactory.getAsync(); - const migrationBranch = await checkConfigMigrationBranch( - config, - migratedConfigData! - ); // null if migration not needed - if (migrationBranch) { - branchList.push(migrationBranch); - await ensureConfigMigrationPr(config, migratedConfigData!); - } - MigratedDataFactory.reset(); - } + await configMigration(config, branchList); await repositoryCache.saveCache(); await pruneStaleBranches(config, branchList); await platform.ensureIssueClosing(