diff --git a/.changeset/patch-clean-add-labels-jsweep.md b/.changeset/patch-clean-add-labels-jsweep.md new file mode 100644 index 00000000000..1c598580753 --- /dev/null +++ b/.changeset/patch-clean-add-labels-jsweep.md @@ -0,0 +1,8 @@ +--- +"gh-aw": patch +--- + +Cleaned `add_labels.cjs` to make the implementation more concise and maintainable, and added a comprehensive test file `add_labels.test.cjs` covering 13 test cases. + +No behavior changes expected; tests added to validate functionality. + diff --git a/actions/setup/js/add_labels.cjs b/actions/setup/js/add_labels.cjs index a0824f8f393..6fb22d5eb48 100644 --- a/actions/setup/js/add_labels.cjs +++ b/actions/setup/js/add_labels.cjs @@ -47,38 +47,20 @@ async function main(config = {}) { processedCount++; - const item = message; - // Determine target issue/PR number - let itemNumber; - if (item.item_number !== undefined) { - itemNumber = parseInt(String(item.item_number), 10); - if (isNaN(itemNumber)) { - core.warning(`Invalid item number: ${item.item_number}`); - return { - success: false, - error: `Invalid item number: ${item.item_number}`, - }; - } - } else { - // Use context issue or PR if available - const contextIssue = context.payload?.issue?.number; - const contextPR = context.payload?.pull_request?.number; - itemNumber = contextIssue || contextPR; - - if (!itemNumber) { - core.warning("No item_number provided and not in issue/PR context"); - return { - success: false, - error: "No issue/PR number available", - }; - } + const itemNumber = message.item_number !== undefined ? parseInt(String(message.item_number), 10) : context.payload?.issue?.number || context.payload?.pull_request?.number; + + if (!itemNumber || isNaN(itemNumber)) { + const errorMsg = message.item_number !== undefined ? `Invalid item number: ${message.item_number}` : "No item_number provided and not in issue/PR context"; + core.warning(errorMsg); + return { + success: false, + error: message.item_number !== undefined ? `Invalid item number: ${message.item_number}` : "No issue/PR number available", + }; } - // Determine context type const contextType = context.payload?.pull_request ? "pull request" : "issue"; - - const requestedLabels = item.labels ?? []; + const requestedLabels = message.labels ?? []; core.info(`Requested labels: ${JSON.stringify(requestedLabels)}`); // Use validation helper to sanitize and validate labels @@ -118,8 +100,7 @@ async function main(config = {}) { try { await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, + ...context.repo, issue_number: itemNumber, labels: uniqueLabels, }); @@ -130,7 +111,7 @@ async function main(config = {}) { success: true, number: itemNumber, labelsAdded: uniqueLabels, - contextType: contextType, + contextType, }; } catch (error) { const errorMessage = getErrorMessage(error); diff --git a/actions/setup/js/add_labels.test.cjs b/actions/setup/js/add_labels.test.cjs new file mode 100644 index 00000000000..67d11e9f0b4 --- /dev/null +++ b/actions/setup/js/add_labels.test.cjs @@ -0,0 +1,346 @@ +// @ts-check +import { describe, it, expect, beforeEach } from "vitest"; +const { main } = require("./add_labels.cjs"); + +describe("add_labels", () => { + let mockCore; + let mockGithub; + let mockContext; + + beforeEach(() => { + // Reset mocks before each test + mockCore = { + info: () => {}, + warning: () => {}, + error: () => {}, + messages: [], + infos: [], + warnings: [], + errors: [], + }; + + // Capture all logged messages + mockCore.info = msg => { + mockCore.infos.push(msg); + mockCore.messages.push({ level: "info", message: msg }); + }; + mockCore.warning = msg => { + mockCore.warnings.push(msg); + mockCore.messages.push({ level: "warning", message: msg }); + }; + mockCore.error = msg => { + mockCore.errors.push(msg); + mockCore.messages.push({ level: "error", message: msg }); + }; + + mockGithub = { + rest: { + issues: { + addLabels: async () => ({}), + }, + }, + }; + + mockContext = { + repo: { + owner: "test-owner", + repo: "test-repo", + }, + payload: { + issue: { + number: 123, + }, + }, + }; + + // Set globals + global.core = mockCore; + global.github = mockGithub; + global.context = mockContext; + }); + + describe("main factory", () => { + it("should create a handler function with default configuration", async () => { + const handler = await main(); + expect(typeof handler).toBe("function"); + }); + + it("should create a handler function with custom configuration", async () => { + const handler = await main({ + allowed: ["bug", "enhancement"], + max: 5, + }); + expect(typeof handler).toBe("function"); + }); + + it("should log configuration on initialization", async () => { + await main({ allowed: ["bug", "enhancement"], max: 3 }); + expect(mockCore.infos.some(msg => msg.includes("max=3"))).toBe(true); + expect(mockCore.infos.some(msg => msg.includes("bug, enhancement"))).toBe(true); + }); + }); + + describe("handleAddLabels", () => { + it("should add labels to an issue using explicit item_number", async () => { + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 456, + labels: ["bug", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.number).toBe(456); + expect(result.labelsAdded).toEqual(["bug", "enhancement"]); + expect(addLabelsCalls.length).toBe(1); + expect(addLabelsCalls[0].issue_number).toBe(456); + expect(addLabelsCalls[0].labels).toEqual(["bug", "enhancement"]); + }); + + it("should add labels to an issue from context when item_number not provided", async () => { + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + labels: ["documentation"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.number).toBe(123); + expect(result.labelsAdded).toEqual(["documentation"]); + expect(result.contextType).toBe("issue"); + }); + + it("should add labels to a pull request from context", async () => { + mockContext.payload = { + pull_request: { + number: 789, + }, + }; + + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + labels: ["needs-review"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.number).toBe(789); + expect(result.contextType).toBe("pull request"); + }); + + it("should handle invalid item_number", async () => { + const handler = await main({ max: 10 }); + + const result = await handler( + { + item_number: "invalid", + labels: ["bug"], + }, + {} + ); + + expect(result.success).toBe(false); + expect(result.error.includes("Invalid item number")).toBe(true); + }); + + it("should handle missing item_number and no context", async () => { + mockContext.payload = {}; + + const handler = await main({ max: 10 }); + + const result = await handler( + { + labels: ["bug"], + }, + {} + ); + + expect(result.success).toBe(false); + expect(result.error.includes("No issue/PR number available")).toBe(true); + }); + + it("should respect max count limit", async () => { + const handler = await main({ max: 2 }); + + // First call succeeds + const result1 = await handler( + { + item_number: 1, + labels: ["bug"], + }, + {} + ); + expect(result1.success).toBe(true); + + // Second call succeeds + const result2 = await handler( + { + item_number: 2, + labels: ["enhancement"], + }, + {} + ); + expect(result2.success).toBe(true); + + // Third call should fail + const result3 = await handler( + { + item_number: 3, + labels: ["documentation"], + }, + {} + ); + expect(result3.success).toBe(false); + expect(result3.error.includes("Max count")).toBe(true); + }); + + it("should filter labels based on allowed list", async () => { + const handler = await main({ + allowed: ["bug", "enhancement"], + max: 10, + }); + + const addLabelsCalls = []; + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "invalid-label", "enhancement"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual(["bug", "enhancement"]); + }); + + it("should handle empty labels array", async () => { + const handler = await main({ max: 10 }); + + const result = await handler( + { + item_number: 100, + labels: [], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual([]); + expect(result.message).toBe("No valid labels found"); + }); + + it("should handle API errors gracefully", async () => { + const handler = await main({ max: 10 }); + + mockGithub.rest.issues.addLabels = async () => { + throw new Error("API Error: Not found"); + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug"], + }, + {} + ); + + expect(result.success).toBe(false); + expect(result.error.includes("API Error")).toBe(true); + }); + + it("should deduplicate labels", async () => { + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: ["bug", "bug", "enhancement", "bug"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded).toEqual(["bug", "enhancement"]); + }); + + it("should sanitize and trim label names", async () => { + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + const result = await handler( + { + item_number: 100, + labels: [" bug ", " enhancement ", "documentation"], + }, + {} + ); + + expect(result.success).toBe(true); + expect(result.labelsAdded.length).toBeGreaterThan(0); + }); + + it("should use spread operator for context.repo", async () => { + const handler = await main({ max: 10 }); + const addLabelsCalls = []; + + mockGithub.rest.issues.addLabels = async params => { + addLabelsCalls.push(params); + return {}; + }; + + await handler( + { + item_number: 100, + labels: ["bug"], + }, + {} + ); + + expect(addLabelsCalls[0].owner).toBe("test-owner"); + expect(addLabelsCalls[0].repo).toBe("test-repo"); + }); + }); +});