Skip to content

[jsweep] Clean add_workflow_run_comment.cjs#26161

Merged
pelikhan merged 1 commit intomainfrom
jsweep/clean-add-workflow-run-comment-0d1886a87675ee5b
Apr 14, 2026
Merged

[jsweep] Clean add_workflow_run_comment.cjs#26161
pelikhan merged 1 commit intomainfrom
jsweep/clean-add-workflow-run-comment-0d1886a87675ee5b

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Summary

Cleaned add_workflow_run_comment.cjs — a github-script context file that creates activation comments with workflow run links on issues, PRs, and discussions.

Changes

add_workflow_run_comment.cjs

Extracted buildCommentBody() helper

  • Moved all comment body assembly (sanitization, lock notice, workflow-id marker, tracker-id marker, reaction type marker) into a dedicated function
  • Separates body construction from API dispatch logic, making each function single-purpose

Extracted postDiscussionComment() helper

  • Unified the two near-identical GraphQL mutation blocks (discussion vs discussion_comment) into one function with an optional replyToNodeId parameter
  • Eliminates ~30 lines of duplicated mutation code

Merged duplicate switch cases

  • issues and issue_comment cases share identical logic → merged with fall-through
  • pull_request and pull_request_review_comment cases share identical logic → merged with fall-through
  • Removes ~15 lines of duplicated code while preserving all behavior

Removed redundant core.error in catch block

  • The catch block was calling core.error followed immediately by core.warning for the same error
  • The comment explicitly says "Don't fail the job - just warn since this is not critical"
  • Removed core.error (which creates a red error annotation) and kept only core.warning

add_workflow_run_comment.test.cjs

Added 11 new test cases (21 → 32 tests):

  • buildCommentBody() — 7 tests covering run URL inclusion, reaction marker, workflow-id marker, tracker-id marker, lock notice (issues vs pull_request), unknown event types
  • postDiscussionComment() — 3 tests covering top-level comment, threaded reply with replyToId, and output setting
  • Missing PR number for pull_request_review_comment — 1 test (gap in original coverage)

Context

  • Execution context: github-script (globals: github, core, context)
  • File: actions/setup/js/add_workflow_run_comment.cjs

Validation ✅

  • Formatting: npm run format:cjs
  • Linting: npm run lint:cjs
  • Type checking: npm run typecheck
  • Tests: npm run test:js — 32 passed ✓ (all 21 original tests + 11 new tests)

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • invalid.example.invalid

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "invalid.example.invalid"

See Network Configuration for more information.

Generated by jsweep - JavaScript Unbloater · ● 2.6M ·

  • expires on Apr 16, 2026, 4:47 AM UTC

- Extract buildCommentBody() helper to separate body assembly from API call logic
- Extract postDiscussionComment() helper to unify discussion/discussion_comment GraphQL calls
- Merge duplicate issues/issue_comment and pull_request/pull_request_review_comment switch cases
- Remove redundant core.error in catch block (keep only core.warning for non-critical errors)
- Add 11 new tests: buildCommentBody(), postDiscussionComment(), missing PR number for pull_request_review_comment

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@pelikhan pelikhan marked this pull request as ready for review April 14, 2026 11:47
Copilot AI review requested due to automatic review settings April 14, 2026 11:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors add_workflow_run_comment.cjs to reduce duplication and improve readability while expanding test coverage for the new helpers and an additional error path.

Changes:

  • Extracted buildCommentBody() to centralize comment-body construction (sanitization + markers + optional lock notice).
  • Extracted postDiscussionComment() to unify GraphQL discussion comment posting (top-level vs reply).
  • Added targeted tests for the new helpers and for missing PR number in pull_request_review_comment events; updated API-error behavior expectation to warning-only.
Show a summary per file
File Description
actions/setup/js/add_workflow_run_comment.cjs Refactors comment construction + discussion posting into helpers; merges duplicated switch cases; removes non-critical core.error.
actions/setup/js/add_workflow_run_comment.test.cjs Adds coverage for new helper functions and validates behavior changes (warning-only on API error, missing PR number case).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions github-actions bot mentioned this pull request Apr 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

🧪 Test Quality Sentinel Report

Test Quality Score: 83/100

Excellent test quality

Metric Value
New/modified tests analyzed 12
✅ Design tests (behavioral contracts) 12 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 5 (42%)
Duplicate test clusters 0
Test inflation detected No (ratio: 1.39 — 96 test lines / 69 prod lines)
🚨 Coding-guideline violations None

Test Classification Details

📋 All 12 tests (click to expand)
Test File Classification Notes
should fail when PR number is missing in pull_request_review_comment event add_workflow_run_comment.test.cjs:231 ✅ Design Error path — asserts setFailed and no API call
should warn but not fail when creating comment with workflow link fails (modified) add_workflow_run_comment.test.cjs:355 ✅ Design Error-severity behavior — warn not error
should include the run URL in the comment body add_workflow_run_comment.test.cjs:491 ✅ Design Verifies URL appears in output body
should always include reaction comment type marker add_workflow_run_comment.test.cjs:497 ✅ Design Verifies required HTML marker in output
should include workflow-id marker when GITHUB_WORKFLOW is set add_workflow_run_comment.test.cjs:503 ✅ Design Conditional output based on env var
should include tracker-id marker when GH_AW_TRACKER_ID is set add_workflow_run_comment.test.cjs:510 ✅ Design Conditional output based on env var
should add lock notice for issues event when GH_AW_LOCK_FOR_AGENT=true add_workflow_run_comment.test.cjs:517 ✅ Design Conditional lock notice in output
should not add lock notice for pull_request events add_workflow_run_comment.test.cjs:524 ✅ Design Edge case — event-type discrimination for lock notice
should use unknown event type description for unrecognized events add_workflow_run_comment.test.cjs:531 ✅ Design Edge case — graceful handling of unknown event types
should post a top-level discussion comment when no replyToNodeId add_workflow_run_comment.test.cjs:540 ✅ Design Default path — correct GraphQL mutation shape
should post a threaded comment when replyToNodeId is provided add_workflow_run_comment.test.cjs:551 ✅ Design Branch path — threaded reply mutation
should set comment outputs after posting add_workflow_run_comment.test.cjs:561 ✅ Design Side-effect contract — outputs set correctly

Flagged Tests — Requires Review

No tests require mandatory review. Minor observation below for optional improvement.

💡 buildCommentBody() suite — env-var teardown hygiene

Observation (non-blocking): Tests that set process.env.GITHUB_WORKFLOW, process.env.GH_AW_TRACKER_ID, and process.env.GH_AW_LOCK_FOR_AGENT don't appear to restore the environment after each test. If the test suite runs in a shared process, env vars set in one test can bleed into later tests.
Suggested improvement: Add an afterEach (or beforeEach) that deletes the env vars, e.g. delete process.env.GH_AW_LOCK_FOR_AGENT.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 tests
  • 🟨 JavaScript (*.test.cjs): 12 tests (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All 12 new/modified tests verify observable behavioral contracts — output contents, error-path handling, conditional markers, and API call shapes. The new buildCommentBody() and postDiscussionComment() suites meaningfully expand coverage for previously-untested exported functions.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 642.7K ·

Copy link
Copy Markdown
Contributor Author

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 83/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 12 new/modified JavaScript (vitest) tests verify behavioral contracts. No coding-guideline violations detected.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

@copilot review all comments

Reviewed all comments:

  • Copilot code review: 0 issues across 2/2 files reviewed
  • Test Quality Sentinel: 83/100 — all new tests verify behavioral contracts, no guideline violations
  • CI: passed ✅

No issues to address.

@github-actions
Copy link
Copy Markdown
Contributor Author

Hey @pelikhan 👋 — great cleanup of add_workflow_run_comment.cjs! The extraction of buildCommentBody() and postDiscussionComment() makes the logic much easier to follow, and merging the duplicate switch cases is a clean improvement. The removal of the redundant core.error in the catch block is also a nice correctness fix.

The PR looks well-structured with thorough test coverage (11 new tests covering the extracted helpers) and a detailed description. This looks ready for maintainer review! ✅

Generated by Contribution Check · ● 1.6M ·

@github-actions github-actions bot added the lgtm label Apr 14, 2026
@pelikhan pelikhan merged commit fc7a080 into main Apr 14, 2026
79 checks passed
@pelikhan pelikhan deleted the jsweep/clean-add-workflow-run-comment-0d1886a87675ee5b branch April 14, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants