[jsweep] Clean add_reaction_and_edit_comment.cjs - #18252
Conversation
- Add {} blocks to all switch cases with const declarations
- Extract EVENT_TYPE_DESCRIPTIONS lookup object (replaces switch in addCommentWithWorkflowLink)
- Remove unused getDiscussionCommentId function
- Extract setCommentOutputs helper to eliminate duplicate output code
- Reuse getDiscussionId in addCommentWithWorkflowLink instead of duplicating GraphQL queries
- Export addCommentWithWorkflowLink, addReaction, addDiscussionReaction for testing
Test improvements:
- Rewrite test file from eval-based/minified code to proper dynamic imports
- Use vi.resetAllMocks() for clean state between tests
- Add 28 comprehensive tests covering all event types and edge cases
- Test exported functions directly instead of via eval
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the add_reaction_and_edit_comment.cjs github-script by removing dead code, reducing duplication, and making key helpers directly testable, along with a full rewrite of the associated test suite to avoid eval and improve coverage/readability.
Changes:
- Refactors the reaction/comment script with scoped
switchcases, shared helpers (EVENT_TYPE_DESCRIPTIONS,setCommentOutputs), and exports for unit testing. - Removes unused discussion-comment helper logic and reuses
getDiscussionId()for discussion comment creation. - Rewrites tests to use dynamic imports (cache-busting) and adds broader coverage across event types, reactions, markers, and error handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/add_reaction_and_edit_comment.cjs | Refactors reaction/comment creation logic, reduces duplication, and exports functions for direct testing. |
| actions/setup/js/add_reaction_and_edit_comment.test.cjs | Replaces eval-based tests with dynamic-import tests and expands coverage across events and error scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -450,13 +374,7 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName) { | |||
| ); | |||
There was a problem hiding this comment.
addCommentWithWorkflowLink() relies on context.payload.comment.node_id for the discussion_comment path but doesn’t validate it before passing it as replyToId. Since the function is now exported (and can be called directly in unit tests/other modules), callers can easily trigger a GraphQL request with replyToId: undefined, yielding a confusing API error. Consider validating commentNodeId (and that the discussion number parsed from endpoint is a real number) and returning/logging a clear ERR_NOT_FOUND/validation message before making the GraphQL request.
Summary
Cleaned and modernized
add_reaction_and_edit_comment.cjs(github-script context).Changes
add_reaction_and_edit_comment.cjs{}blocks to switch cases — allcaseclauses withconstdeclarations now have proper block scope, eliminating lexical declaration scope issuesEVENT_TYPE_DESCRIPTIONSlookup object — replaced a verbose switch statement inaddCommentWithWorkflowLinkwith a single-line lookup, matching the pattern already used inadd_workflow_run_comment.cjsgetDiscussionCommentIdfunction — dead code that was never called anywheresetCommentOutputshelper — eliminates ~8 lines of duplicatedcore.info/core.setOutputcode that appeared in 3 placesgetDiscussionIdinaddCommentWithWorkflowLink— removed duplicate inline GraphQL queries for discussion events; now calls the existing helperaddCommentWithWorkflowLink,addReaction,addDiscussionReaction— enables direct unit testing without evaladd_reaction_and_edit_comment.test.cjsComplete rewrite of the test file:
eval-based test execution — tests now use dynamic imports (import("./...cjs?" + Date.now())) matching the pattern in newer test filesvi.resetAllMocks()instead ofvi.clearAllMocks()— prevents mock queue leakage between testsaddReaction,addDiscussionReaction,addCommentWithWorkflowLink)Context: github-script (uses
core,github,contextglobals)Validation ✅
npm run format:cjs✓npm run lint:cjs✓npm run typecheck✓npm run test:js) ✓Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.org