refactor(migrations): validator (#13073)

* refactor(migrations): implemented migration validator

* refactor(migrations): introduce deprecated flag

* refactor(migrations): tidy

* refactor(migrations): improve validator

* refactor: fix launch.json

* refactor: fix build

* refactor: fix coverage

* refactor: fix coverage

* refactor: introduce new jest custom matcher

* refactor: revert unnecessary changes

* refactor: return override

.vscode/launch.json

@@ -60,6 +60,25 @@
       "protocol": "inspector",
       "skipFiles": ["<node_internals>/**/*.js"]
     },
+    {
+      "type": "node",
+      "request": "launch",
+      "name": "Jest Current Folder",
+      "program": "${workspaceFolder}/node_modules/.bin/jest",
+      "args": [
+        "--runInBand",
+        "--collectCoverage=false",
+        "--testTimeout=100000000",
+        "--roots=${workspaceFolder}/${relativeFileDirname}"
+      ],
+      "console": "integratedTerminal",
+      "windows": {
+        "program": "${workspaceFolder}/node_modules/jest/bin/jest"
+      },
+      "runtimeArgs": ["--preserve-symlinks"],
+      "protocol": "inspector",
+      "skipFiles": ["<node_internals>/**/*.js"]
+    },
     {
       "type": "node",
       "name": "vscode-jest-tests",

jest.config.ts

@@ -30,6 +30,7 @@ const config: InitialOptionsTsJest = {
     'jest-extended/all',
     'expect-more-jest',
     '<rootDir>/test/setup.ts',
+    '<rootDir>/test/to-migrate.ts',
   ],
   snapshotSerializers: ['<rootDir>/test/newline-snapshot-serializer.ts'],
   testEnvironment: 'node',

lib/config/migration.ts

@@ -35,7 +35,7 @@ export function migrateConfig(
         optionTypes[option.name] = option.type;
       });
     }
-    const newConfig = MigrationsService.run(config).migratedConfig;
+    const newConfig = MigrationsService.run(config);
     const migratedConfig = clone(newConfig) as MigratedRenovateConfig;
     const depTypes = [
       'dependencies',

lib/config/migrations/base/abstract-migration.ts

@@ -3,10 +3,9 @@ import type { RenovateConfig } from '../../types';
 import type { Migration } from '../types';
 
 export abstract class AbstractMigration implements Migration {
+  readonly deprecated: boolean = false;
   abstract readonly propertyName: string;
-
   private readonly originalConfig: RenovateConfig;
-
   private readonly migratedConfig: RenovateConfig;
 
   constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {

lib/config/migrations/base/rename-property-migration.ts

@@ -2,6 +2,7 @@ import type { RenovateConfig } from '../../types';
 import { AbstractMigration } from './abstract-migration';
 
 export class RenamePropertyMigration extends AbstractMigration {
+  override readonly deprecated = true;
   readonly propertyName: string;
 
   private readonly newPropertyName: string;
@@ -18,8 +19,6 @@ export class RenamePropertyMigration extends AbstractMigration {
   }
 
   override run(value): void {
-    this.delete(this.propertyName);
-
     this.setSafely(this.newPropertyName, value);
   }
 }

lib/config/migrations/custom/binary-source-migration.spec.ts

@@ -1,14 +1,14 @@
-import { MigrationsService } from '../migrations-service';
+import { BinarySourceMigration } from './binary-source-migration';
 
 describe('config/migrations/custom/binary-source-migration', () => {
   it('should migrate "auto" to "global"', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      binarySource: 'auto',
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      binarySource: 'global',
-    });
+    expect(BinarySourceMigration).toMigrate(
+      {
+        binarySource: 'auto',
+      },
+      {
+        binarySource: 'global',
+      }
+    );
   });
 });

lib/config/migrations/custom/go-mod-tidy-migration.spec.ts

@@ -1,35 +1,35 @@
-import { MigrationsService } from '../migrations-service';
+import { GoModTidyMigration } from './go-mod-tidy-migration';
 
 describe('config/migrations/custom/go-mod-tidy-migration', () => {
   it('should add postUpdateOptions option when true', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      gomodTidy: true,
-      postUpdateOptions: ['test'],
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      postUpdateOptions: ['test', 'gomodTidy'],
-    });
+    expect(GoModTidyMigration).toMigrate(
+      {
+        gomodTidy: true,
+        postUpdateOptions: ['test'],
+      },
+      {
+        postUpdateOptions: ['test', 'gomodTidy'],
+      }
+    );
   });
 
   it('should handle case when postUpdateOptions is not defined ', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      gomodTidy: true,
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      postUpdateOptions: ['gomodTidy'],
-    });
+    expect(GoModTidyMigration).toMigrate(
+      {
+        gomodTidy: true,
+      },
+      {
+        postUpdateOptions: ['gomodTidy'],
+      }
+    );
   });
 
   it('should only remove when false', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      gomodTidy: false,
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({});
+    expect(GoModTidyMigration).toMigrate(
+      {
+        gomodTidy: false,
+      },
+      {}
+    );
   });
 });

lib/config/migrations/custom/go-mod-tidy-migration.ts

@@ -1,13 +1,12 @@
 import { AbstractMigration } from '../base/abstract-migration';
 
 export class GoModTidyMigration extends AbstractMigration {
+  override readonly deprecated = true;
   readonly propertyName = 'gomodTidy';
 
   override run(value): void {
     const postUpdateOptions = this.get('postUpdateOptions');
 
-    this.delete(this.propertyName);
-
     if (value) {
       const newPostUpdateOptions = Array.isArray(postUpdateOptions)
         ? postUpdateOptions.concat(['gomodTidy'])

lib/config/migrations/custom/ignore-node-modules-migration.spec.ts

@@ -1,12 +1,12 @@
-import { MigrationsService } from '../migrations-service';
+import { IgnoreNodeModulesMigration } from './ignore-node-modules-migration';
 
 describe('config/migrations/custom/ignore-node-modules-migration', () => {
   it('should migrate to ignorePaths', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      ignoreNodeModules: true,
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({ ignorePaths: ['node_modules/'] });
+    expect(IgnoreNodeModulesMigration).toMigrate(
+      {
+        ignoreNodeModules: true,
+      },
+      { ignorePaths: ['node_modules/'] }
+    );
   });
 });

lib/config/migrations/custom/ignore-node-modules-migration.ts

@@ -1,11 +1,10 @@
 import { AbstractMigration } from '../base/abstract-migration';
 
 export class IgnoreNodeModulesMigration extends AbstractMigration {
+  override readonly deprecated = true;
   readonly propertyName = 'ignoreNodeModules';
 
   override run(value): void {
-    this.delete(this.propertyName);
-
     this.setSafely('ignorePaths', value ? ['node_modules/'] : []);
   }
 }

lib/config/migrations/custom/required-status-checks-migration.spec.ts

@@ -1,14 +1,14 @@
-import { MigrationsService } from '../migrations-service';
+import { RequiredStatusChecksMigration } from './required-status-checks-migration';
 
 describe('config/migrations/custom/required-status-checks-migration', () => {
   it('should migrate requiredStatusChecks=null to ignoreTests=true', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      requiredStatusChecks: null,
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      ignoreTests: true,
-    });
+    expect(RequiredStatusChecksMigration).toMigrate(
+      {
+        requiredStatusChecks: null,
+      },
+      {
+        ignoreTests: true,
+      }
+    );
   });
 });

lib/config/migrations/custom/required-status-checks-migration.ts

@@ -1,11 +1,10 @@
 import { AbstractMigration } from '../base/abstract-migration';
 
 export class RequiredStatusChecksMigration extends AbstractMigration {
+  override readonly deprecated = true;
   readonly propertyName = 'requiredStatusChecks';
 
   override run(value): void {
-    this.delete(this.propertyName);
-
     if (value === null) {
       this.setSafely('ignoreTests', true);
     }

lib/config/migrations/custom/trust-level-migration.spec.ts

@@ -1,32 +1,32 @@
-import { MigrationsService } from '../migrations-service';
+import { TrustLevelMigration } from './trust-level-migration';
 
 describe('config/migrations/custom/trust-level-migration', () => {
   it('should handle hight level', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      trustLevel: 'high',
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      allowCustomCrateRegistries: true,
-      allowScripts: true,
-      exposeAllEnv: true,
-    });
+    expect(TrustLevelMigration).toMigrate(
+      {
+        trustLevel: 'high',
+      },
+      {
+        allowCustomCrateRegistries: true,
+        allowScripts: true,
+        exposeAllEnv: true,
+      }
+    );
   });
 
   it('should not rewrite provided properties', () => {
-    const { isMigrated, migratedConfig } = MigrationsService.run({
-      allowCustomCrateRegistries: false,
-      allowScripts: false,
-      exposeAllEnv: false,
-      trustLevel: 'high',
-    });
-
-    expect(isMigrated).toBeTrue();
-    expect(migratedConfig).toEqual({
-      allowCustomCrateRegistries: false,
-      allowScripts: false,
-      exposeAllEnv: false,
-    });
+    expect(TrustLevelMigration).toMigrate(
+      {
+        allowCustomCrateRegistries: false,
+        allowScripts: false,
+        exposeAllEnv: false,
+        trustLevel: 'high',
+      },
+      {
+        allowCustomCrateRegistries: false,
+        allowScripts: false,
+        exposeAllEnv: false,
+      }
+    );
   });
 });

lib/config/migrations/custom/trust-level-migration.ts

@@ -1,11 +1,10 @@
 import { AbstractMigration } from '../base/abstract-migration';
 
 export class TrustLevelMigration extends AbstractMigration {
+  override readonly deprecated = true;
   readonly propertyName = 'trustLevel';
 
   override run(value): void {
-    this.delete(this.propertyName);
-
     if (value === 'high') {
       this.setSafely('allowCustomCrateRegistries', true);
       this.setSafely('allowScripts', true);

lib/config/migrations/migrations-service.spec.ts

@@ -8,9 +8,10 @@ describe('config/migrations/migrations-service', () => {
         [property]: 'test',
       };
 
-      const { isMigrated, migratedConfig } =
-        MigrationsService.run(originalConfig);
-      expect(isMigrated).toBeTrue();
+      const migratedConfig = MigrationsService.run(originalConfig);
+      expect(
+        MigrationsService.isMigrated(originalConfig, migratedConfig)
+      ).toBeTrue();
       expect(migratedConfig).toEqual({});
     }
   });
@@ -24,9 +25,10 @@ describe('config/migrations/migrations-service', () => {
         [oldPropertyName]: 'test',
       };
 
-      const { isMigrated, migratedConfig } =
-        MigrationsService.run(originalConfig);
-      expect(isMigrated).toBeTrue();
+      const migratedConfig = MigrationsService.run(originalConfig);
+      expect(
+        MigrationsService.isMigrated(originalConfig, migratedConfig)
+      ).toBeTrue();
       expect(migratedConfig).toEqual({
         [newPropertyName]: 'test',
       });
@@ -39,14 +41,15 @@ describe('config/migrations/migrations-service', () => {
       versionScheme: 'test',
       excludedPackageNames: ['test'],
     };
-    const { isMigrated, migratedConfig } =
-      MigrationsService.run(originalConfig);
+    const migratedConfig = MigrationsService.run(originalConfig);
 
     const mappedProperties = Object.keys(originalConfig).map((property) =>
       MigrationsService.renamedProperties.get(property)
     );
 
-    expect(isMigrated).toBeTrue();
+    expect(
+      MigrationsService.isMigrated(originalConfig, migratedConfig)
+    ).toBeTrue();
     expect(mappedProperties).toEqual(Object.keys(migratedConfig));
   });
 });

lib/config/migrations/migrations-service.ts

@@ -1,5 +1,5 @@
 import { dequal } from 'dequal';
-import type { MigratedConfig, RenovateConfig } from '../types';
+import type { RenovateConfig } from '../types';
 import { RemovePropertyMigration } from './base/remove-property-migration';
 import { RenamePropertyMigration } from './base/rename-property-migration';
 import { BinarySourceMigration } from './custom/binary-source-migration';
@@ -43,26 +43,34 @@ export class MigrationsService {
     TrustLevelMigration,
   ];
 
-  static run(originalConfig: RenovateConfig): MigratedConfig {
+  static run(originalConfig: RenovateConfig): RenovateConfig {
     const migratedConfig: RenovateConfig = {};
-    const migrations = MigrationsService.getMigrations(
-      originalConfig,
-      migratedConfig
-    );
+    const migrations = this.getMigrations(originalConfig, migratedConfig);
 
     for (const [key, value] of Object.entries(originalConfig)) {
       migratedConfig[key] ??= value;
       const migration = migrations.find((item) => item.propertyName === key);
-      migration?.run(value);
+
+      if (migration) {
+        migration.run(value);
+
+        if (migration.deprecated) {
+          delete migratedConfig[key];
+        }
+      }
     }
 
-    return {
-      isMigrated: !dequal(originalConfig, migratedConfig),
-      migratedConfig,
-    };
+    return migratedConfig;
   }
 
-  private static getMigrations(
+  static isMigrated(
+    originalConfig: RenovateConfig,
+    migratedConfig: RenovateConfig
+  ): boolean {
+    return !dequal(originalConfig, migratedConfig);
+  }
+
+  protected static getMigrations(
     originalConfig: RenovateConfig,
     migratedConfig: RenovateConfig
   ): ReadonlyArray<Migration> {

lib/config/migrations/types.ts

@@ -7,6 +7,7 @@ export interface MigrationConstructor {
 }
 
 export interface Migration {
+  readonly deprecated: boolean;
   readonly propertyName: string;
   run(value: unknown): void;
 }

test/to-migrate.ts

@@ -0,0 +1,62 @@
+import { expect } from '@jest/globals';
+import type {
+  Migration,
+  MigrationConstructor,
+} from '../lib/config/migrations/types';
+import type { RenovateConfig } from '../lib/config/types';
+import { MigrationsService } from './../lib/config/migrations/migrations-service';
+
+declare global {
+  // eslint-disable-next-line @typescript-eslint/no-namespace
+  namespace jest {
+    interface Matchers<R> {
+      toMigrate(
+        originalConfig: RenovateConfig,
+        expectedConfig: RenovateConfig,
+        isMigrated?: boolean
+      ): R;
+    }
+  }
+}
+
+expect.extend({
+  toMigrate(
+    CustomMigration: MigrationConstructor,
+    originalConfig: RenovateConfig,
+    expectedConfig: RenovateConfig,
+    isMigrated = true
+  ) {
+    class CustomMigrationsService extends MigrationsService {
+      protected static override getMigrations(
+        original: RenovateConfig,
+        migrated: RenovateConfig
+      ): ReadonlyArray<Migration> {
+        return [new CustomMigration(original, migrated)];
+      }
+    }
+
+    const migratedConfig = CustomMigrationsService.run(originalConfig);
+
+    if (
+      MigrationsService.isMigrated(migratedConfig, originalConfig) !==
+      isMigrated
+    ) {
+      return {
+        message: (): string => `isMigrated should be ${isMigrated}`,
+        pass: false,
+      };
+    }
+
+    if (!this.equals(migratedConfig, expectedConfig)) {
+      return {
+        message: (): string => 'Migration failed',
+        pass: false,
+      };
+    }
+
+    return {
+      message: (): string => 'Migration passed successfully',
+      pass: true,
+    };
+  },
+});