From 8cb771a2176b38d869b83846366fe21cf584aba8 Mon Sep 17 00:00:00 2001 From: Christian Marangi Date: Fri, 26 Apr 2024 18:50:31 +0200 Subject: [PATCH 1/5] feat: improve body parsing logic Current body parsing logic with trim() + split("###") is too fragile and pose problems with some body that contains case with ### in the middle of the line or case with codeblock ``` section. These case will cause the script to parse these as separate section and produce wrong outputs and in some case even prints error assuming things are checkbox and errors out on the concat function. To make the parsing logic more solid, implement a dedicated function and parse with this logic: - We split the body for "\n" - We ignore codeblock ``` section - We check "###" only at the start of the line - We check for "### " (with the space included) as that is the correct section Github issue template expects. With the following change case like: ``` root@OpenWrt:~# cat /boot/config.txt ``` Are correctly parsed as all part of a single section instead of being wrongly treated as different empty sections. Signed-off-by: Christian Marangi --- index.js | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index cab1bce..0ee6160 100644 --- a/index.js +++ b/index.js @@ -111,9 +111,35 @@ export async function run(env, body, fs, core) { return result } - result = body - .trim() - .split("###") + function parseBody(body) { + let result = []; + let ignore = false; + + body.split("\n").reduce((str, line, idx, arr) => { + // ``` Section must be ignored and not parsed as new section + if (line.startsWith("```")) + ignore = !ignore + + // Parse new setion only if they start with ### SPACE + if (!ignore && line.startsWith("### ")) { + result.push(str.trim()) + + return line.replace(/^### /, "")+"\n"; + } + + str += line + "\n" + + // Push the last string if we are at the end of lines + if (arr.length - 1 == idx) + result.push(str.trim()) + + return str; + }, "") + + return result; + } + + result = parseBody(body) .filter(Boolean) .map((line) => { return line From 48ade791d2c2fd7ed7b102f8c3069da00d5099fb Mon Sep 17 00:00:00 2001 From: Christian Marangi Date: Fri, 26 Apr 2024 18:57:19 +0200 Subject: [PATCH 2/5] feat: add additional test for section with #### Add additional test for section with #### to prevent and catch in future code change that would produce wrong parsing. Signed-off-by: Christian Marangi --- .../paragraph-confusing-####/expected.json | 3 ++ fixtures/paragraph-confusing-####/form.yml | 9 +++++ .../paragraph-confusing-####/issue-body.md | 3 ++ fixtures/paragraph-confusing-####/issue.js | 6 +++ test.spec.js | 39 +++++++++++++++++++ 5 files changed, 60 insertions(+) create mode 100644 fixtures/paragraph-confusing-####/expected.json create mode 100644 fixtures/paragraph-confusing-####/form.yml create mode 100644 fixtures/paragraph-confusing-####/issue-body.md create mode 100644 fixtures/paragraph-confusing-####/issue.js diff --git a/fixtures/paragraph-confusing-####/expected.json b/fixtures/paragraph-confusing-####/expected.json new file mode 100644 index 0000000..34954d8 --- /dev/null +++ b/fixtures/paragraph-confusing-####/expected.json @@ -0,0 +1,3 @@ +{ + "textarea-one": "Textarea input text 1 ####" +} diff --git a/fixtures/paragraph-confusing-####/form.yml b/fixtures/paragraph-confusing-####/form.yml new file mode 100644 index 0000000..cb5bf9f --- /dev/null +++ b/fixtures/paragraph-confusing-####/form.yml @@ -0,0 +1,9 @@ +body: + - type: textarea + id: textarea-one + attributes: + label: My textarea input + - type: textarea + id: textarea-two + attributes: + label: Another textarea input diff --git a/fixtures/paragraph-confusing-####/issue-body.md b/fixtures/paragraph-confusing-####/issue-body.md new file mode 100644 index 0000000..38a4b14 --- /dev/null +++ b/fixtures/paragraph-confusing-####/issue-body.md @@ -0,0 +1,3 @@ +### My textarea input + +Textarea input text 1 #### diff --git a/fixtures/paragraph-confusing-####/issue.js b/fixtures/paragraph-confusing-####/issue.js new file mode 100644 index 0000000..6851c74 --- /dev/null +++ b/fixtures/paragraph-confusing-####/issue.js @@ -0,0 +1,6 @@ +const { resolve } = require("path"); +const { readFileSync } = require("fs"); + +const issueBodyPath = resolve(__dirname, "issue-body.md"); + +module.exports = readFileSync(issueBodyPath, "utf-8") diff --git a/test.spec.js b/test.spec.js index a7d380d..5891324 100644 --- a/test.spec.js +++ b/test.spec.js @@ -184,6 +184,45 @@ it("multiple paragraphs", () => { expect(core.setOutput.mock.calls.length).toBe(3) }); +it("paragraph with confusing ####", () => { + const expectedOutput = require("./fixtures/paragraph-confusing-####/expected.json"); + const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); + + // mock ENV + const env = { + HOME: "", + }; + + // mock event payload + const eventPayload = require("./fixtures/paragraph-confusing-####/issue"); + + // mock fs + const fs = { + readFileSync(path, encoding) { + expect(path).toBe(""); + expect(encoding).toBe("utf8"); + return readFileSync("fixtures/paragraph-confusing-####/form.yml", "utf-8"); + }, + writeFileSync(path, content) { + expect(path).toBe("/issue-parser-result.json"); + expect(content).toBe(expectedOutputJson); + }, + }; + + // mock core + const core = { + getInput: jest.fn(() => ''), + setOutput: jest.fn(), + }; + + run(env, eventPayload, fs, core); + + expect(core.getInput).toHaveBeenCalledWith('template-path') + expect(core.setOutput).toHaveBeenCalledWith('jsonString', JSON.stringify(expectedOutput, null, 2)) + expect(core.setOutput).toHaveBeenCalledWith('issueparser_textarea-one', 'Textarea input text 1 ####') + expect(core.setOutput.mock.calls.length).toBe(2) +}); + it("blank", () => { const expectedOutput = loadJson("./fixtures/blank/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); From f28daf5aa5e0a6b000a6f92dcc75803d47514297 Mon Sep 17 00:00:00 2001 From: Christian Marangi Date: Fri, 26 Apr 2024 18:58:26 +0200 Subject: [PATCH 3/5] feat: add additional test for codeblock ``` ignore Add test for codeblock ``` ignore to prevent and catch in future code change that would produce wrong parsing. Signed-off-by: Christian Marangi --- fixtures/paragraph-ignore-```/expected.json | 3 ++ fixtures/paragraph-ignore-```/form.yml | 9 +++++ fixtures/paragraph-ignore-```/issue-body.md | 7 ++++ fixtures/paragraph-ignore-```/issue.js | 6 ++++ test.spec.js | 39 +++++++++++++++++++++ 5 files changed, 64 insertions(+) create mode 100644 fixtures/paragraph-ignore-```/expected.json create mode 100644 fixtures/paragraph-ignore-```/form.yml create mode 100644 fixtures/paragraph-ignore-```/issue-body.md create mode 100644 fixtures/paragraph-ignore-```/issue.js diff --git a/fixtures/paragraph-ignore-```/expected.json b/fixtures/paragraph-ignore-```/expected.json new file mode 100644 index 0000000..2f8fdda --- /dev/null +++ b/fixtures/paragraph-ignore-```/expected.json @@ -0,0 +1,3 @@ +{ + "textarea-one": "Textarea input text 1\n\n```\n### To be ignored tag\n```" +} diff --git a/fixtures/paragraph-ignore-```/form.yml b/fixtures/paragraph-ignore-```/form.yml new file mode 100644 index 0000000..cb5bf9f --- /dev/null +++ b/fixtures/paragraph-ignore-```/form.yml @@ -0,0 +1,9 @@ +body: + - type: textarea + id: textarea-one + attributes: + label: My textarea input + - type: textarea + id: textarea-two + attributes: + label: Another textarea input diff --git a/fixtures/paragraph-ignore-```/issue-body.md b/fixtures/paragraph-ignore-```/issue-body.md new file mode 100644 index 0000000..ebe88ce --- /dev/null +++ b/fixtures/paragraph-ignore-```/issue-body.md @@ -0,0 +1,7 @@ +### My textarea input + +Textarea input text 1 + +``` +### To be ignored tag +``` diff --git a/fixtures/paragraph-ignore-```/issue.js b/fixtures/paragraph-ignore-```/issue.js new file mode 100644 index 0000000..6851c74 --- /dev/null +++ b/fixtures/paragraph-ignore-```/issue.js @@ -0,0 +1,6 @@ +const { resolve } = require("path"); +const { readFileSync } = require("fs"); + +const issueBodyPath = resolve(__dirname, "issue-body.md"); + +module.exports = readFileSync(issueBodyPath, "utf-8") diff --git a/test.spec.js b/test.spec.js index 5891324..e77cf70 100644 --- a/test.spec.js +++ b/test.spec.js @@ -223,6 +223,45 @@ it("paragraph with confusing ####", () => { expect(core.setOutput.mock.calls.length).toBe(2) }); +it("paragraph with ``` section", () => { + const expectedOutput = require("./fixtures/paragraph-ignore-```/expected.json"); + const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); + + // mock ENV + const env = { + HOME: "", + }; + + // mock event payload + const eventPayload = require("./fixtures/paragraph-ignore-```/issue"); + + // mock fs + const fs = { + readFileSync(path, encoding) { + expect(path).toBe(""); + expect(encoding).toBe("utf8"); + return readFileSync("fixtures/paragraph-ignore-```/form.yml", "utf-8"); + }, + writeFileSync(path, content) { + expect(path).toBe("/issue-parser-result.json"); + expect(content).toBe(expectedOutputJson); + }, + }; + + // mock core + const core = { + getInput: jest.fn(() => ''), + setOutput: jest.fn(), + }; + + run(env, eventPayload, fs, core); + + expect(core.getInput).toHaveBeenCalledWith('template-path') + expect(core.setOutput).toHaveBeenCalledWith('jsonString', JSON.stringify(expectedOutput, null, 2)) + expect(core.setOutput).toHaveBeenCalledWith('issueparser_textarea-one', 'Textarea input text 1\n\n```\n### To be ignored tag\n```') + expect(core.setOutput.mock.calls.length).toBe(2) +}); + it("blank", () => { const expectedOutput = loadJson("./fixtures/blank/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); From d70e6a412dd369d6708559f25939cdc441d6c405 Mon Sep 17 00:00:00 2001 From: Christian Marangi Date: Fri, 31 Jan 2025 13:49:27 +0100 Subject: [PATCH 4/5] feat: add additional test for codeblock ```sh ignore Add test for codeblock ```sh ignore to prevent and catch in future code change that would produce wrong parsing. This is a variant of the ``` test that makes the code block section target specific code. Signed-off-by: Christian Marangi --- fixtures/paragraph-ignore-```sh/expected.json | 3 ++ fixtures/paragraph-ignore-```sh/form.yml | 9 +++++ fixtures/paragraph-ignore-```sh/issue-body.md | 7 ++++ fixtures/paragraph-ignore-```sh/issue.js | 6 +++ test.spec.js | 39 +++++++++++++++++++ 5 files changed, 64 insertions(+) create mode 100644 fixtures/paragraph-ignore-```sh/expected.json create mode 100644 fixtures/paragraph-ignore-```sh/form.yml create mode 100644 fixtures/paragraph-ignore-```sh/issue-body.md create mode 100644 fixtures/paragraph-ignore-```sh/issue.js diff --git a/fixtures/paragraph-ignore-```sh/expected.json b/fixtures/paragraph-ignore-```sh/expected.json new file mode 100644 index 0000000..1026cc4 --- /dev/null +++ b/fixtures/paragraph-ignore-```sh/expected.json @@ -0,0 +1,3 @@ +{ + "textarea-one": "Textarea input text 1\n\n```sh\n### To be ignored tag\n```" +} diff --git a/fixtures/paragraph-ignore-```sh/form.yml b/fixtures/paragraph-ignore-```sh/form.yml new file mode 100644 index 0000000..cb5bf9f --- /dev/null +++ b/fixtures/paragraph-ignore-```sh/form.yml @@ -0,0 +1,9 @@ +body: + - type: textarea + id: textarea-one + attributes: + label: My textarea input + - type: textarea + id: textarea-two + attributes: + label: Another textarea input diff --git a/fixtures/paragraph-ignore-```sh/issue-body.md b/fixtures/paragraph-ignore-```sh/issue-body.md new file mode 100644 index 0000000..cf4981e --- /dev/null +++ b/fixtures/paragraph-ignore-```sh/issue-body.md @@ -0,0 +1,7 @@ +### My textarea input + +Textarea input text 1 + +```sh +### To be ignored tag +``` diff --git a/fixtures/paragraph-ignore-```sh/issue.js b/fixtures/paragraph-ignore-```sh/issue.js new file mode 100644 index 0000000..6851c74 --- /dev/null +++ b/fixtures/paragraph-ignore-```sh/issue.js @@ -0,0 +1,6 @@ +const { resolve } = require("path"); +const { readFileSync } = require("fs"); + +const issueBodyPath = resolve(__dirname, "issue-body.md"); + +module.exports = readFileSync(issueBodyPath, "utf-8") diff --git a/test.spec.js b/test.spec.js index e77cf70..d195c1a 100644 --- a/test.spec.js +++ b/test.spec.js @@ -262,6 +262,45 @@ it("paragraph with ``` section", () => { expect(core.setOutput.mock.calls.length).toBe(2) }); +it("paragraph with ```sh section", () => { + const expectedOutput = require("./fixtures/paragraph-ignore-```sh/expected.json"); + const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); + + // mock ENV + const env = { + HOME: "", + }; + + // mock event payload + const eventPayload = require("./fixtures/paragraph-ignore-```sh/issue"); + + // mock fs + const fs = { + readFileSync(path, encoding) { + expect(path).toBe(""); + expect(encoding).toBe("utf8"); + return readFileSync("fixtures/paragraph-ignore-```sh/form.yml", "utf-8"); + }, + writeFileSync(path, content) { + expect(path).toBe("/issue-parser-result.json"); + expect(content).toBe(expectedOutputJson); + }, + }; + + // mock core + const core = { + getInput: jest.fn(() => ''), + setOutput: jest.fn(), + }; + + run(env, eventPayload, fs, core); + + expect(core.getInput).toHaveBeenCalledWith('template-path') + expect(core.setOutput).toHaveBeenCalledWith('jsonString', JSON.stringify(expectedOutput, null, 2)) + expect(core.setOutput).toHaveBeenCalledWith('issueparser_textarea-one', 'Textarea input text 1\n\n```sh\n### To be ignored tag\n```') + expect(core.setOutput.mock.calls.length).toBe(2) +}); + it("blank", () => { const expectedOutput = loadJson("./fixtures/blank/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); From daac19fda189457c6a890cd371124aa0a0558b67 Mon Sep 17 00:00:00 2001 From: Gregor Martynus <39992+gr2m@users.noreply.github.com> Date: Wed, 22 Apr 2026 07:52:22 -0700 Subject: [PATCH 5/5] test: convert new fixtures and tests to ESM The fixtures and test blocks added in this PR used CommonJS (require / module.exports). After rebasing onto main, which converted the codebase to ESM in #105, they need to use import / export default and the loadJson helper. --- fixtures/paragraph-confusing-####/issue.js | 8 +++++--- fixtures/paragraph-ignore-```/issue.js | 8 +++++--- fixtures/paragraph-ignore-```sh/issue.js | 8 +++++--- test.spec.js | 15 +++++++++------ 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/fixtures/paragraph-confusing-####/issue.js b/fixtures/paragraph-confusing-####/issue.js index 6851c74..d8098d4 100644 --- a/fixtures/paragraph-confusing-####/issue.js +++ b/fixtures/paragraph-confusing-####/issue.js @@ -1,6 +1,8 @@ -const { resolve } = require("path"); -const { readFileSync } = require("fs"); +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +const __dirname = fileURLToPath(new URL(".", import.meta.url)); const issueBodyPath = resolve(__dirname, "issue-body.md"); -module.exports = readFileSync(issueBodyPath, "utf-8") +export default readFileSync(issueBodyPath, "utf-8"); diff --git a/fixtures/paragraph-ignore-```/issue.js b/fixtures/paragraph-ignore-```/issue.js index 6851c74..d8098d4 100644 --- a/fixtures/paragraph-ignore-```/issue.js +++ b/fixtures/paragraph-ignore-```/issue.js @@ -1,6 +1,8 @@ -const { resolve } = require("path"); -const { readFileSync } = require("fs"); +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +const __dirname = fileURLToPath(new URL(".", import.meta.url)); const issueBodyPath = resolve(__dirname, "issue-body.md"); -module.exports = readFileSync(issueBodyPath, "utf-8") +export default readFileSync(issueBodyPath, "utf-8"); diff --git a/fixtures/paragraph-ignore-```sh/issue.js b/fixtures/paragraph-ignore-```sh/issue.js index 6851c74..d8098d4 100644 --- a/fixtures/paragraph-ignore-```sh/issue.js +++ b/fixtures/paragraph-ignore-```sh/issue.js @@ -1,6 +1,8 @@ -const { resolve } = require("path"); -const { readFileSync } = require("fs"); +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +const __dirname = fileURLToPath(new URL(".", import.meta.url)); const issueBodyPath = resolve(__dirname, "issue-body.md"); -module.exports = readFileSync(issueBodyPath, "utf-8") +export default readFileSync(issueBodyPath, "utf-8"); diff --git a/test.spec.js b/test.spec.js index d195c1a..5ec0985 100644 --- a/test.spec.js +++ b/test.spec.js @@ -7,6 +7,9 @@ import readmeExampleIssue from "./fixtures/readme-example/issue.js"; import fullExampleIssue from "./fixtures/full-example/issue.js"; import mismatchedParsingIssue from "./fixtures/mismatched-parsing/issue.js"; import multipleParagraphsIssue from "./fixtures/multiple-paragraphs/issue.js"; +import paragraphConfusingHashesIssue from "./fixtures/paragraph-confusing-####/issue.js"; +import paragraphIgnoreCodeblockIssue from "./fixtures/paragraph-ignore-```/issue.js"; +import paragraphIgnoreCodeblockShIssue from "./fixtures/paragraph-ignore-```sh/issue.js"; function loadJson(path) { return JSON.parse(readFileSync(path, "utf-8")); @@ -185,7 +188,7 @@ it("multiple paragraphs", () => { }); it("paragraph with confusing ####", () => { - const expectedOutput = require("./fixtures/paragraph-confusing-####/expected.json"); + const expectedOutput = loadJson("./fixtures/paragraph-confusing-####/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); // mock ENV @@ -194,7 +197,7 @@ it("paragraph with confusing ####", () => { }; // mock event payload - const eventPayload = require("./fixtures/paragraph-confusing-####/issue"); + const eventPayload = paragraphConfusingHashesIssue; // mock fs const fs = { @@ -224,7 +227,7 @@ it("paragraph with confusing ####", () => { }); it("paragraph with ``` section", () => { - const expectedOutput = require("./fixtures/paragraph-ignore-```/expected.json"); + const expectedOutput = loadJson("./fixtures/paragraph-ignore-```/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); // mock ENV @@ -233,7 +236,7 @@ it("paragraph with ``` section", () => { }; // mock event payload - const eventPayload = require("./fixtures/paragraph-ignore-```/issue"); + const eventPayload = paragraphIgnoreCodeblockIssue; // mock fs const fs = { @@ -263,7 +266,7 @@ it("paragraph with ``` section", () => { }); it("paragraph with ```sh section", () => { - const expectedOutput = require("./fixtures/paragraph-ignore-```sh/expected.json"); + const expectedOutput = loadJson("./fixtures/paragraph-ignore-```sh/expected.json"); const expectedOutputJson = JSON.stringify(expectedOutput, null, 2); // mock ENV @@ -272,7 +275,7 @@ it("paragraph with ```sh section", () => { }; // mock event payload - const eventPayload = require("./fixtures/paragraph-ignore-```sh/issue"); + const eventPayload = paragraphIgnoreCodeblockShIssue; // mock fs const fs = {