Skip to content
Permalink
Browse files
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>
  • Loading branch information
3 people committed Sep 12, 2021
1 parent 0a5cee1 commit 7801ae7c1652426de10557555fc7c71dbfd47f71
Showing with 206 additions and 331 deletions.
  1. +9 −12 docs/usage/configuration-options.md
  2. +1 −1 docs/usage/key-concepts/automerge.md
  3. +2 −0 lib/config/migration.ts
  4. +18 −0 lib/config/migrations/index.ts
  5. +18 −0 lib/config/migrations/migration.ts
  6. +21 −0 lib/config/migrations/required-status-checks-migration.spec.ts
  7. +11 −0 lib/config/migrations/required-status-checks-migration.ts
  8. +4 −7 lib/config/options/index.ts
  9. +1 −1 lib/config/presets/__snapshots__/index.spec.ts.snap
  10. +2 −2 lib/config/presets/internal/default.ts
  11. +1 −1 lib/config/types.ts
  12. +5 −15 lib/platform/azure/index.spec.ts
  13. +1 −11 lib/platform/azure/index.ts
  14. +7 −11 lib/platform/bitbucket-server/index.spec.ts
  15. +2 −11 lib/platform/bitbucket-server/index.ts
  16. +0 −32 lib/platform/bitbucket/__snapshots__/index.spec.ts.snap
  17. +6 −22 lib/platform/bitbucket/index.spec.ts
  18. +1 −12 lib/platform/bitbucket/index.ts
  19. +3 −15 lib/platform/gitea/index.spec.ts
  20. +1 −13 lib/platform/gitea/index.ts
  21. +1 −49 lib/platform/github/__snapshots__/index.spec.ts.snap
  22. +7 −20 lib/platform/github/index.spec.ts
  23. +1 −12 lib/platform/github/index.ts
  24. +10 −10 lib/platform/gitlab/__snapshots__/index.spec.ts.snap
  25. +11 −19 lib/platform/gitlab/index.spec.ts
  26. +2 −12 lib/platform/gitlab/index.ts
  27. +1 −4 lib/platform/types.ts
  28. +3 −2 lib/workers/branch/automerge.ts
  29. +2 −2 lib/workers/branch/index.spec.ts
  30. +2 −2 lib/workers/branch/index.ts
  31. +9 −0 lib/workers/branch/status-checks.spec.ts
  32. +19 −0 lib/workers/branch/status-checks.ts
  33. +6 −5 lib/workers/pr/automerge.ts
  34. +3 −3 lib/workers/pr/body/config-description.ts
  35. +14 −24 lib/workers/pr/index.ts
  36. +1 −1 lib/workers/repository/process/write.spec.ts
@@ -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.
@@ -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
@@ -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];
@@ -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;
}
@@ -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];
}
}
@@ -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();
});
});
@@ -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;
}
}
}
@@ -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',
@@ -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",
@@ -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:
@@ -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;
@@ -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);
});
});
@@ -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) {
@@ -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();
});
});
@@ -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');
@@ -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 {

0 comments on commit 7801ae7

Please sign in to comment.