fix(config-migration): runtime error when comparing json5 strings (#1…

…9870)
renovatebot · Jan 17, 2023 · 5dc806b · 5dc806b
1 parent 6679d7e
commit 5dc806b
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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;
 }