Skip to content

Commit

Permalink
fix: Fix target parsing and emitting for publish issues (#5026)
Browse files Browse the repository at this point in the history
* pain

* no formatter changes

* even less formatter changes
  • Loading branch information
lforst authored Feb 10, 2025
1 parent 6de13a1 commit 82c14c6
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 51 deletions.
23 changes: 11 additions & 12 deletions src/modules/__tests__/details-from-context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-env jest */

const detailsFromContext = require("../details-from-context.js");
const { detailsFromContext } = require("../details-from-context.js");

const inputsArgs = {
context: {
Expand All @@ -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"],
Expand All @@ -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 () => {
Expand Down
55 changes: 54 additions & 1 deletion src/modules/__tests__/update-issue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down Expand Up @@ -120,6 +120,7 @@ describe.each([false, true])("state file exists: %s", (stateFileExists) => {
- [ ] docker[latest]
- [x] lol
- [ ] hey
",
"issue_number": "211",
"owner": "getsentry",
Expand Down Expand Up @@ -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`);
});
});
37 changes: 29 additions & 8 deletions src/modules/details-from-context.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
};
62 changes: 33 additions & 29 deletions src/modules/update-issue.js
Original file line number Diff line number Diff line change
@@ -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`;
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -69,6 +73,6 @@ async function updateIssue({ context, octokit, inputs }) {
name: "accepted",
}),
]);
};
}

module.exports = updateIssue;
module.exports = { updateIssue, transformIssueBody };
2 changes: 1 addition & 1 deletion src/publish/inputs.js
Original file line number Diff line number Diff line change
@@ -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({
Expand Down

0 comments on commit 82c14c6

Please sign in to comment.