From 395bd46cd849daf31175f9c16cfd2a87cb831c40 Mon Sep 17 00:00:00 2001 From: Maksim Sharipov Date: Sun, 15 Aug 2021 11:21:53 +0200 Subject: [PATCH] feat(config): migrate requiredStatusChecks to ignoreTests --- docs/usage/automerge-configuration.md | 2 +- docs/usage/configuration-options.md | 21 ++++---- lib/config/migration.ts | 8 +++ lib/config/migrations/migration.ts | 18 +++++++ .../required-status-checks-migration.spec.ts | 21 ++++++++ .../required-status-checks-migration.ts | 14 ++++++ lib/config/options/index.ts | 11 ++-- .../presets/__snapshots__/index.spec.ts.snap | 2 +- lib/config/presets/internal/default.ts | 4 +- lib/config/types.ts | 2 +- lib/platform/azure/index.spec.ts | 19 +++---- lib/platform/azure/index.ts | 10 +--- lib/platform/bitbucket-server/index.spec.ts | 18 +++---- lib/platform/bitbucket-server/index.ts | 7 ++- .../__snapshots__/index.spec.ts.snap | 16 ------ lib/platform/bitbucket/index.spec.ts | 23 +++------ lib/platform/bitbucket/index.ts | 10 +--- lib/platform/gitea/index.spec.ts | 16 ++---- lib/platform/gitea/index.ts | 9 +--- .../github/__snapshots__/index.spec.ts.snap | 50 +------------------ lib/platform/github/index.spec.ts | 27 +++------- lib/platform/github/index.ts | 10 +--- .../gitlab/__snapshots__/index.spec.ts.snap | 20 ++++---- lib/platform/gitlab/index.spec.ts | 30 +++++------ lib/platform/gitlab/index.ts | 12 ++--- lib/platform/types.ts | 2 +- lib/workers/branch/automerge.ts | 2 +- lib/workers/branch/index.spec.ts | 4 +- lib/workers/branch/index.ts | 4 +- lib/workers/pr/automerge.ts | 15 +++--- lib/workers/pr/body/config-description.ts | 2 +- lib/workers/pr/index.ts | 2 +- lib/workers/repository/process/write.spec.ts | 2 +- 33 files changed, 168 insertions(+), 245 deletions(-) create mode 100644 lib/config/migrations/migration.ts create mode 100644 lib/config/migrations/required-status-checks-migration.spec.ts create mode 100644 lib/config/migrations/required-status-checks-migration.ts diff --git a/docs/usage/automerge-configuration.md b/docs/usage/automerge-configuration.md index 533a3018f13be6..572263691837a7 100644 --- a/docs/usage/automerge-configuration.md +++ b/docs/usage/automerge-configuration.md @@ -117,7 +117,7 @@ If you see "Automerge: Disabled by config" then it means you need a config chang ### 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/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 4b5cc2992b5e43..fedc5ed3b837d5 100644 --- 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: @@ -1078,6 +1078,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`). @@ -2116,17 +2124,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/lib/config/migration.ts b/lib/config/migration.ts index d336a965b978e4..c13d265f1d0842 100644 --- a/lib/config/migration.ts +++ b/lib/config/migration.ts @@ -4,6 +4,8 @@ import { dequal } from 'dequal'; import { logger } from '../logger'; import { clone } from '../util/clone'; import { getGlobalConfig } from './global'; +import type { Migration } from './migrations/migration'; +import { RequiredStatusChecksMigration } from './migrations/required-status-checks-migration'; import { getOptions } from './options'; import { removedPresets } from './presets/common'; import type { @@ -57,6 +59,12 @@ export function migrateConfig( 'peerDependencies', ]; const { migratePresets } = getGlobalConfig(); + const migrations: Migration[] = [ + new RequiredStatusChecksMigration(config, migratedConfig), + ]; + migrations.forEach((migration) => { + migration.migrate(); + }); for (const [key, val] of Object.entries(config)) { if (removedOptions.includes(key)) { delete migratedConfig[key]; diff --git a/lib/config/migrations/migration.ts b/lib/config/migrations/migration.ts new file mode 100644 index 00000000000000..9c4ce334cff02f --- /dev/null +++ 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(): RenovateConfig; + + 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 new file mode 100644 index 00000000000000..afe9019ee573f3 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..037ced008635a1 --- /dev/null +++ b/lib/config/migrations/required-status-checks-migration.ts @@ -0,0 +1,14 @@ +import { RenovateConfig } from '../types'; +import { Migration } from './migration'; + +export class RequiredStatusChecksMigration extends Migration { + public migrate(): RenovateConfig { + this.delete('requiredStatusChecks'); + + if (this.originalConfig.requiredStatusChecks === null) { + this.migratedConfig.ignoreTests = true; + } + + return this.migratedConfig; + } +} diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index b796b0990ddd56..a6fc636fd68c8f 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1303,13 +1303,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 index 63987db40e2a8e..ac7b6c8d96f8bd 100644 --- 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 devDependencies 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 index 0c2b8000a6a062..8775157db93752 100644 --- a/lib/config/presets/internal/default.ts +++ b/lib/config/presets/internal/default.ts @@ -310,11 +310,11 @@ export const presets: Record = { }, 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 index 459458b7200bfa..a2260aa44fe07d 100644 --- 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 index 90e2361480b256..673e06f4f9b958 100644 --- a/lib/platform/azure/index.spec.ts +++ b/lib/platform/azure/index.spec.ts @@ -474,17 +474,12 @@ describe('platform/azure/index', () => { expect(res).toBeNull(); }); }); - describe('getBranchStatus(branchName, requiredStatusChecks)', () => { - it('return success if requiredStatusChecks null', async () => { + describe('getBranchStatus(branchName, ignoreTests)', () => { + it('return success if ignoreTests true', async () => { await initRepo('some/repo'); - const res = await azure.getBranchStatus('somebranch', null); + const res = await azure.getBranchStatus('somebranch', true); 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); - }); it('should pass through success', async () => { await initRepo({ repository: 'some/repo' }); azureApi.gitApi.mockImplementationOnce( @@ -494,7 +489,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 +501,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 +513,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 +525,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 index 27efa594855b81..9a489d67db7063 100644 --- a/lib/platform/azure/index.ts +++ b/lib/platform/azure/index.ts @@ -334,18 +334,12 @@ export async function getBranchStatusCheck( export async function getBranchStatus( branchName: string, - requiredStatusChecks: string[] + ignoreTests = false ): Promise { logger.debug(`getBranchStatus(${branchName})`); - if (!requiredStatusChecks) { - // null means disable status checks, so it always succeeds + if (ignoreTests) { 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 index 1002b9aafa2719..43402754930551 100644 --- a/lib/platform/bitbucket-server/index.spec.ts +++ b/lib/platform/bitbucket-server/index.spec.ts @@ -1754,11 +1754,11 @@ Followed by some information. failed: 0, }); - expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual( + expect(await bitbucket.getBranchStatus('somebranch')).toEqual( BranchStatus.green ); - expect(await bitbucket.getBranchStatus('somebranch')).toEqual( + expect(await bitbucket.getBranchStatus('somebranch', true)).toEqual( BranchStatus.green ); @@ -1777,7 +1777,7 @@ Followed by some information. failed: 0, }); - expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual( + expect(await bitbucket.getBranchStatus('somebranch')).toEqual( BranchStatus.yellow ); @@ -1791,7 +1791,7 @@ Followed by some information. failed: 0, }); - expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual( + expect(await bitbucket.getBranchStatus('somebranch')).toEqual( BranchStatus.yellow ); @@ -1810,7 +1810,7 @@ Followed by some information. failed: 1, }); - expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual( + expect(await bitbucket.getBranchStatus('somebranch')).toEqual( BranchStatus.red ); @@ -1820,7 +1820,7 @@ Followed by some information. ) .replyWithError('requst-failed'); - expect(await bitbucket.getBranchStatus('somebranch', [])).toEqual( + expect(await bitbucket.getBranchStatus('somebranch')).toEqual( BranchStatus.red ); @@ -1830,9 +1830,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 index 44bc93d61a6a4f..46aca1a8393a4a 100644 --- a/lib/platform/bitbucket-server/index.ts +++ b/lib/platform/bitbucket-server/index.ts @@ -399,14 +399,13 @@ async function getStatus( // https://docs.atlassian.com/bitbucket-server/rest/6.0.0/bitbucket-build-rest.html#idp2 export async function getBranchStatus( branchName: string, - requiredStatusChecks?: string[] | null + ignoreTests = false ): Promise { logger.debug( - `getBranchStatus(${branchName}, requiredStatusChecks=${!!requiredStatusChecks})` + `getBranchStatus(${branchName}, ignoreTests=${Boolean(ignoreTests)})` ); - if (!requiredStatusChecks) { - // null means disable status checks, so it always succeeds + if (ignoreTests) { logger.debug('Status checks disabled = returning "success"'); return BranchStatus.green; } diff --git a/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap b/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap index 6931731cdb83da..674d2c3a448704 100644 --- a/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap +++ b/lib/platform/bitbucket/__snapshots__/index.spec.ts.snap @@ -573,22 +573,6 @@ Array [ ] `; -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 { diff --git a/lib/platform/bitbucket/index.spec.ts b/lib/platform/bitbucket/index.spec.ts index 2711cf3cc7a998..bc5f370f3ce4c2 100644 --- a/lib/platform/bitbucket/index.spec.ts +++ b/lib/platform/bitbucket/index.spec.ts @@ -188,18 +188,11 @@ describe('platform/bitbucket/index', () => { describe('getBranchStatus()', () => { it('getBranchStatus 1', async () => { await initRepoMock(); - expect(await bitbucket.getBranchStatus('master', null)).toBe( + expect(await bitbucket.getBranchStatus('master', true)).toBe( BranchStatus.green ); expect(httpMock.getTrace()).toMatchSnapshot(); }); - it('getBranchStatus 2', async () => { - await initRepoMock(); - expect(await bitbucket.getBranchStatus('master', ['foo'])).toBe( - BranchStatus.red - ); - expect(httpMock.getTrace()).toMatchSnapshot(); - }); it('getBranchStatus 3', async () => { const scope = await initRepoMock(); scope @@ -219,9 +212,7 @@ describe('platform/bitbucket/index', () => { }, ], }); - expect(await bitbucket.getBranchStatus('master', [])).toBe( - BranchStatus.red - ); + expect(await bitbucket.getBranchStatus('master')).toBe(BranchStatus.red); expect(httpMock.getTrace()).toMatchSnapshot(); }); it('getBranchStatus 4', async () => { @@ -246,7 +237,7 @@ describe('platform/bitbucket/index', () => { }, ], }); - expect(await bitbucket.getBranchStatus('branch', [])).toBe( + expect(await bitbucket.getBranchStatus('branch')).toBe( BranchStatus.green ); expect(httpMock.getTrace()).toMatchSnapshot(); @@ -273,7 +264,7 @@ describe('platform/bitbucket/index', () => { }, ], }); - expect(await bitbucket.getBranchStatus('pending/branch', [])).toBe( + expect(await bitbucket.getBranchStatus('pending/branch')).toBe( BranchStatus.yellow ); expect(httpMock.getTrace()).toMatchSnapshot(); @@ -297,9 +288,9 @@ describe('platform/bitbucket/index', () => { .reply(200, { values: [], }); - expect( - await bitbucket.getBranchStatus('branch-with-empty-status', []) - ).toBe(BranchStatus.yellow); + expect(await bitbucket.getBranchStatus('branch-with-empty-status')).toBe( + BranchStatus.yellow + ); expect(httpMock.getTrace()).toMatchSnapshot(); }); }); diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts index 98471047aa87f7..248e6b1e0fba18 100644 --- a/lib/platform/bitbucket/index.ts +++ b/lib/platform/bitbucket/index.ts @@ -331,19 +331,13 @@ async function getStatus( // Returns the combined status for a branch. export async function getBranchStatus( branchName: string, - requiredStatusChecks?: string[] + ignoreTests = false ): Promise { logger.debug(`getBranchStatus(${branchName})`); - if (!requiredStatusChecks) { - // null means disable status checks, so it always succeeds + if (ignoreTests) { logger.debug('Status checks disabled = returning "success"'); return BranchStatus.green; } - if (requiredStatusChecks.length) { - // This is Unsupported - logger.warn({ requiredStatusChecks }, `Unsupported requiredStatusChecks`); - return BranchStatus.red; - } const statuses = await getStatus(branchName); logger.debug({ branch: branchName, statuses }, 'branch status check result'); if (!statuses.length) { diff --git a/lib/platform/gitea/index.spec.ts b/lib/platform/gitea/index.spec.ts index 2c41ac96a9cd02..d95fa95f53cd0b 100644 --- a/lib/platform/gitea/index.spec.ts +++ b/lib/platform/gitea/index.spec.ts @@ -402,21 +402,15 @@ describe('platform/gitea/index', () => { }) ); - return gitea.getBranchStatus('some-branch', []); + return gitea.getBranchStatus('some-branch'); }; - it('should return success if requiredStatusChecks null', async () => { - expect(await gitea.getBranchStatus('some-branch', null)).toEqual( + it('should return success if ignoreTests true', async () => { + expect(await gitea.getBranchStatus('some-branch', true)).toEqual( BranchStatus.green ); }); - it('should return failed if unsupported requiredStatusChecks', async () => { - expect(await gitea.getBranchStatus('some-branch', ['foo'])).toEqual( - BranchStatus.red - ); - }); - it('should return yellow for unknown result', async () => { expect(await getBranchStatus('unknown')).toEqual(BranchStatus.yellow); }); @@ -436,7 +430,7 @@ describe('platform/gitea/index', () => { it('should abort when branch status returns 404', async () => { helper.getCombinedCommitStatus.mockRejectedValueOnce({ statusCode: 404 }); - await expect(gitea.getBranchStatus('some-branch', [])).rejects.toThrow( + await expect(gitea.getBranchStatus('some-branch')).rejects.toThrow( REPOSITORY_CHANGED ); }); @@ -446,7 +440,7 @@ describe('platform/gitea/index', () => { new Error('getCombinedCommitStatus()') ); - await expect(gitea.getBranchStatus('some-branch', [])).rejects.toThrow( + await expect(gitea.getBranchStatus('some-branch')).rejects.toThrow( 'getCombinedCommitStatus()' ); }); diff --git a/lib/platform/gitea/index.ts b/lib/platform/gitea/index.ts index c7d831e6b007a7..c13a19247da5f5 100644 --- a/lib/platform/gitea/index.ts +++ b/lib/platform/gitea/index.ts @@ -354,17 +354,12 @@ const platform: Platform = { async getBranchStatus( branchName: string, - requiredStatusChecks?: string[] | null + ignoreTests = false ): Promise { - if (!requiredStatusChecks) { + if (ignoreTests) { return BranchStatus.green; } - if (Array.isArray(requiredStatusChecks) && requiredStatusChecks.length) { - logger.warn({ requiredStatusChecks }, 'Unsupported requiredStatusChecks'); - return BranchStatus.red; - } - let ccs: helper.CombinedCommitStatus; try { ccs = await helper.getCombinedCommitStatus(config.repository, branchName); diff --git a/lib/platform/github/__snapshots__/index.spec.ts.snap b/lib/platform/github/__snapshots__/index.spec.ts.snap index 37ff4dedb235e2..d6bf460391abd4 100644 --- a/lib/platform/github/__snapshots__/index.spec.ts.snap +++ b/lib/platform/github/__snapshots__/index.spec.ts.snap @@ -4347,55 +4347,7 @@ Array [ ] `; -exports[`platform/github/index getBranchStatus() return failed if unsupported requiredStatusChecks 1`] = ` -Array [ - Object { - "graphql": Object { - "query": Object { - "__vars": Object { - "$name": "String!", - "$owner": "String!", - }, - "repository": Object { - "__args": Object { - "name": "$name", - "owner": "$owner", - }, - "defaultBranchRef": Object { - "name": null, - "target": Object { - "oid": null, - }, - }, - "isArchived": null, - "isFork": null, - "mergeCommitAllowed": null, - "nameWithOwner": null, - "rebaseMergeAllowed": null, - "squashMergeAllowed": null, - }, - }, - "variables": Object { - "name": "repo", - "owner": "some", - }, - }, - "headers": Object { - "accept": "application/vnd.github.v3+json", - "accept-encoding": "gzip, deflate, br", - "authorization": "token abc123", - "content-length": "351", - "content-type": "application/json", - "host": "api.github.com", - "user-agent": "RenovateBot/0.0.0-semantic-release (https://github.com/renovatebot/renovate)", - }, - "method": "POST", - "url": "https://api.github.com/graphql", - }, -] -`; - -exports[`platform/github/index getBranchStatus() returns success if requiredStatusChecks null 1`] = ` +exports[`platform/github/index getBranchStatus() returns success if ignoreTests true 1`] = ` Array [ Object { "graphql": Object { diff --git a/lib/platform/github/index.spec.ts b/lib/platform/github/index.spec.ts index 99a5463b1a61b6..d75b7ece5ec6ca 100644 --- a/lib/platform/github/index.spec.ts +++ b/lib/platform/github/index.spec.ts @@ -679,28 +679,17 @@ describe('platform/github/index', () => { }); }); describe('getBranchStatus()', () => { - it('returns success if requiredStatusChecks null', async () => { + it('returns success if ignoreTests true', async () => { const scope = httpMock.scope(githubApiHost); initRepoMock(scope, 'some/repo'); await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', null); + const res = await github.getBranchStatus('somebranch', true); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); - it('return failed if unsupported requiredStatusChecks', async () => { - const scope = httpMock.scope(githubApiHost); - initRepoMock(scope, 'some/repo'); - - await github.initRepo({ - repository: 'some/repo', - } as any); - const res = await github.getBranchStatus('somebranch', ['foo']); - expect(res).toEqual(BranchStatus.red); - expect(httpMock.getTrace()).toMatchSnapshot(); - }); it('should pass through success', async () => { const scope = httpMock.scope(githubApiHost); initRepoMock(scope, 'some/repo'); @@ -715,7 +704,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -733,7 +722,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.red); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -750,7 +739,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.yellow); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -784,7 +773,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.red); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -824,7 +813,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -857,7 +846,7 @@ describe('platform/github/index', () => { await github.initRepo({ repository: 'some/repo', } as any); - const res = await github.getBranchStatus('somebranch', []); + const res = await github.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.yellow); expect(httpMock.getTrace()).toMatchSnapshot(); }); diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts index a4c7070edbd720..4edaa2d0454d8a 100644 --- a/lib/platform/github/index.ts +++ b/lib/platform/github/index.ts @@ -804,19 +804,13 @@ async function getStatus( // Returns the combined status for a branch. export async function getBranchStatus( branchName: string, - requiredStatusChecks: any[] | undefined + ignoreTests = false ): Promise { logger.debug(`getBranchStatus(${branchName})`); - if (!requiredStatusChecks) { - // null means disable status checks, so it always succeeds + if (ignoreTests) { logger.debug('Status checks disabled = returning "success"'); return BranchStatus.green; } - if (requiredStatusChecks.length) { - // This is Unsupported - logger.warn({ requiredStatusChecks }, `Unsupported requiredStatusChecks`); - return BranchStatus.red; - } let commitStatus: CombinedBranchStatus; try { commitStatus = await getStatus(branchName); diff --git a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap index 2a334d52630101..8c4cccc473a87a 100644 --- a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap +++ b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap @@ -1852,7 +1852,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) maps custom statuses to yellow 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) maps custom statuses to yellow 1`] = ` Array [ Object { "headers": Object { @@ -1879,7 +1879,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns failure if any mandatory jobs fails 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns failure if any mandatory jobs fails 1`] = ` Array [ Object { "headers": Object { @@ -1906,7 +1906,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns failure if any mandatory jobs fails and one job is skipped 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns failure if any mandatory jobs fails and one job is skipped 1`] = ` Array [ Object { "headers": Object { @@ -1933,7 +1933,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns pending if no results 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns pending if no results 1`] = ` Array [ Object { "headers": Object { @@ -1960,7 +1960,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns success if all are optional 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns success if all are optional 1`] = ` Array [ Object { "headers": Object { @@ -1987,7 +1987,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns success if all are success 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns success if all are success 1`] = ` Array [ Object { "headers": Object { @@ -2014,7 +2014,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns success if job is skipped 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns success if job is skipped 1`] = ` Array [ Object { "headers": Object { @@ -2041,7 +2041,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns success if optional jobs fail 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns success if optional jobs fail 1`] = ` Array [ Object { "headers": Object { @@ -2068,7 +2068,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) returns yellow if there are no jobs expect skipped 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) returns yellow if there are no jobs expect skipped 1`] = ` Array [ Object { "headers": Object { @@ -2095,7 +2095,7 @@ Array [ ] `; -exports[`platform/gitlab/index getBranchStatus(branchName, requiredStatusChecks) throws repository-changed 1`] = ` +exports[`platform/gitlab/index getBranchStatus(branchName, ignoreTests) throws repository-changed 1`] = ` Array [ Object { "headers": Object { diff --git a/lib/platform/gitlab/index.spec.ts b/lib/platform/gitlab/index.spec.ts index ba8effbea106bd..45f32fb5019798 100644 --- a/lib/platform/gitlab/index.spec.ts +++ b/lib/platform/gitlab/index.spec.ts @@ -514,15 +514,11 @@ describe('platform/gitlab/index', () => { expect(httpMock.getTrace()).toMatchSnapshot(); }); }); - describe('getBranchStatus(branchName, requiredStatusChecks)', () => { - it('returns success if requiredStatusChecks null', async () => { - const res = await gitlab.getBranchStatus('somebranch', null); + describe('getBranchStatus(branchName, ignoreTests)', () => { + it('returns success if ignoreTests true', async () => { + const res = await gitlab.getBranchStatus('somebranch', true); expect(res).toEqual(BranchStatus.green); }); - it('return failed if unsupported requiredStatusChecks', async () => { - const res = await gitlab.getBranchStatus('somebranch', ['foo']); - expect(res).toEqual(BranchStatus.red); - }); it('returns pending if no results', async () => { const scope = await initRepo(); scope @@ -530,7 +526,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, []); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.yellow); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -541,7 +537,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'success' }, { status: 'success' }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -555,7 +551,7 @@ describe('platform/gitlab/index', () => { { status: 'success' }, { status: 'failed', allow_failure: true }, ]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -566,7 +562,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'failed', allow_failure: true }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -577,7 +573,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'success' }, { status: 'skipped' }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.green); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -588,7 +584,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'skipped' }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.yellow); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -599,7 +595,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'skipped' }, { status: 'failed' }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.red); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -614,7 +610,7 @@ describe('platform/gitlab/index', () => { { status: 'failed', allow_failure: true }, { status: 'failed' }, ]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.red); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -625,7 +621,7 @@ describe('platform/gitlab/index', () => { '/api/v4/projects/some%2Frepo/repository/commits/0d9c7726c3d628b7e28af234595cfd20febdbf8e/statuses' ) .reply(200, [{ status: 'success' }, { status: 'foo' }]); - const res = await gitlab.getBranchStatus('somebranch', []); + const res = await gitlab.getBranchStatus('somebranch'); expect(res).toEqual(BranchStatus.yellow); expect(httpMock.getTrace()).toMatchSnapshot(); }); @@ -633,7 +629,7 @@ describe('platform/gitlab/index', () => { expect.assertions(2); git.branchExists.mockReturnValue(false); await initRepo(); - await expect(gitlab.getBranchStatus('somebranch', [])).rejects.toThrow( + await expect(gitlab.getBranchStatus('somebranch')).rejects.toThrow( REPOSITORY_CHANGED ); expect(httpMock.getTrace()).toMatchSnapshot(); diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts index bd2e7afd6d2250..3b4b6e0f55f094 100644 --- a/lib/platform/gitlab/index.ts +++ b/lib/platform/gitlab/index.ts @@ -364,18 +364,12 @@ const gitlabToRenovateStatusMapping: Record = { // Returns the combined status for a branch. export async function getBranchStatus( branchName: string, - requiredStatusChecks?: string[] | null + ignoreTests = false ): Promise { logger.debug(`getBranchStatus(${branchName})`); - if (!requiredStatusChecks) { - // null means disable status checks, so it always succeeds + if (ignoreTests) { return BranchStatus.green; } - if (Array.isArray(requiredStatusChecks) && requiredStatusChecks.length) { - // This is Unsupported - logger.warn({ requiredStatusChecks }, `Unsupported requiredStatusChecks`); - return BranchStatus.red; - } if (!git.branchExists(branchName)) { throw new Error(REPOSITORY_CHANGED); @@ -599,7 +593,7 @@ export async function getPr(iid: number): Promise { pr.canMerge = false; pr.isConflicted = true; } else if (pr.state === PrState.Open) { - const branchStatus = await getBranchStatus(pr.sourceBranch, []); + const branchStatus = await getBranchStatus(pr.sourceBranch); if (branchStatus === BranchStatus.green) { pr.canMerge = true; } diff --git a/lib/platform/types.ts b/lib/platform/types.ts index 04981cff72e02a..5e203d853ce090 100644 --- a/lib/platform/types.ts +++ b/lib/platform/types.ts @@ -185,7 +185,7 @@ export interface Platform { refreshPr?(number: number): Promise; getBranchStatus( branchName: string, - requiredStatusChecks?: string[] | null + ignoreTests?: boolean ): Promise; getBranchPr(branchName: string): Promise; initPlatform(config: PlatformParams): Promise; diff --git a/lib/workers/branch/automerge.ts b/lib/workers/branch/automerge.ts index c44dfc78a6fb90..280c1a05928466 100644 --- a/lib/workers/branch/automerge.ts +++ b/lib/workers/branch/automerge.ts @@ -27,7 +27,7 @@ export async function tryBranchAutomerge( } const branchStatus = await platform.getBranchStatus( config.branchName, - config.requiredStatusChecks + config.ignoreTests ); if (branchStatus === BranchStatus.green) { logger.debug(`Automerging branch`); diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts index 3e4697fea274be..99ed6c93b5c30a 100644 --- a/lib/workers/branch/index.spec.ts +++ b/lib/workers/branch/index.spec.ts @@ -410,7 +410,7 @@ describe('workers/branch/index', () => { automerge.tryBranchAutomerge.mockResolvedValueOnce('automerged'); await branchWorker.processBranch({ ...config, - requiredStatusChecks: null, + ignoreTests: true, }); expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(1); expect(prWorker.ensurePr).toHaveBeenCalledTimes(0); @@ -535,7 +535,7 @@ describe('workers/branch/index', () => { expect( await branchWorker.processBranch({ ...config, - requiredStatusChecks: null, + ignoreTests: true, prCreation: 'not-pending', }) ).toMatchSnapshot(); diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index 78802336d76b38..96b33b94932d84 100644 --- a/lib/workers/branch/index.ts +++ b/lib/workers/branch/index.ts @@ -464,7 +464,7 @@ export async function processBranch( !dependencyDashboardCheck && !config.rebaseRequested && commitSha && - (config.requiredStatusChecks?.length || config.prCreation !== 'immediate') + config.prCreation !== 'immediate' ) { logger.debug({ commitSha }, `Branch status pending`); return { @@ -475,7 +475,7 @@ export async function processBranch( } // Try to automerge branch and finish if successful, but only if branch already existed before this run - if (branchExists || !config.requiredStatusChecks) { + if (branchExists || config.ignoreTests) { const mergeStatus = await tryBranchAutomerge(config); logger.debug(`mergeStatus=${mergeStatus}`); if (mergeStatus === 'automerged') { diff --git a/lib/workers/pr/automerge.ts b/lib/workers/pr/automerge.ts index 30d4d5bb72e3a1..3d370ca4698b21 100644 --- a/lib/workers/pr/automerge.ts +++ b/lib/workers/pr/automerge.ts @@ -26,11 +26,11 @@ export async function checkAutoMerge( ): Promise { logger.trace({ config }, 'checkAutoMerge'); const { - branchName, - automergeType, - automergeStrategy, automergeComment, - requiredStatusChecks, + automergeStrategy, + automergeType, + branchName, + ignoreTests, rebaseRequested, } = config; // Return if PR not ready for automerge @@ -41,7 +41,7 @@ export async function checkAutoMerge( prAutomergeBlockReason: PrAutomergeBlockReason.Conflicted, }; } - if (requiredStatusChecks && pr.canMerge !== true) { + if (!ignoreTests && pr.canMerge !== true) { logger.debug( { canMergeReason: pr.canMergeReason }, 'PR is not ready for merge' @@ -51,10 +51,7 @@ export async function checkAutoMerge( prAutomergeBlockReason: PrAutomergeBlockReason.PlatformNotReady, }; } - const branchStatus = await platform.getBranchStatus( - branchName, - requiredStatusChecks - ); + const branchStatus = await platform.getBranchStatus(branchName, ignoreTests); if (branchStatus !== BranchStatus.green) { logger.debug( `PR is not ready for merge (branch status is ${branchStatus})` diff --git a/lib/workers/pr/body/config-description.ts b/lib/workers/pr/body/config-description.ts index 804a19fbcddfbb..6c41016a6c88cc 100644 --- a/lib/workers/pr/body/config-description.ts +++ b/lib/workers/pr/body/config-description.ts @@ -28,7 +28,7 @@ export async function getPrConfigDescription( if (config.automerge) { const branchStatus = await platform.getBranchStatus( config.branchName, - config.requiredStatusChecks + config.ignoreTests ); // istanbul ignore if if (branchStatus === BranchStatus.red) { diff --git a/lib/workers/pr/index.ts b/lib/workers/pr/index.ts index b3cf078daef997..328db9c391a50a 100644 --- a/lib/workers/pr/index.ts +++ b/lib/workers/pr/index.ts @@ -169,7 +169,7 @@ export async function ensurePr( if (!branchStatus) { branchStatus = await platform.getBranchStatus( branchName, - config.requiredStatusChecks + config.ignoreTests ); logger.debug({ branchStatus, branchName }, 'getBranchStatus() result'); } diff --git a/lib/workers/repository/process/write.spec.ts b/lib/workers/repository/process/write.spec.ts index 5b57436fd477b8..e818bc2aaf3f45 100644 --- a/lib/workers/repository/process/write.spec.ts +++ b/lib/workers/repository/process/write.spec.ts @@ -27,7 +27,7 @@ describe('workers/repository/process/write', () => { const branches: BranchConfig[] = [ {}, {}, - { automergeType: 'pr-comment', requiredStatusChecks: null }, + { automergeType: 'pr-comment', ignoreTests: true }, {}, {}, ] as never;