[test-improver] Improve tests for mcp/connection package

[github-actions]

Test Improvements: internal/mcp/connection_test.go

File Analyzed

• Test File: internal/mcp/connection_test.go
• Package: internal/mcp
• Lines of Code: 1,020 → 977 (net -43 lines, all noise removed)

Improvements Made

1. Better Testing Patterns (Idiomatic Testify)

Replaced four different manual error-checking patterns with proper testify assertions:

• ✅ t.Errorf("Failed to read request body: %v", err) → assert.NoError(t, err, "Failed to read request body")
• ✅ t.Fatalf("Failed to create connection: %v", err) → require.True(t, tt.expectError, ...) (correct semantics: fails test immediately if connection creation was unexpected)
• ✅ t.Error("Expected error but got none") → require.Error(t, err, "Expected an error but got none")
• ✅ t.Errorf("Expected error to contain '%s', got: %v", ...) → assert.Contains(t, err.Error(), tt.errorSubstring, ...)

Before (in TestHTTPRequest_ErrorResponses):

if err != nil && tt.expectError {
    if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
        t.Errorf("Expected error to contain '%s', got: %v", tt.errorSubstring, err)
    }
    return
}
if err != nil {
    t.Fatalf("Failed to create connection: %v", err)
}
// ...
if tt.expectError {
    if err == nil {
        t.Error("Expected error but got none")
    } else if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
        t.Errorf("Expected error to contain '%s', got: %v", tt.errorSubstring, err)
    }
} else {
    if err != nil {
        t.Errorf("Expected no error but got: %v", err)
    }
}

After:

if err != nil {
    require.True(t, tt.expectError, "Unexpected error creating connection: %v", err)
    if tt.errorSubstring != "" {
        assert.Contains(t, err.Error(), tt.errorSubstring, "Error should contain expected substring")
    }
    return
}
// ...
if tt.expectError {
    require.Error(t, err, "Expected an error but got none")
    if tt.errorSubstring != "" {
        assert.Contains(t, err.Error(), tt.errorSubstring, "Error should contain expected substring")
    }
} else {
    assert.NoError(t, err)
}

2. Removed Redundant Helper Functions

• ✅ Removed containsSubstring(s, substr string) bool — manually reimplemented what assert.Contains already does
• ✅ Removed stringContains(s, substr string) bool — a hand-rolled substring search used only by containsSubstring

These helpers (17 lines) duplicated functionality already available in testify's assert.Contains.

3. Cleaner Assertion Style in TestConnection_IsHTTP

• ✅ if conn.GetHTTPURL() != testServer.URL { t.Errorf(...) } → assert.Equal(t, testServer.URL, conn.GetHTTPURL(), ...)
• ✅ if returnedHeaders[k] != v { t.Errorf(...) } → assert.Equal(t, v, returnedHeaders[k], "Header %s should match", k)

Why These Changes?

The internal/mcp/connection_test.go file was one of the few test files in the codebase that still used raw t.Errorf/t.Fatalf calls instead of the testify assertions already imported and used elsewhere in the same file. The manual patterns also included two custom helper functions (containsSubstring, stringContains) that re-implemented string-contains logic already available as assert.Contains.

These improvements make test failures produce clearer output (testify shows expected vs. actual values), eliminate dead code, and make the test code consistent with the rest of the codebase.

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Generated by Test Improver · ● 10.9M · ◷

[Copilot]

Pull request overview

This PR modernizes and simplifies the internal/mcp/connection_test.go test suite by replacing ad-hoc error checks and custom substring helpers with idiomatic Testify assert/require assertions, improving failure clarity and removing duplicated logic.

Changes:

• Replaced manual t.Errorf/t.Fatalf error-handling branches with assert/require assertions in several tests.
• Removed redundant string-substring helper functions in favor of assert.Contains.
• Simplified equality checks in TestConnection_IsHTTP using assert.Equal.

Show a summary per file

File	Description
internal/mcp/connection_test.go	Refactors tests to use consistent Testify assertions and removes redundant helpers for clearer, more maintainable test code.

Copilot's findings

Tip

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

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