From 1bc09b0bfcbefb503f2440865bc99d6fc0d71e8e Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Sat, 8 May 2021 14:38:27 +0200 Subject: [PATCH] feat: internalChecksFilter (#9796) --- docs/usage/configuration-options.md | 12 ++ lib/config/definitions.ts | 7 + lib/manager/types.ts | 2 + .../branch/__snapshots__/index.spec.ts.snap | 7 + lib/workers/branch/index.spec.ts | 13 ++ lib/workers/branch/index.ts | 7 + .../__snapshots__/filter-checks.spec.ts.snap | 103 ++++++++++++ .../process/lookup/filter-checks.spec.ts | 154 ++++++++++++++++++ .../process/lookup/filter-checks.ts | 81 +++++++++ .../repository/process/lookup/index.spec.ts | 63 +++++++ .../repository/process/lookup/index.ts | 20 ++- 11 files changed, 468 insertions(+), 1 deletion(-) create mode 100644 lib/workers/repository/process/lookup/__snapshots__/filter-checks.spec.ts.snap create mode 100644 lib/workers/repository/process/lookup/filter-checks.spec.ts create mode 100644 lib/workers/repository/process/lookup/filter-checks.ts diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 5449af59ea..dde6ef708c 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1040,6 +1040,18 @@ If you wish for Renovate to process only select paths in the repository, use `in Alternatively, if you need to just _exclude_ certain paths in the repository then consider `ignorePaths` instead. If you are more interested in including only certain package managers (e.g. `npm`), then consider `enabledManagers` instead. +## internalChecksFilter + +This setting determines whether Renovate controls when and how filtering of internal checks are performed, particularly when multiple versions of the same update type are available. +Currently this applies to the `stabilityDays` check only. + +- `none`: No filtering will be performed, and the highest release will be used regardless of whether it's pending or not +- `strict`: All pending releases will be filtered. PRs will be skipped unless a non-pending version is available +- `flexible`: Similar to strict, but in the case where all versions are pending then a PR will be created with the highest pending version + +The `flexible` mode can result in "flapping" of Pull Requests, where e.g. a pending PR with version `1.0.3` is first released but then downgraded to `1.0.2` once it passes `stabilityDays`. +We recommend that you use the `strict` mode, and enable the `dependencyDashboard` so that you have visibility into suppressed PRs. + ## java Use this configuration option for shared config across all java projects (Gradle and Maven). diff --git a/lib/config/definitions.ts b/lib/config/definitions.ts index b57e37c96d..b117c02b7f 100644 --- a/lib/config/definitions.ts +++ b/lib/config/definitions.ts @@ -1185,6 +1185,13 @@ const options: RenovateOptions[] = [ type: 'integer', default: 0, }, + { + name: 'internalChecksFilter', + description: 'When/how to filter based on internal checks.', + type: 'string', + allowedValues: ['strict', 'flexible', 'none'], + default: 'none', + }, { name: 'prCreation', description: 'When to create the PR for a branch.', diff --git a/lib/manager/types.ts b/lib/manager/types.ts index 5cccc8a481..6d91ac4886 100644 --- a/lib/manager/types.ts +++ b/lib/manager/types.ts @@ -146,6 +146,8 @@ export interface LookupUpdate { newMinor?: number; newValue: string; semanticCommitType?: string; + pendingChecks?: string[]; + pendingVersions?: string[]; newVersion?: string; updateType?: UpdateType; } diff --git a/lib/workers/branch/__snapshots__/index.spec.ts.snap b/lib/workers/branch/__snapshots__/index.spec.ts.snap index ea2f5e0d51..596958e7a2 100644 --- a/lib/workers/branch/__snapshots__/index.spec.ts.snap +++ b/lib/workers/branch/__snapshots__/index.spec.ts.snap @@ -119,6 +119,13 @@ Object { } `; +exports[`workers/branch/index processBranch returns if pending checks 1`] = ` +Object { + "branchExists": false, + "result": "pending", +} +`; + exports[`workers/branch/index processBranch returns if pr creation limit exceeded and branch exists 1`] = ` Object { "branchExists": true, diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts index 32db6ec901..d14c901d56 100644 --- a/lib/workers/branch/index.spec.ts +++ b/lib/workers/branch/index.spec.ts @@ -324,6 +324,19 @@ describe(getName(), () => { commit.commitFilesToBranch.mockResolvedValueOnce(null); expect(await branchWorker.processBranch(config)).toMatchSnapshot(); }); + + it('returns if pending checks', async () => { + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ + ...updatedPackageFiles, + }); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + config.pendingChecks = ['stabilityDays']; + expect(await branchWorker.processBranch(config)).toMatchSnapshot(); + }); + it('returns if branch automerged', async () => { getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ updatedPackageFiles: [{}], diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index 57a6549755..36b8199c2f 100644 --- a/lib/workers/branch/index.ts +++ b/lib/workers/branch/index.ts @@ -120,6 +120,13 @@ export async function processBranch( logger.debug('Reached commits limit - skipping branch'); return { branchExists, result: BranchResult.CommitLimitReached }; } + if ( + !branchExists && + branchConfig.pendingChecks && + !dependencyDashboardCheck + ) { + return { branchExists: false, result: BranchResult.Pending }; + } if (branchExists) { logger.debug('Checking if PR has been edited'); const branchIsModified = await isBranchModified(config.branchName); diff --git a/lib/workers/repository/process/lookup/__snapshots__/filter-checks.spec.ts.snap b/lib/workers/repository/process/lookup/__snapshots__/filter-checks.spec.ts.snap new file mode 100644 index 0000000000..f272e20082 --- /dev/null +++ b/lib/workers/repository/process/lookup/__snapshots__/filter-checks.spec.ts.snap @@ -0,0 +1,103 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() picks up stabilityDays settings from hostRules 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [], + "release": Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() picks up stabilityDays settings from updateType 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [ + Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, + ], + "release": Object { + "releaseTimestamp": "2021-01-05T00:00:00.000Z", + "version": "1.0.3", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() returns latest release if internalChecksFilter=none 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [], + "release": Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() returns non-latest release if internalChecksFilter=flexible and some pass checks 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [ + Object { + "releaseTimestamp": "2021-01-05T00:00:00.000Z", + "version": "1.0.3", + }, + Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, + ], + "release": Object { + "releaseTimestamp": "2021-01-03T00:00:00.000Z", + "version": "1.0.2", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() returns non-latest release if internalChecksFilter=strict and some pass checks 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [ + Object { + "releaseTimestamp": "2021-01-05T00:00:00.000Z", + "version": "1.0.3", + }, + Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, + ], + "release": Object { + "releaseTimestamp": "2021-01-03T00:00:00.000Z", + "version": "1.0.2", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() returns non-pending latest release if internalChecksFilter=flexible and none pass checks 1`] = ` +Object { + "pendingChecks": Array [], + "pendingReleases": Array [], + "release": Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, +} +`; + +exports[`workers/repository/process/lookup/filter-checks .filterInternalChecks() returns pending latest release if internalChecksFilter=strict and none pass checks 1`] = ` +Object { + "pendingChecks": Array [ + "stabilityDays", + ], + "pendingReleases": Array [], + "release": Object { + "releaseTimestamp": "2021-01-07T00:00:00.000Z", + "version": "1.0.4", + }, +} +`; diff --git a/lib/workers/repository/process/lookup/filter-checks.spec.ts b/lib/workers/repository/process/lookup/filter-checks.spec.ts new file mode 100644 index 0000000000..3299a12d68 --- /dev/null +++ b/lib/workers/repository/process/lookup/filter-checks.spec.ts @@ -0,0 +1,154 @@ +import { getConfig, getName, mocked } from '../../../../../test/util'; +import type { Release } from '../../../../datasource'; +import { clone } from '../../../../util/clone'; +import * as _dateUtil from '../../../../util/date'; +import * as allVersioning from '../../../../versioning'; +import { filterInternalChecks } from './filter-checks'; +import type { LookupUpdateConfig, UpdateResult } from './types'; + +jest.mock('../../../../util/date'); + +const dateUtil = mocked(_dateUtil); + +let config: Partial<LookupUpdateConfig & UpdateResult>; + +const versioning = allVersioning.get('semver'); + +const releases: Release[] = [ + { + version: '1.0.1', + releaseTimestamp: '2021-01-01T00:00:01.000Z', + }, + { + version: '1.0.2', + releaseTimestamp: '2021-01-03T00:00:00.000Z', + }, + { + version: '1.0.3', + releaseTimestamp: '2021-01-05T00:00:00.000Z', + }, + { + version: '1.0.4', + releaseTimestamp: '2021-01-07T00:00:00.000Z', + }, +]; + +describe(getName(), () => { + let sortedReleases: Release[]; + beforeEach(() => { + config = getConfig(); + config.currentVersion = '1.0.0'; + sortedReleases = clone(releases); + jest.resetAllMocks(); + dateUtil.getElapsedDays.mockReturnValueOnce(3); + dateUtil.getElapsedDays.mockReturnValueOnce(5); + dateUtil.getElapsedDays.mockReturnValueOnce(7); + dateUtil.getElapsedDays.mockReturnValueOnce(9); + }); + + describe('.filterInternalChecks()', () => { + it('returns latest release if internalChecksFilter=none', () => { + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(0); + expect(res.release.version).toEqual('1.0.4'); + }); + + it('returns non-pending latest release if internalChecksFilter=flexible and none pass checks', () => { + config.internalChecksFilter = 'flexible'; + config.stabilityDays = 10; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(0); + expect(res.release.version).toEqual('1.0.4'); + }); + + it('returns pending latest release if internalChecksFilter=strict and none pass checks', () => { + config.internalChecksFilter = 'strict'; + config.stabilityDays = 10; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(1); + expect(res.pendingReleases).toHaveLength(0); + expect(res.release.version).toEqual('1.0.4'); + }); + + it('returns non-latest release if internalChecksFilter=strict and some pass checks', () => { + config.internalChecksFilter = 'strict'; + config.stabilityDays = 6; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(2); + expect(res.release.version).toEqual('1.0.2'); + }); + + it('returns non-latest release if internalChecksFilter=flexible and some pass checks', () => { + config.internalChecksFilter = 'strict'; + config.stabilityDays = 6; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(2); + expect(res.release.version).toEqual('1.0.2'); + }); + + it('picks up stabilityDays settings from hostRules', () => { + config.internalChecksFilter = 'strict'; + config.stabilityDays = 6; + config.packageRules = [{ matchUpdateTypes: ['patch'], stabilityDays: 1 }]; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(0); + expect(res.release.version).toEqual('1.0.4'); + }); + + it('picks up stabilityDays settings from updateType', () => { + config.internalChecksFilter = 'strict'; + config.patch = { stabilityDays: 4 }; + const res = filterInternalChecks( + config, + versioning, + 'patch', + sortedReleases + ); + expect(res).toMatchSnapshot(); + expect(res.pendingChecks).toHaveLength(0); + expect(res.pendingReleases).toHaveLength(1); + expect(res.release.version).toEqual('1.0.3'); + }); + }); +}); diff --git a/lib/workers/repository/process/lookup/filter-checks.ts b/lib/workers/repository/process/lookup/filter-checks.ts new file mode 100644 index 0000000000..f850c691b5 --- /dev/null +++ b/lib/workers/repository/process/lookup/filter-checks.ts @@ -0,0 +1,81 @@ +import is from '@sindresorhus/is'; +import { mergeChildConfig } from '../../../../config'; +import type { Release } from '../../../../datasource'; +import { logger } from '../../../../logger'; +import { getElapsedDays } from '../../../../util/date'; +import { applyPackageRules } from '../../../../util/package-rules'; +import type { VersioningApi } from '../../../../versioning'; +import type { LookupUpdateConfig, UpdateResult } from './types'; +import { getUpdateType } from './update-type'; + +export interface InternalChecksResult { + release: Release; + pendingChecks?: string[]; + pendingReleases?: Release[]; +} + +export function filterInternalChecks( + config: Partial<LookupUpdateConfig & UpdateResult>, + versioning: VersioningApi, + bucket: string, + sortedReleases: Release[] +): InternalChecksResult { + const { currentVersion, depName, internalChecksFilter } = config; + let release: Release; + const pendingChecks: string[] = []; + let pendingReleases: Release[] = []; + if (internalChecksFilter === 'none') { + // Don't care if stabilityDays are unmet + release = sortedReleases.pop(); + } else { + // iterate through releases from highest to lowest, looking for the first which will pass checks if present + for (const candidateRelease of sortedReleases.reverse()) { + // merge the release data into dependency config + let releaseConfig = mergeChildConfig(config, candidateRelease); + // calculate updateType and then apply it + releaseConfig.updateType = getUpdateType( + releaseConfig, + versioning, + currentVersion, + candidateRelease.version + ); + releaseConfig = mergeChildConfig( + releaseConfig, + releaseConfig[releaseConfig.updateType] + ); + // Apply packageRules in case any apply to updateType + releaseConfig = applyPackageRules(releaseConfig); + // Now check for a stabilityDays config + const { stabilityDays, releaseTimestamp } = releaseConfig; + if (is.integer(stabilityDays) && releaseTimestamp) { + if (getElapsedDays(releaseTimestamp) < stabilityDays) { + // Skip it if it doesn't pass checks + logger.debug( + `Release ${candidateRelease.version} is pending status checks` + ); + pendingReleases.unshift(candidateRelease); + continue; // eslint-disable-line no-continue + } + } + // If we get to here, then the release is OK and we can stop iterating + release = candidateRelease; + break; + } + if (!release) { + if (pendingReleases.length) { + // If all releases were pending then just take the highest + logger.debug( + { depName, bucket }, + 'All releases are pending - using latest' + ); + release = pendingReleases.pop(); + // None are pending anymore because we took the latest, so empty the array + pendingReleases = []; + if (internalChecksFilter === 'strict') { + pendingChecks.push('stabilityDays'); + } + } + } + } + return { release, pendingChecks, pendingReleases }; +} diff --git a/lib/workers/repository/process/lookup/index.spec.ts b/lib/workers/repository/process/lookup/index.spec.ts index 02e887e6c2..352a2233e5 100644 --- a/lib/workers/repository/process/lookup/index.spec.ts +++ b/lib/workers/repository/process/lookup/index.spec.ts @@ -656,6 +656,69 @@ describe(getName(), () => { }); expect((await lookup.lookupUpdates(config)).updates).toMatchSnapshot(); }); + + it('should return pendingChecks', async () => { + config.currentValue = '1.4.4'; + config.depName = 'some/action'; + config.datasource = datasourceGithubReleases.id; + config.stabilityDays = 14; + config.internalChecksFilter = 'strict'; + const yesterday = new Date(); + yesterday.setDate(yesterday.getDate() - 1); + const lastWeek = new Date(); + lastWeek.setDate(lastWeek.getDate() - 7); + githubReleases.getReleases.mockResolvedValueOnce({ + releases: [ + { + version: '1.4.4', + }, + { + version: '1.4.5', + releaseTimestamp: lastWeek.toISOString(), + }, + { + version: '1.4.6', + releaseTimestamp: yesterday.toISOString(), + }, + ], + }); + const res = await lookup.lookupUpdates(config); + expect(res.updates).toHaveLength(1); + expect(res.updates[0].newVersion).toEqual('1.4.6'); + expect(res.updates[0].pendingChecks).toHaveLength(1); + }); + + it('should return pendingVersions', async () => { + config.currentValue = '1.4.4'; + config.depName = 'some/action'; + config.datasource = datasourceGithubReleases.id; + config.stabilityDays = 3; + config.internalChecksFilter = 'strict'; + const yesterday = new Date(); + yesterday.setDate(yesterday.getDate() - 1); + const lastWeek = new Date(); + lastWeek.setDate(lastWeek.getDate() - 7); + githubReleases.getReleases.mockResolvedValueOnce({ + releases: [ + { + version: '1.4.4', + }, + { + version: '1.4.5', + releaseTimestamp: lastWeek.toISOString(), + }, + { + version: '1.4.6', + releaseTimestamp: yesterday.toISOString(), + }, + ], + }); + const res = await lookup.lookupUpdates(config); + expect(res.updates).toHaveLength(1); + expect(res.updates[0].newVersion).toEqual('1.4.5'); + expect(res.updates[0].pendingVersions).toHaveLength(1); + }); + it('should allow unstable versions if the ignoreUnstable=false', async () => { config.currentValue = '2.5.16'; config.ignoreUnstable = false; diff --git a/lib/workers/repository/process/lookup/index.ts b/lib/workers/repository/process/lookup/index.ts index 75402e7717..589d923c9a 100644 --- a/lib/workers/repository/process/lookup/index.ts +++ b/lib/workers/repository/process/lookup/index.ts @@ -1,3 +1,4 @@ +import { mergeChildConfig } from '../../../../config'; import type { ValidationMessage } from '../../../../config/types'; import { Release, @@ -17,6 +18,7 @@ import * as allVersioning from '../../../../versioning'; import { getBucket } from './bucket'; import { getCurrentVersion } from './current'; import { filterVersions } from './filter'; +import { filterInternalChecks } from './filter-checks'; import { generateUpdate } from './generate'; import { getRollbackUpdate } from './rollback'; import type { LookupUpdateConfig, UpdateResult } from './types'; @@ -201,11 +203,21 @@ export async function lookupUpdates( buckets[bucket] = [release]; } } + const depResultConfig = mergeChildConfig(config, res); for (const [bucket, releases] of Object.entries(buckets)) { const sortedReleases = releases.sort((r1, r2) => versioning.sortVersions(r1.version, r2.version) ); - const release = sortedReleases.pop(); + const { release, pendingChecks, pendingReleases } = filterInternalChecks( + depResultConfig, + versioning, + bucket, + sortedReleases + ); + // istanbul ignore next + if (!release) { + return res; + } const newVersion = release.version; const update = generateUpdate( config, @@ -214,6 +226,12 @@ export async function lookupUpdates( bucket, release ); + if (pendingChecks.length) { + update.pendingChecks = pendingChecks; + } + if (pendingReleases.length) { + update.pendingVersions = pendingReleases.map((r) => r.version); + } if (!update.newValue || update.newValue === currentValue) { if (!lockedVersion) { continue; // eslint-disable-line no-continue -- GitLab