Skip to content

[jsweep] Clean update_pr_description_helpers.cjs #36575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Jun 3, 2026
+62 −9
Merged

[jsweep] Clean update_pr_description_helpers.cjs #36575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions actions/setup/js/update_pr_description_helpers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,20 @@ function updateBody(params) {
if (operation === "replace-island") {
// Try to find existing island for this workflow ID
const island = findIsland(currentBody, workflowId);
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);
const islandContent = `${startMarker}\n${contentWithCaution}${aiFooter}${workflowIdMarker}\n${endMarker}`;

if (island.found) {
// Replace the island content
core.info(`Operation: replace-island (updating existing island for workflow ${workflowId})`);
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);
const islandContent = `${startMarker}\n${contentWithCaution}${aiFooter}${workflowIdMarker}\n${endMarker}`;

const before = currentBody.substring(0, island.startIndex);
const after = currentBody.substring(island.endIndex);
return before + islandContent + after;
} else {
// Island not found, fall back to append mode
core.info(`Operation: replace-island (island not found for workflow ${workflowId}, falling back to append)`);
const startMarker = buildIslandStartMarker(workflowId);
const endMarker = buildIslandEndMarker(workflowId);
const islandContent = `${startMarker}\n${contentWithCaution}${aiFooter}${workflowIdMarker}\n${endMarker}`;
const appendSection = `\n\n---\n\n${islandContent}`;
return currentBody + appendSection;
return currentBody + `\n\n---\n\n${islandContent}`;
}
}

Expand Down
58 changes: 58 additions & 0 deletions actions/setup/js/update_pr_description_helpers.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,4 +496,62 @@ describe("update_pr_description_helpers.cjs", () => {
expect(result).toContain("<!-- gh-aw-island-end:test-workflow -->");
});
});

describe("historyUrl parameter", () => {
it("should pass historyUrl to the footer when provided", () => {
const result = updateBody({
currentBody: "Original",
newContent: "New content",
operation: "append",
workflowName: "Test",
runUrl: "https://github.com/test/actions/runs/123",
workflowId: "test-workflow",
historyUrl: "https://github.com/search?q=test",
});
expect(result).toContain("New content");
// historyUrl should be included in the footer
expect(result).toContain("https://github.com/search?q=test");
Copy link
Copy Markdown
Contributor Author

@github-actions github-actions Bot Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/zoom-out] The historyUrl test verifies that the URL appears somewhere in the output, but does not assert it lands specifically inside the generated footer. If historyUrl were accidentally placed in the main content, the test would still pass.

💡 Suggestion

Consider asserting on the specific footer fragment that carries the URL:

expect(result).toContain(`[◷](https://github.com/search?q=test)`);
// or
expect(result).toMatch(/Generated by.*https:\/\/github\.com\/search/);

This guards against regressions where the URL is injected in the wrong location.

});

it("should work without historyUrl (optional parameter)", () => {
const result = updateBody({
currentBody: "Original",
newContent: "New content",
operation: "append",
workflowName: "Test",
runUrl: "https://github.com/test/actions/runs/123",
workflowId: "test-workflow",
});
expect(result).toContain("New content");
expect(result).toContain("Original");
});
});

describe("workflowIdMarker - includeFooter false", () => {
it("should include workflow ID marker when includeFooter is false and workflowId is provided", () => {
const result = updateBody({
currentBody: "Original",
newContent: "New content",
operation: "append",
workflowName: "Test",
runUrl: "https://github.com/test/actions/runs/123",
workflowId: "my-workflow",
includeFooter: false,
});
expect(result).toContain("<!-- gh-aw-workflow-id: my-workflow -->");
});

it("should not include workflow ID marker when includeFooter is false and workflowId is empty", () => {
const result = updateBody({
currentBody: "Original",
newContent: "New content",
operation: "append",
workflowName: "Test",
runUrl: "https://github.com/test/actions/runs/123",
workflowId: "",
includeFooter: false,
});
expect(result).not.toContain("gh-aw-workflow-id");
});
});
});
Loading