From 5dc806bb8b72df57dfcc28b8ec40836c4fa54940 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti <97394622+Gabriel-Ladzaretti@users.noreply.github.com> Date: Tue, 17 Jan 2023 06:50:02 +0200 Subject: [PATCH] fix(config-migration): runtime error when comparing json5 strings (#19870) --- .../branch/__fixtures__/renovate.json | 1 + .../config-migration/branch/rebase.spec.ts | 145 ++++++++++++------ .../config-migration/branch/rebase.ts | 3 +- 3 files changed, 99 insertions(+), 50 deletions(-) diff --git a/lib/workers/repository/config-migration/branch/__fixtures__/renovate.json b/lib/workers/repository/config-migration/branch/__fixtures__/renovate.json index c7a91ea389..685fe51fc7 100644 --- a/lib/workers/repository/config-migration/branch/__fixtures__/renovate.json +++ b/lib/workers/repository/config-migration/branch/__fixtures__/renovate.json @@ -1,4 +1,5 @@ { + "$schema": "https://docs.renovatebot.com/renovate-schema.json", "extends": [ ":separateMajorReleases", ":prImmediately", diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index 7a8fadfeed..fbfa847574 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -1,4 +1,5 @@ import type { Indent } from 'detect-indent'; +import JSON5 from 'json5'; import { Fixtures } from '../../../../../test/fixtures'; import { RenovateConfig, @@ -32,22 +33,22 @@ describe('workers/repository/config-migration/branch/rebase', () => { }); describe('rebaseMigrationBranch()', () => { - const raw = Fixtures.getJson('./renovate.json'); + const repoConfig = Fixtures.getJson('./renovate.json'); const indent = ' '; - const renovateConfig = JSON.stringify(raw, undefined, indent) + '\n'; - const filename = 'renovate.json'; - + const renovateConfigJson = + JSON.stringify(repoConfig, undefined, indent) + '\n'; + const renovateConfigJson5 = + JSON5.stringify(repoConfig, undefined, indent) + '\n'; let config: RenovateConfig; - let migratedConfigData: MigratedData; + const migratedConfigData: MigratedData = { + content: '', + filename: '', + indent: partial<Indent>({}), + }; beforeEach(() => { jest.resetAllMocks(); GlobalConfig.reset(); - migratedConfigData = { - content: renovateConfig, - filename, - indent: partial<Indent>({}), - }; config = { ...getConfig(), repository: 'some/repo', @@ -58,62 +59,108 @@ describe('workers/repository/config-migration/branch/rebase', () => { it('does not rebase modified branch', async () => { git.isBranchModified.mockResolvedValueOnce(true); - await rebaseMigrationBranch(config, migratedConfigData); - expect(checkoutBranch).toHaveBeenCalledTimes(0); - expect(git.commitFiles).toHaveBeenCalledTimes(0); - }); - it('does nothing if branch is up to date', async () => { - git.getFile - .mockResolvedValueOnce(renovateConfig) - .mockResolvedValueOnce(renovateConfig); await rebaseMigrationBranch(config, migratedConfigData); + expect(checkoutBranch).toHaveBeenCalledTimes(0); expect(git.commitFiles).toHaveBeenCalledTimes(0); }); - it('rebases migration branch', async () => { - git.isBranchBehindBase.mockResolvedValueOnce(true); - await rebaseMigrationBranch(config, migratedConfigData); - expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); - expect(git.commitFiles).toHaveBeenCalledTimes(1); - }); + it.each([ + ['renovate.json', renovateConfigJson], + ['renovate.json5', renovateConfigJson5], + ])( + 'does nothing if branch is up to date (%s)', + async (filename, rawConfig) => { + git.getFile + .mockResolvedValueOnce(rawConfig) + .mockResolvedValueOnce(rawConfig); + migratedConfigData.filename = filename; + migratedConfigData.content = rawConfig; + + await rebaseMigrationBranch(config, migratedConfigData); + + expect(checkoutBranch).toHaveBeenCalledTimes(0); + expect(git.commitFiles).toHaveBeenCalledTimes(0); + } + ); - it('applies prettier formatting when rebasing the migration branch ', async () => { - const formatted = formattedMigratedData.content; - prettierSpy.mockResolvedValueOnce(formattedMigratedData.content); + it.each([ + ['renovate.json', renovateConfigJson], + ['renovate.json5', renovateConfigJson5], + ])('rebases migration branch (%s)', async (filename, rawConfig) => { git.isBranchBehindBase.mockResolvedValueOnce(true); + migratedConfigData.filename = filename; + migratedConfigData.content = rawConfig; + 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', - }); - git.isBranchBehindBase.mockResolvedValueOnce(true); - await rebaseMigrationBranch(config, migratedConfigData); - expect(checkoutBranch).toHaveBeenCalledTimes(0); - expect(git.commitFiles).toHaveBeenCalledTimes(0); - }); + it.each([ + ['renovate.json', renovateConfigJson], + ['renovate.json5', renovateConfigJson5], + ])( + 'applies prettier formatting when rebasing the migration branch (%s)', + async (filename, rawConfig) => { + const formatted = formattedMigratedData.content; + prettierSpy.mockResolvedValueOnce(formattedMigratedData.content); + git.isBranchBehindBase.mockResolvedValueOnce(true); + migratedConfigData.filename = filename; + migratedConfigData.content = rawConfig; - it('rebases via platform', async () => { + 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: filename, + contents: formatted, + }, + ], + message: `Migrate config ${filename}`, + platformCommit: false, + }); + } + ); + + it.each([ + ['renovate.json', renovateConfigJson], + ['renovate.json5', renovateConfigJson5], + ])( + 'does not rebases migration branch when in dryRun is on (%s)', + async (filename, rawConfig) => { + GlobalConfig.set({ + dryRun: 'full', + }); + git.isBranchBehindBase.mockResolvedValueOnce(true); + migratedConfigData.filename = filename; + migratedConfigData.content = rawConfig; + + await rebaseMigrationBranch(config, migratedConfigData); + + expect(checkoutBranch).toHaveBeenCalledTimes(0); + expect(git.commitFiles).toHaveBeenCalledTimes(0); + } + ); + + it.each([ + ['renovate.json', renovateConfigJson], + ['renovate.json5', renovateConfigJson5], + ])('rebases via platform (%s)', async (filename, rawConfig) => { config.platformCommit = true; git.isBranchBehindBase.mockResolvedValueOnce(true); + migratedConfigData.filename = filename; + migratedConfigData.content = rawConfig; + await rebaseMigrationBranch(config, migratedConfigData); + expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); expect(platform.commitFiles).toHaveBeenCalledTimes(1); }); diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 52ca3be7c1..b4980c1aff 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -1,3 +1,4 @@ +import JSON5 from 'json5'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -78,5 +79,5 @@ export function jsonStripWhitespaces(json: string | null): string | null { * * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#parameters */ - return quickStringify(JSON.parse(json)) ?? null; + return quickStringify(JSON5.parse(json)) ?? null; } -- GitLab