Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 12 additions & 3 deletions actions/setup/js/pr_review_buffer.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,20 @@ function createReviewBuffer() {
* - "always" (default): Always include footer
* - "none": Never include footer
* - "if-body": Only include footer if review body is non-empty
* Note: Boolean values are converted to strings in the Go compiler before reaching JavaScript.
* @param {string} value - Footer mode string
* Also accepts boolean values for backward compatibility:
* - true → "always"
* - false → "none"
* Note: create-pull-request-review-comment.footer is converted to a string in Go,
* but submit-pull-request-review.footer and global footer are still emitted as booleans.
* @param {string|boolean} value - Footer mode string or boolean
*/
function setFooterMode(value) {
if (typeof value === "string") {
if (typeof value === "boolean") {
// Normalize boolean to string mode (backward compatibility)
const normalized = value ? "always" : "none";
core.info(`Normalized boolean footer config (${value}) to mode: "${normalized}"`);
footerMode = normalized;
} else if (typeof value === "string") {
// Validate string mode
if (value === "always" || value === "none" || value === "if-body") {
footerMode = value;
Expand Down
103 changes: 103 additions & 0 deletions actions/setup/js/pr_review_buffer.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,109 @@ describe("pr_review_buffer (factory pattern)", () => {
expect(callArgs.body).not.toContain("test-workflow");
});

it("should normalize boolean false to 'none' mode", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review body", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode(false);

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 505,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-505",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should NOT be included because false maps to "none"
expect(callArgs.body).toBe("Review body");
expect(callArgs.body).not.toContain("test-workflow");
// Verify normalization was logged
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('Normalized boolean footer config (false) to mode: "none"'));
});

it("should normalize boolean true to 'always' mode", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("", "APPROVE"); // Empty body
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
buffer.setFooterMode(true);

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 506,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-506",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should be included because true maps to "always"
expect(callArgs.body).toContain("test-workflow");
// Verify normalization was logged
expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('Normalized boolean footer config (true) to mode: "always"'));
});

it("should handle setIncludeFooter(false) backward compatibility", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review body", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
// Use the backward-compatible alias with boolean (the original API contract)
buffer.setIncludeFooter(false);

mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 508,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-508",
},
});

const result = await buffer.submitReview();

expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer should NOT be included because false maps to "none"
expect(callArgs.body).toBe("Review body");
expect(callArgs.body).not.toContain("test-workflow");
});

Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

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

Consider adding a test case for setIncludeFooter(true) to ensure complete coverage of the backward compatibility alias with both boolean values. While setIncludeFooter is just an alias to setFooterMode (which already has tests for both true and false), having explicit tests for both boolean values through the backward compatibility API would make the test suite more complete and self-documenting.

Suggested change
it("should handle setIncludeFooter(true) backward compatibility", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("Review body", "COMMENT");
buffer.setReviewContext({
repo: "owner/repo",
repoParts: { owner: "owner", repo: "repo" },
pullRequestNumber: 42,
pullRequest: { head: { sha: "abc123" } },
});
buffer.setFooterContext({
workflowName: "test-workflow",
runUrl: "https://github.com/owner/repo/actions/runs/123",
workflowSource: "owner/repo/workflows/test.md@v1",
workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md",
});
// Use the backward-compatible alias with boolean (the original API contract)
buffer.setIncludeFooter(true);
mockGithub.rest.pulls.createReview.mockResolvedValue({
data: {
id: 509,
html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-509",
},
});
const result = await buffer.submitReview();
expect(result.success).toBe(true);
const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0];
// Footer SHOULD be included because true maps to "always"
expect(callArgs.body).toContain("test-workflow");
});

Copilot uses AI. Check for mistakes.
it("should default to 'always' for invalid string mode", async () => {
buffer.addComment({ path: "test.js", line: 1, body: "comment" });
buffer.setReviewMetadata("", "APPROVE");
Expand Down
Loading