From 7713e34ac094d1f18b665410aa214ff36a35f3f6 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Feb 2025 14:31:15 +0100 Subject: [PATCH 1/3] pain --- src/modules/__tests__/details-from-context.js | 72 +++++++++---------- src/modules/__tests__/update-issue.js | 65 +++++++++++++++-- src/modules/details-from-context.js | 37 +++++++--- src/modules/update-issue.js | 62 ++++++++-------- src/publish/inputs.js | 17 +++-- src/publish/update-issue.js | 6 +- 6 files changed, 167 insertions(+), 92 deletions(-) diff --git a/src/modules/__tests__/details-from-context.js b/src/modules/__tests__/details-from-context.js index 31c0644..7be1889 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,34 +37,31 @@ 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 () => { - const result = await detailsFromContext({ - context: { - repo: { owner: "getsentry", repo: "publish" }, - payload: { - issue: { - number: "123", - title: "publish: getsentry/sentry-forked-django-stubs@4.2.6+sentry1", - body: "Requested by: @example", - labels: [], - } - } - } - }); - expect(result.version).toEqual('4.2.6+sentry1'); + const result = await detailsFromContext({ + context: { + repo: { owner: "getsentry", repo: "publish" }, + payload: { + issue: { + number: "123", + title: "publish: getsentry/sentry-forked-django-stubs@4.2.6+sentry1", + body: "Requested by: @example", + labels: [], + }, + }, + }, + }); + expect(result.version).toEqual("4.2.6+sentry1"); }); const defaultTargetInputsArgs = { @@ -94,20 +93,17 @@ Assign the **accepted** label to this issue to approve the release. test("Do not extract merge_target value if its a default value", async () => { const result = await detailsFromContext(defaultTargetInputsArgs); expect(result).toEqual({ - dry_run: "", - merge_target: "", - path: ".", - repo: "sentry", - requester: "BYK", - targets: [ - "github", - "docker[latest]", - ], - version: "21.3.1", - }); + dry_run: "", + merge_target: "", + path: ".", + repo: "sentry", + requester: "BYK", + targets: ["github", "docker[latest]"], + version: "21.3.1", + }); }); test("throw error when context is missing the issue payload", async () => { const fn = () => detailsFromContext({ context: {} }); - expect(fn).rejects.toThrow('Issue context is not defined'); + expect(fn).rejects.toThrow("Issue context is not defined"); }); diff --git a/src/modules/__tests__/update-issue.js b/src/modules/__tests__/update-issue.js index 0e9f53b..1f0f6cb 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" }, @@ -83,7 +83,9 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { if (stateFileExists) { test("restore publish state", async () => { - expect(updateTargetsArgs.octokit.rest.issues.get).toHaveBeenCalledTimes(1); + expect(updateTargetsArgs.octokit.rest.issues.get).toHaveBeenCalledTimes( + 1 + ); expect(updateTargetsArgs.octokit.rest.issues.get.mock.calls[0]) .toMatchInlineSnapshot(` Array [ @@ -95,9 +97,9 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { ] `); - expect(updateTargetsArgs.octokit.rest.issues.update).toHaveBeenCalledTimes( - 1 - ); + expect( + updateTargetsArgs.octokit.rest.issues.update + ).toHaveBeenCalledTimes(1); expect(updateTargetsArgs.octokit.rest.issues.update.mock.calls[0]) .toMatchInlineSnapshot(` Array [ @@ -120,6 +122,7 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { - [ ] docker[latest] - [x] lol - [ ] hey + ", "issue_number": "211", "owner": "getsentry", @@ -147,3 +150,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..354b917 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 target 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 target 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..12f141e 100644 --- a/src/publish/inputs.js +++ b/src/publish/inputs.js @@ -1,13 +1,12 @@ -const github = require('@actions/github'); -const core = require('@actions/core'); -const detailsFromContext = require('../modules/details-from-context'); +const github = require("@actions/github"); +const core = require("@actions/core"); +const { detailsFromContext } = require("../modules/details-from-context"); async function inputs() { - const result = await detailsFromContext({ - context: github.context, - }); - core.setOutput('result', result); + const result = await detailsFromContext({ + context: github.context, + }); + core.setOutput("result", result); } - -inputs(); \ No newline at end of file +inputs(); diff --git a/src/publish/update-issue.js b/src/publish/update-issue.js index d9b9f8b..84737f5 100644 --- a/src/publish/update-issue.js +++ b/src/publish/update-issue.js @@ -1,6 +1,6 @@ -const updateIssue = require('../modules/update-issue.js'); -const {getGitHubToken} = require('../libs/github'); -const github = require('@actions/github'); +const { updateIssue } = require("../modules/update-issue.js"); +const { getGitHubToken } = require("../libs/github"); +const github = require("@actions/github"); const context = github.context; const octokit = github.getOctokit(getGitHubToken()); From 55800d66fc674599dd53003c27d50a7f20754a5e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Feb 2025 15:30:14 +0100 Subject: [PATCH 2/3] no formatter changes --- src/modules/__tests__/details-from-context.js | 15 +++++++++------ src/modules/__tests__/update-issue.js | 10 ++++------ src/modules/details-from-context.js | 4 ++-- src/publish/inputs.js | 9 +++++---- src/publish/update-issue.js | 6 +++--- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/modules/__tests__/details-from-context.js b/src/modules/__tests__/details-from-context.js index 7be1889..e4e17c1 100644 --- a/src/modules/__tests__/details-from-context.js +++ b/src/modules/__tests__/details-from-context.js @@ -57,11 +57,11 @@ test("can parse version containing +", async () => { title: "publish: getsentry/sentry-forked-django-stubs@4.2.6+sentry1", body: "Requested by: @example", labels: [], - }, - }, - }, + } + } + } }); - expect(result.version).toEqual("4.2.6+sentry1"); + expect(result.version).toEqual('4.2.6+sentry1'); }); const defaultTargetInputsArgs = { @@ -98,12 +98,15 @@ test("Do not extract merge_target value if its a default value", async () => { path: ".", repo: "sentry", requester: "BYK", - targets: ["github", "docker[latest]"], + targets: [ + "github", + "docker[latest]", + ], version: "21.3.1", }); }); test("throw error when context is missing the issue payload", async () => { const fn = () => detailsFromContext({ context: {} }); - expect(fn).rejects.toThrow("Issue context is not defined"); + expect(fn).rejects.toThrow('Issue context is not defined'); }); diff --git a/src/modules/__tests__/update-issue.js b/src/modules/__tests__/update-issue.js index 1f0f6cb..8e22d70 100644 --- a/src/modules/__tests__/update-issue.js +++ b/src/modules/__tests__/update-issue.js @@ -83,9 +83,7 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { if (stateFileExists) { test("restore publish state", async () => { - expect(updateTargetsArgs.octokit.rest.issues.get).toHaveBeenCalledTimes( - 1 - ); + expect(updateTargetsArgs.octokit.rest.issues.get).toHaveBeenCalledTimes(1); expect(updateTargetsArgs.octokit.rest.issues.get.mock.calls[0]) .toMatchInlineSnapshot(` Array [ @@ -97,9 +95,9 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => { ] `); - expect( - updateTargetsArgs.octokit.rest.issues.update - ).toHaveBeenCalledTimes(1); + expect(updateTargetsArgs.octokit.rest.issues.update).toHaveBeenCalledTimes( + 1 + ); expect(updateTargetsArgs.octokit.rest.issues.update.mock.calls[0]) .toMatchInlineSnapshot(` Array [ diff --git a/src/modules/details-from-context.js b/src/modules/details-from-context.js index 354b917..9db9651 100644 --- a/src/modules/details-from-context.js +++ b/src/modules/details-from-context.js @@ -5,13 +5,13 @@ const TARGETS_SECTION_PARSER_REGEX = /^(?!### Targets$\s)(?: *- \[[ x]\] \S+\s*$ /** * 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 target is captured within a capture group. + * 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 target is captured within a capture group. + * The "id" of the targets is captured within a capture group. */ const CHECKED_TARGETS_PARSER_REGEX = /^\s*- \[x\] (\S+)/gim; diff --git a/src/publish/inputs.js b/src/publish/inputs.js index 12f141e..79c0802 100644 --- a/src/publish/inputs.js +++ b/src/publish/inputs.js @@ -1,12 +1,13 @@ -const github = require("@actions/github"); -const core = require("@actions/core"); -const { detailsFromContext } = require("../modules/details-from-context"); +const github = require('@actions/github'); +const core = require('@actions/core'); +const { detailsFromContext } = require('../modules/details-from-context'); async function inputs() { const result = await detailsFromContext({ context: github.context, }); - core.setOutput("result", result); + core.setOutput('result', result); } + inputs(); diff --git a/src/publish/update-issue.js b/src/publish/update-issue.js index 84737f5..d9b9f8b 100644 --- a/src/publish/update-issue.js +++ b/src/publish/update-issue.js @@ -1,6 +1,6 @@ -const { updateIssue } = require("../modules/update-issue.js"); -const { getGitHubToken } = require("../libs/github"); -const github = require("@actions/github"); +const updateIssue = require('../modules/update-issue.js'); +const {getGitHubToken} = require('../libs/github'); +const github = require('@actions/github'); const context = github.context; const octokit = github.getOctokit(getGitHubToken()); From 3d5af9eb9244565d2604fd3257de9ca775607d9c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 10 Feb 2025 15:32:20 +0100 Subject: [PATCH 3/3] even less formatter changes --- src/modules/__tests__/details-from-context.js | 34 +++++++++---------- src/publish/inputs.js | 8 ++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/modules/__tests__/details-from-context.js b/src/modules/__tests__/details-from-context.js index e4e17c1..7008bc7 100644 --- a/src/modules/__tests__/details-from-context.js +++ b/src/modules/__tests__/details-from-context.js @@ -48,19 +48,19 @@ test("parse inputs", async () => { }); test("can parse version containing +", async () => { - const result = await detailsFromContext({ - context: { - repo: { owner: "getsentry", repo: "publish" }, - payload: { - issue: { - number: "123", - title: "publish: getsentry/sentry-forked-django-stubs@4.2.6+sentry1", - body: "Requested by: @example", - labels: [], + const result = await detailsFromContext({ + context: { + repo: { owner: "getsentry", repo: "publish" }, + payload: { + issue: { + number: "123", + title: "publish: getsentry/sentry-forked-django-stubs@4.2.6+sentry1", + body: "Requested by: @example", + labels: [], } } } - }); + }); expect(result.version).toEqual('4.2.6+sentry1'); }); @@ -93,17 +93,17 @@ Assign the **accepted** label to this issue to approve the release. test("Do not extract merge_target value if its a default value", async () => { const result = await detailsFromContext(defaultTargetInputsArgs); expect(result).toEqual({ - dry_run: "", - merge_target: "", - path: ".", - repo: "sentry", - requester: "BYK", + dry_run: "", + merge_target: "", + path: ".", + repo: "sentry", + requester: "BYK", targets: [ "github", "docker[latest]", ], - version: "21.3.1", - }); + version: "21.3.1", + }); }); test("throw error when context is missing the issue payload", async () => { diff --git a/src/publish/inputs.js b/src/publish/inputs.js index 79c0802..0640d9e 100644 --- a/src/publish/inputs.js +++ b/src/publish/inputs.js @@ -3,11 +3,11 @@ const core = require('@actions/core'); const { detailsFromContext } = require('../modules/details-from-context'); async function inputs() { - const result = await detailsFromContext({ - context: github.context, - }); + const result = await detailsFromContext({ + context: github.context, + }); core.setOutput('result', result); } -inputs(); +inputs(); \ No newline at end of file