From 57aaa788c317bebffbc6e4dfbe3720053d088bac Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 13 Sep 2024 18:48:37 +0200 Subject: [PATCH 01/20] Remove changelogs & update table from body to fit max length. --- .../repository/update/pr/body/index.ts | 57 ++++++++++++++++++- lib/workers/repository/update/pr/index.ts | 9 +-- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/lib/workers/repository/update/pr/body/index.ts b/lib/workers/repository/update/pr/body/index.ts index 1c1e5d8d1f9e88..5588f243efa128 100644 --- a/lib/workers/repository/update/pr/body/index.ts +++ b/lib/workers/repository/update/pr/body/index.ts @@ -70,6 +70,17 @@ interface PrBodyConfig { appendExtra?: string | null | undefined; rebasingNotice?: string; debugData: PrDebugData; + bodyMaxLength: number; +} + +interface PrBodyContent { + body: string; + comments?: PrComment[]; +} + +interface PrComment { + title: string; + content: string; } const rebasingRegex = regEx(/\*\*Rebasing\*\*: .*/); @@ -78,7 +89,7 @@ export function getPrBody( branchConfig: BranchConfig, prBodyConfig: PrBodyConfig, config: RenovateConfig, -): string { +): PrBodyContent { massageUpdateMetadata(branchConfig); let warnings = ''; warnings += getWarnings(branchConfig); @@ -89,7 +100,7 @@ export function getPrBody( branchConfig.dependencyDashboard, ); } - const content = { + let content = { header: getPrHeader(branchConfig), table: getPrUpdatesTable(branchConfig), warnings, @@ -100,6 +111,48 @@ export function getPrBody( footer: getPrFooter(branchConfig), }; + let prBody = createPrBody(content, branchConfig, prBodyConfig); + + if (prBody.length <= prBodyConfig.bodyMaxLength) { + return { body: prBody }; + } + + const changelogs = content.changelogs; + content = { + ...content, + changelogs: 'Please see comment below for changelogs', + }; + + prBody = createPrBody(content, branchConfig, prBodyConfig); + + if (prBody.length <= prBodyConfig.bodyMaxLength) { + return { + body: prBody, + comments: [{ title: 'Release Notes', content: changelogs }], + }; + } + + const table = content.table; + content = { + ...content, + table: 'Please see comment below for updates', + }; + prBody = createPrBody(content, branchConfig, prBodyConfig); + + return { + body: prBody, + comments: [ + { title: 'Release Notes', content: changelogs }, + { title: 'Updates', content: table }, + ], + }; +} + +function createPrBody( + content: Record, + branchConfig: BranchConfig, + prBodyConfig: PrBodyConfig, +): string { let prBody = ''; if (branchConfig.prBodyTemplate) { const prBodyTemplate = branchConfig.prBodyTemplate; diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index f57b619c02acb1..2cd700e66ec670 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -329,6 +329,7 @@ export async function ensurePr( const prBody = getPrBody( config, { + bodyMaxLength: platform.maxBodyLength(), debugData: updatePrDebugData( config.baseBranch, prepareLabels(config), // include labels in debug data @@ -356,7 +357,7 @@ export async function ensurePr( const existingPrTitle = stripEmojis(existingPr.title); const existingPrBodyHash = existingPr.bodyStruct?.hash; const newPrTitle = stripEmojis(prTitle); - const newPrBodyHash = hashBody(prBody); + const newPrBodyHash = hashBody(prBody.body); const prInitialLabels = existingPr.bodyStruct?.debugData?.labels; const prCurrentLabels = existingPr.labels; @@ -385,7 +386,7 @@ export async function ensurePr( const updatePrConfig: UpdatePrConfig = { number: existingPr.number, prTitle, - prBody, + prBody: prBody.body, platformPrOptions: getPlatformPrOptions(config), }; // PR must need updating @@ -460,7 +461,7 @@ export async function ensurePr( type: 'with-pr', pr: { ...existingPr, - bodyStruct: getPrBodyStruct(prBody), + bodyStruct: getPrBodyStruct(prBody.body), title: prTitle, targetBranch: config.baseBranch, }, @@ -488,7 +489,7 @@ export async function ensurePr( sourceBranch: branchName, targetBranch: config.baseBranch, prTitle, - prBody, + prBody: prBody.body, labels: prepareLabels(config), platformPrOptions: getPlatformPrOptions(config), draftPR: !!config.draftPR, From 994b18ec5ba42f70447f50e47b8591d224f906cd Mon Sep 17 00:00:00 2001 From: Bianco Veigel Date: Fri, 13 Sep 2024 19:48:35 +0200 Subject: [PATCH 02/20] remove truncate from massageMarkdown --- lib/modules/platform/azure/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/modules/platform/azure/index.ts b/lib/modules/platform/azure/index.ts index 13356c9211da50..ac41240964f5a0 100644 --- a/lib/modules/platform/azure/index.ts +++ b/lib/modules/platform/azure/index.ts @@ -42,7 +42,6 @@ import type { UpdatePrConfig, } from '../types'; import { getNewBranchName, repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import * as azureApi from './azure-got-wrapper'; import * as azureHelper from './azure-helper'; import type { AzurePr } from './types'; @@ -811,7 +810,7 @@ export async function mergePr({ export function massageMarkdown(input: string): string { // Remove any HTML we use - return smartTruncate(input, maxBodyLength()) + return input .replace( 'you tick the rebase/retry checkbox', 'rename PR to start with "rebase!"', From c254f7a21ea535a6d353b2e1b8022ed3b6e46977 Mon Sep 17 00:00:00 2001 From: Bianco Veigel Date: Fri, 13 Sep 2024 20:32:23 +0200 Subject: [PATCH 03/20] add comments for skipped body parts --- lib/workers/repository/update/pr/index.ts | 32 ++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 2cd700e66ec670..d7c385752b1e1d 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -368,7 +368,28 @@ export async function ensurePr( prCurrentLabels, configuredLabels, ); - + let topics: string[]; + + if (prBody.comments) { + topics = prBody.comments.map((x) => x.title); + for (const comment of prBody.comments) { + await platform.ensureComment({ + number: existingPr.number, + topic: comment.title, + content: comment.content, + }); + } + } + const topicsToDelete = ['Release Notes', 'Updates'].filter( + (x) => !topics.includes(x), + ); + for (const topic of topicsToDelete) { + await platform.ensureCommentRemoval({ + number: existingPr.number, + type: 'by-topic', + topic, + }); + } if ( existingPr?.targetBranch === config.baseBranch && existingPrTitle === newPrTitle && @@ -495,6 +516,15 @@ export async function ensurePr( draftPR: !!config.draftPR, milestone: config.milestone, }); + if (pr && prBody.comments) { + for (const comment of prBody.comments) { + await platform.ensureComment({ + number: pr.number, + topic: comment.title, + content: comment.content, + }); + } + } incLimitedValue('PullRequests'); logger.info({ pr: pr?.number, prTitle }, 'PR created'); From 7085017b4a7d022cf7325fc47a7be688481f8023 Mon Sep 17 00:00:00 2001 From: Bianco Veigel Date: Fri, 13 Sep 2024 21:44:27 +0200 Subject: [PATCH 04/20] do not create empty comments --- .../repository/update/pr/body/index.ts | 56 ++++++++----------- lib/workers/repository/update/pr/index.ts | 1 - 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/lib/workers/repository/update/pr/body/index.ts b/lib/workers/repository/update/pr/body/index.ts index 5588f243efa128..1d1934c8ab0af5 100644 --- a/lib/workers/repository/update/pr/body/index.ts +++ b/lib/workers/repository/update/pr/body/index.ts @@ -70,12 +70,11 @@ interface PrBodyConfig { appendExtra?: string | null | undefined; rebasingNotice?: string; debugData: PrDebugData; - bodyMaxLength: number; } interface PrBodyContent { body: string; - comments?: PrComment[]; + comments: PrComment[]; } interface PrComment { @@ -100,7 +99,7 @@ export function getPrBody( branchConfig.dependencyDashboard, ); } - let content = { + const content = { header: getPrHeader(branchConfig), table: getPrUpdatesTable(branchConfig), warnings, @@ -111,41 +110,34 @@ export function getPrBody( footer: getPrFooter(branchConfig), }; - let prBody = createPrBody(content, branchConfig, prBodyConfig); - - if (prBody.length <= prBodyConfig.bodyMaxLength) { - return { body: prBody }; - } - - const changelogs = content.changelogs; - content = { - ...content, - changelogs: 'Please see comment below for changelogs', + const result: PrBodyContent = { + body: createPrBody(content, branchConfig, prBodyConfig), + comments: [], }; + if (result.body.length <= platform.maxBodyLength()) { + return result; + } - prBody = createPrBody(content, branchConfig, prBodyConfig); + if (content.changelogs) { + result.comments.push({ + title: 'Release Notes', + content: content.changelogs, + }); + content.changelogs = 'Please see comment below for changelogs'; - if (prBody.length <= prBodyConfig.bodyMaxLength) { - return { - body: prBody, - comments: [{ title: 'Release Notes', content: changelogs }], - }; + result.body = createPrBody(content, branchConfig, prBodyConfig); + if (result.body.length <= platform.maxBodyLength()) { + return result; + } } - const table = content.table; - content = { - ...content, - table: 'Please see comment below for updates', - }; - prBody = createPrBody(content, branchConfig, prBodyConfig); + if (content.table) { + result.comments.push({ title: 'Updates', content: content.table }); + content.table = 'Please see comment below for updates'; - return { - body: prBody, - comments: [ - { title: 'Release Notes', content: changelogs }, - { title: 'Updates', content: table }, - ], - }; + result.body = createPrBody(content, branchConfig, prBodyConfig); + } + return result; } function createPrBody( diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index d7c385752b1e1d..d35f1c067b0793 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -329,7 +329,6 @@ export async function ensurePr( const prBody = getPrBody( config, { - bodyMaxLength: platform.maxBodyLength(), debugData: updatePrDebugData( config.baseBranch, prepareLabels(config), // include labels in debug data From 08a35271f4c6fe123f911fc3e2304eb7fa4222f6 Mon Sep 17 00:00:00 2001 From: Bianco Veigel Date: Fri, 13 Sep 2024 21:56:02 +0200 Subject: [PATCH 05/20] move smartTruncate from platform to createPrBody --- lib/modules/platform/bitbucket-server/index.ts | 3 +-- lib/modules/platform/bitbucket/index.ts | 3 +-- lib/modules/platform/gerrit/index.ts | 3 +-- lib/modules/platform/gitea/index.ts | 3 +-- lib/modules/platform/github/index.ts | 5 ++--- lib/modules/platform/gitlab/index.ts | 3 +-- lib/workers/repository/config-migration/pr/index.ts | 2 ++ lib/workers/repository/dependency-dashboard.ts | 6 +++++- lib/workers/repository/onboarding/pr/index.ts | 2 ++ lib/workers/repository/update/branch/index.ts | 2 ++ lib/workers/repository/update/pr/body/index.ts | 2 ++ lib/workers/repository/update/pr/index.ts | 2 ++ 12 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/modules/platform/bitbucket-server/index.ts b/lib/modules/platform/bitbucket-server/index.ts index 3811ee5e09ce18..9e5f9cebbcd451 100644 --- a/lib/modules/platform/bitbucket-server/index.ts +++ b/lib/modules/platform/bitbucket-server/index.ts @@ -39,7 +39,6 @@ import type { UpdatePrConfig, } from '../types'; import { getNewBranchName, repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import { UserSchema } from './schema'; import type { BbsConfig, @@ -1104,7 +1103,7 @@ export async function mergePr({ export function massageMarkdown(input: string): string { logger.debug(`massageMarkdown(${input.split(newlineRegex)[0]})`); // Remove any HTML we use - return smartTruncate(input, maxBodyLength()) + return input .replace( 'you tick the rebase/retry checkbox', 'rename PR to start with "rebase!"', diff --git a/lib/modules/platform/bitbucket/index.ts b/lib/modules/platform/bitbucket/index.ts index 4ebd23684d6031..b52d64c362a86f 100644 --- a/lib/modules/platform/bitbucket/index.ts +++ b/lib/modules/platform/bitbucket/index.ts @@ -31,7 +31,6 @@ import type { UpdatePrConfig, } from '../types'; import { repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import { readOnlyIssueBody } from '../utils/read-only-issue-body'; import * as comments from './comments'; import { BitbucketPrCache } from './pr-cache'; @@ -570,7 +569,7 @@ async function closeIssue(issueNumber: number): Promise { export function massageMarkdown(input: string): string { // Remove any HTML we use - return smartTruncate(input, maxBodyLength()) + return input .replace( 'you tick the rebase/retry checkbox', 'by renaming this PR to start with "rebase!"', diff --git a/lib/modules/platform/gerrit/index.ts b/lib/modules/platform/gerrit/index.ts index 20360a761d497d..18649f868b5867 100644 --- a/lib/modules/platform/gerrit/index.ts +++ b/lib/modules/platform/gerrit/index.ts @@ -25,7 +25,6 @@ import type { } from '../types'; import { repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import { readOnlyIssueBody } from '../utils/read-only-issue-body'; import { client } from './client'; import { configureScm } from './scm'; @@ -396,7 +395,7 @@ export async function ensureComment( export function massageMarkdown(prBody: string): string { //TODO: do more Gerrit specific replacements? - return smartTruncate(readOnlyIssueBody(prBody), maxBodyLength()) + return readOnlyIssueBody(prBody) .replace(regEx(/Pull Request(s)?/g), 'Change-Request$1') .replace(regEx(/\bPR(s)?\b/g), 'Change-Request$1') .replace(regEx(/<\/?summary>/g), '**') diff --git a/lib/modules/platform/gitea/index.ts b/lib/modules/platform/gitea/index.ts index d67cf0ceea290e..2fd9fd45cf7e42 100644 --- a/lib/modules/platform/gitea/index.ts +++ b/lib/modules/platform/gitea/index.ts @@ -39,7 +39,6 @@ import type { UpdatePrConfig, } from '../types'; import { repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import * as helper from './gitea-helper'; import { giteaHttp } from './gitea-helper'; import { GiteaPrCache } from './pr-cache'; @@ -998,7 +997,7 @@ const platform: Platform = { }, massageMarkdown(prBody: string): string { - return smartTruncate(smartLinks(prBody), maxBodyLength()); + return smartLinks(prBody); }, maxBodyLength, diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index 23a8210574053a..e7c766dfeb08f0 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -66,7 +66,6 @@ import type { } from '../types'; import { repoFingerprint } from '../util'; import { normalizeNamePerEcosystem } from '../utils/github-alerts'; -import { smartTruncate } from '../utils/pr-body'; import { remoteBranchExists } from './branch'; import { coerceRestPr, githubApi } from './common'; import { @@ -1941,7 +1940,7 @@ export async function mergePr({ export function massageMarkdown(input: string): string { if (platformConfig.isGhe) { - return smartTruncate(input, maxBodyLength()); + return input; } const massagedInput = massageMarkdownLinks(input) // to be safe, replace all github.com links with redirect.github.com @@ -1961,7 +1960,7 @@ export function massageMarkdown(input: string): string { .replace('> ⚠ **Warning**\n> \n', '> [!WARNING]\n') .replace('> ⚠️ **Warning**\n> \n', '> [!WARNING]\n') .replace('> ❗ **Important**\n> \n', '> [!IMPORTANT]\n'); - return smartTruncate(massagedInput, maxBodyLength()); + return massagedInput; } export function maxBodyLength(): number { diff --git a/lib/modules/platform/gitlab/index.ts b/lib/modules/platform/gitlab/index.ts index 264a05bb10b7ed..39d1df4d76eb32 100644 --- a/lib/modules/platform/gitlab/index.ts +++ b/lib/modules/platform/gitlab/index.ts @@ -54,7 +54,6 @@ import type { UpdatePrConfig, } from '../types'; import { repoFingerprint } from '../util'; -import { smartTruncate } from '../utils/pr-body'; import { getMemberUserIDs, getMemberUsernames, @@ -904,7 +903,7 @@ export function massageMarkdown(input: string): string { .replace(regEx(/\]\(\.\.\/pull\//g), '](!') // Strip unicode null characters as GitLab markdown does not permit them .replace(regEx(/\u0000/g), ''); // eslint-disable-line no-control-regex - return smartTruncate(desc, maxBodyLength()); + return desc; } export function maxBodyLength(): number { diff --git a/lib/workers/repository/config-migration/pr/index.ts b/lib/workers/repository/config-migration/pr/index.ts index 53a3ccb57744cd..55b3ad93e22f3e 100644 --- a/lib/workers/repository/config-migration/pr/index.ts +++ b/lib/workers/repository/config-migration/pr/index.ts @@ -5,6 +5,7 @@ import { logger } from '../../../../logger'; import { platform } from '../../../../modules/platform'; import { hashBody } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; +import { smartTruncate } from '../../../../modules/platform/utils/pr-body'; import { emojify } from '../../../../util/emoji'; import { coerceString } from '../../../../util/string'; import * as template from '../../../../util/template'; @@ -64,6 +65,7 @@ ${ logger.trace({ prBody }, 'prBody'); prBody = platform.massageMarkdown(prBody); + prBody = smartTruncate(prBody, platform.maxBodyLength()); if (existingPr) { logger.debug('Found open migration PR'); diff --git a/lib/workers/repository/dependency-dashboard.ts b/lib/workers/repository/dependency-dashboard.ts index d23e5e70beb298..d7cafc4df6f4ee 100644 --- a/lib/workers/repository/dependency-dashboard.ts +++ b/lib/workers/repository/dependency-dashboard.ts @@ -4,6 +4,7 @@ import type { RenovateConfig } from '../../config/types'; import { logger } from '../../logger'; import type { PackageFile } from '../../modules/manager/types'; import { platform } from '../../modules/platform'; +import { smartTruncate } from '../../modules/platform/utils/pr-body'; import { regEx } from '../../util/regex'; import { coerceString } from '../../util/string'; import * as template from '../../util/template'; @@ -515,7 +516,10 @@ export async function ensureDependencyDashboard( await platform.ensureIssue({ title: config.dependencyDashboardTitle!, reuseTitle, - body: platform.massageMarkdown(issueBody), + body: smartTruncate( + platform.massageMarkdown(issueBody), + platform.maxBodyLength(), + ), labels: config.dependencyDashboardLabels, confidential: config.confidential, }); diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 3150d381324d9a..74588379432a3c 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -7,6 +7,7 @@ import { platform } from '../../../../modules/platform'; import { ensureComment } from '../../../../modules/platform/comment'; import { hashBody } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; +import { smartTruncate } from '../../../../modules/platform/utils/pr-body'; import { emojify } from '../../../../util/emoji'; import { getFile } from '../../../../util/git'; import { toSha256 } from '../../../../util/hash'; @@ -148,6 +149,7 @@ If you need any further assistance then you can also [request help here](${ logger.trace('prBody:\n' + prBody); prBody = platform.massageMarkdown(prBody); + prBody = smartTruncate(prBody, platform.maxBodyLength()); if (existingPr) { logger.debug('Found open onboarding PR'); diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 4f35e4ad3667e9..47c3faf219e97b 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -23,6 +23,7 @@ import { ensureCommentRemoval, } from '../../../../modules/platform/comment'; import { scm } from '../../../../modules/platform/scm'; +import { smartTruncate } from '../../../../modules/platform/utils/pr-body'; import { ExternalHostError } from '../../../../types/errors/external-host-error'; import { getElapsedMs } from '../../../../util/date'; import { emojify } from '../../../../util/emoji'; @@ -871,6 +872,7 @@ export async function processBranch( content += `\`\`\`\n${error.stderr!}\n\`\`\`\n\n`; }); content = platform.massageMarkdown(content); + content = smartTruncate(content, platform.maxBodyLength()); if ( !( config.suppressNotifications!.includes('artifactErrors') || diff --git a/lib/workers/repository/update/pr/body/index.ts b/lib/workers/repository/update/pr/body/index.ts index 1d1934c8ab0af5..bc329fb8b4f9fb 100644 --- a/lib/workers/repository/update/pr/body/index.ts +++ b/lib/workers/repository/update/pr/body/index.ts @@ -1,6 +1,7 @@ import type { RenovateConfig } from '../../../../../config/types'; import type { PrDebugData } from '../../../../../modules/platform'; import { platform } from '../../../../../modules/platform'; +import { smartTruncate } from '../../../../../modules/platform/utils/pr-body'; import { regEx } from '../../../../../util/regex'; import { toBase64 } from '../../../../../util/string'; import * as template from '../../../../../util/template'; @@ -137,6 +138,7 @@ export function getPrBody( result.body = createPrBody(content, branchConfig, prBodyConfig); } + result.body = smartTruncate(result.body, platform.maxBodyLength()); return result; } diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index d35f1c067b0793..8cb235508a3212 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -21,6 +21,7 @@ import { hashBody, } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; +import { smartTruncate } from '../../../../modules/platform/utils/pr-body'; import { ExternalHostError } from '../../../../types/errors/external-host-error'; import { getElapsedHours } from '../../../../util/date'; import { stripEmojis } from '../../../../util/emoji'; @@ -561,6 +562,7 @@ export async function ensurePr( content += '\n___\n * Branch has one or more failed status checks'; } content = platform.massageMarkdown(content); + content = smartTruncate(content, platform.maxBodyLength()); logger.debug('Adding branch automerge failure message to PR'); if (GlobalConfig.get('dryRun')) { logger.info(`DRY-RUN: Would add comment to PR #${pr.number}`); From 09b865632ddae4b093950c7c1bb72eae143d3d9d Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 13 Sep 2024 23:16:38 +0200 Subject: [PATCH 06/20] fix uninitialized topics array --- lib/workers/repository/update/pr/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/pr/index.ts b/lib/workers/repository/update/pr/index.ts index 8cb235508a3212..b214d4aed7c4b1 100644 --- a/lib/workers/repository/update/pr/index.ts +++ b/lib/workers/repository/update/pr/index.ts @@ -368,7 +368,7 @@ export async function ensurePr( prCurrentLabels, configuredLabels, ); - let topics: string[]; + let topics: string[] = []; if (prBody.comments) { topics = prBody.comments.map((x) => x.title); From 76f868deddc18203faeed6cfb57472fb42fd0797 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 13 Sep 2024 23:36:28 +0200 Subject: [PATCH 07/20] Fix tests --- lib/modules/platform/gitlab/index.spec.ts | 20 ++++++------------- .../repository/update/pr/body/index.spec.ts | 15 ++++++++------ .../repository/update/pr/index.spec.ts | 9 +++++---- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/modules/platform/gitlab/index.spec.ts b/lib/modules/platform/gitlab/index.spec.ts index 671e64080280d8..b642c439806de9 100644 --- a/lib/modules/platform/gitlab/index.spec.ts +++ b/lib/modules/platform/gitlab/index.spec.ts @@ -3068,25 +3068,17 @@ These updates have all been created already. Click a checkbox below to force a r expect(gitlab.massageMarkdown(prBody)).toMatchSnapshot(); expect(smartTruncate).not.toHaveBeenCalled(); }); + }); - it('truncates description if too low API version', async () => { - jest.doMock('../utils/pr-body'); - const { smartTruncate } = await import('../utils/pr-body'); - + describe('maxBodyLength()', () => { + it('maxBodyLength is 25000 if too low API version', async () => { await initFakePlatform('13.3.0'); - gitlab.massageMarkdown(prBody); - expect(smartTruncate).toHaveBeenCalledTimes(1); - expect(smartTruncate).toHaveBeenCalledWith(expect.any(String), 25000); + expect(gitlab.maxBodyLength()).toBe(25000); }); - it('truncates description for API version gt 13.4', async () => { - jest.doMock('../utils/pr-body'); - const { smartTruncate } = await import('../utils/pr-body'); - + it('maxBodyLength is 1000000 description for API version gt 13.4', async () => { await initFakePlatform('13.4.1'); - gitlab.massageMarkdown(prBody); - expect(smartTruncate).toHaveBeenCalledTimes(1); - expect(smartTruncate).toHaveBeenCalledWith(expect.any(String), 1000000); + expect(gitlab.maxBodyLength()).toBe(1000000); }); }); diff --git a/lib/workers/repository/update/pr/body/index.spec.ts b/lib/workers/repository/update/pr/body/index.spec.ts index eb5149a2685a1f..fd526ab2b35247 100644 --- a/lib/workers/repository/update/pr/body/index.spec.ts +++ b/lib/workers/repository/update/pr/body/index.spec.ts @@ -51,6 +51,7 @@ describe('workers/repository/update/pr/body/index', () => { }); it('handles empty template', () => { + platform.maxBodyLength.mockReturnValueOnce(3.14); const res = getPrBody( { manager: 'some-manager', @@ -67,7 +68,7 @@ describe('workers/repository/update/pr/body/index', () => { }, {}, ); - expect(res).toBeEmptyString(); + expect(res).toStrictEqual({ body: '', comments: [] }); }); it('massages upgrades', () => { @@ -182,8 +183,8 @@ describe('workers/repository/update/pr/body/index', () => { }, {}, ); - expect(res).toContain('PR BODY'); - expect(res).toContain(`\n Some body`, - ); + comments: [], + }); config.labels = ['new_label']; const res = await ensurePr(config); From b046ec52f663d5c8348fa8844e5d7bf37368fab5 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 00:36:22 +0200 Subject: [PATCH 08/20] Mock massageMarkdown() --- lib/workers/repository/config-migration/pr/index.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/workers/repository/config-migration/pr/index.spec.ts b/lib/workers/repository/config-migration/pr/index.spec.ts index 162d60b5ce07e2..3571557e86cc46 100644 --- a/lib/workers/repository/config-migration/pr/index.spec.ts +++ b/lib/workers/repository/config-migration/pr/index.spec.ts @@ -262,6 +262,7 @@ describe('workers/repository/config-migration/pr/index', () => { errors: [{ message: 'A pull request already exists' }], }; platform.createPr.mockRejectedValue(err); + platform.massageMarkdown.mockImplementationOnce((x) => x); await expect(ensureConfigMigrationPr(config, migratedData)).toResolve(); expect(logger.warn).toHaveBeenCalledWith( { err }, From fd9111ce031d0233384458c53964f5634d392a66 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 14:56:02 +0200 Subject: [PATCH 09/20] Add tests for maxBodyLength() --- lib/modules/platform/azure/index.spec.ts | 6 ++++++ lib/modules/platform/bitbucket-server/index.spec.ts | 6 ++++++ lib/modules/platform/bitbucket/index.spec.ts | 6 ++++++ lib/modules/platform/codecommit/index.spec.ts | 6 ++++++ lib/modules/platform/gerrit/index.spec.ts | 6 ++++++ lib/modules/platform/gitea/index.spec.ts | 6 ++++++ lib/modules/platform/github/index.spec.ts | 6 ++++++ 7 files changed, 42 insertions(+) diff --git a/lib/modules/platform/azure/index.spec.ts b/lib/modules/platform/azure/index.spec.ts index bfeb72ebd043d1..51abef384e63c2 100644 --- a/lib/modules/platform/azure/index.spec.ts +++ b/lib/modules/platform/azure/index.spec.ts @@ -1985,4 +1985,10 @@ describe('modules/platform/azure/index', () => { expect(res).toBeNull(); }); }); + + describe('maxBodyLength()', () => { + it('returns 4000', () => { + expect(azure.maxBodyLength()).toBe(4000); + }); + }); }); diff --git a/lib/modules/platform/bitbucket-server/index.spec.ts b/lib/modules/platform/bitbucket-server/index.spec.ts index 61bdbb2a287bb8..1a5bc15e9dcf95 100644 --- a/lib/modules/platform/bitbucket-server/index.spec.ts +++ b/lib/modules/platform/bitbucket-server/index.spec.ts @@ -2470,6 +2470,12 @@ Followed by some information. await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow(); }); }); + + describe('maxBodyLength()', () => { + it('returns 30000', () => { + expect(bitbucket.maxBodyLength()).toBe(30000); + }); + }); }); }); }); diff --git a/lib/modules/platform/bitbucket/index.spec.ts b/lib/modules/platform/bitbucket/index.spec.ts index 9de64419b236dc..18b0cb76cff28d 100644 --- a/lib/modules/platform/bitbucket/index.spec.ts +++ b/lib/modules/platform/bitbucket/index.spec.ts @@ -1896,4 +1896,10 @@ describe('modules/platform/bitbucket/index', () => { await expect(bitbucket.getJsonFile('file.json')).rejects.toThrow(); }); }); + + describe('maxBodyLength()', () => { + it('returns 50000', () => { + expect(bitbucket.maxBodyLength()).toBe(50000); + }); + }); }); diff --git a/lib/modules/platform/codecommit/index.spec.ts b/lib/modules/platform/codecommit/index.spec.ts index 75ff969775db8f..758a55c380ef54 100644 --- a/lib/modules/platform/codecommit/index.spec.ts +++ b/lib/modules/platform/codecommit/index.spec.ts @@ -1327,4 +1327,10 @@ describe('modules/platform/codecommit/index', () => { ); }); }); + + describe('maxBodyLength()', () => { + it('returns Infinity', () => { + expect(codeCommit.maxBodyLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/gerrit/index.spec.ts b/lib/modules/platform/gerrit/index.spec.ts index 025f1d1e2e3f7e..7b0cc2262a8e45 100644 --- a/lib/modules/platform/gerrit/index.spec.ts +++ b/lib/modules/platform/gerrit/index.spec.ts @@ -792,4 +792,10 @@ describe('modules/platform/gerrit/index', () => { await expect(gerrit.getIssueList()).resolves.toStrictEqual([]); }); }); + + describe('maxBodyLength()', () => { + it('returns 16384', () => { + expect(gerrit.maxBodyLength()).toBe(16384); + }); + }); }); diff --git a/lib/modules/platform/gitea/index.spec.ts b/lib/modules/platform/gitea/index.spec.ts index 198ea0962ad7a5..ed3979070b4848 100644 --- a/lib/modules/platform/gitea/index.spec.ts +++ b/lib/modules/platform/gitea/index.spec.ts @@ -2989,4 +2989,10 @@ describe('modules/platform/gitea/index', () => { await expect(gitea.getJsonFile('file.json')).rejects.toThrow(); }); }); + + describe('maxBodyLength()', () => { + it('returns 1000000', () => { + expect(gitea.maxBodyLength()).toBe(1000000); + }); + }); }); diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index 47127a5504c371..73988451a3bd92 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -4203,4 +4203,10 @@ describe('modules/platform/github/index', () => { expect(res).toBeNull(); }); }); + + describe('maxBodyLength()', () => { + it('returns 60000', () => { + expect(github.maxBodyLength()).toBe(60000); + }); + }); }); From 90b600f2a9dcec8a1ecdd090919acb3979fefa09 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 19:51:57 +0200 Subject: [PATCH 10/20] add comments tests for getPrBody() --- .../repository/update/pr/body/index.spec.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/lib/workers/repository/update/pr/body/index.spec.ts b/lib/workers/repository/update/pr/body/index.spec.ts index fd526ab2b35247..c4b633a28be250 100644 --- a/lib/workers/repository/update/pr/body/index.spec.ts +++ b/lib/workers/repository/update/pr/body/index.spec.ts @@ -289,5 +289,71 @@ describe('workers/repository/update/pr/body/index', () => { 'Please check the Dependency Dashboard for more information\n\n---'; expect(res.body).toBe(expected); }); + + it('returns changelog as comment if body is larger than maxBodyLength() of the platform', () => { + platform.maxBodyLength.mockReturnValue('{{{header}}}'.length); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce((_) => '{{{header}}}'); + template.compile.mockImplementation((x) => x); + + const res = getPrBody( + { + manager: 'some-manager', + baseBranch: 'base', + branchName: 'some-branch', + upgrades: [], + prBodyTemplate: '{{{header}}}{{{changelogs}}}', + }, + { + debugData: { + updatedInVer: '1.2.3', + createdInVer: '1.2.3', + targetBranch: 'base', + }, + }, + {}, + ); + + expect(res).toStrictEqual({ + body: '{{{header}}}', + comments: [{ content: 'getChangelogs', title: 'Release Notes' }], + }); + }); + + it('returns changelog & update table as comments if body is larger than maxBodyLength() of the platform', () => { + platform.maxBodyLength.mockReturnValue('{{{header}}}'.length); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce( + (_) => '{{{header}}}{{{table}}}', + ); + platform.massageMarkdown.mockImplementationOnce((_) => '{{{header}}}'); + template.compile.mockImplementation((x) => x); + + const res = getPrBody( + { + manager: 'some-manager', + baseBranch: 'base', + branchName: 'some-branch', + upgrades: [], + prBodyTemplate: '{{{header}}}{{{table}}}{{{changelogs}}}', + }, + { + debugData: { + updatedInVer: '1.2.3', + createdInVer: '1.2.3', + targetBranch: 'base', + }, + }, + {}, + ); + + expect(res).toStrictEqual({ + body: '{{{header}}}', + comments: [ + { content: 'getChangelogs', title: 'Release Notes' }, + { content: 'getPrUpdatesTable', title: 'Updates' }, + ], + }); + }); }); }); From e1fb895932c8aa93006833a549dec8d3ec19bfd9 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 22:26:00 +0200 Subject: [PATCH 11/20] adjust type of comment title --- lib/workers/repository/update/pr/body/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/update/pr/body/index.ts b/lib/workers/repository/update/pr/body/index.ts index bc329fb8b4f9fb..9c881f8a90cc9e 100644 --- a/lib/workers/repository/update/pr/body/index.ts +++ b/lib/workers/repository/update/pr/body/index.ts @@ -79,7 +79,7 @@ interface PrBodyContent { } interface PrComment { - title: string; + title: 'Release Notes' | 'Updates'; content: string; } From 1e40481b757bd292713f0ce174140610c852206a Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 22:39:03 +0200 Subject: [PATCH 12/20] add tests for comment creation on platform --- .../repository/update/pr/index.spec.ts | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/lib/workers/repository/update/pr/index.spec.ts b/lib/workers/repository/update/pr/index.spec.ts index b98c3453cd9e3c..5303586c0f20ba 100644 --- a/lib/workers/repository/update/pr/index.spec.ts +++ b/lib/workers/repository/update/pr/index.spec.ts @@ -227,6 +227,29 @@ describe('workers/repository/update/pr/index', () => { expect(prCache.setPrCache).toHaveBeenCalled(); }); + it('creates comments if prBody contains comments', async () => { + platform.createPr.mockResolvedValueOnce(pr); + prBody.getPrBody.mockReturnValueOnce({ + body: 'body', + comments: [ + { content: 'content', title: 'Release Notes' }, + { content: 'content', title: 'Updates' }, + ], + }); + await ensurePr(config); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Release Notes', + content: 'content', + }); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Updates', + content: 'content', + }); + expect(platform.ensureCommentRemoval).not.toHaveBeenCalled(); + }); + describe('Error handling', () => { it('handles unknown error', async () => { const err = new Error('unknown'); @@ -484,6 +507,50 @@ describe('workers/repository/update/pr/index', () => { 'Pull Request #123 does not need updating', ); }); + + it('creates comments if prBody contains comments on existing Pr', async () => { + platform.getBranchPr.mockResolvedValueOnce(pr); + prBody.getPrBody.mockReturnValueOnce({ + body: 'body', + comments: [ + { content: 'content', title: 'Release Notes' }, + { content: 'content', title: 'Updates' }, + ], + }); + await ensurePr(config); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Release Notes', + content: 'content', + }); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Updates', + content: 'content', + }); + expect(platform.ensureCommentRemoval).not.toHaveBeenCalled(); + }); + + it('removes comments if prBody does not contains comments', async () => { + platform.getBranchPr.mockResolvedValueOnce(pr); + platform.createPr.mockResolvedValueOnce(pr); + prBody.getPrBody.mockReturnValueOnce({ + body: 'body', + comments: [], + }); + await ensurePr(config); + expect(platform.ensureComment).not.toHaveBeenCalled(); + expect(platform.ensureCommentRemoval).toHaveBeenCalledWith({ + number: expect.anything(), + type: 'by-topic', + topic: 'Release Notes', + }); + expect(platform.ensureCommentRemoval).toHaveBeenCalledWith({ + number: expect.anything(), + type: 'by-topic', + topic: 'Updates', + }); + }); }); describe('dry-run', () => { From eb26afcb80179ebbdb25f1ca19650ea49205973c Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Sat, 14 Sep 2024 22:54:57 +0200 Subject: [PATCH 13/20] Move massageMarkdown() mock to be mocked for both 'ensureConfigMigrationPr() throws' tests --- lib/workers/repository/config-migration/pr/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/workers/repository/config-migration/pr/index.spec.ts b/lib/workers/repository/config-migration/pr/index.spec.ts index 3571557e86cc46..f8f328ab282091 100644 --- a/lib/workers/repository/config-migration/pr/index.spec.ts +++ b/lib/workers/repository/config-migration/pr/index.spec.ts @@ -249,6 +249,7 @@ describe('workers/repository/config-migration/pr/index', () => { beforeEach(() => { GlobalConfig.reset(); scm.deleteBranch.mockResolvedValue(); + platform.massageMarkdown.mockImplementationOnce((x) => x); }); it('throws when trying to create a new PR', async () => { @@ -262,7 +263,6 @@ describe('workers/repository/config-migration/pr/index', () => { errors: [{ message: 'A pull request already exists' }], }; platform.createPr.mockRejectedValue(err); - platform.massageMarkdown.mockImplementationOnce((x) => x); await expect(ensureConfigMigrationPr(config, migratedData)).toResolve(); expect(logger.warn).toHaveBeenCalledWith( { err }, From 33558acca36c06fe661c0350645e9854d0024d29 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 20 Sep 2024 22:48:31 +0200 Subject: [PATCH 14/20] Move PR List and/or Detected Package Files to comments if prBody limit is reached --- lib/workers/repository/onboarding/pr/index.ts | 216 ++++++++++++++---- 1 file changed, 170 insertions(+), 46 deletions(-) diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 74588379432a3c..86bd99c1378537 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -31,6 +31,28 @@ import { getBaseBranchDesc } from './base-branch'; import { getConfigDesc } from './config-description'; import { getPrList } from './pr-list'; +interface PrContent { + packageFiles: string; + config: string; + warnings: string; + errors: string; + baseBranch: string; + prList: string; + prHeader: string; + prFooter: string; + onboardingConfigHashComment: string; +} + +interface PrBodyContent { + body: string; + comments: PrComment[]; +} + +interface PrComment { + title: 'PR List' | 'Package Files'; + content: string; +} + export async function ensureOnboardingPr( config: RenovateConfig, packageFiles: Record | null, @@ -106,55 +128,41 @@ If you need any further assistance then you can also [request help here](${ `, ); prTemplate += rebaseCheckBox; - let prBody = prTemplate; - if (packageFiles && Object.entries(packageFiles).length) { - let files: string[] = []; - for (const [manager, managerFiles] of Object.entries(packageFiles)) { - files = files.concat( - managerFiles.map((file) => ` * \`${file.packageFile}\` (${manager})`), - ); - } - prBody = - prBody.replace( - '{{PACKAGE FILES}}', - '### Detected Package Files\n\n' + files.join('\n'), - ) + '\n'; - } else { - prBody = prBody.replace('{{PACKAGE FILES}}\n', ''); - } - let configDesc = ''; - if (GlobalConfig.get('dryRun')) { - // TODO: types (#22198) - logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); - } else { - configDesc = getConfigDesc(config, packageFiles!); - } - prBody = prBody.replace('{{CONFIG}}\n', configDesc); - prBody = prBody.replace( - '{{WARNINGS}}\n', - getWarnings(config) + getDepWarningsOnboardingPR(packageFiles!, config), + const prBody = getPrBody( + prTemplate, + packageFiles, + config, + branches, + onboardingConfigHashComment, ); - prBody = prBody.replace('{{ERRORS}}\n', getErrors(config)); - prBody = prBody.replace('{{BASEBRANCH}}\n', getBaseBranchDesc(config)); - prBody = prBody.replace('{{PRLIST}}\n', getPrList(config, branches)); - if (is.string(config.prHeader)) { - prBody = `${template.compile(config.prHeader, config)}\n\n${prBody}`; - } - if (is.string(config.prFooter)) { - prBody = `${prBody}\n---\n\n${template.compile(config.prFooter, config)}\n`; - } - - prBody += onboardingConfigHashComment; - - logger.trace('prBody:\n' + prBody); - - prBody = platform.massageMarkdown(prBody); - prBody = smartTruncate(prBody, platform.maxBodyLength()); if (existingPr) { logger.debug('Found open onboarding PR'); + let topics: string[] = []; + + if (prBody.comments) { + topics = prBody.comments.map((x) => x.title); + for (const comment of prBody.comments) { + await platform.ensureComment({ + number: existingPr.number, + topic: comment.title, + content: comment.content, + }); + } + } + const topicsToDelete = ['PR List', 'Package Files'].filter( + (x) => !topics.includes(x), + ); + for (const topic of topicsToDelete) { + await platform.ensureCommentRemoval({ + number: existingPr.number, + type: 'by-topic', + topic, + }); + } + // Check if existing PR needs updating - const prBodyHash = hashBody(prBody); + const prBodyHash = hashBody(prBody.body); if (existingPr.bodyStruct?.hash === prBodyHash) { logger.debug(`Pull Request #${existingPr.number} does not need updating`); return; @@ -166,7 +174,7 @@ If you need any further assistance then you can also [request help here](${ await platform.updatePr({ number: existingPr.number, prTitle: existingPr.title, - prBody, + prBody: prBody.body, }); logger.info({ pr: existingPr.number }, 'Onboarding PR updated'); } @@ -187,13 +195,22 @@ If you need any further assistance then you can also [request help here](${ sourceBranch: config.onboardingBranch!, targetBranch: config.defaultBranch!, prTitle, - prBody, + prBody: prBody.body, labels, platformPrOptions: getPlatformPrOptions({ ...config, automerge: false, }), }); + if (pr && prBody.comments) { + for (const comment of prBody.comments) { + await platform.ensureComment({ + number: pr.number, + topic: comment.title, + content: comment.content, + }); + } + } logger.info( { pr: `Pull Request #${pr!.number}` }, 'Onboarding PR created', @@ -217,6 +234,113 @@ If you need any further assistance then you can also [request help here](${ } } +function getPrBody( + prTemplate: string, + packageFiles: Record | null, + config: RenovateConfig, + branches: BranchConfig[], + onboardingConfigHashComment: string, +): PrBodyContent { + let packageFilesContent = ''; + if (packageFiles && Object.entries(packageFiles).length) { + let files: string[] = []; + for (const [manager, managerFiles] of Object.entries(packageFiles)) { + files = files.concat( + managerFiles.map((file) => ` * \`${file.packageFile}\` (${manager})`), + ); + } + packageFilesContent = + '\n### Detected Package Files\n\n' + files.join('\n') + '\n'; + } + + let configDesc = ''; + if (GlobalConfig.get('dryRun')) { + // TODO: types (#22198) + logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); + } else { + configDesc = getConfigDesc(config, packageFiles!); + } + + let prHeader = ''; + if (is.string(config.prHeader)) { + prHeader = template.compile(config.prHeader, config); + } + let prFooter = ''; + if (is.string(config.prFooter)) { + prFooter = template.compile(config.prFooter, config); + } + + const content = { + packageFiles: packageFilesContent, //todo \n mit ins template + config: configDesc, //todo \n mit ins template + warnings: + getWarnings(config) + getDepWarningsOnboardingPR(packageFiles!, config), + errors: getErrors(config), + baseBranch: getBaseBranchDesc(config), + prList: getPrList(config, branches), + prHeader, + prFooter, + onboardingConfigHashComment, + }; + + const result: PrBodyContent = { + body: createPrBody(prTemplate, content), + comments: [], + }; + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + + if (content.prList) { + result.comments.push({ + title: 'PR List', + content: content.prList, + }); + content.prList = 'Please see comment below for what to expect'; + + result.body = createPrBody(prTemplate, content); + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + } + + if (content.packageFiles) { + result.comments.push({ + title: 'Package Files', + content: content.packageFiles, + }); + content.packageFiles = + 'Please see comment below for detected Package Files'; + + result.body = createPrBody(prTemplate, content); + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + } + + result.body = smartTruncate(result.body, platform.maxBodyLength()); + return result; +} + +function createPrBody(template: string, content: PrContent): string { + let prBody = template.replace('{{PACKAGE FILES}}\n', content.packageFiles); + prBody = prBody.replace('{{CONFIG}}\n', content.config); + prBody = prBody.replace('{{WARNINGS}}\n', content.warnings); + prBody = prBody.replace('{{ERRORS}}\n', content.errors); + prBody = prBody.replace('{{BASEBRANCH}}\n', content.baseBranch); + prBody = prBody.replace('{{PRLIST}}\n', content.prList); + //footer + if (content.prHeader) { + prBody = `${content.prHeader}\n\n${prBody}`; + } + if (content.prFooter) { + prBody = `${prBody}\n---\n\n${content.prFooter}\n`; + } + + prBody = platform.massageMarkdown(prBody); + return prBody; +} + function getRebaseCheckbox(onboardingRebaseCheckbox?: boolean): string { let rebaseCheckBox = ''; if (onboardingRebaseCheckbox) { From fb0330750a3dda995783a7169ab648080fb5826b Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 20 Sep 2024 23:20:46 +0200 Subject: [PATCH 15/20] Add missing onboardingConfigHashComment --- lib/workers/repository/onboarding/pr/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 86bd99c1378537..db11b0f5d13e88 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -250,7 +250,7 @@ function getPrBody( ); } packageFilesContent = - '\n### Detected Package Files\n\n' + files.join('\n') + '\n'; + '### Detected Package Files\n\n' + files.join('\n') + '\n'; } let configDesc = ''; @@ -310,7 +310,7 @@ function getPrBody( content: content.packageFiles, }); content.packageFiles = - 'Please see comment below for detected Package Files'; + 'Please see comment below for detected Package Files\n'; result.body = createPrBody(prTemplate, content); if (result.body.length <= platform.maxBodyLength()) { @@ -336,7 +336,7 @@ function createPrBody(template: string, content: PrContent): string { if (content.prFooter) { prBody = `${prBody}\n---\n\n${content.prFooter}\n`; } - + prBody += content.onboardingConfigHashComment; prBody = platform.massageMarkdown(prBody); return prBody; } From 6e436af5a51fedb28d83dc14ebf6b2e9eac17597 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 20 Sep 2024 23:21:16 +0200 Subject: [PATCH 16/20] adjust existing tests --- .../pr/__snapshots__/index.spec.ts.snap | 22 ++----------------- .../repository/onboarding/pr/index.spec.ts | 1 + 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap b/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap index 625b52c907cee7..0150ae77e4b343 100644 --- a/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap +++ b/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap @@ -1,9 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with empty footer and header(onboardingRebaseCheckbox="false") 1`] = ` -" - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. +"Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. 🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. @@ -23,19 +21,12 @@ It looks like your repository dependencies are already up-to-date and no Pull Re ❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - ---- - - - " `; exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with empty footer and header(onboardingRebaseCheckbox="true") 1`] = ` -" - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. +"Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. 🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. @@ -60,11 +51,6 @@ If you need any further assistance then you can also [request help here](https:/ - [ ] If you want to rebase/retry this PR, click this checkbox. - ---- - - - " `; @@ -92,7 +78,6 @@ It looks like your repository dependencies are already up-to-date and no Pull Re ❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - --- And this is a footer for repository:test baseBranch:some-branch @@ -129,7 +114,6 @@ If you need any further assistance then you can also [request help here](https:/ - [ ] If you want to rebase/retry this PR, click this checkbox. - --- And this is a footer for repository:test baseBranch:some-branch @@ -163,7 +147,6 @@ It looks like your repository dependencies are already up-to-date and no Pull Re ❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - --- There should be several empty lines at the end of the PR @@ -205,7 +188,6 @@ If you need any further assistance then you can also [request help here](https:/ - [ ] If you want to rebase/retry this PR, click this checkbox. - --- There should be several empty lines at the end of the PR diff --git a/lib/workers/repository/onboarding/pr/index.spec.ts b/lib/workers/repository/onboarding/pr/index.spec.ts index ecf18987c0c5c6..53b163b4f70699 100644 --- a/lib/workers/repository/onboarding/pr/index.spec.ts +++ b/lib/workers/repository/onboarding/pr/index.spec.ts @@ -35,6 +35,7 @@ describe('workers/repository/onboarding/pr/index', () => { branches = []; platform.massageMarkdown.mockImplementation((input) => input); platform.createPr.mockResolvedValueOnce(partial()); + platform.maxBodyLength.mockImplementationOnce(() => Infinity); GlobalConfig.reset(); }); From fdac7143eab9018d1dae09396b73c4a10b3a23f9 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Fri, 20 Sep 2024 23:54:36 +0200 Subject: [PATCH 17/20] Refactor testing --- .../pr/__snapshots__/index.spec.ts.snap | 200 -------------- .../config-description.spec.ts.snap | 10 +- .../pr/{ => body}/base-branch.spec.ts | 6 +- .../onboarding/pr/{ => body}/base-branch.ts | 2 +- .../pr/{ => body}/config-description.spec.ts | 8 +- .../pr/{ => body}/config-description.ts | 10 +- .../onboarding/pr/body/index.spec.ts | 179 +++++++++++++ .../repository/onboarding/pr/body/index.ts | 146 ++++++++++ .../onboarding/pr/{ => body}/pr-list.spec.ts | 8 +- .../onboarding/pr/{ => body}/pr-list.ts | 10 +- .../repository/onboarding/pr/index.spec.ts | 252 ++++++++++-------- lib/workers/repository/onboarding/pr/index.ts | 141 +--------- 12 files changed, 491 insertions(+), 481 deletions(-) delete mode 100644 lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap rename lib/workers/repository/onboarding/pr/{ => body}/__snapshots__/config-description.spec.ts.snap (75%) rename lib/workers/repository/onboarding/pr/{ => body}/base-branch.spec.ts (82%) rename lib/workers/repository/onboarding/pr/{ => body}/base-branch.ts (87%) rename lib/workers/repository/onboarding/pr/{ => body}/config-description.spec.ts (92%) rename lib/workers/repository/onboarding/pr/{ => body}/config-description.ts (86%) create mode 100644 lib/workers/repository/onboarding/pr/body/index.spec.ts create mode 100644 lib/workers/repository/onboarding/pr/body/index.ts rename lib/workers/repository/onboarding/pr/{ => body}/pr-list.spec.ts (93%) rename lib/workers/repository/onboarding/pr/{ => body}/pr-list.ts (89%) diff --git a/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap b/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap deleted file mode 100644 index 0150ae77e4b343..00000000000000 --- a/lib/workers/repository/onboarding/pr/__snapshots__/index.spec.ts.snap +++ /dev/null @@ -1,200 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with empty footer and header(onboardingRebaseCheckbox="false") 1`] = ` -"Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - - -" -`; - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with empty footer and header(onboardingRebaseCheckbox="true") 1`] = ` -"Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - - ---- - - - [ ] If you want to rebase/retry this PR, click this checkbox. - - -" -`; - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with footer and header using templating(onboardingRebaseCheckbox="false") 1`] = ` -"This is a header for platform:github - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - ---- - -And this is a footer for repository:test baseBranch:some-branch - - -" -`; - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with footer and header using templating(onboardingRebaseCheckbox="true") 1`] = ` -"This is a header for platform:github - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - - ---- - - - [ ] If you want to rebase/retry this PR, click this checkbox. - ---- - -And this is a footer for repository:test baseBranch:some-branch - - -" -`; - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with footer and header with trailing and leading newlines(onboardingRebaseCheckbox="false") 1`] = ` -" - -This should not be the first line of the PR - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - ---- - -There should be several empty lines at the end of the PR - - - - - -" -`; - -exports[`workers/repository/onboarding/pr/index ensureOnboardingPr() creates PR with footer and header with trailing and leading newlines(onboardingRebaseCheckbox="true") 1`] = ` -" - -This should not be the first line of the PR - -Welcome to [Renovate](https://github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. - -🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. - - - ---- -### Detected Package Files - - * \`package.json\` (npm) - -### What to Expect - -It looks like your repository dependencies are already up-to-date and no Pull Requests will be necessary right away. - ---- - -❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. -If you need any further assistance then you can also [request help here](https://github.com/renovatebot/renovate/discussions). - - ---- - - - [ ] If you want to rebase/retry this PR, click this checkbox. - ---- - -There should be several empty lines at the end of the PR - - - - - -" -`; diff --git a/lib/workers/repository/onboarding/pr/__snapshots__/config-description.spec.ts.snap b/lib/workers/repository/onboarding/pr/body/__snapshots__/config-description.spec.ts.snap similarity index 75% rename from lib/workers/repository/onboarding/pr/__snapshots__/config-description.spec.ts.snap rename to lib/workers/repository/onboarding/pr/body/__snapshots__/config-description.spec.ts.snap index 1d338c6d487a4d..de58e26a9826ef 100644 --- a/lib/workers/repository/onboarding/pr/__snapshots__/config-description.spec.ts.snap +++ b/lib/workers/repository/onboarding/pr/body/__snapshots__/config-description.spec.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`workers/repository/onboarding/pr/config-description getConfigDesc() contains the onboardingConfigFileName if set 1`] = ` +exports[`workers/repository/onboarding/pr/body/config-description getConfigDesc() contains the onboardingConfigFileName if set 1`] = ` " ### Configuration Summary @@ -15,7 +15,7 @@ Based on the default config's presets, Renovate will: " `; -exports[`workers/repository/onboarding/pr/config-description getConfigDesc() falls back to "renovate.json" if onboardingConfigFileName is not set 1`] = ` +exports[`workers/repository/onboarding/pr/body/config-description getConfigDesc() falls back to "renovate.json" if onboardingConfigFileName is not set 1`] = ` " ### Configuration Summary @@ -30,7 +30,7 @@ Based on the default config's presets, Renovate will: " `; -exports[`workers/repository/onboarding/pr/config-description getConfigDesc() falls back to "renovate.json" if onboardingConfigFileName is not valid 1`] = ` +exports[`workers/repository/onboarding/pr/body/config-description getConfigDesc() falls back to "renovate.json" if onboardingConfigFileName is not valid 1`] = ` " ### Configuration Summary @@ -45,7 +45,7 @@ Based on the default config's presets, Renovate will: " `; -exports[`workers/repository/onboarding/pr/config-description getConfigDesc() returns a full list 1`] = ` +exports[`workers/repository/onboarding/pr/body/config-description getConfigDesc() returns a full list 1`] = ` " ### Configuration Summary @@ -63,7 +63,7 @@ Based on the default config's presets, Renovate will: " `; -exports[`workers/repository/onboarding/pr/config-description getConfigDesc() include retry/refresh checkbox message only if onboardingRebaseCheckbox is true 1`] = ` +exports[`workers/repository/onboarding/pr/body/config-description getConfigDesc() include retry/refresh checkbox message only if onboardingRebaseCheckbox is true 1`] = ` " ### Configuration Summary diff --git a/lib/workers/repository/onboarding/pr/base-branch.spec.ts b/lib/workers/repository/onboarding/pr/body/base-branch.spec.ts similarity index 82% rename from lib/workers/repository/onboarding/pr/base-branch.spec.ts rename to lib/workers/repository/onboarding/pr/body/base-branch.spec.ts index 803ab8ce786724..52fd756795f8be 100644 --- a/lib/workers/repository/onboarding/pr/base-branch.spec.ts +++ b/lib/workers/repository/onboarding/pr/body/base-branch.spec.ts @@ -1,8 +1,8 @@ -import type { RenovateConfig } from '../../../../../test/util'; -import { partial } from '../../../../../test/util'; +import type { RenovateConfig } from '../../../../../../test/util'; +import { partial } from '../../../../../../test/util'; import { getBaseBranchDesc } from './base-branch'; -describe('workers/repository/onboarding/pr/base-branch', () => { +describe('workers/repository/onboarding/pr/body/base-branch', () => { describe('getBaseBranchDesc()', () => { let config: RenovateConfig; diff --git a/lib/workers/repository/onboarding/pr/base-branch.ts b/lib/workers/repository/onboarding/pr/body/base-branch.ts similarity index 87% rename from lib/workers/repository/onboarding/pr/base-branch.ts rename to lib/workers/repository/onboarding/pr/body/base-branch.ts index 0e574e18dc6fc8..448b5d036ade7b 100644 --- a/lib/workers/repository/onboarding/pr/base-branch.ts +++ b/lib/workers/repository/onboarding/pr/body/base-branch.ts @@ -1,4 +1,4 @@ -import type { RenovateConfig } from '../../../../config/types'; +import type { RenovateConfig } from '../../../../../config/types'; export function getBaseBranchDesc(config: RenovateConfig): string { // Describe base branch only if it's configured diff --git a/lib/workers/repository/onboarding/pr/config-description.spec.ts b/lib/workers/repository/onboarding/pr/body/config-description.spec.ts similarity index 92% rename from lib/workers/repository/onboarding/pr/config-description.spec.ts rename to lib/workers/repository/onboarding/pr/body/config-description.spec.ts index 399e5f8a7dd88e..9d38c5ab3cbe8a 100644 --- a/lib/workers/repository/onboarding/pr/config-description.spec.ts +++ b/lib/workers/repository/onboarding/pr/body/config-description.spec.ts @@ -1,9 +1,9 @@ -import type { RenovateConfig } from '../../../../../test/util'; -import { partial } from '../../../../../test/util'; -import type { PackageFile } from '../../../../modules/manager/types'; +import type { RenovateConfig } from '../../../../../../test/util'; +import { partial } from '../../../../../../test/util'; +import type { PackageFile } from '../../../../../modules/manager/types'; import { getConfigDesc } from './config-description'; -describe('workers/repository/onboarding/pr/config-description', () => { +describe('workers/repository/onboarding/pr/body/config-description', () => { describe('getConfigDesc()', () => { let config: RenovateConfig; diff --git a/lib/workers/repository/onboarding/pr/config-description.ts b/lib/workers/repository/onboarding/pr/body/config-description.ts similarity index 86% rename from lib/workers/repository/onboarding/pr/config-description.ts rename to lib/workers/repository/onboarding/pr/body/config-description.ts index e1c092c88e0d79..53b74cdc76ee80 100644 --- a/lib/workers/repository/onboarding/pr/config-description.ts +++ b/lib/workers/repository/onboarding/pr/body/config-description.ts @@ -1,9 +1,9 @@ import is from '@sindresorhus/is'; -import { configFileNames } from '../../../../config/app-strings'; -import type { RenovateConfig } from '../../../../config/types'; -import { logger } from '../../../../logger'; -import type { PackageFile } from '../../../../modules/manager/types'; -import { emojify } from '../../../../util/emoji'; +import { configFileNames } from '../../../../../config/app-strings'; +import type { RenovateConfig } from '../../../../../config/types'; +import { logger } from '../../../../../logger'; +import type { PackageFile } from '../../../../../modules/manager/types'; +import { emojify } from '../../../../../util/emoji'; const defaultConfigFile = configFileNames[0]; diff --git a/lib/workers/repository/onboarding/pr/body/index.spec.ts b/lib/workers/repository/onboarding/pr/body/index.spec.ts new file mode 100644 index 00000000000000..7eafdd47ac314c --- /dev/null +++ b/lib/workers/repository/onboarding/pr/body/index.spec.ts @@ -0,0 +1,179 @@ +import type { RenovateConfig } from '../../../../../../test/util'; +import { mocked, platform } from '../../../../../../test/util'; +import { getConfig } from '../../../../../config/defaults'; +import { GlobalConfig } from '../../../../../config/global'; +import { logger } from '../../../../../logger'; +import type { PackageFile } from '../../../../../modules/manager/types'; +import type { BranchConfig } from '../../../../types'; +import * as _baseBranch from './base-branch'; +import * as _configDescription from './config-description'; +import * as _prList from './pr-list'; +import { getPrBody } from '.'; + +jest.mock('./pr-list'); +const prList = mocked(_prList); + +jest.mock('./config-description'); +const configDescription = mocked(_configDescription); + +jest.mock('./base-branch'); +const baseBranch = mocked(_baseBranch); + +describe('workers/repository/onboarding/pr/body/index', () => { + describe('getPrBody', () => { + let config: RenovateConfig; + let packageFiles: Record; + let branches: BranchConfig[]; + + beforeEach(() => { + config = { + ...getConfig(), + errors: [], + warnings: [], + description: [], + prFooter: undefined, + prHeader: undefined, + }; + packageFiles = {}; + branches = []; + + prList.getPrList.mockReturnValueOnce('getPrList'); + configDescription.getConfigDesc.mockReturnValueOnce('getConfigDesc'); + baseBranch.getBaseBranchDesc.mockReturnValueOnce('getBaseBranchDesc'); + platform.massageMarkdown.mockImplementation((x) => x); + GlobalConfig.reset(); + }); + + it('creates body without comments if maxbodylength is long enough', () => { + platform.maxBodyLength.mockReturnValueOnce(Infinity); + const template = 'PrBody'; + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(res.body).toBe('PrBody'); + expect(res.comments).toEqual([]); + }); + + it('creates body with prFooter', () => { + platform.maxBodyLength.mockReturnValue(Infinity); + const template = 'PrBody'; + + const res = getPrBody( + template, + packageFiles, + { ...config, prFooter: 'Footer' }, + branches, + '', + ); + + expect(res.body).toContain('Footer'); + }); + + it('creates body with prHeader', () => { + platform.maxBodyLength.mockReturnValue(Infinity); + const template = 'PrBody'; + + const res = getPrBody( + template, + packageFiles, + { ...config, prHeader: 'Header' }, + branches, + '', + ); + + expect(res.body).toStartWith('Header'); + }); + + it('creates body with Pr List in comment', () => { + platform.maxBodyLength.mockReturnValue('PrBody'.length); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce((_) => 'PrBody'); + const template = 'PrBody{{PRLIST}}'; + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(res).toStrictEqual({ + body: 'PrBody', + comments: [{ title: 'PR List', content: 'getPrList' }], + }); + }); + + it('creates body with Pr List & Package Files in comments', () => { + platform.maxBodyLength.mockReturnValue('PrBody'.length); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce( + (x) => 'PrBody{{PACKAGE FILES}}', + ); + platform.massageMarkdown.mockImplementationOnce((_) => 'PrBody'); + packageFiles = { npm: [{ packageFile: 'package.json', deps: [] }] }; + const template = 'PrBody{{PRLIST}}{{PACKAGE FILES}}'; + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(res).toStrictEqual({ + body: 'PrBody', + comments: [ + { title: 'PR List', content: 'getPrList' }, + { + title: 'Package Files', + content: '### Detected Package Files\n\n * `package.json` (npm)\n', + }, + ], + }); + }); + + it('creates & truncates body if body is too long', () => { + platform.maxBodyLength.mockReturnValue(2); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce( + (x) => 'PrBody{{PACKAGE FILES}}', + ); + platform.massageMarkdown.mockImplementationOnce((_) => 'PrBody'); + packageFiles = { npm: [{ packageFile: 'package.json', deps: [] }] }; + const template = 'PrBody{{PRLIST}}{{PACKAGE FILES}}'; + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(res.body).toBe('Pr'); + }); + + it('creates body with empty configDescription if dryRun', () => { + platform.maxBodyLength.mockReturnValueOnce(Infinity); + const template = '{{CONFIG}}\n'; + GlobalConfig.set({ dryRun: 'full' }); + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(logger.info).toHaveBeenCalledWith( + 'DRY-RUN: Would check branch renovate/configure', + ); + expect(res.body).toBe(''); + }); + + it('creates body with footer and header using templating', () => { + platform.maxBodyLength.mockReturnValue(Infinity); + const template = ''; + + const res = getPrBody( + template, + packageFiles, + { + ...config, + prFooter: + 'And this is a footer for repository:{{repository}} baseBranch:{{baseBranch}}', + prHeader: 'This is a header for platform:{{platform}}', + baseBranch: 'some-branch', + repository: 'test', + }, + branches, + '', + ); + + expect(res.body).toStartWith('This is a header for platform:github'); + + expect(res.body).toEndWith( + 'And this is a footer for repository:test baseBranch:some-branch\n', + ); + }); + }); +}); diff --git a/lib/workers/repository/onboarding/pr/body/index.ts b/lib/workers/repository/onboarding/pr/body/index.ts new file mode 100644 index 00000000000000..15f7d212eeb9eb --- /dev/null +++ b/lib/workers/repository/onboarding/pr/body/index.ts @@ -0,0 +1,146 @@ +import is from '@sindresorhus/is'; +import { GlobalConfig } from '../../../../../config/global'; +import type { RenovateConfig } from '../../../../../config/types'; +import { logger } from '../../../../../logger'; +import type { PackageFile } from '../../../../../modules/manager/types'; +import { platform } from '../../../../../modules/platform'; +import { smartTruncate } from '../../../../../modules/platform/utils/pr-body'; +import * as template from '../../../../../util/template'; +import type { BranchConfig } from '../../../../types'; + +import { + getDepWarningsOnboardingPR, + getErrors, + getWarnings, +} from '../../../errors-warnings'; +import { getBaseBranchDesc } from './base-branch'; +import { getConfigDesc } from './config-description'; +import { getPrList } from './pr-list'; + +interface PrContent { + packageFiles: string; + config: string; + warnings: string; + errors: string; + baseBranch: string; + prList: string; + prHeader: string; + prFooter: string; + onboardingConfigHashComment: string; +} + +interface PrBodyContent { + body: string; + comments: PrComment[]; +} + +interface PrComment { + title: 'PR List' | 'Package Files'; + content: string; +} + +export function getPrBody( + prTemplate: string, + packageFiles: Record | null, + config: RenovateConfig, + branches: BranchConfig[], + onboardingConfigHashComment: string, +): PrBodyContent { + let packageFilesContent = ''; + if (packageFiles && Object.entries(packageFiles).length) { + let files: string[] = []; + for (const [manager, managerFiles] of Object.entries(packageFiles)) { + files = files.concat( + managerFiles.map((file) => ` * \`${file.packageFile}\` (${manager})`), + ); + } + packageFilesContent = + '### Detected Package Files\n\n' + files.join('\n') + '\n'; + } + + let configDesc = ''; + if (GlobalConfig.get('dryRun')) { + // TODO: types (#22198) + logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); + } else { + configDesc = getConfigDesc(config, packageFiles!); + } + + let prHeader = ''; + if (is.string(config.prHeader)) { + prHeader = template.compile(config.prHeader, config); + } + let prFooter = ''; + if (is.string(config.prFooter)) { + prFooter = template.compile(config.prFooter, config); + } + + const content = { + packageFiles: packageFilesContent, + config: configDesc, + warnings: + getWarnings(config) + getDepWarningsOnboardingPR(packageFiles!, config), + errors: getErrors(config), + baseBranch: getBaseBranchDesc(config), + prList: getPrList(config, branches), + prHeader, + prFooter, + onboardingConfigHashComment, + }; + + const result: PrBodyContent = { + body: createPrBody(prTemplate, content), + comments: [], + }; + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + + if (content.prList) { + result.comments.push({ + title: 'PR List', + content: content.prList, + }); + content.prList = 'Please see comment below for what to expect'; + + result.body = createPrBody(prTemplate, content); + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + } + + if (content.packageFiles) { + result.comments.push({ + title: 'Package Files', + content: content.packageFiles, + }); + content.packageFiles = + 'Please see comment below for detected Package Files\n'; + + result.body = createPrBody(prTemplate, content); + if (result.body.length <= platform.maxBodyLength()) { + return result; + } + } + + result.body = smartTruncate(result.body, platform.maxBodyLength()); + return result; +} + +function createPrBody(template: string, content: PrContent): string { + let prBody = template.replace('{{PACKAGE FILES}}\n', content.packageFiles); + prBody = prBody.replace('{{CONFIG}}\n', content.config); + prBody = prBody.replace('{{WARNINGS}}\n', content.warnings); + prBody = prBody.replace('{{ERRORS}}\n', content.errors); + prBody = prBody.replace('{{BASEBRANCH}}\n', content.baseBranch); + prBody = prBody.replace('{{PRLIST}}\n', content.prList); + if (content.prHeader) { + prBody = `${content.prHeader}\n\n${prBody}`; + } + if (content.prFooter) { + prBody = `${prBody}\n---\n\n${content.prFooter}\n`; + } + prBody += content.onboardingConfigHashComment; + prBody = platform.massageMarkdown(prBody); + return prBody; +} diff --git a/lib/workers/repository/onboarding/pr/pr-list.spec.ts b/lib/workers/repository/onboarding/pr/body/pr-list.spec.ts similarity index 93% rename from lib/workers/repository/onboarding/pr/pr-list.spec.ts rename to lib/workers/repository/onboarding/pr/body/pr-list.spec.ts index 6e1637628b170e..676044f4de75e8 100644 --- a/lib/workers/repository/onboarding/pr/pr-list.spec.ts +++ b/lib/workers/repository/onboarding/pr/body/pr-list.spec.ts @@ -1,9 +1,9 @@ -import type { RenovateConfig } from '../../../../../test/util'; -import { partial } from '../../../../../test/util'; -import type { BranchConfig } from '../../../types'; +import type { RenovateConfig } from '../../../../../../test/util'; +import { partial } from '../../../../../../test/util'; +import type { BranchConfig } from '../../../../types'; import { getPrList } from './pr-list'; -describe('workers/repository/onboarding/pr/pr-list', () => { +describe('workers/repository/onboarding/pr/body/pr-list', () => { describe('getPrList()', () => { let config: RenovateConfig; diff --git a/lib/workers/repository/onboarding/pr/pr-list.ts b/lib/workers/repository/onboarding/pr/body/pr-list.ts similarity index 89% rename from lib/workers/repository/onboarding/pr/pr-list.ts rename to lib/workers/repository/onboarding/pr/body/pr-list.ts index 3411b0a37fca46..ba5c49ce3db7c1 100644 --- a/lib/workers/repository/onboarding/pr/pr-list.ts +++ b/lib/workers/repository/onboarding/pr/body/pr-list.ts @@ -1,8 +1,8 @@ -import type { RenovateConfig } from '../../../../config/types'; -import { logger } from '../../../../logger'; -import { emojify } from '../../../../util/emoji'; -import { regEx } from '../../../../util/regex'; -import type { BranchConfig } from '../../../types'; +import type { RenovateConfig } from '../../../../../config/types'; +import { logger } from '../../../../../logger'; +import { emojify } from '../../../../../util/emoji'; +import { regEx } from '../../../../../util/regex'; +import type { BranchConfig } from '../../../../types'; export function getPrList( config: RenovateConfig, diff --git a/lib/workers/repository/onboarding/pr/index.spec.ts b/lib/workers/repository/onboarding/pr/index.spec.ts index 53b163b4f70699..5531223bbd0c00 100644 --- a/lib/workers/repository/onboarding/pr/index.spec.ts +++ b/lib/workers/repository/onboarding/pr/index.spec.ts @@ -1,6 +1,6 @@ import type { RequestError, Response } from 'got'; import type { RenovateConfig } from '../../../../../test/util'; -import { partial, platform, scm } from '../../../../../test/util'; +import { mocked, partial, platform, scm } from '../../../../../test/util'; import { getConfig } from '../../../../config/defaults'; import { GlobalConfig } from '../../../../config/global'; import { logger } from '../../../../logger'; @@ -9,10 +9,14 @@ import type { Pr } from '../../../../modules/platform'; import * as memCache from '../../../../util/cache/memory'; import type { BranchConfig } from '../../../types'; import { OnboardingState } from '../common'; +import * as _prBody from './body'; import { ensureOnboardingPr } from '.'; jest.mock('../../../../util/git'); +jest.mock('./body'); +const prBody = mocked(_prBody); + describe('workers/repository/onboarding/pr/index', () => { describe('ensureOnboardingPr()', () => { let config: RenovateConfig; @@ -20,7 +24,7 @@ describe('workers/repository/onboarding/pr/index', () => { let branches: BranchConfig[]; const bodyStruct = { - hash: '6aa71f8cb7b1503b883485c8f5bd564b31923b9c7fa765abe2a7338af40e03b1', + hash: '230d8358dc8e8890b4c58deeb62912ee2f20357ae92a5cc861b98e68fe31acb5', }; beforeEach(() => { @@ -34,13 +38,21 @@ describe('workers/repository/onboarding/pr/index', () => { packageFiles = { npm: [{ packageFile: 'package.json', deps: [] }] }; branches = []; platform.massageMarkdown.mockImplementation((input) => input); - platform.createPr.mockResolvedValueOnce(partial()); - platform.maxBodyLength.mockImplementationOnce(() => Infinity); + platform.createPr.mockResolvedValueOnce(partial({ number: 3.14 })); + platform.maxBodyLength.mockReturnValueOnce(Infinity); + prBody.getPrBody.mockReturnValue({ + body: 'body', + comments: [ + { content: 'content', title: 'PR List' }, + { content: 'content', title: 'Package Files' }, + ], + }); GlobalConfig.reset(); }); it('returns if onboarded', async () => { config.repoIsOnboarded = true; + await expect( ensureOnboardingPr(config, packageFiles, branches), ).resolves.not.toThrow(); @@ -61,6 +73,7 @@ describe('workers/repository/onboarding/pr/index', () => { config.repoIsOnboarded = false; config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; OnboardingState.prUpdateRequested = prUpdateRequested; + await expect( ensureOnboardingPr(config, packageFiles, branches), ).resolves.not.toThrow(); @@ -71,6 +84,7 @@ describe('workers/repository/onboarding/pr/index', () => { it('creates PR', async () => { await ensureOnboardingPr(config, packageFiles, branches); + expect(platform.createPr).toHaveBeenCalledTimes(1); }); @@ -84,6 +98,7 @@ describe('workers/repository/onboarding/pr/index', () => { packageFiles, branches, ); + expect(platform.createPr).toHaveBeenCalledWith( expect.objectContaining({ prTitle: 'chore: Configure Renovate', @@ -101,6 +116,7 @@ describe('workers/repository/onboarding/pr/index', () => { packageFiles, branches, ); + expect(platform.createPr).toHaveBeenCalledTimes(1); expect(platform.createPr.mock.calls[0][0].labels).toEqual([ 'additional-label', @@ -108,112 +124,20 @@ describe('workers/repository/onboarding/pr/index', () => { ]); }); - it.each` - onboardingRebaseCheckbox - ${false} - ${true} - `( - 'creates PR with empty footer and header' + - '(onboardingRebaseCheckbox="$onboardingRebaseCheckbox")', - async ({ onboardingRebaseCheckbox }) => { - config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; - OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" - await ensureOnboardingPr( - { - ...config, - prHeader: '', - prFooter: '', - }, - packageFiles, - branches, - ); - expect(platform.createPr).toHaveBeenCalledTimes(1); - expect(platform.createPr.mock.calls[0][0].prBody).toMatchSnapshot(); - }, - ); - - it.each` - onboardingRebaseCheckbox - ${false} - ${true} - `( - 'creates PR with footer and header with trailing and leading newlines' + - '(onboardingRebaseCheckbox="$onboardingRebaseCheckbox")', - async ({ onboardingRebaseCheckbox }) => { - config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; - OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" - await ensureOnboardingPr( - { - ...config, - prHeader: '\r\r\nThis should not be the first line of the PR', - prFooter: - 'There should be several empty lines at the end of the PR\r\n\n\n', - }, - packageFiles, - branches, - ); - expect(platform.createPr).toHaveBeenCalledTimes(1); - expect(platform.createPr.mock.calls[0][0].prBody).toMatchSnapshot(); - }, - ); + it('returns if PR does not need updating', async () => { + OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" + platform.getBranchPr.mockResolvedValue( + partial({ + title: 'Configure Renovate', + bodyStruct, + }), + ); - it.each` - onboardingRebaseCheckbox - ${false} - ${true} - `( - 'creates PR with footer and header using templating' + - '(onboardingRebaseCheckbox="$onboardingRebaseCheckbox")', - async ({ onboardingRebaseCheckbox }) => { - config.baseBranch = 'some-branch'; - config.repository = 'test'; - config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; - config.onboardingConfigFileName = undefined; // checks the case when fileName isn't available - OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" - await ensureOnboardingPr( - { - ...config, - prHeader: 'This is a header for platform:{{platform}}', - prFooter: - 'And this is a footer for repository:{{repository}} baseBranch:{{baseBranch}}', - }, - packageFiles, - branches, - ); - expect(platform.createPr).toHaveBeenCalledTimes(1); - expect(platform.createPr.mock.calls[0][0].prBody).toMatch( - /platform:github/, - ); - expect(platform.createPr.mock.calls[0][0].prBody).toMatch( - /repository:test/, - ); - expect(platform.createPr.mock.calls[0][0].prBody).toMatchSnapshot(); - }, - ); + await ensureOnboardingPr(config, packageFiles, branches); - it.each` - onboardingRebaseCheckbox - ${false} - ${true} - `( - 'returns if PR does not need updating' + - '(onboardingRebaseCheckbox="$onboardingRebaseCheckbox")', - async ({ onboardingRebaseCheckbox }) => { - const hash = - '30029ee05ed80b34d2f743afda6e78fe20247a1eedaa9ce6a8070045c229ebfa'; // no rebase checkbox PR hash - config.onboardingRebaseCheckbox = onboardingRebaseCheckbox; - OnboardingState.prUpdateRequested = true; // case 'false' is tested in "breaks early when onboarding" - platform.getBranchPr.mockResolvedValue( - partial({ - title: 'Configure Renovate', - bodyStruct: onboardingRebaseCheckbox ? bodyStruct : { hash }, - }), - ); - await ensureOnboardingPr(config, packageFiles, branches); - expect(platform.createPr).toHaveBeenCalledTimes(0); - expect(platform.updatePr).toHaveBeenCalledTimes(0); - }, - ); + expect(platform.createPr).toHaveBeenCalledTimes(0); + expect(platform.updatePr).toHaveBeenCalledTimes(0); + }); it('ensures comment, when PR is conflicted', async () => { config.baseBranch = 'some-branch'; @@ -224,7 +148,9 @@ describe('workers/repository/onboarding/pr/index', () => { }), ); scm.isBranchConflicted.mockResolvedValueOnce(true); + await ensureOnboardingPr(config, {}, branches); + expect(platform.ensureComment).toHaveBeenCalledTimes(1); expect(platform.createPr).toHaveBeenCalledTimes(0); expect(platform.updatePr).toHaveBeenCalledTimes(0); @@ -238,29 +164,38 @@ describe('workers/repository/onboarding/pr/index', () => { bodyStruct, }), ); + prBody.getPrBody.mockReturnValueOnce({ + body: 'changed Body', + comments: [], + }); + await ensureOnboardingPr(config, {}, branches); + expect(platform.createPr).toHaveBeenCalledTimes(0); expect(platform.updatePr).toHaveBeenCalledTimes(1); }); it('creates PR (no require config)', async () => { config.requireConfig = 'optional'; + await ensureOnboardingPr(config, packageFiles, branches); + expect(platform.createPr).toHaveBeenCalledTimes(1); }); it('creates PR (require config)', async () => { config.requireConfig = 'required'; + await ensureOnboardingPr(config, packageFiles, branches); + expect(platform.createPr).toHaveBeenCalledTimes(1); }); it('dryrun of creates PR', async () => { GlobalConfig.set({ dryRun: 'full' }); + await ensureOnboardingPr(config, packageFiles, branches); - expect(logger.info).toHaveBeenCalledWith( - 'DRY-RUN: Would check branch renovate/configure', - ); + expect(logger.info).toHaveBeenLastCalledWith( 'DRY-RUN: Would create onboarding PR', ); @@ -274,15 +209,102 @@ describe('workers/repository/onboarding/pr/index', () => { bodyStruct, }), ); + prBody.getPrBody.mockReturnValueOnce({ + body: 'changed Body', + comments: [], + }); + await ensureOnboardingPr(config, packageFiles, branches); - expect(logger.info).toHaveBeenCalledWith( - 'DRY-RUN: Would check branch renovate/configure', - ); + expect(logger.info).toHaveBeenLastCalledWith( 'DRY-RUN: Would update onboarding PR', ); }); + it('creates comments if prBody contains comments', async () => { + prBody.getPrBody.mockReturnValueOnce({ + body: 'body', + comments: [ + { content: 'content', title: 'PR List' }, + { content: 'content', title: 'Package Files' }, + ], + }); + + await ensureOnboardingPr(config, packageFiles, branches); + + expect(platform.createPr).toHaveBeenCalledTimes(1); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'PR List', + content: 'content', + }); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Package Files', + content: 'content', + }); + expect(platform.ensureCommentRemoval).not.toHaveBeenCalled(); + }); + + it('creates comments if prBody contains comments on existing Pr', async () => { + prBody.getPrBody.mockReturnValueOnce({ + body: 'body', + comments: [ + { content: 'content', title: 'PR List' }, + { content: 'content', title: 'Package Files' }, + ], + }); + platform.getBranchPr.mockResolvedValueOnce( + partial({ + title: 'Configure Renovate', + bodyStruct, + number: 1234, + }), + ); + + await ensureOnboardingPr(config, packageFiles, branches); + + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'PR List', + content: 'content', + }); + expect(platform.ensureComment).toHaveBeenCalledWith({ + number: expect.anything(), + topic: 'Package Files', + content: 'content', + }); + expect(platform.ensureCommentRemoval).not.toHaveBeenCalled(); + }); + + it('removes comments if prBody does not contains comments', async () => { + platform.getBranchPr.mockResolvedValueOnce( + partial({ + title: 'Configure Renovate', + bodyStruct, + number: 1234, + }), + ); + prBody.getPrBody.mockReturnValue({ + body: 'body', + comments: [], + }); + + await ensureOnboardingPr(config, packageFiles, branches); + + expect(platform.ensureComment).not.toHaveBeenCalled(); + expect(platform.ensureCommentRemoval).toHaveBeenCalledWith({ + number: expect.anything(), + type: 'by-topic', + topic: 'PR List', + }); + expect(platform.ensureCommentRemoval).toHaveBeenCalledWith({ + number: expect.anything(), + type: 'by-topic', + topic: 'Package Files', + }); + }); + describe('ensureOnboardingPr() throws', () => { const response = partial({ statusCode: 422 }); const err = partial({ response }); @@ -295,6 +317,7 @@ describe('workers/repository/onboarding/pr/index', () => { it('throws when trying to create a new PR', async () => { platform.createPr.mockRejectedValueOnce(err); + await expect( ensureOnboardingPr(config, packageFiles, branches), ).toReject(); @@ -306,6 +329,7 @@ describe('workers/repository/onboarding/pr/index', () => { errors: [{ message: 'A pull request already exists' }], }; platform.createPr.mockRejectedValueOnce(err); + await expect( ensureOnboardingPr(config, packageFiles, branches), ).toResolve(); diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index db11b0f5d13e88..7a49437da7d225 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -1,4 +1,3 @@ -import is from '@sindresorhus/is'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -7,17 +6,10 @@ import { platform } from '../../../../modules/platform'; import { ensureComment } from '../../../../modules/platform/comment'; import { hashBody } from '../../../../modules/platform/pr-body'; import { scm } from '../../../../modules/platform/scm'; -import { smartTruncate } from '../../../../modules/platform/utils/pr-body'; import { emojify } from '../../../../util/emoji'; import { getFile } from '../../../../util/git'; import { toSha256 } from '../../../../util/hash'; -import * as template from '../../../../util/template'; import type { BranchConfig } from '../../../types'; -import { - getDepWarningsOnboardingPR, - getErrors, - getWarnings, -} from '../../errors-warnings'; import { getPlatformPrOptions } from '../../update/pr'; import { prepareLabels } from '../../update/pr/labels'; import { addParticipants } from '../../update/pr/participants'; @@ -27,31 +19,7 @@ import { defaultConfigFile, getSemanticCommitPrTitle, } from '../common'; -import { getBaseBranchDesc } from './base-branch'; -import { getConfigDesc } from './config-description'; -import { getPrList } from './pr-list'; - -interface PrContent { - packageFiles: string; - config: string; - warnings: string; - errors: string; - baseBranch: string; - prList: string; - prHeader: string; - prFooter: string; - onboardingConfigHashComment: string; -} - -interface PrBodyContent { - body: string; - comments: PrComment[]; -} - -interface PrComment { - title: 'PR List' | 'Package Files'; - content: string; -} +import { getPrBody } from './body'; export async function ensureOnboardingPr( config: RenovateConfig, @@ -234,113 +202,6 @@ If you need any further assistance then you can also [request help here](${ } } -function getPrBody( - prTemplate: string, - packageFiles: Record | null, - config: RenovateConfig, - branches: BranchConfig[], - onboardingConfigHashComment: string, -): PrBodyContent { - let packageFilesContent = ''; - if (packageFiles && Object.entries(packageFiles).length) { - let files: string[] = []; - for (const [manager, managerFiles] of Object.entries(packageFiles)) { - files = files.concat( - managerFiles.map((file) => ` * \`${file.packageFile}\` (${manager})`), - ); - } - packageFilesContent = - '### Detected Package Files\n\n' + files.join('\n') + '\n'; - } - - let configDesc = ''; - if (GlobalConfig.get('dryRun')) { - // TODO: types (#22198) - logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); - } else { - configDesc = getConfigDesc(config, packageFiles!); - } - - let prHeader = ''; - if (is.string(config.prHeader)) { - prHeader = template.compile(config.prHeader, config); - } - let prFooter = ''; - if (is.string(config.prFooter)) { - prFooter = template.compile(config.prFooter, config); - } - - const content = { - packageFiles: packageFilesContent, //todo \n mit ins template - config: configDesc, //todo \n mit ins template - warnings: - getWarnings(config) + getDepWarningsOnboardingPR(packageFiles!, config), - errors: getErrors(config), - baseBranch: getBaseBranchDesc(config), - prList: getPrList(config, branches), - prHeader, - prFooter, - onboardingConfigHashComment, - }; - - const result: PrBodyContent = { - body: createPrBody(prTemplate, content), - comments: [], - }; - if (result.body.length <= platform.maxBodyLength()) { - return result; - } - - if (content.prList) { - result.comments.push({ - title: 'PR List', - content: content.prList, - }); - content.prList = 'Please see comment below for what to expect'; - - result.body = createPrBody(prTemplate, content); - if (result.body.length <= platform.maxBodyLength()) { - return result; - } - } - - if (content.packageFiles) { - result.comments.push({ - title: 'Package Files', - content: content.packageFiles, - }); - content.packageFiles = - 'Please see comment below for detected Package Files\n'; - - result.body = createPrBody(prTemplate, content); - if (result.body.length <= platform.maxBodyLength()) { - return result; - } - } - - result.body = smartTruncate(result.body, platform.maxBodyLength()); - return result; -} - -function createPrBody(template: string, content: PrContent): string { - let prBody = template.replace('{{PACKAGE FILES}}\n', content.packageFiles); - prBody = prBody.replace('{{CONFIG}}\n', content.config); - prBody = prBody.replace('{{WARNINGS}}\n', content.warnings); - prBody = prBody.replace('{{ERRORS}}\n', content.errors); - prBody = prBody.replace('{{BASEBRANCH}}\n', content.baseBranch); - prBody = prBody.replace('{{PRLIST}}\n', content.prList); - //footer - if (content.prHeader) { - prBody = `${content.prHeader}\n\n${prBody}`; - } - if (content.prFooter) { - prBody = `${prBody}\n---\n\n${content.prFooter}\n`; - } - prBody += content.onboardingConfigHashComment; - prBody = platform.massageMarkdown(prBody); - return prBody; -} - function getRebaseCheckbox(onboardingRebaseCheckbox?: boolean): string { let rebaseCheckBox = ''; if (onboardingRebaseCheckbox) { From 23bede968b2e4eb41f639b780f2545b7e5e58d62 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Tue, 24 Sep 2024 16:07:07 +0200 Subject: [PATCH 18/20] Truncate comments if comment gets too long --- lib/modules/platform/azure/index.ts | 4 ++++ lib/modules/platform/bitbucket-server/index.ts | 4 ++++ lib/modules/platform/bitbucket/index.ts | 4 ++++ lib/modules/platform/codecommit/index.ts | 4 ++++ lib/modules/platform/gerrit/index.ts | 4 ++++ lib/modules/platform/gitea/index.ts | 5 +++++ lib/modules/platform/github/index.ts | 4 ++++ lib/modules/platform/gitlab/index.ts | 4 ++++ lib/modules/platform/local/index.ts | 4 ++++ lib/modules/platform/types.ts | 1 + lib/workers/repository/onboarding/pr/body/index.ts | 4 ++-- lib/workers/repository/update/pr/body/index.ts | 7 +++++-- 12 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/modules/platform/azure/index.ts b/lib/modules/platform/azure/index.ts index ac41240964f5a0..937f57a12f10e0 100644 --- a/lib/modules/platform/azure/index.ts +++ b/lib/modules/platform/azure/index.ts @@ -827,6 +827,10 @@ export function maxBodyLength(): number { return 4000; } +export function maxCommentLength(): number { + return 150000; +} + /* istanbul ignore next */ export function findIssue(): Promise { // TODO: Needs implementation (#9592) diff --git a/lib/modules/platform/bitbucket-server/index.ts b/lib/modules/platform/bitbucket-server/index.ts index 9e5f9cebbcd451..e5e3211de24232 100644 --- a/lib/modules/platform/bitbucket-server/index.ts +++ b/lib/modules/platform/bitbucket-server/index.ts @@ -1121,3 +1121,7 @@ export function massageMarkdown(input: string): string { export function maxBodyLength(): number { return 30000; } + +export function maxCommentLength(): number { + return Infinity; +} diff --git a/lib/modules/platform/bitbucket/index.ts b/lib/modules/platform/bitbucket/index.ts index b52d64c362a86f..0284f9a0660d0d 100644 --- a/lib/modules/platform/bitbucket/index.ts +++ b/lib/modules/platform/bitbucket/index.ts @@ -589,6 +589,10 @@ export function maxBodyLength(): number { return 50000; } +export function maxCommentLength(): number { + return Infinity; +} + export async function ensureIssue({ title, reuseTitle, diff --git a/lib/modules/platform/codecommit/index.ts b/lib/modules/platform/codecommit/index.ts index c10e5d16012af4..a38134c2feec9f 100644 --- a/lib/modules/platform/codecommit/index.ts +++ b/lib/modules/platform/codecommit/index.ts @@ -329,6 +329,10 @@ export function maxBodyLength(): number { return Infinity; } +export function maxCommentLength(): number { + return Infinity; +} + export async function getJsonFile( fileName: string, repoName?: string, diff --git a/lib/modules/platform/gerrit/index.ts b/lib/modules/platform/gerrit/index.ts index 18649f868b5867..fa26286bb34746 100644 --- a/lib/modules/platform/gerrit/index.ts +++ b/lib/modules/platform/gerrit/index.ts @@ -422,6 +422,10 @@ export function maxBodyLength(): number { return 16384; //TODO: check the real gerrit limit (max. chars) } +export function maxCommentLength(): number { + return Infinity; +} + export function deleteLabel(number: number, label: string): Promise { return Promise.resolve(); } diff --git a/lib/modules/platform/gitea/index.ts b/lib/modules/platform/gitea/index.ts index 2fd9fd45cf7e42..8d0e00c86faa1d 100644 --- a/lib/modules/platform/gitea/index.ts +++ b/lib/modules/platform/gitea/index.ts @@ -1001,12 +1001,17 @@ const platform: Platform = { }, maxBodyLength, + maxCommentLength, }; export function maxBodyLength(): number { return 1000000; } +export function maxCommentLength(): number { + return Infinity; +} + /* eslint-disable @typescript-eslint/unbound-method */ export const { addAssignees, diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index e7c766dfeb08f0..e1d2bd37f4bcf6 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -1967,6 +1967,10 @@ export function maxBodyLength(): number { return GitHubMaxPrBodyLen; } +export function maxCommentLength(): number { + return Infinity; +} + export async function getVulnerabilityAlerts(): Promise { if (config.hasVulnerabilityAlertsEnabled === false) { logger.debug('No vulnerability alerts enabled for repo'); diff --git a/lib/modules/platform/gitlab/index.ts b/lib/modules/platform/gitlab/index.ts index 39d1df4d76eb32..7dc082f55f5038 100644 --- a/lib/modules/platform/gitlab/index.ts +++ b/lib/modules/platform/gitlab/index.ts @@ -918,6 +918,10 @@ export function maxBodyLength(): number { } } +export function maxCommentLength(): number { + return Infinity; +} + // Branch function matchesState(state: string, desiredState: string): boolean { diff --git a/lib/modules/platform/local/index.ts b/lib/modules/platform/local/index.ts index c3646ebb4f8241..f421d034a15d2f 100644 --- a/lib/modules/platform/local/index.ts +++ b/lib/modules/platform/local/index.ts @@ -69,6 +69,10 @@ export function maxBodyLength(): number { return Infinity; } +export function maxCommentLength(): number { + return Infinity; +} + export function updatePr(): Promise { return Promise.resolve(); } diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 8a98fe9b4b2cc0..0d9d7c8591f159 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -279,6 +279,7 @@ export interface Platform { expandGroupMembers?(reviewersOrAssignees: string[]): Promise; maxBodyLength(): number; + maxCommentLength(): number; } export interface PlatformScm { diff --git a/lib/workers/repository/onboarding/pr/body/index.ts b/lib/workers/repository/onboarding/pr/body/index.ts index 15f7d212eeb9eb..91071dce1a53a7 100644 --- a/lib/workers/repository/onboarding/pr/body/index.ts +++ b/lib/workers/repository/onboarding/pr/body/index.ts @@ -99,7 +99,7 @@ export function getPrBody( if (content.prList) { result.comments.push({ title: 'PR List', - content: content.prList, + content: smartTruncate(content.prList, platform.maxCommentLength()), }); content.prList = 'Please see comment below for what to expect'; @@ -112,7 +112,7 @@ export function getPrBody( if (content.packageFiles) { result.comments.push({ title: 'Package Files', - content: content.packageFiles, + content: smartTruncate(content.packageFiles, platform.maxCommentLength()), }); content.packageFiles = 'Please see comment below for detected Package Files\n'; diff --git a/lib/workers/repository/update/pr/body/index.ts b/lib/workers/repository/update/pr/body/index.ts index 9c881f8a90cc9e..c32e1405668875 100644 --- a/lib/workers/repository/update/pr/body/index.ts +++ b/lib/workers/repository/update/pr/body/index.ts @@ -122,7 +122,7 @@ export function getPrBody( if (content.changelogs) { result.comments.push({ title: 'Release Notes', - content: content.changelogs, + content: smartTruncate(content.changelogs, platform.maxCommentLength()), }); content.changelogs = 'Please see comment below for changelogs'; @@ -133,7 +133,10 @@ export function getPrBody( } if (content.table) { - result.comments.push({ title: 'Updates', content: content.table }); + result.comments.push({ + title: 'Updates', + content: smartTruncate(content.table, platform.maxCommentLength()), + }); content.table = 'Please see comment below for updates'; result.body = createPrBody(content, branchConfig, prBodyConfig); From fa46c370fa403f2a11b3c451069837780b9535d5 Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Tue, 24 Sep 2024 18:02:36 +0200 Subject: [PATCH 19/20] Add tests for maxCommentLength --- lib/modules/platform/azure/index.spec.ts | 6 ++++++ lib/modules/platform/bitbucket-server/index.spec.ts | 6 ++++++ lib/modules/platform/bitbucket/index.spec.ts | 6 ++++++ lib/modules/platform/codecommit/index.spec.ts | 6 ++++++ lib/modules/platform/gerrit/index.spec.ts | 6 ++++++ lib/modules/platform/gitea/index.spec.ts | 6 ++++++ lib/modules/platform/github/index.spec.ts | 6 ++++++ lib/modules/platform/gitlab/index.spec.ts | 6 ++++++ lib/modules/platform/local/index.spec.ts | 4 ++++ 9 files changed, 52 insertions(+) diff --git a/lib/modules/platform/azure/index.spec.ts b/lib/modules/platform/azure/index.spec.ts index 51abef384e63c2..f96de6bc1da6e2 100644 --- a/lib/modules/platform/azure/index.spec.ts +++ b/lib/modules/platform/azure/index.spec.ts @@ -1991,4 +1991,10 @@ describe('modules/platform/azure/index', () => { expect(azure.maxBodyLength()).toBe(4000); }); }); + + describe('maxCommentLength()', () => { + it('returns 150000', () => { + expect(azure.maxCommentLength()).toBe(150000); + }); + }); }); diff --git a/lib/modules/platform/bitbucket-server/index.spec.ts b/lib/modules/platform/bitbucket-server/index.spec.ts index 1a5bc15e9dcf95..f8c954c600fc6c 100644 --- a/lib/modules/platform/bitbucket-server/index.spec.ts +++ b/lib/modules/platform/bitbucket-server/index.spec.ts @@ -2476,6 +2476,12 @@ Followed by some information. expect(bitbucket.maxBodyLength()).toBe(30000); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(bitbucket.maxCommentLength()).toBe(Infinity); + }); + }); }); }); }); diff --git a/lib/modules/platform/bitbucket/index.spec.ts b/lib/modules/platform/bitbucket/index.spec.ts index 18b0cb76cff28d..c253e11c174932 100644 --- a/lib/modules/platform/bitbucket/index.spec.ts +++ b/lib/modules/platform/bitbucket/index.spec.ts @@ -1902,4 +1902,10 @@ describe('modules/platform/bitbucket/index', () => { expect(bitbucket.maxBodyLength()).toBe(50000); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(bitbucket.maxCommentLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/codecommit/index.spec.ts b/lib/modules/platform/codecommit/index.spec.ts index 758a55c380ef54..0645b0f46c7e42 100644 --- a/lib/modules/platform/codecommit/index.spec.ts +++ b/lib/modules/platform/codecommit/index.spec.ts @@ -1333,4 +1333,10 @@ describe('modules/platform/codecommit/index', () => { expect(codeCommit.maxBodyLength()).toBe(Infinity); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(codeCommit.maxCommentLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/gerrit/index.spec.ts b/lib/modules/platform/gerrit/index.spec.ts index 7b0cc2262a8e45..8be8ea682cd20b 100644 --- a/lib/modules/platform/gerrit/index.spec.ts +++ b/lib/modules/platform/gerrit/index.spec.ts @@ -798,4 +798,10 @@ describe('modules/platform/gerrit/index', () => { expect(gerrit.maxBodyLength()).toBe(16384); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(gerrit.maxCommentLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/gitea/index.spec.ts b/lib/modules/platform/gitea/index.spec.ts index ed3979070b4848..4cef3dfe68dcbb 100644 --- a/lib/modules/platform/gitea/index.spec.ts +++ b/lib/modules/platform/gitea/index.spec.ts @@ -2995,4 +2995,10 @@ describe('modules/platform/gitea/index', () => { expect(gitea.maxBodyLength()).toBe(1000000); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(gitea.maxCommentLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index 73988451a3bd92..311f752ee74881 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -4209,4 +4209,10 @@ describe('modules/platform/github/index', () => { expect(github.maxBodyLength()).toBe(60000); }); }); + + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(github.maxCommentLength()).toBe(Infinity); + }); + }); }); diff --git a/lib/modules/platform/gitlab/index.spec.ts b/lib/modules/platform/gitlab/index.spec.ts index b642c439806de9..74b51fd8f0f3ae 100644 --- a/lib/modules/platform/gitlab/index.spec.ts +++ b/lib/modules/platform/gitlab/index.spec.ts @@ -3082,6 +3082,12 @@ These updates have all been created already. Click a checkbox below to force a r }); }); + describe('maxCommentLength()', () => { + it('returns Infinity', () => { + expect(gitlab.maxCommentLength()).toBe(Infinity); + }); + }); + describe('deleteLabel(issueNo, label)', () => { it('should delete the label', async () => { httpMock diff --git a/lib/modules/platform/local/index.spec.ts b/lib/modules/platform/local/index.spec.ts index 140f38ec6f509d..edb38d57608fb4 100644 --- a/lib/modules/platform/local/index.spec.ts +++ b/lib/modules/platform/local/index.spec.ts @@ -69,6 +69,10 @@ describe('modules/platform/local/index', () => { expect(platform.maxBodyLength()).toBe(Infinity); }); + it('maxCommentLength', () => { + expect(platform.maxCommentLength()).toBe(Infinity); + }); + it('updatePr', async () => { expect(await platform.updatePr()).toBeUndefined(); }); From 8cf0284c8b71030796e380bd1ec594c254f5393d Mon Sep 17 00:00:00 2001 From: Timo Walter Date: Tue, 24 Sep 2024 22:43:16 +0200 Subject: [PATCH 20/20] add tests for comment length --- .../onboarding/pr/body/index.spec.ts | 25 +++++++++++++ .../repository/update/pr/body/index.spec.ts | 37 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/lib/workers/repository/onboarding/pr/body/index.spec.ts b/lib/workers/repository/onboarding/pr/body/index.spec.ts index 7eafdd47ac314c..8637c6d0b7ad46 100644 --- a/lib/workers/repository/onboarding/pr/body/index.spec.ts +++ b/lib/workers/repository/onboarding/pr/body/index.spec.ts @@ -175,5 +175,30 @@ describe('workers/repository/onboarding/pr/body/index', () => { 'And this is a footer for repository:test baseBranch:some-branch\n', ); }); + + it('creates & truncates comments if comment is too long', () => { + platform.maxBodyLength.mockReturnValue('PrBody'.length); + platform.maxCommentLength.mockReturnValue(3); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce( + (x) => 'PrBody{{PACKAGE FILES}}', + ); + platform.massageMarkdown.mockImplementationOnce((_) => 'PrBody'); + packageFiles = { npm: [{ packageFile: 'package.json', deps: [] }] }; + const template = 'PrBody{{PRLIST}}{{PACKAGE FILES}}'; + + const res = getPrBody(template, packageFiles, config, branches, ''); + + expect(res).toStrictEqual({ + body: 'PrBody', + comments: [ + { title: 'PR List', content: 'get' }, + { + title: 'Package Files', + content: '###', + }, + ], + }); + }); }); }); diff --git a/lib/workers/repository/update/pr/body/index.spec.ts b/lib/workers/repository/update/pr/body/index.spec.ts index c4b633a28be250..318829586b96e7 100644 --- a/lib/workers/repository/update/pr/body/index.spec.ts +++ b/lib/workers/repository/update/pr/body/index.spec.ts @@ -355,5 +355,42 @@ describe('workers/repository/update/pr/body/index', () => { ], }); }); + + it('returns & truncates comments if comment is too long', () => { + platform.maxBodyLength.mockReturnValue('{{{header}}}'.length); + platform.maxCommentLength.mockReturnValue(3); + platform.massageMarkdown.mockImplementationOnce((x) => x); + platform.massageMarkdown.mockImplementationOnce( + (_) => '{{{header}}}{{{table}}}', + ); + platform.massageMarkdown.mockImplementationOnce((_) => '{{{header}}}'); + template.compile.mockImplementation((x) => x); + + const res = getPrBody( + { + manager: 'some-manager', + baseBranch: 'base', + branchName: 'some-branch', + upgrades: [], + prBodyTemplate: '{{{header}}}{{{table}}}{{{changelogs}}}', + }, + { + debugData: { + updatedInVer: '1.2.3', + createdInVer: '1.2.3', + targetBranch: 'base', + }, + }, + {}, + ); + + expect(res).toStrictEqual({ + body: '{{{header}}}', + comments: [ + { content: 'get', title: 'Release Notes' }, + { content: 'get', title: 'Updates' }, + ], + }); + }); }); });