From 70a49def74555e0bef0a57d0b97dc437f3947f61 Mon Sep 17 00:00:00 2001
From: Gabriel-Ladzaretti
<97394622+Gabriel-Ladzaretti@users.noreply.github.com>
Date: Mon, 29 Aug 2022 23:08:35 +0300
Subject: [PATCH] refactor(repo/config-migration): use cached raw config file
from `detectRepoFileConfig` (#17311)
Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
---
.../branch/migrated-data.spec.ts | 32 ++++++++----
.../config-migration/branch/migrated-data.ts | 15 +++---
.../config-migration/branch/rebase.ts | 11 +---
.../repository/config-migration/index.spec.ts | 51 +++++++++++++++++++
.../repository/config-migration/index.ts | 22 ++++++++
lib/workers/repository/finalise/index.ts | 18 +------
6 files changed, 109 insertions(+), 40 deletions(-)
create mode 100644 lib/workers/repository/config-migration/index.spec.ts
create mode 100644 lib/workers/repository/config-migration/index.ts
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 49978519f2..b43ca2b6f8 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 da22bb5f4b..a6062a49a3 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 fc3817f2b4..1da3459f0a 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 0000000000..0a4380e34c
--- /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 0000000000..33a401d063
--- /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 5a588bda29..0c8f93dcf7 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(
--
GitLab