From 7c737ba6fa760b029f48a05de2b46aa2941488b0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 13 Jun 2026 05:27:56 +0000 Subject: [PATCH 1/2] jsweep: clean update_handler_factory.cjs - Split long destructuring onto multiple lines for readability - Replace forEach configParts building with spread + filter + map - Break long withRetry call and ternary metadata captures across multiple lines - Add 5 new tests: staged mode, buildUpdateData skipped, deferred resolution, itemFilter skip, itemFilter proceed (34 tests total, up from 29) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- actions/setup/js/update_handler_factory.cjs | 16 +-- .../setup/js/update_handler_factory.test.cjs | 127 ++++++++++++++++++ 2 files changed, 134 insertions(+), 9 deletions(-) diff --git a/actions/setup/js/update_handler_factory.cjs b/actions/setup/js/update_handler_factory.cjs index 1630e3aef83..35cac8684c0 100644 --- a/actions/setup/js/update_handler_factory.cjs +++ b/actions/setup/js/update_handler_factory.cjs @@ -138,15 +138,13 @@ function createUpdateHandlerFactory(handlerConfig) { // Check if we're in staged mode const isStaged = isStagedMode(config); - // Build configuration log message - const configParts = [`max=${maxCount}`, `target=${updateTarget}`]; - - // Add additional config items to log - Object.entries(additionalConfig).forEach(([key, value]) => { - if (config[key] !== undefined) { - configParts.push(`${key}=${config[key]}`); - } - }); + const configParts = [ + `max=${maxCount}`, + `target=${updateTarget}`, + ...Object.entries(additionalConfig) + .filter(([key]) => config[key] !== undefined) + .map(([key]) => `${key}=${config[key]}`), + ]; core.info(`Update ${itemTypeName} configuration: ${configParts.join(", ")}`); diff --git a/actions/setup/js/update_handler_factory.test.cjs b/actions/setup/js/update_handler_factory.test.cjs index ff9ddb88f70..f86ce51d9eb 100644 --- a/actions/setup/js/update_handler_factory.test.cjs +++ b/actions/setup/js/update_handler_factory.test.cjs @@ -806,5 +806,132 @@ describe("update_handler_factory.cjs", () => { // The log should mention "body" even though _rawBody starts with underscore expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('"body"')); }); + + it("should return staged result when config.staged is true", async () => { + const mockExecuteUpdate = vi.fn(); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "New title" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({ staged: true }); + const result = await handler({ title: "New title" }); + + expect(result.success).toBe(true); + expect(result.staged).toBe(true); + expect(result.previewInfo).toMatchObject({ number: 42 }); + // executeUpdate must NOT be called in staged mode + expect(mockExecuteUpdate).not.toHaveBeenCalled(); + }); + + it("should return skipped result when buildUpdateData returns skipped:true", async () => { + const mockExecuteUpdate = vi.fn(); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ + success: true, + skipped: true, + reason: "No supported fields provided", + }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + }); + + const handler = await handlerFactory({}); + const result = await handler({ title: "Test" }); + + expect(result.success).toBe(true); + expect(result.skipped).toBe(true); + expect(result.reason).toBe("No supported fields provided"); + expect(mockExecuteUpdate).not.toHaveBeenCalled(); + }); + + it("should propagate deferred flag from resolveItemNumber", async () => { + const mockExecuteUpdate = vi.fn(); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ + success: false, + deferred: true, + error: "Temporary ID not yet resolved: aw_pending", + }), + buildUpdateData: vi.fn(), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn(), + }); + + const handler = await handlerFactory({}); + const result = await handler({ issue_number: "aw_pending" }); + + expect(result.success).toBe(false); + expect(result.deferred).toBe(true); + expect(result.error).toContain("aw_pending"); + expect(mockExecuteUpdate).not.toHaveBeenCalled(); + }); + + it("should call itemFilter and skip update when filter returns a result", async () => { + const mockExecuteUpdate = vi.fn(); + const mockItemFilter = vi.fn().mockResolvedValue({ + success: false, + skipped: true, + reason: "Required label not present", + }); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + itemFilter: mockItemFilter, + }); + + const handler = await handlerFactory({}); + const result = await handler({ title: "Test" }); + + expect(mockItemFilter).toHaveBeenCalled(); + expect(result.success).toBe(false); + expect(result.skipped).toBe(true); + expect(result.reason).toBe("Required label not present"); + expect(mockExecuteUpdate).not.toHaveBeenCalled(); + }); + + it("should call itemFilter and proceed when filter returns null", async () => { + const mockExecuteUpdate = vi.fn().mockResolvedValue({ html_url: "https://example.com", title: "Updated" }); + const mockItemFilter = vi.fn().mockResolvedValue(null); + + const handlerFactory = factoryModule.createUpdateHandlerFactory({ + itemType: "update_test", + itemTypeName: "test item", + supportsPR: false, + resolveItemNumber: vi.fn().mockReturnValue({ success: true, number: 42 }), + buildUpdateData: vi.fn().mockReturnValue({ success: true, data: { title: "Test" } }), + executeUpdate: mockExecuteUpdate, + formatSuccessResult: vi.fn().mockReturnValue({ success: true }), + itemFilter: mockItemFilter, + }); + + const handler = await handlerFactory({}); + const result = await handler({ title: "Test" }); + + expect(mockItemFilter).toHaveBeenCalled(); + expect(result.success).toBe(true); + expect(mockExecuteUpdate).toHaveBeenCalled(); + }); }); }); From 2db7e74f3d6a76367d223fc4ecceafac8cee8f7a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 13 Jun 2026 10:40:11 +0000 Subject: [PATCH 2/2] Fix review feedback in update handler tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/update_handler_factory.test.cjs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/update_handler_factory.test.cjs b/actions/setup/js/update_handler_factory.test.cjs index f86ce51d9eb..0b3c55517c6 100644 --- a/actions/setup/js/update_handler_factory.test.cjs +++ b/actions/setup/js/update_handler_factory.test.cjs @@ -806,7 +806,9 @@ describe("update_handler_factory.cjs", () => { // The log should mention "body" even though _rawBody starts with underscore expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('"body"')); }); + }); + describe("handler control flow", () => { it("should return staged result when config.staged is true", async () => { const mockExecuteUpdate = vi.fn(); @@ -887,7 +889,7 @@ describe("update_handler_factory.cjs", () => { const mockItemFilter = vi.fn().mockResolvedValue({ success: false, skipped: true, - reason: "Required label not present", + error: "Required label not present", }); const handlerFactory = factoryModule.createUpdateHandlerFactory({ @@ -907,7 +909,7 @@ describe("update_handler_factory.cjs", () => { expect(mockItemFilter).toHaveBeenCalled(); expect(result.success).toBe(false); expect(result.skipped).toBe(true); - expect(result.reason).toBe("Required label not present"); + expect(result.error).toBe("Required label not present"); expect(mockExecuteUpdate).not.toHaveBeenCalled(); });