Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

A skipped test in assign_to_agent.test.cjs was failing because assignAgentToIssue() returned false on permission errors instead of throwing, causing the original error message to be lost and replaced with a generic "Failed to assign via GraphQL" error.

Changes

assign_agent_helpers.cjs:

  • Changed assignAgentToIssue() to throw the original error instead of returning false when permission-related mutations fail
  • Preserves error message ("Resource not accessible by integration") for summary generation logic

assign_to_agent.test.cjs:

  • Re-enabled skipped test by removing .skip()
  • Added 4th mock for fallback mutation (test requires: findAgent, getIssueDetails, primary mutation, fallback mutation)
  • Added explicit mockGithub.graphql.mockReset() to fix test isolation issue where mock state from previous tests interfered

Example

Before:

// assignAgentToIssue returned false, losing error context
if (!success) {
  throw new Error("Failed to assign copilot via GraphQL");  // Generic error
}

After:

// assignAgentToIssue throws original error
throw error;  // Preserves "Resource not accessible by integration"

This allows the summary generation code at line 197 to detect permission errors and include helpful troubleshooting guidance.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Code Quality] Fix skipped test for assign_to_agent permission error handling</issue_title>
<issue_description>## Description

The test file actions/setup/js/assign_to_agent.test.cjs contains a skipped test for permission error summary generation (line 470). The test is disabled because the mock setup doesn't work correctly with eval() and error propagation fails in the test environment.

Suggested Changes

  1. Fix mock setup: Refactor test to avoid issues with eval() in test environment
  2. Improve error propagation: Ensure errors from getIssueDetails are properly propagated
  3. Alternative testing approach: Consider using dependency injection or test doubles instead of eval()
  4. Re-enable test: Remove .skip() once test is working correctly

Files Affected

  • actions/setup/js/assign_to_agent.test.cjs (lines 470-478 - fix and re-enable test)
  • actions/setup/js/assign_to_agent.cjs (may need refactoring for testability)

Success Criteria

  • Test runs successfully without .skip()
  • Permission error summary generation is properly tested
  • Mock setup works correctly with all code paths
  • Error propagation from getIssueDetails is verified
  • Test is maintainable and doesn't rely on fragile eval() patterns

Source

Skipped test with TODO comment in actions/setup/js/assign_to_agent.test.cjs:470-472:

it.skip("should generate permission error summary when appropriate", async () => {
  // TODO: This test needs to be fixed - the mock setup doesn't work correctly with eval()
  // The error from getIssueDetails is not being propagated properly in the test environment

Priority

Medium - Test coverage gap that should be addressed for code quality

AI generated by Discussion Task Miner - Code Quality Improvement Agent

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits January 14, 2026 17:06
…andling

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
… error propagation

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix skipped test for assign_to_agent permission error handling Fix skipped test for assign_to_agent permission error handling Jan 14, 2026
Copilot AI requested a review from mnkiefer January 14, 2026 17:30
@pelikhan pelikhan marked this pull request as ready for review January 14, 2026 22:51
@pelikhan pelikhan closed this Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Fix skipped test for assign_to_agent permission error handling

3 participants