From ba8b19c6aaf8134e55fff643e36d7a2c9fc58ca6 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Thu, 6 Oct 2022 01:15:17 +0300 Subject: [PATCH 1/7] move applyPrettierFormatting to the commiting phase --- .../config-migration/branch/create.spec.ts | 29 ++++++ .../config-migration/branch/create.ts | 7 +- .../branch/migrated-data.spec.ts | 96 ++++++++++++------- .../config-migration/branch/migrated-data.ts | 83 ++++++++-------- .../config-migration/branch/rebase.spec.ts | 33 ++++++- .../config-migration/branch/rebase.ts | 24 ++++- 6 files changed, 195 insertions(+), 77 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/create.spec.ts b/lib/workers/repository/config-migration/branch/create.spec.ts index 3672f6833be9a5..abfa45bd1ac180 100644 --- a/lib/workers/repository/config-migration/branch/create.spec.ts +++ b/lib/workers/repository/config-migration/branch/create.spec.ts @@ -3,9 +3,14 @@ import { RenovateConfig, getConfig, platform } from '../../../../../test/util'; import { checkoutBranch, commitFiles } from '../../../../util/git'; import { createConfigMigrationBranch } from './create'; import type { MigratedData } from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; jest.mock('../../../../util/git'); +const formattedMigratedData = Fixtures.getJson( + './migrated-data-formatted.json' +); + describe('workers/repository/config-migration/branch/create', () => { const raw = Fixtures.getJson('./renovate.json'); const indent = ' '; @@ -24,6 +29,11 @@ describe('workers/repository/config-migration/branch/create', () => { }); describe('createConfigMigrationBranch', () => { + const prettierSpy = jest.spyOn( + MigratedDataFactory, + 'applyPrettierFormatting' + ); + it('applies the default commit message', async () => { await createConfigMigrationBranch(config, migratedConfigData); expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); @@ -178,6 +188,25 @@ describe('workers/repository/config-migration/branch/create', () => { platformCommit: false, }); }); + + it('applies prettier formatting to the committed content', async () => { + const formatted = formattedMigratedData.content; + prettierSpy.mockResolvedValueOnce(formattedMigratedData.content); + await createConfigMigrationBranch(config, migratedConfigData); + expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); + expect(commitFiles).toHaveBeenCalledWith({ + branchName: 'renovate/migrate-config', + files: [ + { + type: 'addition', + path: 'renovate.json', + contents: formatted, + }, + ], + message: 'Migrate config renovate.json', + platformCommit: false, + }); + }); }); }); }); diff --git a/lib/workers/repository/config-migration/branch/create.ts b/lib/workers/repository/config-migration/branch/create.ts index 0e8bbe74dccfb1..5e5bc5b3223387 100644 --- a/lib/workers/repository/config-migration/branch/create.ts +++ b/lib/workers/repository/config-migration/branch/create.ts @@ -5,6 +5,7 @@ import { commitAndPush } from '../../../../modules/platform/commit'; import { checkoutBranch } from '../../../../util/git'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function createConfigMigrationBranch( @@ -12,7 +13,6 @@ export async function createConfigMigrationBranch( migratedConfigData: MigratedData ): Promise { logger.debug('createConfigMigrationBranch()'); - const contents = migratedConfigData.content; const configFileName = migratedConfigData.filename; logger.debug('Creating config migration branch'); @@ -30,6 +30,11 @@ export async function createConfigMigrationBranch( } await checkoutBranch(config.defaultBranch!); + let contents = migratedConfigData.content; + const prettified = await MigratedDataFactory.applyPrettierFormatting(); + if (prettified) { + contents = prettified; + } return commitAndPush({ branchName: getMigrationBranchName(config), files: [ 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 b43ca2b6f801e9..ecc445e53739a9 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts @@ -7,7 +7,7 @@ import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; import { getFileList } from '../../../../util/git'; import { detectRepoFileConfig } from '../../init/merge'; -import { MigratedDataFactory, applyPrettierFormatting } from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; jest.mock('../../../../config/migration'); jest.mock('../../../../util/git'); @@ -41,7 +41,6 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { isMigrated: true, migratedConfig: migratedConfigObj, }); - mockedFunction(getFileList).mockResolvedValue([]); }); it('Calls getAsync a first when migration not needed', async () => { @@ -119,49 +118,82 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { 'MigratedDataFactory.getAsync() Error initializing renovate MigratedData' ); }); + }); - it('format and migrate a JSON config file', async () => { + describe(' MigratedDataFactory.applyPrettierFormatting', () => { + beforeAll(() => { + mockedFunction(detectIndent).mockReturnValueOnce({ + type: 'space', + amount: 2, + indent: ' ', + }); mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json', + configFileRaw: rawNonMigrated, + }); + mockedFunction(migrateConfig).mockReturnValueOnce({ + isMigrated: true, + migratedConfig: migratedConfigObj, }); - mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - formattedMigratedData - ); }); - it('should not stop run for invalid package.json', async () => { - mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ - configFileName: 'renovate.json', - configFileRaw: 'abci', - }); - mockedFunction(readLocalFile).mockResolvedValueOnce(rawNonMigrated); - MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + beforeEach(() => { + mockedFunction(getFileList).mockResolvedValue([]); }); - it('should not stop run for readLocalFile error', async () => { - mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ - configFileName: 'renovate.json', - configFileRaw: 'abci', - }); + it('returns null if invoked before factory is initialized', async () => { + await expect( + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toBeNull(); + }); + + it('does not format when no prettier config is present', async () => { + const unformatted = migratedData.content; + mockedFunction(readLocalFile).mockResolvedValueOnce(null); + await MigratedDataFactory.getAsync(); + await expect( + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toEqual(unformatted); + }); + + it('does not format when failing to fetch package.json file', async () => { + const unformatted = migratedData.content; mockedFunction(readLocalFile).mockRejectedValueOnce(null); - MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + await MigratedDataFactory.getAsync(); + await expect( + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toEqual(unformatted); + }); + + it('does not format when there is an invalid package.json file', async () => { + const unformatted = migratedData.content; + mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json'); + await MigratedDataFactory.getAsync(); + await expect( + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toEqual(unformatted); + }); + + it('formats when prettier config file is found', async () => { + const formatted = formattedMigratedData.content; + mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); + await MigratedDataFactory.getAsync(); + await expect( + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toEqual(formatted); }); - it('return original content if its invalid', async () => { + it('formats when finds prettier config inside the package.json file', async () => { + const formatted = formattedMigratedData.content; + mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ + configFileName: 'renovate.json', + }); + mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}'); + await MigratedDataFactory.getAsync(); await expect( - applyPrettierFormatting(`{"name":"Rahul"`, 'json', { - indent: ' ', - amount: 2, - }) - ).resolves.toBe(`{"name":"Rahul"`); + MigratedDataFactory.applyPrettierFormatting() + ).resolves.toEqual(formatted); }); }); }); diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index a6062a49a38f04..106cbd632185ae 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -11,6 +11,7 @@ export interface MigratedData { content: string; filename: string; } + interface Indent { amount: number; indent: string; @@ -30,42 +31,10 @@ const prettierConfigFilenames = new Set([ '.prettierrc.toml', ]); -export async function applyPrettierFormatting( - content: string, - parser: string, - indent: Indent -): Promise { - const fileList = await getFileList(); - let prettierExists = fileList.some((file) => - prettierConfigFilenames.has(file) - ); - if (!prettierExists) { - try { - const packageJsonContent = await readLocalFile('package.json', 'utf8'); - prettierExists = - packageJsonContent && JSON.parse(packageJsonContent).prettier; - } catch { - logger.warn( - 'applyPrettierFormatting() - Error processing package.json file' - ); - } - } - - if (!prettierExists) { - return content; - } - const options = { - parser, - tabWidth: indent.amount === 0 ? 2 : indent.amount, - useTabs: indent.type === 'tab', - }; - - return prettier.format(content, options); -} - export class MigratedDataFactory { // singleton private static data: MigratedData | null; + private static indent: Indent; public static async getAsync(): Promise { if (this.data) { @@ -81,6 +50,44 @@ export class MigratedDataFactory { return this.data; } + public static async applyPrettierFormatting(): Promise { + logger.trace('applyPrettierFormatting() - START'); + if (!this.data) { + return null; + } + + const { content, filename } = this.data; + const indent = this.indent; + const fileList = await getFileList(); + let prettierExists = fileList.some((file) => + prettierConfigFilenames.has(file) + ); + + if (!prettierExists) { + try { + const packageJsonContent = await readLocalFile('package.json', 'utf8'); + prettierExists = + packageJsonContent && JSON.parse(packageJsonContent).prettier; + } catch { + logger.warn( + 'applyPrettierFormatting() - Error processing package.json file' + ); + } + } + + if (!prettierExists) { + return content; + } + const options = { + parser: filename.endsWith('.json5') ? 'json5' : 'json', + tabWidth: indent.amount === 0 ? 2 : indent.amount, + useTabs: indent.type === 'tab', + }; + + logger.trace('applyPrettierFormatting() - END'); + return prettier.format(content, options); + } + public static reset(): void { this.data = null; } @@ -105,8 +112,8 @@ export class MigratedDataFactory { // indent defaults to 2 spaces // TODO #7154 - const indent = detectIndent(raw!); - const indentSpace = indent.indent ?? ' '; + this.indent = detectIndent(raw!); + const indentSpace = this.indent.indent ?? ' '; const filename = configFileName!; let content: string; @@ -116,12 +123,6 @@ export class MigratedDataFactory { content = JSON.stringify(migratedConfig, undefined, indentSpace); } - // format if prettier is found in the user's repo - content = await applyPrettierFormatting( - content, - filename.endsWith('.json5') ? 'json5' : 'json', - indent - ); if (!content.endsWith('\n')) { content += '\n'; } diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index 81ab12b7f5e494..718fe80d9754a0 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -6,13 +6,23 @@ import { platform, } from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; -import { checkoutBranch } from '../../../../util/git'; +import { checkoutBranch, commitFiles } from '../../../../util/git'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; import { rebaseMigrationBranch } from './rebase'; jest.mock('../../../../util/git'); +const formattedMigratedData = Fixtures.getJson( + './migrated-data-formatted.json' +); + describe('workers/repository/config-migration/branch/rebase', () => { + const prettierSpy = jest.spyOn( + MigratedDataFactory, + 'applyPrettierFormatting' + ); + beforeAll(() => { GlobalConfig.set({ localDir: '', @@ -63,6 +73,27 @@ describe('workers/repository/config-migration/branch/rebase', () => { expect(git.commitFiles).toHaveBeenCalledTimes(1); }); + it('applies prettier formatting when rebasing the migration branch ', async () => { + const formatted = formattedMigratedData.content; + prettierSpy.mockResolvedValueOnce(formattedMigratedData.content); + git.isBranchBehindBase.mockResolvedValueOnce(true); + await rebaseMigrationBranch(config, migratedConfigData); + expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); + expect(git.commitFiles).toHaveBeenCalledTimes(1); + expect(commitFiles).toHaveBeenCalledWith({ + branchName: 'renovate/migrate-config', + files: [ + { + type: 'addition', + path: 'renovate.json', + contents: formatted, + }, + ], + message: 'Migrate config renovate.json', + platformCommit: false, + }); + }); + it('does not rebases migration branch when in dryRun is on', async () => { GlobalConfig.set({ dryRun: 'full', diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 06a3800915a6f8..46f61606c5f7fd 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -1,3 +1,4 @@ +import hasha from 'hasha'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -7,8 +8,10 @@ import { getFile, isBranchModified, } from '../../../../util/git'; +import { regEx } from '../../../../util/regex'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function rebaseMigrationBranch( @@ -22,9 +25,9 @@ export async function rebaseMigrationBranch( return null; } const configFileName = migratedConfigData.filename; - const contents = migratedConfigData.content; + let contents = migratedConfigData.content; const existingContents = await getFile(configFileName, branchName); - if (contents === existingContents) { + if (hash(contents) === hash(existingContents)) { logger.debug('Migration branch is up to date'); return null; } @@ -42,6 +45,10 @@ export async function rebaseMigrationBranch( const commitMessage = commitMessageFactory.getCommitMessage(); await checkoutBranch(config.defaultBranch!); + const prettified = await MigratedDataFactory.applyPrettierFormatting(); + if (prettified) { + contents = prettified; + } return commitAndPush({ branchName, files: [ @@ -55,3 +62,16 @@ export async function rebaseMigrationBranch( platformCommit: !!config.platformCommit, }); } + +function stripWhitespaces(str: string): string { + const whitespacesRe = regEx(/\s/g); + return str.replace(whitespacesRe, ''); +} + +function hash(str: string | null): string | null { + if (!str) { + return null; + } + const stripped = stripWhitespaces(str); + return hasha(stripped, { algorithm: 'sha256' }); +} From 9f7a640b48e568780276223c2c3435e5956c9e33 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Thu, 6 Oct 2022 17:05:25 +0300 Subject: [PATCH 2/7] apply code review suggestions --- .../config-migration/branch/migrated-data.ts | 68 ++++++++++--------- .../config-migration/branch/rebase.ts | 7 +- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index 106cbd632185ae..5a67b31cea4662 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -51,41 +51,47 @@ export class MigratedDataFactory { } public static async applyPrettierFormatting(): Promise { - logger.trace('applyPrettierFormatting() - START'); - if (!this.data) { - return null; - } + try { + logger.trace('applyPrettierFormatting() - START'); + if (!this.data) { + return null; + } - const { content, filename } = this.data; - const indent = this.indent; - const fileList = await getFileList(); - let prettierExists = fileList.some((file) => - prettierConfigFilenames.has(file) - ); - - if (!prettierExists) { - try { - const packageJsonContent = await readLocalFile('package.json', 'utf8'); - prettierExists = - packageJsonContent && JSON.parse(packageJsonContent).prettier; - } catch { - logger.warn( - 'applyPrettierFormatting() - Error processing package.json file' - ); + const { content, filename } = this.data; + const indent = this.indent; + const fileList = await getFileList(); + let prettierExists = fileList.some((file) => + prettierConfigFilenames.has(file) + ); + + if (!prettierExists) { + try { + const packageJsonContent = await readLocalFile( + 'package.json', + 'utf8' + ); + prettierExists = + packageJsonContent && JSON.parse(packageJsonContent).prettier; + } catch { + logger.warn( + 'applyPrettierFormatting() - Error processing package.json file' + ); + } } - } - if (!prettierExists) { - return content; + if (!prettierExists) { + return content; + } + const options = { + parser: filename.endsWith('.json5') ? 'json5' : 'json', + tabWidth: indent.amount === 0 ? 2 : indent.amount, + useTabs: indent.type === 'tab', + }; + + return prettier.format(content, options); + } finally { + logger.trace('applyPrettierFormatting() - END'); } - const options = { - parser: filename.endsWith('.json5') ? 'json5' : 'json', - tabWidth: indent.amount === 0 ? 2 : indent.amount, - useTabs: indent.type === 'tab', - }; - - logger.trace('applyPrettierFormatting() - END'); - return prettier.format(content, options); } public static reset(): void { diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 46f61606c5f7fd..47bb7fbbcaf693 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -8,7 +8,7 @@ import { getFile, isBranchModified, } from '../../../../util/git'; -import { regEx } from '../../../../util/regex'; +import { quickStringify } from '../../../../util/stringify'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; import { MigratedDataFactory } from './migrated-data'; @@ -64,8 +64,7 @@ export async function rebaseMigrationBranch( } function stripWhitespaces(str: string): string { - const whitespacesRe = regEx(/\s/g); - return str.replace(whitespacesRe, ''); + return quickStringify(JSON.parse(str)); } function hash(str: string | null): string | null { @@ -73,5 +72,5 @@ function hash(str: string | null): string | null { return null; } const stripped = stripWhitespaces(str); - return hasha(stripped, { algorithm: 'sha256' }); + return hasha(stripped); } From 64583a1d6eadee283ee15d1ea13ddfc389d21dba Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Sun, 16 Oct 2022 01:31:30 +0300 Subject: [PATCH 3/7] extract applyPrettier to the global scope rename getAsync to get --- .../branch/__fixtures__/migrated-data.json | 7 +- .../branch/__fixtures__/migrated-data.json5 | 7 +- .../config-migration/branch/create.spec.ts | 46 +++------ .../config-migration/branch/create.ts | 11 +-- .../branch/migrated-data.spec.ts | 76 +++++++------- .../config-migration/branch/migrated-data.ts | 98 +++++++++---------- .../config-migration/branch/rebase.spec.ts | 15 +-- .../config-migration/branch/rebase.ts | 11 ++- .../repository/config-migration/index.spec.ts | 8 +- .../repository/config-migration/index.ts | 2 +- .../config-migration/pr/index.spec.ts | 3 + 11 files changed, 140 insertions(+), 144 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json index 9febe12a31cb7f..440ee5392db4da 100644 --- a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json +++ b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json @@ -1,4 +1,9 @@ { + "content": "{\n \"extends\": [\n \":separateMajorReleases\",\n \":prImmediately\",\n \":renovatePrefix\",\n \":semanticPrefixFixDepsChoreOthers\",\n \":updateNotScheduled\",\n \":automergeDisabled\",\n \":maintainLockFilesDisabled\",\n \":autodetectPinVersions\",\n \"group:monorepos\"\n ],\n \"onboarding\": false,\n \"rangeStrategy\": \"replace\",\n \"semanticCommits\": \"enabled\",\n \"timezone\": \"US/Central\",\n \"baseBranches\": [\n \"main\"\n ]\n}\n", "filename": "renovate.json", - "content": "{\n \"extends\": [\n \":separateMajorReleases\",\n \":prImmediately\",\n \":renovatePrefix\",\n \":semanticPrefixFixDepsChoreOthers\",\n \":updateNotScheduled\",\n \":automergeDisabled\",\n \":maintainLockFilesDisabled\",\n \":autodetectPinVersions\",\n \"group:monorepos\"\n ],\n \"onboarding\": false,\n \"rangeStrategy\": \"replace\",\n \"semanticCommits\": \"enabled\",\n \"timezone\": \"US/Central\",\n \"baseBranches\": [\n \"main\"\n ]\n}\n" + "indent": { + "amount": 2, + "indent": " ", + "type": "space" + } } diff --git a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 index 0540d1f8ccb642..259972dafa88f5 100644 --- a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 +++ b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 @@ -1,4 +1,9 @@ { + "content": "{\n extends: [\n ':separateMajorReleases',\n ':prImmediately',\n ':renovatePrefix',\n ':semanticPrefixFixDepsChoreOthers',\n ':updateNotScheduled',\n ':automergeDisabled',\n ':maintainLockFilesDisabled',\n ':autodetectPinVersions',\n 'group:monorepos',\n ],\n onboarding: false,\n rangeStrategy: 'replace',\n semanticCommits: 'enabled',\n timezone: 'US/Central',\n baseBranches: [\n 'main',\n ],\n}\n", "filename": "renovate.json5", - "content": "{\n extends: [\n ':separateMajorReleases',\n ':prImmediately',\n ':renovatePrefix',\n ':semanticPrefixFixDepsChoreOthers',\n ':updateNotScheduled',\n ':automergeDisabled',\n ':maintainLockFilesDisabled',\n ':autodetectPinVersions',\n 'group:monorepos',\n ],\n onboarding: false,\n rangeStrategy: 'replace',\n semanticCommits: 'enabled',\n timezone: 'US/Central',\n baseBranches: [\n 'main',\n ],\n}\n" + "indent": { + "amount": 2, + "indent": " ", + "type": "space" + } } diff --git a/lib/workers/repository/config-migration/branch/create.spec.ts b/lib/workers/repository/config-migration/branch/create.spec.ts index abfa45bd1ac180..adeac4742f9279 100644 --- a/lib/workers/repository/config-migration/branch/create.spec.ts +++ b/lib/workers/repository/config-migration/branch/create.spec.ts @@ -1,21 +1,24 @@ +import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../../test/fixtures'; -import { RenovateConfig, getConfig, platform } from '../../../../../test/util'; +import { + RenovateConfig, + getConfig, + partial, + platform, +} from '../../../../../test/util'; import { checkoutBranch, commitFiles } from '../../../../util/git'; import { createConfigMigrationBranch } from './create'; import type { MigratedData } from './migrated-data'; -import { MigratedDataFactory } from './migrated-data'; +import * as MigratedDataModule from './migrated-data'; jest.mock('../../../../util/git'); -const formattedMigratedData = Fixtures.getJson( - './migrated-data-formatted.json' -); - describe('workers/repository/config-migration/branch/create', () => { const raw = Fixtures.getJson('./renovate.json'); const indent = ' '; const renovateConfig = JSON.stringify(raw, undefined, indent) + '\n'; const filename = 'renovate.json'; + const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); let config: RenovateConfig; let migratedConfigData: MigratedData; @@ -25,15 +28,15 @@ describe('workers/repository/config-migration/branch/create', () => { config = getConfig(); config.baseBranch = 'dev'; config.defaultBranch = 'master'; - migratedConfigData = { content: renovateConfig, filename }; + migratedConfigData = { + content: renovateConfig, + filename, + indent: partial({}), + }; + prettierSpy.mockResolvedValueOnce(migratedConfigData.content); }); describe('createConfigMigrationBranch', () => { - const prettierSpy = jest.spyOn( - MigratedDataFactory, - 'applyPrettierFormatting' - ); - it('applies the default commit message', async () => { await createConfigMigrationBranch(config, migratedConfigData); expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); @@ -188,25 +191,6 @@ describe('workers/repository/config-migration/branch/create', () => { platformCommit: false, }); }); - - it('applies prettier formatting to the committed content', async () => { - const formatted = formattedMigratedData.content; - prettierSpy.mockResolvedValueOnce(formattedMigratedData.content); - await createConfigMigrationBranch(config, migratedConfigData); - expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); - expect(commitFiles).toHaveBeenCalledWith({ - branchName: 'renovate/migrate-config', - files: [ - { - type: 'addition', - path: 'renovate.json', - contents: formatted, - }, - ], - message: 'Migrate config renovate.json', - platformCommit: false, - }); - }); }); }); }); diff --git a/lib/workers/repository/config-migration/branch/create.ts b/lib/workers/repository/config-migration/branch/create.ts index 5e5bc5b3223387..285823426d56e9 100644 --- a/lib/workers/repository/config-migration/branch/create.ts +++ b/lib/workers/repository/config-migration/branch/create.ts @@ -1,3 +1,4 @@ +import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -5,7 +6,7 @@ import { commitAndPush } from '../../../../modules/platform/commit'; import { checkoutBranch } from '../../../../util/git'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; -import { MigratedDataFactory } from './migrated-data'; +import { PrettierParser, applyPrettierFormatting } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function createConfigMigrationBranch( @@ -30,11 +31,9 @@ export async function createConfigMigrationBranch( } await checkoutBranch(config.defaultBranch!); - let contents = migratedConfigData.content; - const prettified = await MigratedDataFactory.applyPrettierFormatting(); - if (prettified) { - contents = prettified; - } + const { content, filename, indent } = migratedConfigData; + const parser = upath.extname(filename).replace('.', '') as PrettierParser; + const contents = await applyPrettierFormatting(content, parser, indent); return commitAndPush({ branchName: getMigrationBranchName(config), files: [ 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 ecc445e53739a9..55fd85b4234faf 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts @@ -7,7 +7,7 @@ import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; import { getFileList } from '../../../../util/git'; import { detectRepoFileConfig } from '../../init/merge'; -import { MigratedDataFactory } from './migrated-data'; +import { MigratedDataFactory, applyPrettierFormatting } from './migrated-data'; jest.mock('../../../../config/migration'); jest.mock('../../../../util/git'); @@ -48,53 +48,49 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { isMigrated: false, migratedConfig: {}, }); - await expect(MigratedDataFactory.getAsync()).resolves.toBeNull(); + await expect(MigratedDataFactory.get()).resolves.toBeNull(); }); it('Calls getAsync a first time to initialize the factory', async () => { - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); expect(detectRepoFileConfig).toHaveBeenCalledTimes(1); }); it('Calls getAsync a second time to get the saved data from before', async () => { - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); expect(detectRepoFileConfig).toHaveBeenCalledTimes(0); }); describe('MigratedData class', () => { it('gets the filename from the class instance', async () => { - const data = await MigratedDataFactory.getAsync(); + const data = await MigratedDataFactory.get(); expect(data?.filename).toBe('renovate.json'); }); it('gets the content from the class instance', async () => { - const data = await MigratedDataFactory.getAsync(); + const data = await MigratedDataFactory.get(); expect(data?.content).toBe(migratedData.content); }); }); it('Resets the factory and gets a new value', async () => { MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); }); it('Resets the factory and gets a new value with default indentation', async () => { - mockedFunction(detectIndent).mockReturnValueOnce({ + const indent = { type: undefined, amount: 0, // TODO: incompatible types (#7154) indent: null as never, - }); + }; + mockedFunction(detectIndent).mockReturnValueOnce(indent); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( - migratedData - ); + await expect(MigratedDataFactory.get()).resolves.toEqual({ + ...migratedData, + indent, + }); }); it('Migrate a JSON5 config file', async () => { @@ -103,7 +99,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { configFileRaw: rawNonMigratedJson5, }); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toEqual( + await expect(MigratedDataFactory.get()).resolves.toEqual( migratedDataJson5 ); }); @@ -112,7 +108,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { const err = new Error('error-message'); mockedFunction(detectRepoFileConfig).mockRejectedValueOnce(err); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.getAsync()).resolves.toBeNull(); + await expect(MigratedDataFactory.get()).resolves.toBeNull(); expect(logger.debug).toHaveBeenCalledWith( { err }, 'MigratedDataFactory.getAsync() Error initializing renovate MigratedData' @@ -142,57 +138,59 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { mockedFunction(getFileList).mockResolvedValue([]); }); - it('returns null if invoked before factory is initialized', async () => { - await expect( - MigratedDataFactory.applyPrettierFormatting() - ).resolves.toBeNull(); - }); - it('does not format when no prettier config is present', async () => { - const unformatted = migratedData.content; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce(null); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('does not format when failing to fetch package.json file', async () => { - const unformatted = migratedData.content; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockRejectedValueOnce(null); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('does not format when there is an invalid package.json file', async () => { - const unformatted = migratedData.content; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json'); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('formats when prettier config file is found', async () => { + const { content, filename } = migratedData; const formatted = formattedMigratedData.content; mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting( + content, + filename.endsWith('json') ? 'json' : 'json5' + ) ).resolves.toEqual(formatted); }); it('formats when finds prettier config inside the package.json file', async () => { + const { content, filename } = migratedData; const formatted = formattedMigratedData.content; mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json', }); mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}'); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting( + content, + filename.endsWith('json5') ? 'json5' : 'json' + ) ).resolves.toEqual(formatted); }); }); diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index 5a67b31cea4662..66b6e6080bbf93 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -1,6 +1,6 @@ import detectIndent from 'detect-indent'; import JSON5 from 'json5'; -import prettier from 'prettier'; +import prettier, { BuiltInParserName } from 'prettier'; import { migrateConfig } from '../../../../config/migration'; import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; @@ -10,6 +10,7 @@ import { detectRepoFileConfig } from '../../init/merge'; export interface MigratedData { content: string; filename: string; + indent: Indent; } interface Indent { @@ -31,12 +32,51 @@ const prettierConfigFilenames = new Set([ '.prettierrc.toml', ]); +export type PrettierParser = BuiltInParserName; +export async function applyPrettierFormatting( + content: string, + parser: PrettierParser, + indent?: Indent +): Promise { + try { + logger.trace('applyPrettierFormatting - START'); + const fileList = await getFileList(); + let prettierExists = fileList.some((file) => + prettierConfigFilenames.has(file) + ); + + if (!prettierExists) { + try { + const packageJsonContent = await readLocalFile('package.json', 'utf8'); + prettierExists = + packageJsonContent && JSON.parse(packageJsonContent).prettier; + } catch { + logger.warn( + 'applyPrettierFormatting - Error processing package.json file' + ); + } + } + + if (!prettierExists) { + return content; + } + const options = { + parser, + tabWidth: indent?.amount === 0 ? 2 : indent?.amount, + useTabs: indent?.type === 'tab', + }; + + return prettier.format(content, options); + } finally { + logger.trace('applyPrettierFormatting - END'); + } +} + export class MigratedDataFactory { // singleton private static data: MigratedData | null; - private static indent: Indent; - public static async getAsync(): Promise { + static async get(): Promise { if (this.data) { return this.data; } @@ -50,51 +90,7 @@ export class MigratedDataFactory { return this.data; } - public static async applyPrettierFormatting(): Promise { - try { - logger.trace('applyPrettierFormatting() - START'); - if (!this.data) { - return null; - } - - const { content, filename } = this.data; - const indent = this.indent; - const fileList = await getFileList(); - let prettierExists = fileList.some((file) => - prettierConfigFilenames.has(file) - ); - - if (!prettierExists) { - try { - const packageJsonContent = await readLocalFile( - 'package.json', - 'utf8' - ); - prettierExists = - packageJsonContent && JSON.parse(packageJsonContent).prettier; - } catch { - logger.warn( - 'applyPrettierFormatting() - Error processing package.json file' - ); - } - } - - if (!prettierExists) { - return content; - } - const options = { - parser: filename.endsWith('.json5') ? 'json5' : 'json', - tabWidth: indent.amount === 0 ? 2 : indent.amount, - useTabs: indent.type === 'tab', - }; - - return prettier.format(content, options); - } finally { - logger.trace('applyPrettierFormatting() - END'); - } - } - - public static reset(): void { + static reset(): void { this.data = null; } @@ -118,8 +114,8 @@ export class MigratedDataFactory { // indent defaults to 2 spaces // TODO #7154 - this.indent = detectIndent(raw!); - const indentSpace = this.indent.indent ?? ' '; + const indent = detectIndent(raw!); + const indentSpace = indent.indent ?? ' '; const filename = configFileName!; let content: string; @@ -133,7 +129,7 @@ export class MigratedDataFactory { content += '\n'; } - res = { content, filename }; + res = { content, filename, indent }; } catch (err) { logger.debug( { err }, diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index 718fe80d9754a0..70993b0d348b7e 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -1,13 +1,15 @@ +import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../../test/fixtures'; import { RenovateConfig, getConfig, git, + partial, platform, } from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; import { checkoutBranch, commitFiles } from '../../../../util/git'; -import { MigratedDataFactory } from './migrated-data'; +import * as MigratedDataModule from './migrated-data'; import type { MigratedData } from './migrated-data'; import { rebaseMigrationBranch } from './rebase'; @@ -18,10 +20,7 @@ const formattedMigratedData = Fixtures.getJson( ); describe('workers/repository/config-migration/branch/rebase', () => { - const prettierSpy = jest.spyOn( - MigratedDataFactory, - 'applyPrettierFormatting' - ); + const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); beforeAll(() => { GlobalConfig.set({ @@ -41,7 +40,11 @@ describe('workers/repository/config-migration/branch/rebase', () => { beforeEach(() => { jest.resetAllMocks(); GlobalConfig.reset(); - migratedConfigData = { content: renovateConfig, filename }; + migratedConfigData = { + content: renovateConfig, + filename, + indent: partial({}), + }; config = { ...getConfig(), repository: 'some/repo', diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 47bb7fbbcaf693..e638eb387bb4c2 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -1,4 +1,5 @@ import hasha from 'hasha'; +import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -11,7 +12,7 @@ import { import { quickStringify } from '../../../../util/stringify'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; -import { MigratedDataFactory } from './migrated-data'; +import { PrettierParser, applyPrettierFormatting } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function rebaseMigrationBranch( @@ -45,10 +46,10 @@ export async function rebaseMigrationBranch( const commitMessage = commitMessageFactory.getCommitMessage(); await checkoutBranch(config.defaultBranch!); - const prettified = await MigratedDataFactory.applyPrettierFormatting(); - if (prettified) { - contents = prettified; - } + + const { content, filename, indent } = migratedConfigData; + const parser = upath.extname(filename).replace('.', '') as PrettierParser; + contents = await applyPrettierFormatting(content, parser, indent); return commitAndPush({ branchName, files: [ diff --git a/lib/workers/repository/config-migration/index.spec.ts b/lib/workers/repository/config-migration/index.spec.ts index ad29c89f68e57a..6b1512647da54f 100644 --- a/lib/workers/repository/config-migration/index.spec.ts +++ b/lib/workers/repository/config-migration/index.spec.ts @@ -1,5 +1,6 @@ +import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../test/fixtures'; -import { getConfig, mockedFunction } from '../../../../test/util'; +import { getConfig, mockedFunction, partial } from '../../../../test/util'; import { checkConfigMigrationBranch } from './branch'; import { MigratedDataFactory } from './branch/migrated-data'; import { ensureConfigMigrationPr } from './pr'; @@ -20,15 +21,16 @@ const config = { describe('workers/repository/config-migration/index', () => { beforeEach(() => { jest.resetAllMocks(); - mockedFunction(MigratedDataFactory.getAsync).mockResolvedValue({ + mockedFunction(MigratedDataFactory.get).mockResolvedValue({ filename, content, + indent: partial({}), }); }); it('does nothing when config migration is disabled', async () => { await configMigration({ ...config, configMigration: false }, []); - expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); + expect(MigratedDataFactory.get).toHaveBeenCalledTimes(0); expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); diff --git a/lib/workers/repository/config-migration/index.ts b/lib/workers/repository/config-migration/index.ts index 33a401d0635750..eddcd14fdb14cc 100644 --- a/lib/workers/repository/config-migration/index.ts +++ b/lib/workers/repository/config-migration/index.ts @@ -8,7 +8,7 @@ export async function configMigration( branchList: string[] ): Promise { if (config.configMigration) { - const migratedConfigData = await MigratedDataFactory.getAsync(); + const migratedConfigData = await MigratedDataFactory.get(); const migrationBranch = await checkConfigMigrationBranch( config, migratedConfigData diff --git a/lib/workers/repository/config-migration/pr/index.spec.ts b/lib/workers/repository/config-migration/pr/index.spec.ts index 6b2053ae61ccb9..48eaacbcc2a9e5 100644 --- a/lib/workers/repository/config-migration/pr/index.spec.ts +++ b/lib/workers/repository/config-migration/pr/index.spec.ts @@ -1,3 +1,4 @@ +import type { Indent } from 'detect-indent'; import type { RequestError, Response } from 'got'; import { mock } from 'jest-mock-extended'; import { Fixtures } from '../../../../../test/fixtures'; @@ -30,6 +31,7 @@ describe('workers/repository/config-migration/pr/index', () => { const migratedData: MigratedData = { content: migratedContent, filename: configFileName, + indent: partial({}), }; let config: RenovateConfig; @@ -166,6 +168,7 @@ describe('workers/repository/config-migration/pr/index', () => { await ensureConfigMigrationPr(config, { content: migratedContent, filename: 'renovate.json5', + indent: partial({}), }); expect(platform.createPr).toHaveBeenCalledTimes(1); expect(platform.createPr.mock.calls[0][0].prBody).toMatchSnapshot(); From 99c92a9953b93968ee088b50ea01034559bf40a4 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Sun, 23 Oct 2022 23:58:07 +0300 Subject: [PATCH 4/7] add wrapper function for applyPrettierFormatting Co-authored-by: Michael Kriese --- .../config-migration/branch/create.spec.ts | 7 ++++-- .../config-migration/branch/create.ts | 9 ++++---- .../config-migration/branch/migrated-data.ts | 11 ++++++++++ .../config-migration/branch/rebase.spec.ts | 7 ++++-- .../config-migration/branch/rebase.ts | 22 ++++++------------- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/create.spec.ts b/lib/workers/repository/config-migration/branch/create.spec.ts index adeac4742f9279..cdb2576326979e 100644 --- a/lib/workers/repository/config-migration/branch/create.spec.ts +++ b/lib/workers/repository/config-migration/branch/create.spec.ts @@ -8,8 +8,8 @@ import { } from '../../../../../test/util'; import { checkoutBranch, commitFiles } from '../../../../util/git'; import { createConfigMigrationBranch } from './create'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; -import * as MigratedDataModule from './migrated-data'; jest.mock('../../../../util/git'); @@ -18,7 +18,10 @@ describe('workers/repository/config-migration/branch/create', () => { const indent = ' '; const renovateConfig = JSON.stringify(raw, undefined, indent) + '\n'; const filename = 'renovate.json'; - const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); + const prettierSpy = jest.spyOn( + MigratedDataFactory, + 'applyPrettierFormatting' + ); let config: RenovateConfig; let migratedConfigData: MigratedData; diff --git a/lib/workers/repository/config-migration/branch/create.ts b/lib/workers/repository/config-migration/branch/create.ts index 285823426d56e9..cd337ad37bfa6d 100644 --- a/lib/workers/repository/config-migration/branch/create.ts +++ b/lib/workers/repository/config-migration/branch/create.ts @@ -1,4 +1,3 @@ -import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -6,7 +5,7 @@ import { commitAndPush } from '../../../../modules/platform/commit'; import { checkoutBranch } from '../../../../util/git'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; -import { PrettierParser, applyPrettierFormatting } from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function createConfigMigrationBranch( @@ -31,9 +30,9 @@ export async function createConfigMigrationBranch( } await checkoutBranch(config.defaultBranch!); - const { content, filename, indent } = migratedConfigData; - const parser = upath.extname(filename).replace('.', '') as PrettierParser; - const contents = await applyPrettierFormatting(content, parser, indent); + const contents = await MigratedDataFactory.applyPrettierFormatting( + migratedConfigData + ); return commitAndPush({ branchName: getMigrationBranchName(config), files: [ diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index 66b6e6080bbf93..30c4279a16a26d 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -1,6 +1,7 @@ import detectIndent from 'detect-indent'; import JSON5 from 'json5'; import prettier, { BuiltInParserName } from 'prettier'; +import upath from 'upath'; import { migrateConfig } from '../../../../config/migration'; import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; @@ -33,6 +34,7 @@ const prettierConfigFilenames = new Set([ ]); export type PrettierParser = BuiltInParserName; + export async function applyPrettierFormatting( content: string, parser: PrettierParser, @@ -94,6 +96,15 @@ export class MigratedDataFactory { this.data = null; } + static applyPrettierFormatting({ + content, + filename, + indent, + }: MigratedData): Promise { + const parser = upath.extname(filename).replace('.', '') as PrettierParser; + return applyPrettierFormatting(content, parser, indent); + } + private static async build(): Promise { let res: MigratedData | null = null; try { diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index 70993b0d348b7e..a9f647c99845bf 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -9,7 +9,7 @@ import { } from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; import { checkoutBranch, commitFiles } from '../../../../util/git'; -import * as MigratedDataModule from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; import { rebaseMigrationBranch } from './rebase'; @@ -20,7 +20,10 @@ const formattedMigratedData = Fixtures.getJson( ); describe('workers/repository/config-migration/branch/rebase', () => { - const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); + const prettierSpy = jest.spyOn( + MigratedDataFactory, + 'applyPrettierFormatting' + ); beforeAll(() => { GlobalConfig.set({ diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index e638eb387bb4c2..a8f56d234538e3 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -1,5 +1,3 @@ -import hasha from 'hasha'; -import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -12,7 +10,7 @@ import { import { quickStringify } from '../../../../util/stringify'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; -import { PrettierParser, applyPrettierFormatting } from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function rebaseMigrationBranch( @@ -28,7 +26,7 @@ export async function rebaseMigrationBranch( const configFileName = migratedConfigData.filename; let contents = migratedConfigData.content; const existingContents = await getFile(configFileName, branchName); - if (hash(contents) === hash(existingContents)) { + if (stripWhitespaces(contents) === stripWhitespaces(existingContents)) { logger.debug('Migration branch is up to date'); return null; } @@ -46,10 +44,9 @@ export async function rebaseMigrationBranch( const commitMessage = commitMessageFactory.getCommitMessage(); await checkoutBranch(config.defaultBranch!); - - const { content, filename, indent } = migratedConfigData; - const parser = upath.extname(filename).replace('.', '') as PrettierParser; - contents = await applyPrettierFormatting(content, parser, indent); + contents = await MigratedDataFactory.applyPrettierFormatting( + migratedConfigData + ); return commitAndPush({ branchName, files: [ @@ -64,14 +61,9 @@ export async function rebaseMigrationBranch( }); } -function stripWhitespaces(str: string): string { - return quickStringify(JSON.parse(str)); -} - -function hash(str: string | null): string | null { +function stripWhitespaces(str: string | null): string | null { if (!str) { return null; } - const stripped = stripWhitespaces(str); - return hasha(stripped); + return quickStringify(JSON.parse(str)); } From 862d9dec44e26f130cb3205fc1be1c0df7e732f8 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Tue, 25 Oct 2022 11:23:51 +0300 Subject: [PATCH 5/7] update migrated-data.spec.ts --- .../branch/migrated-data.spec.ts | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) 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 55fd85b4234faf..d6cd36a6415941 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts @@ -7,7 +7,7 @@ import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; import { getFileList } from '../../../../util/git'; import { detectRepoFileConfig } from '../../init/merge'; -import { MigratedDataFactory, applyPrettierFormatting } from './migrated-data'; +import { MigratedDataFactory } from './migrated-data'; jest.mock('../../../../config/migration'); jest.mock('../../../../util/git'); @@ -116,7 +116,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { }); }); - describe(' MigratedDataFactory.applyPrettierFormatting', () => { + describe('MigratedDataFactory.applyPrettierFormatting', () => { beforeAll(() => { mockedFunction(detectIndent).mockReturnValueOnce({ type: 'space', @@ -139,47 +139,42 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { }); it('does not format when no prettier config is present', async () => { - const { content: unformatted, filename } = migratedData; + const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce(null); await MigratedDataFactory.get(); await expect( - applyPrettierFormatting(unformatted, filename) + MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); }); it('does not format when failing to fetch package.json file', async () => { - const { content: unformatted, filename } = migratedData; + const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockRejectedValueOnce(null); await MigratedDataFactory.get(); await expect( - applyPrettierFormatting(unformatted, filename) + MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); }); it('does not format when there is an invalid package.json file', async () => { - const { content: unformatted, filename } = migratedData; + const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json'); await MigratedDataFactory.get(); await expect( - applyPrettierFormatting(unformatted, filename) + MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); }); it('formats when prettier config file is found', async () => { - const { content, filename } = migratedData; const formatted = formattedMigratedData.content; mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); await MigratedDataFactory.get(); await expect( - applyPrettierFormatting( - content, - filename.endsWith('json') ? 'json' : 'json5' - ) + MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(formatted); }); it('formats when finds prettier config inside the package.json file', async () => { - const { content, filename } = migratedData; const formatted = formattedMigratedData.content; mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({ configFileName: 'renovate.json', @@ -187,10 +182,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}'); await MigratedDataFactory.get(); await expect( - applyPrettierFormatting( - content, - filename.endsWith('json5') ? 'json5' : 'json' - ) + MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(formatted); }); }); From ef865fa7c8f119ec7cf207e40e9ca09c4829d1e5 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Wed, 2 Nov 2022 23:06:10 +0200 Subject: [PATCH 6/7] add comments to and rename stripWhitespaces helper func --- .../config-migration/branch/rebase.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index a8f56d234538e3..7a6a700ca8234a 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -26,7 +26,9 @@ export async function rebaseMigrationBranch( const configFileName = migratedConfigData.filename; let contents = migratedConfigData.content; const existingContents = await getFile(configFileName, branchName); - if (stripWhitespaces(contents) === stripWhitespaces(existingContents)) { + if ( + jsonStripWhitespaces(contents) === jsonStripWhitespaces(existingContents) + ) { logger.debug('Migration branch is up to date'); return null; } @@ -61,9 +63,20 @@ export async function rebaseMigrationBranch( }); } -function stripWhitespaces(str: string | null): string | null { - if (!str) { +/** + * @param json a JSON string + * @return a minimal json string. i.e. does not contain any formatting/whitespaces + */ +function jsonStripWhitespaces(json: string | null): string | null { + if (!json) { return null; } - return quickStringify(JSON.parse(str)); + /** + * JSON.stringify(value, replacer, space): + * If "space" is anything other than a string or number — + * for example, is null or not provided — no white space is used. + * + * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#parameters + */ + return quickStringify(JSON.parse(json)); } From 42990de4d7d745e7ca1c3e29dabfa32d32f2fe18 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Tue, 8 Nov 2022 13:01:29 +0200 Subject: [PATCH 7/7] revert getAsync rename --- .../branch/migrated-data.spec.ts | 34 +++++++++++-------- .../config-migration/branch/migrated-data.ts | 2 +- .../repository/config-migration/index.spec.ts | 4 +-- .../repository/config-migration/index.ts | 2 +- 4 files changed, 24 insertions(+), 18 deletions(-) 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 d6cd36a6415941..09f7bcecc13af8 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts @@ -48,34 +48,40 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { isMigrated: false, migratedConfig: {}, }); - await expect(MigratedDataFactory.get()).resolves.toBeNull(); + await expect(MigratedDataFactory.getAsync()).resolves.toBeNull(); }); it('Calls getAsync a first time to initialize the factory', async () => { - await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); + await expect(MigratedDataFactory.getAsync()).resolves.toEqual( + migratedData + ); expect(detectRepoFileConfig).toHaveBeenCalledTimes(1); }); it('Calls getAsync a second time to get the saved data from before', async () => { - await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); + await expect(MigratedDataFactory.getAsync()).resolves.toEqual( + migratedData + ); expect(detectRepoFileConfig).toHaveBeenCalledTimes(0); }); describe('MigratedData class', () => { it('gets the filename from the class instance', async () => { - const data = await MigratedDataFactory.get(); + const data = await MigratedDataFactory.getAsync(); expect(data?.filename).toBe('renovate.json'); }); it('gets the content from the class instance', async () => { - const data = await MigratedDataFactory.get(); + const data = await MigratedDataFactory.getAsync(); expect(data?.content).toBe(migratedData.content); }); }); it('Resets the factory and gets a new value', async () => { MigratedDataFactory.reset(); - await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData); + await expect(MigratedDataFactory.getAsync()).resolves.toEqual( + migratedData + ); }); it('Resets the factory and gets a new value with default indentation', async () => { @@ -87,7 +93,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { }; mockedFunction(detectIndent).mockReturnValueOnce(indent); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.get()).resolves.toEqual({ + await expect(MigratedDataFactory.getAsync()).resolves.toEqual({ ...migratedData, indent, }); @@ -99,7 +105,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { configFileRaw: rawNonMigratedJson5, }); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.get()).resolves.toEqual( + await expect(MigratedDataFactory.getAsync()).resolves.toEqual( migratedDataJson5 ); }); @@ -108,7 +114,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { const err = new Error('error-message'); mockedFunction(detectRepoFileConfig).mockRejectedValueOnce(err); MigratedDataFactory.reset(); - await expect(MigratedDataFactory.get()).resolves.toBeNull(); + await expect(MigratedDataFactory.getAsync()).resolves.toBeNull(); expect(logger.debug).toHaveBeenCalledWith( { err }, 'MigratedDataFactory.getAsync() Error initializing renovate MigratedData' @@ -141,7 +147,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('does not format when no prettier config is present', async () => { const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce(null); - await MigratedDataFactory.get(); + await MigratedDataFactory.getAsync(); await expect( MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); @@ -150,7 +156,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('does not format when failing to fetch package.json file', async () => { const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockRejectedValueOnce(null); - await MigratedDataFactory.get(); + await MigratedDataFactory.getAsync(); await expect( MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); @@ -159,7 +165,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('does not format when there is an invalid package.json file', async () => { const { content: unformatted } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json'); - await MigratedDataFactory.get(); + await MigratedDataFactory.getAsync(); await expect( MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(unformatted); @@ -168,7 +174,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { it('formats when prettier config file is found', async () => { const formatted = formattedMigratedData.content; mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); - await MigratedDataFactory.get(); + await MigratedDataFactory.getAsync(); await expect( MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(formatted); @@ -180,7 +186,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { configFileName: 'renovate.json', }); mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}'); - await MigratedDataFactory.get(); + await MigratedDataFactory.getAsync(); await expect( MigratedDataFactory.applyPrettierFormatting(migratedData) ).resolves.toEqual(formatted); diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index 30c4279a16a26d..0a06ebca7f36ff 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -78,7 +78,7 @@ export class MigratedDataFactory { // singleton private static data: MigratedData | null; - static async get(): Promise { + static async getAsync(): Promise { if (this.data) { return this.data; } diff --git a/lib/workers/repository/config-migration/index.spec.ts b/lib/workers/repository/config-migration/index.spec.ts index 6b1512647da54f..f15f4bd75d5d75 100644 --- a/lib/workers/repository/config-migration/index.spec.ts +++ b/lib/workers/repository/config-migration/index.spec.ts @@ -21,7 +21,7 @@ const config = { describe('workers/repository/config-migration/index', () => { beforeEach(() => { jest.resetAllMocks(); - mockedFunction(MigratedDataFactory.get).mockResolvedValue({ + mockedFunction(MigratedDataFactory.getAsync).mockResolvedValue({ filename, content, indent: partial({}), @@ -30,7 +30,7 @@ describe('workers/repository/config-migration/index', () => { it('does nothing when config migration is disabled', async () => { await configMigration({ ...config, configMigration: false }, []); - expect(MigratedDataFactory.get).toHaveBeenCalledTimes(0); + expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); diff --git a/lib/workers/repository/config-migration/index.ts b/lib/workers/repository/config-migration/index.ts index eddcd14fdb14cc..33a401d0635750 100644 --- a/lib/workers/repository/config-migration/index.ts +++ b/lib/workers/repository/config-migration/index.ts @@ -8,7 +8,7 @@ export async function configMigration( branchList: string[] ): Promise { if (config.configMigration) { - const migratedConfigData = await MigratedDataFactory.get(); + const migratedConfigData = await MigratedDataFactory.getAsync(); const migrationBranch = await checkConfigMigrationBranch( config, migratedConfigData