From 88000a4f9be5cdd45a43abd421777d788d14b663 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Tue, 30 Jan 2024 13:58:17 +0545 Subject: [PATCH] fix(workers/pr): improve deduplication in updates-table (#26771) Co-authored-by: Michael Kriese --- .../update/pr/body/updates-table.spec.ts | 129 +++++++++++++++++- .../update/pr/body/updates-table.ts | 62 ++++++++- 2 files changed, 184 insertions(+), 7 deletions(-) diff --git a/lib/workers/repository/update/pr/body/updates-table.spec.ts b/lib/workers/repository/update/pr/body/updates-table.spec.ts index db9823035d90b0..40eec486fd281f 100644 --- a/lib/workers/repository/update/pr/body/updates-table.spec.ts +++ b/lib/workers/repository/update/pr/body/updates-table.spec.ts @@ -87,13 +87,42 @@ describe('workers/repository/update/pr/body/updates-table', () => { displayFrom: '^6.2.3', displayTo: '6.2.3', }); + // TODO #22198 allow or filter undefined const upgrade3 = undefined as never; + + // duplicate of upgrade2 + const upgrade4 = partial({ + manager: 'some-manager', + branchName: 'some-branch', + prBodyDefinitions: { + Package: '{{{depNameLinked}}}', + Type: '{{{depType}}}', + Update: '{{{updateType}}}', + 'Current value': '{{{currentValue}}}', + 'New value': '{{{newValue}}}', + Change: '`{{{displayFrom}}}` -> `{{{displayTo}}}`', + Pending: '{{{displayPending}}}', + References: '{{{references}}}', + 'Package file': '{{{packageFile}}}', + }, + updateType: 'pin', + depNameLinked: + '[mocha](https://mochajs.org/) ([source](https://github.com/mochajs/mocha))', + depType: 'devDependencies', + depName: 'mocha', + currentValue: '^6.2.3', + newValue: '6.2.3', + currentVersion: '6.2.3', + newVersion: '6.2.3', + displayFrom: '^6.2.3', + displayTo: '6.2.3', + }); const configObj: BranchConfig = { manager: 'some-manager', branchName: 'some-branch', baseBranch: 'base', - upgrades: [upgrade0, upgrade1, upgrade2, upgrade3], + upgrades: [upgrade0, upgrade1, upgrade2, upgrade3, upgrade4], prBodyColumns: ['Package', 'Type', 'Update', 'Change', 'Pending'], prBodyDefinitions: { Package: '{{{depNameLinked}}}', @@ -122,4 +151,102 @@ describe('workers/repository/update/pr/body/updates-table', () => { '\n', ); }); + + it('selects the best upgrade incase of duplicate table rows', () => { + const upgrade1 = partial({ + manager: 'some-manager', + branchName: 'some-branch', + prBodyDefinitions: { + Package: '{{{depNameLinked}}}', + Type: '{{{depType}}}', + Update: '{{{updateType}}}', + 'Current value': '{{{currentValue}}}', + 'New value': '{{{newValue}}}', + Change: '`{{{displayFrom}}}` -> `{{{displayTo}}}`', + }, + updateType: 'pin', + depNameLinked: + '[mocha](https://mochajs.org/) ([source](https://github.com/mochajs/mocha))', + depType: 'devDependencies', + depName: 'mocha', + currentValue: '^6.2.3', + newValue: '6.2.3', + displayFrom: '^6.2.3', + displayTo: '6.2.3', + }); + + // duplicate of upgrade1 + const upgrade2 = partial({ + manager: 'some-manager', + branchName: 'some-branch', + prBodyDefinitions: { + Package: '{{{depNameLinked}}}', + Type: '{{{depType}}}', + Update: '{{{updateType}}}', + 'Current value': '{{{currentValue}}}', + 'New value': '{{{newValue}}}', + Change: + "[{{#if displayFrom}}`{{{displayFrom}}}` -> {{else}}{{#if currentValue}}`{{{currentValue}}}` -> {{/if}}{{/if}}{{#if displayTo}}`{{{displayTo}}}`{{else}}`{{{newValue}}}`{{/if}}]({{#if depName}}https://renovatebot.com/diffs/npm/{{replace '/' '%2f' depName}}/{{{currentVersion}}}/{{{newVersion}}}{{/if}})", + }, + updateType: 'pin', + depNameLinked: + '[mocha](https://mochajs.org/) ([source](https://github.com/mochajs/mocha))', + depType: 'devDependencies', + depName: 'mocha', + currentValue: '^6.2.3', + newValue: '6.2.3', + currentVersion: '6.2.3', + newVersion: '6.2.3', + displayFrom: '^6.2.3', + displayTo: '6.2.3', + }); + + // duplicate of upgrade1 + const upgrade3 = partial({ + manager: 'some-manager', + branchName: 'some-branch', + updateType: 'pin', + prBodyDefinitions: { + Pending: '{{{displayPending}}}', + }, + depNameLinked: + '[mocha](https://mochajs.org/) ([source](https://github.com/mochajs/mocha))', + depType: 'devDependencies', + depName: 'mocha', + currentValue: '^6.2.3', + newValue: '6.2.3', + displayFrom: '^6.2.3', + displayTo: '6.2.3', + displayPending: 'some-string', + }); + + const configObj: BranchConfig = { + manager: 'some-manager', + branchName: 'some-branch', + baseBranch: 'base', + upgrades: [upgrade1, upgrade2, upgrade3], + prBodyColumns: ['Package', 'Type', 'Update', 'Change', 'Pending'], + prBodyDefinitions: { + Package: '{{{depNameLinked}}}', + Type: '{{{depType}}}', + Update: '{{{updateType}}}', + 'Current value': '{{{currentValue}}}', + 'New value': '{{{newValue}}}', + Change: 'All locks refreshed', + Pending: '{{{displayPending}}}', + }, + }; + const result = getPrUpdatesTable(configObj); + expect(result).toMatch( + '\n' + + '\n' + + 'This PR contains the following updates:\n' + + '\n' + + '| Package | Type | Update | Change |\n' + + '|---|---|---|---|\n' + + '| [mocha](https://mochajs.org/) ([source](https://github.com/mochajs/mocha)) | devDependencies | pin | [`^6.2.3` -> `6.2.3`](https://renovatebot.com/diffs/npm/mocha/6.2.3/6.2.3) |\n' + + '\n' + + '\n', + ); + }); }); diff --git a/lib/workers/repository/update/pr/body/updates-table.ts b/lib/workers/repository/update/pr/body/updates-table.ts index a70828289eab7d..dc338ac0d6567d 100644 --- a/lib/workers/repository/update/pr/body/updates-table.ts +++ b/lib/workers/repository/update/pr/body/updates-table.ts @@ -44,11 +44,19 @@ export function getPrUpdatesTable(config: BranchConfig): string { logger.warn('getPrUpdatesTable - prBodyColumns is undefined'); return ''; } - const tableValues = config.upgrades - .filter((upgrade) => upgrade !== undefined) - .map((upgrade) => { + + const tableKeyValuePairs: Record> = {}; + for (const upgrade of config.upgrades) { + if (upgrade) { + // Create a key based on the properties which are significant in the updates table + const key = `${upgrade.depName ?? ''}_${upgrade.depType ?? ''}_${ + upgrade.newValue ?? upgrade.newVersion ?? '' + }_${upgrade.currentValue ?? upgrade.currentVersion ?? ''}_${ + upgrade.updateType + }`; + const res: Record = {}; - const rowDefinition = getRowDefinition(config.prBodyColumns!, upgrade); + const rowDefinition = getRowDefinition(config.prBodyColumns, upgrade); for (const column of rowDefinition) { const { header, value } = column; try { @@ -64,8 +72,22 @@ export function getPrUpdatesTable(config: BranchConfig): string { logger.warn({ header, value, err }, 'Handlebars compilation error'); } } - return res; - }); + + if (tableKeyValuePairs[key]) { + // compare the duplicate upgrades as per their table values + // and select one with better values + tableKeyValuePairs[key] = compareTableValues( + tableKeyValuePairs[key], + res, + config.prBodyColumns, + ); + } else { + tableKeyValuePairs[key] = res; + } + } + } + + const tableValues = Object.values(tableKeyValuePairs); const tableColumns = getNonEmptyColumns(config.prBodyColumns, tableValues); let res = '\n\nThis PR contains the following updates:\n\n'; res += '| ' + tableColumns.join(' | ') + ' |\n'; @@ -89,3 +111,31 @@ export function getPrUpdatesTable(config: BranchConfig): string { res += '\n\n'; return res; } + +// return the row with better table values +function compareTableValues( + a: Record, + b: Record, + prBodyColumns: string[], +): Record { + let score = 0; + + for (const header of prBodyColumns) { + if (!b[header] && !a[header]) { + continue; + } + if (!b[header]) { + score--; + continue; + } + if (!a[header]) { + score++; + continue; + } + + if (a[header] !== b[header]) { + a[header].length < b[header].length ? score++ : score--; + } + } + return score > 0 ? b : a; +}