Skip to content

Conversation

@dsarno
Copy link
Owner

@dsarno dsarno commented Jan 1, 2026

Motivation

  • Remove duplicated message formatting logic and centralize it for reuse across the codebase.
  • Make the test suite use the production formatter instead of maintaining a separate copy of the logic.
  • Improve maintainability by providing a single FormatTestResultMessage helper in RunTests.

Description

  • Added internal static string FormatTestResultMessage(string mode, TestRunResult result) to MCPForUnity/Editor/Tools/RunTests.cs and replaced the inline message construction with a call to this method.
  • Updated TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RunTestsTests.cs to call MCPForUnity.Editor.Tools.RunTests.FormatTestResultMessage directly and removed the duplicated local helper.
  • Preserved the behavior of appending the "(No tests matched the specified filters)" warning when result.Total == 0.

Testing

  • No automated tests were executed as part of this change.
  • Unit test code was updated to call the production formatter, but the tests were not run in this rollout.

Codex Task

@coderabbitai
Copy link

coderabbitai bot commented Jan 1, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dsarno dsarno merged commit 74adad3 into optimize-run-tests Jan 1, 2026
3 checks passed
@greptile-apps
Copy link

greptile-apps bot commented Jan 1, 2026

Greptile Summary

Extracted message formatting logic into a reusable FormatTestResultMessage method to eliminate code duplication between production and test code.

  • Added internal static FormatTestResultMessage(string mode, TestRunResult result) to RunTests.cs
  • Updated test suite to call the production formatter instead of maintaining a duplicate local helper
  • Preserved existing behavior including the "No tests matched the specified filters" warning for empty results

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Clean refactoring that extracts duplicated logic into a shared method with no behavioral changes. Tests were updated to use the production code path, improving maintainability
  • No files require special attention

Important Files Changed

Filename Overview
MCPForUnity/Editor/Tools/RunTests.cs Extracted FormatTestResultMessage method to centralize message formatting logic, replacing inline string construction
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/RunTestsTests.cs Removed duplicated helper method and updated tests to call production FormatTestResultMessage directly

Sequence Diagram

sequenceDiagram
    participant Client
    participant RunTests
    participant TestService
    participant FormatMethod as FormatTestResultMessage

    Client->>RunTests: HandleCommand(params)
    RunTests->>RunTests: Parse mode, timeout, filters
    RunTests->>TestService: RunTestsAsync(mode, filters)
    TestService-->>RunTests: TestRunResult
    RunTests->>FormatMethod: FormatTestResultMessage(mode, result)
    FormatMethod->>FormatMethod: Build message string
    alt result.Total == 0
        FormatMethod->>FormatMethod: Append filter warning
    end
    FormatMethod-->>RunTests: Formatted message
    RunTests->>RunTests: ToSerializable(result)
    RunTests-->>Client: SuccessResponse(message, data)
Loading

@dsarno dsarno deleted the codex/refactor-formattestresultmessage-logic branch January 2, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants