feat(config): migrate requiredStatusChecks to ignoreTests (#11355)

* feat(config): migrate requiredStatusChecks to ignoreTests * fix(config): restore order of props * feat(config): add applyMigrations function * feat(platform): check ignoreTests param in worker * feat(config): rename getBranchStatus to resolveBranchStatus Co-authored-by: Michael Kriese <michael.kriese@visualon.de> Co-authored-by: Rhys Arkins <rhys@arkins.net>
renovatebot · Sep 12, 2021 · 7801ae7 · 7801ae7
1 parent 0a5cee1
commit 7801ae7
Show file tree

Hide file tree

Showing 36 changed files with 206 additions and 331 deletions.
diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md
@@ -201,7 +201,7 @@ In that case Renovate first creates a branch and associated Pull Request, and th
 If by the next run the PR is already behind the base branch it will be automatically rebased, because Renovate only automerges branches which are up-to-date and green.
 If Renovate is scheduled for hourly runs on the repository but commits are made every 15 minutes to the main branch, then an automerge like this will keep getting deferred with every rebase.
 
-Note: if you have no tests but still want Renovate to automerge, you need to add `"requiredStatusChecks": null` to your configuration.
+Note: if you have no tests but still want Renovate to automerge, you need to add `"ignoreTests": true` to your configuration.
 
 If you prefer that Renovate more silently automerge _without_ Pull Requests at all, you can configure `"automergeType": "branch"`. In this case Renovate will:
 
@@ -1103,6 +1103,14 @@ It would take the entire `"config:base"` preset - which contains a lot of sub-pr
 
 Applicable for npm and Composer only for now. Set this to `true` if running scripts causes problems.
 
+## ignoreTests
+
+Currently Renovate's default behavior is to only automerge if every status check has succeeded.
+
+Setting this option to `true` means that Renovate will ignore _all_ status checks.
+You can set this if you don't have any status checks but still want Renovate to automerge PRs.
+Beware: configuring Renovate to automerge without any tests can lead to broken builds on your base branch, please think again before enabling this!
+
 ## ignoreUnstable
 
 By default, Renovate won't update any package versions to unstable versions (e.g. `4.0.0-rc3`) unless the current version has the same `major.minor.patch` and was _already_ unstable (e.g. it was already on `4.0.0-rc2`).
@@ -2141,17 +2149,6 @@ In case there is a need to configure them manually, it can be done using this `r
 
 The field supports multiple URLs however it is datasource-dependent on whether only the first is used or multiple.
 
-## requiredStatusChecks
-
-Currently Renovate's default behavior is to only automerge if every status check has succeeded.
-
-Setting this option to `null` means that Renovate will ignore _all_ status checks.
-You can set this if you don't have any status checks but still want Renovate to automerge PRs.
-Beware: configuring Renovate to automerge without any tests can lead to broken builds on your base branch, please think again before enabling this!
-
-In future, this might be configurable to allow certain status checks to be ignored/required.
-See [issue 1853 at the Renovate repository](https://github.com/renovatebot/renovate/issues/1853) for more details.
-
 ## respectLatest
 
 Similar to `ignoreUnstable`, this option controls whether to update to versions that are greater than the version tagged as `latest` in the repository.

diff --git a/docs/usage/key-concepts/automerge.md b/docs/usage/key-concepts/automerge.md
@@ -117,7 +117,7 @@ If you see "Automerge: Disabled by config" it means you need to make a config ch
 ### Absence of tests
 
 By default, Renovate will not automerge until it sees passing status checks / check runs for the branch.
-If you have no tests but still want Renovate to automerge, you need to add `"requiredStatusChecks": null` to your configuration.
+If you have no tests but still want Renovate to automerge, you need to add `"ignoreTests": true` to your configuration.
 However, we strongly recommend you have tests in any project where you are regularly updating dependencies.
 
 ### Committer restrictions

diff --git a/lib/config/migration.ts b/lib/config/migration.ts
@@ -4,6 +4,7 @@ import { dequal } from 'dequal';
 import { logger } from '../logger';
 import { clone } from '../util/clone';
 import { getGlobalConfig } from './global';
+import { applyMigrations } from './migrations';
 import { getOptions } from './options';
 import { removedPresets } from './presets/common';
 import type {
@@ -57,6 +58,7 @@ export function migrateConfig(
       'peerDependencies',
     ];
     const { migratePresets } = getGlobalConfig();
+    applyMigrations(config, migratedConfig);
     for (const [key, val] of Object.entries(config)) {
       if (removedOptions.includes(key)) {
         delete migratedConfig[key];

diff --git a/lib/config/migrations/index.ts b/lib/config/migrations/index.ts
@@ -0,0 +1,18 @@
+import { RenovateConfig } from '../types';
+import type { Migration } from './migration';
+import { RequiredStatusChecksMigration } from './required-status-checks-migration';
+
+export function applyMigrations(
+  originalConfig: RenovateConfig,
+  migratedConfig: RenovateConfig
+): RenovateConfig {
+  const migrations: Migration[] = [
+    new RequiredStatusChecksMigration(originalConfig, migratedConfig),
+  ];
+
+  for (const migration of migrations) {
+    migration.migrate();
+  }
+
+  return migratedConfig;
+}
diff --git a/lib/config/migrations/migration.ts b/lib/config/migrations/migration.ts
@@ -0,0 +1,18 @@
+import type { RenovateConfig } from '../types';
+
+export abstract class Migration {
+  protected readonly originalConfig: RenovateConfig;
+
+  protected readonly migratedConfig: RenovateConfig;
+
+  constructor(originalConfig: RenovateConfig, migratedConfig: RenovateConfig) {
+    this.originalConfig = originalConfig;
+    this.migratedConfig = migratedConfig;
+  }
+
+  abstract migrate(): void;
+
+  protected delete(property: string): void {
+    delete this.migratedConfig[property];
+  }
+}
diff --git a/lib/config/migrations/required-status-checks-migration.spec.ts b/lib/config/migrations/required-status-checks-migration.spec.ts
@@ -0,0 +1,21 @@
+import { RequiredStatusChecksMigration } from './required-status-checks-migration';
+
+describe('config/migrations/required-status-checks-migration', () => {
+  it('should migrate requiredStatusChecks=null to ignoreTests=true', () => {
+    const originalConfig: any = {
+      requiredStatusChecks: null,
+    };
+    const migratedConfig: any = {
+      requiredStatusChecks: null,
+    };
+    const migration = new RequiredStatusChecksMigration(
+      originalConfig,
+      migratedConfig
+    );
+
+    expect(migratedConfig.requiredStatusChecks).toBeNull();
+    migration.migrate();
+    expect(migratedConfig.requiredStatusChecks).toBeUndefined();
+    expect(migratedConfig.ignoreTests).toBeTrue();
+  });
+});
diff --git a/lib/config/migrations/required-status-checks-migration.ts b/lib/config/migrations/required-status-checks-migration.ts
@@ -0,0 +1,11 @@
+import { Migration } from './migration';
+
+export class RequiredStatusChecksMigration extends Migration {
+  public migrate(): void {
+    this.delete('requiredStatusChecks');
+
+    if (this.originalConfig.requiredStatusChecks === null) {
+      this.migratedConfig.ignoreTests = true;
+    }
+  }
+}
diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts
@@ -1310,13 +1310,10 @@ const options: RenovateOptions[] = [
     default: 'automergeComment',
   },
   {
-    name: 'requiredStatusChecks',
-    description:
-      'List of status checks that must pass before automerging. Set to null to enable automerging without tests.',
-    type: 'array',
-    subType: 'string',
-    cli: false,
-    env: false,
+    name: 'ignoreTests',
+    description: 'Set to true to enable automerging without tests.',
+    type: 'boolean',
+    default: false,
   },
   {
     name: 'transitiveRemediation',

diff --git a/lib/config/presets/__snapshots__/index.spec.ts.snap b/lib/config/presets/__snapshots__/index.spec.ts.snap
@@ -33,6 +33,7 @@ Object {
     "Require all status checks to pass before any automerging",
     "Pin dependency versions for <code>devDependencies</code> and retain semver ranges for others",
   ],
+  "ignoreTests": false,
   "ignoreUnstable": true,
   "labels": Array [
     "dependencies",
@@ -85,7 +86,6 @@ Object {
   ],
   "prCreation": "immediate",
   "rebaseWhen": "behind-base-branch",
-  "requiredStatusChecks": Array [],
   "respectLatest": true,
   "schedule": Array [
     "before 8am",

diff --git a/lib/config/presets/internal/default.ts b/lib/config/presets/internal/default.ts
@@ -316,11 +316,11 @@ export const presets: Record<string, Preset> = {
   },
   automergeRequireAllStatusChecks: {
     description: 'Require all status checks to pass before any automerging',
-    requiredStatusChecks: [],
+    ignoreTests: false,
   },
   skipStatusChecks: {
     description: 'Skip status checks and automerge right away',
-    requiredStatusChecks: null,
+    ignoreTests: true,
   },
   maintainLockFilesDisabled: {
     description:

diff --git a/lib/config/types.ts b/lib/config/types.ts
@@ -38,6 +38,7 @@ export interface RenovateSharedConfig {
   includePaths?: string[];
   ignoreDeps?: string[];
   ignorePaths?: string[];
+  ignoreTests?: boolean;
   labels?: string[];
   addLabels?: string[];
   dependencyDashboardApproval?: boolean;
@@ -55,7 +56,6 @@ export interface RenovateSharedConfig {
   recreateClosed?: boolean;
   repository?: string;
   repositoryCache?: RepositoryCacheConfig;
-  requiredStatusChecks?: string[];
   schedule?: string[];
   semanticCommits?: 'auto' | 'enabled' | 'disabled';
   semanticCommitScope?: string;

diff --git a/lib/platform/azure/index.spec.ts b/lib/platform/azure/index.spec.ts
@@ -474,17 +474,7 @@ describe('platform/azure/index', () => {
       expect(res).toBeNull();
     });
   });
-  describe('getBranchStatus(branchName, requiredStatusChecks)', () => {
-    it('return success if requiredStatusChecks null', async () => {
-      await initRepo('some/repo');
-      const res = await azure.getBranchStatus('somebranch', null);
-      expect(res).toEqual(BranchStatus.green);
-    });
-    it('return failed if unsupported requiredStatusChecks', async () => {
-      await initRepo('some/repo');
-      const res = await azure.getBranchStatus('somebranch', ['foo']);
-      expect(res).toEqual(BranchStatus.red);
-    });
+  describe('getBranchStatus(branchName, ignoreTests)', () => {
     it('should pass through success', async () => {
       await initRepo({ repository: 'some/repo' });
       azureApi.gitApi.mockImplementationOnce(
@@ -494,7 +484,7 @@ describe('platform/azure/index', () => {
             getStatuses: jest.fn(() => [{ state: GitStatusState.Succeeded }]),
           } as any)
       );
-      const res = await azure.getBranchStatus('somebranch', []);
+      const res = await azure.getBranchStatus('somebranch');
       expect(res).toEqual(BranchStatus.green);
     });
     it('should pass through failed', async () => {
@@ -506,7 +496,7 @@ describe('platform/azure/index', () => {
             getStatuses: jest.fn(() => [{ state: GitStatusState.Error }]),
           } as any)
       );
-      const res = await azure.getBranchStatus('somebranch', []);
+      const res = await azure.getBranchStatus('somebranch');
       expect(res).toEqual(BranchStatus.red);
     });
     it('should pass through pending', async () => {
@@ -518,7 +508,7 @@ describe('platform/azure/index', () => {
             getStatuses: jest.fn(() => [{ state: GitStatusState.Pending }]),
           } as any)
       );
-      const res = await azure.getBranchStatus('somebranch', []);
+      const res = await azure.getBranchStatus('somebranch');
       expect(res).toEqual(BranchStatus.yellow);
     });
     it('should fall back to yellow if no statuses returned', async () => {
@@ -530,7 +520,7 @@ describe('platform/azure/index', () => {
             getStatuses: jest.fn(() => []),
           } as any)
       );
-      const res = await azure.getBranchStatus('somebranch', []);
+      const res = await azure.getBranchStatus('somebranch');
       expect(res).toEqual(BranchStatus.yellow);
     });
   });

diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts
@@ -331,19 +331,9 @@ export async function getBranchStatusCheck(
 }
 
 export async function getBranchStatus(
-  branchName: string,
-  requiredStatusChecks: string[]
+  branchName: string
 ): Promise<BranchStatus> {
   logger.debug(`getBranchStatus(${branchName})`);
-  if (!requiredStatusChecks) {
-    // null means disable status checks, so it always succeeds
-    return BranchStatus.green;
-  }
-  if (requiredStatusChecks.length) {
-    // This is Unsupported
-    logger.warn({ requiredStatusChecks }, `Unsupported requiredStatusChecks`);
-    return BranchStatus.red;
-  }
   const statuses = await getStatusCheck(branchName);
   logger.debug({ branch: branchName, statuses }, 'branch status check result');
   if (!statuses.length) {

diff --git a/lib/platform/bitbucket-server/index.spec.ts b/lib/platform/bitbucket-server/index.spec.ts
@@ -1749,10 +1749,6 @@ Followed by some information.
               failed: 0,
             });
 
-          expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual(
-            BranchStatus.green
-          );
-
           expect(await bitbucket.getBranchStatus('somebranch')).toEqual(
             BranchStatus.green
           );
@@ -1772,7 +1768,7 @@ Followed by some information.
               failed: 0,
             });
 
-          expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual(
+          expect(await bitbucket.getBranchStatus('somebranch')).toEqual(
             BranchStatus.yellow
           );
 
@@ -1786,7 +1782,7 @@ Followed by some information.
               failed: 0,
             });
 
-          expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual(
+          expect(await bitbucket.getBranchStatus('somebranch')).toEqual(
             BranchStatus.yellow
           );
 
@@ -1805,7 +1801,7 @@ Followed by some information.
               failed: 1,
             });
 
-          expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual(
+          expect(await bitbucket.getBranchStatus('somebranch')).toEqual(
             BranchStatus.red
           );
 
@@ -1815,7 +1811,7 @@ Followed by some information.
             )
             .replyWithError('requst-failed');
 
-          expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual(
+          expect(await bitbucket.getBranchStatus('somebranch')).toEqual(
             BranchStatus.red
           );
 
@@ -1825,9 +1821,9 @@ Followed by some information.
         it('throws repository-changed', async () => {
           git.branchExists.mockReturnValue(false);
           await initRepo();
-          await expect(
-            bitbucket.getBranchStatus('somebranch', [])
-          ).rejects.toThrow(REPOSITORY_CHANGED);
+          await expect(bitbucket.getBranchStatus('somebranch')).rejects.toThrow(
+            REPOSITORY_CHANGED
+          );
           expect(httpMock.getTrace()).toMatchSnapshot();
         });
       });

diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts
@@ -396,18 +396,9 @@ async function getStatus(
 // umbrella for status checks
 // https://docs.atlassian.com/bitbucket-server/rest/6.0.0/bitbucket-build-rest.html#idp2
 export async function getBranchStatus(
-  branchName: string,
-  requiredStatusChecks?: string[] | null
+  branchName: string
 ): Promise<BranchStatus> {
-  logger.debug(
-    `getBranchStatus(${branchName}, requiredStatusChecks=${!!requiredStatusChecks})`
-  );
-
-  if (!requiredStatusChecks) {
-    // null means disable status checks, so it always succeeds
-    logger.debug('Status checks disabled = returning "success"');
-    return BranchStatus.green;
-  }
+  logger.debug(`getBranchStatus(${branchName})`);
 
   if (!git.branchExists(branchName)) {
     logger.debug('Branch does not exist - cannot fetch status');

diff --git a/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap b/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap
@@ -557,38 +557,6 @@ Array [
 ]
 `;
 
-exports[`platform/bitbucket/index getBranchStatus() getBranchStatus 1 1`] = `
-Array [
-  Object {
-    "headers": Object {
-      "accept": "application/json",
-      "accept-encoding": "gzip, deflate, br",
-      "authorization": "Basic YWJjOjEyMw==",
-      "host": "api.bitbucket.org",
-      "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)",
-    },
-    "method": "GET",
-    "url": "https://api.bitbucket.org/2.0/repositories/some/repo",
-  },
-]
-`;
-
-exports[`platform/bitbucket/index getBranchStatus() getBranchStatus 2 1`] = `
-Array [
-  Object {
-    "headers": Object {
-      "accept": "application/json",
-      "accept-encoding": "gzip, deflate, br",
-      "authorization": "Basic YWJjOjEyMw==",
-      "host": "api.bitbucket.org",
-      "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)",
-    },
-    "method": "GET",
-    "url": "https://api.bitbucket.org/2.0/repositories/some/repo",
-  },
-]
-`;
-
 exports[`platform/bitbucket/index getBranchStatus() getBranchStatus 3 1`] = `
 Array [
   Object {