From 82c14c6742035edb4543435bfaf092a02afeeb4e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Feb 2025 15:44:17 +0100 Subject: [PATCH] fix: Fix target parsing and emitting for publish issues (#5026) * pain * no formatter changes * even less formatter changes --- src/modules/__tests__/details-from-context.js | 23 ++++--- src/modules/__tests__/update-issue.js | 55 +++++++++++++++- src/modules/details-from-context.js | 37 ++++++++--- src/modules/update-issue.js | 62 ++++++++++--------- src/publish/inputs.js | 2 +- 5 files changed, 128 insertions(+), 51 deletions(-) diff --git a/src/modules/__tests__/details-from-context.js b/src/modules/__tests__/details-from-context.js index 31c0644..7008bc7 100644 --- a/src/modules/__tests__/details-from-context.js +++ b/src/modules/__tests__/details-from-context.js @@ -1,6 +1,6 @@ /* eslint-env jest */ -const detailsFromContext = require("../details-from-context.js"); +const { detailsFromContext } = require("../details-from-context.js"); const inputsArgs = { context: { @@ -24,6 +24,8 @@ Assign the **accepted** label to this issue to approve the release. - [x] github\r - [ ] pypi\r - [ ] docker[release] + - [ ] npm[@sentry/opentelemetry] + - [x] npm[@sentry/node] - [x] docker[latest]\r `, labels: ["accepted"], @@ -35,17 +37,14 @@ Assign the **accepted** label to this issue to approve the release. test("parse inputs", async () => { const result = await detailsFromContext(inputsArgs); expect(result).toEqual({ - dry_run: "", - merge_target: "custom-branch", - path: ".", - repo: "sentry", - requester: "BYK", - targets: [ - "github", - "docker[latest]", - ], - version: "21.3.1", - }); + dry_run: "", + merge_target: "custom-branch", + path: ".", + repo: "sentry", + requester: "BYK", + targets: ["github", "npm[@sentry/node]", "docker[latest]"], + version: "21.3.1", + }); }); test("can parse version containing +", async () => { diff --git a/src/modules/__tests__/update-issue.js b/src/modules/__tests__/update-issue.js index 0e9f53b..8e22d70 100644 --- a/src/modules/__tests__/update-issue.js +++ b/src/modules/__tests__/update-issue.js @@ -3,7 +3,7 @@ jest.mock("fs"); const fs = require("fs"); -const updateIssue = require("../update-issue.js"); +const { updateIssue, transformIssueBody } = require("../update-issue.js"); const updateTargetsArgs = { inputs: { repo: "sentry", version: "21.3.1" }, @@ -120,6 +120,7 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { - [ ] docker[latest] - [x] lol - [ ] hey + ", "issue_number": "211", "owner": "getsentry", @@ -147,3 +148,55 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { }); }); }); + +describe("transformIssueBody", () => { + it("should correctly transform an issue body", () => { + const result = transformIssueBody( + { + published: { + "npm[@sentry/node]": true, + "aws-lambda": true, + github: false, + foo: true, + }, + }, + `Requested by: @lforst + +Merge target: (default) + +Quick links: +- [View changes](https://github.com/getsentry/sentry-elixir/compare/2f5876adf89822cc75199576966df4fd587f68e9...refs/heads/release/10.7.2) +- [View check runs](https://github.com/getsentry/sentry-elixir/commit/fcbc69b88481a95532d10a6162a243107fabb96a/checks/) +Assign the **accepted** label to this issue to approve the release. +Leave a comment containing \`#retract\` under this issue to retract the release (original issuer only). + +### Targets + + - [ ] npm[@sentry/node] + - [x] aws-lambda + - [x] github + + +Targets marked with a checkbox have already been executed. Administrators can manually tick a checkbox to force craft to skip it.\n` + ); + + expect(result).toBe(`Requested by: @lforst + +Merge target: (default) + +Quick links: +- [View changes](https://github.com/getsentry/sentry-elixir/compare/2f5876adf89822cc75199576966df4fd587f68e9...refs/heads/release/10.7.2) +- [View check runs](https://github.com/getsentry/sentry-elixir/commit/fcbc69b88481a95532d10a6162a243107fabb96a/checks/) +Assign the **accepted** label to this issue to approve the release. +Leave a comment containing \`#retract\` under this issue to retract the release (original issuer only). + +### Targets + +- [x] npm[@sentry/node] +- [x] aws-lambda +- [ ] github +- [x] foo + +Targets marked with a checkbox have already been executed. Administrators can manually tick a checkbox to force craft to skip it.\n`); + }); +}); diff --git a/src/modules/details-from-context.js b/src/modules/details-from-context.js index bf6b7f1..9db9651 100644 --- a/src/modules/details-from-context.js +++ b/src/modules/details-from-context.js @@ -1,3 +1,21 @@ +/** + * Matches the entire "Targets" section of a github publish issue body. + */ +const TARGETS_SECTION_PARSER_REGEX = /^(?!### Targets$\s)(?: *- \[[ x]\] \S+\s*$(?:\r?\n)?)+/m; + +/** + * Matches all targets of a github publish issue body in a section that was already matched and extracted with `TARGETS_PARSER_REGEX`. + * The "id" of the targets is captured within a capture group. + */ +const TARGETS_PARSER_REGEX = /^\s*- \[[ x]\] (\S+)/gim; + +/** + * Matches checked targets of a github publish issue body in a section that was already matched and extracted with `TARGETS_PARSER_REGEX`. + * The "id" of the targets is captured within a capture group. + */ +const CHECKED_TARGETS_PARSER_REGEX = /^\s*- \[x\] (\S+)/gim; + + async function detailsFromContext({ context }) { if (!context || !context.payload || !context.payload.issue) { throw new Error('Issue context is not defined'); @@ -27,14 +45,12 @@ async function detailsFromContext({ context }) { merge_target = mergeTargetMatch.groups.merge_target || ""; } - const targetsParser = /^(?!### Targets$\s)(?: *- \[[ x]\] [\w.[\]-]+$(?:\r?\n)?)+/m; - const targetsMatch = context.payload.issue.body.match(targetsParser); + const targetsMatch = context.payload.issue.body.match(TARGETS_SECTION_PARSER_REGEX); let targets; if (targetsMatch) { - const targetMatcher = /^ *- \[x\] ([\w.[\]-]+)$(?:\r?\n)?/gim; - targets = Array.from(targetsMatch[0].matchAll(targetMatcher)).map( - (x) => x[1] - ); + targets = Array.from( + targetsMatch[0].matchAll(CHECKED_TARGETS_PARSER_REGEX) + ).map((x) => x[1]); } return { @@ -45,6 +61,11 @@ async function detailsFromContext({ context }) { requester, targets, }; -}; +} -module.exports = detailsFromContext; +module.exports = { + detailsFromContext, + TARGETS_SECTION_PARSER_REGEX, + TARGETS_PARSER_REGEX, + CHECKED_TARGETS_PARSER_REGEX, +}; diff --git a/src/modules/update-issue.js b/src/modules/update-issue.js index 36c8400..896ded9 100644 --- a/src/modules/update-issue.js +++ b/src/modules/update-issue.js @@ -1,4 +1,8 @@ const fs = require("fs"); +const { + TARGETS_SECTION_PARSER_REGEX, + TARGETS_PARSER_REGEX, +} = require("./details-from-context"); async function updateTargets({octokit, version, publishRepo, issue_number}) { const CRAFT_STATE_FILE_PATH = `${process.env.GITHUB_WORKSPACE}/__repo__/.craft-publish-${version}.json`; @@ -21,33 +25,7 @@ async function updateTargets({octokit, version, publishRepo, issue_number}) { craftStateRequest, ]); - const targetsParser = /^(?!### Targets$\s)^(?: *- \[[ x]\] [\w.[\]-]+[ ]*$(?:\r?\n)?)+/m; - const declaredTargets = new Set(); - let leadingSpaces = " "; - const newIssueBody = issue.body.replace(targetsParser, (targetsSection) => { - let targetsText = targetsSection.trimRight(); - const targetMatcher = /^( *)- \[[ x]\] ([\w.[\]-]+)$/gim; - targetsText = targetsText.replace( - targetMatcher, - (_match, spaces, target) => { - leadingSpaces = spaces; - declaredTargets.add(target); - const x = craftState.published[target] ? "x" : " "; - return `${spaces}- [${x}] ${target}`; - } - ); - const unlistedTargets = Object.keys(craftState.published) - .filter((target) => !declaredTargets.has(target)) - .map( - (target) => - `${leadingSpaces}- [${ - craftState.published[target] ? "x" : " " - }] ${target}` - ) - .join("\n"); - targetsText += `\n${unlistedTargets}\n`; - return targetsText; - }); + const newIssueBody = transformIssueBody(craftState, issue.body); await octokit.rest.issues.update({ ...publishRepo, @@ -56,6 +34,32 @@ async function updateTargets({octokit, version, publishRepo, issue_number}) { }); } +function transformIssueBody(craftState, issueBody) { + const declaredTargets = new Set(); + return issueBody.replace( + TARGETS_SECTION_PARSER_REGEX, + (targetsSection) => { + let targetsText = targetsSection.trimRight(); + targetsText = targetsText.replace( + TARGETS_PARSER_REGEX, + (_match, targetId) => { + declaredTargets.add(targetId); + const x = craftState.published[targetId] ? "x" : " "; + return `- [${x}] ${targetId}`; + } + ); + const unlistedTargets = Object.keys(craftState.published) + .filter((target) => !declaredTargets.has(target)) + .map( + (target) => + `- [${craftState.published[target] ? "x" : " "}] ${target}` + ) + .join("\n") + '\n'; + targetsText += `\n${unlistedTargets}\n`; + return targetsText; + }); +} + async function updateIssue({ context, octokit, inputs }) { const { version } = inputs; const { repo: publishRepo } = context; @@ -69,6 +73,6 @@ async function updateIssue({ context, octokit, inputs }) { name: "accepted", }), ]); -}; +} -module.exports = updateIssue; \ No newline at end of file +module.exports = { updateIssue, transformIssueBody }; diff --git a/src/publish/inputs.js b/src/publish/inputs.js index 4806c00..0640d9e 100644 --- a/src/publish/inputs.js +++ b/src/publish/inputs.js @@ -1,6 +1,6 @@ const github = require('@actions/github'); const core = require('@actions/core'); -const detailsFromContext = require('../modules/details-from-context'); +const { detailsFromContext } = require('../modules/details-from-context'); async function inputs() { const result = await detailsFromContext({