[go-fan] Go Module Review: stretchr/testify

[github-actions]

🐹 Go Fan Report: stretchr/testify

Module Overview

github.com/stretchr/testify v1.11.1 is Go's most popular testing library, providing expressive assertion functions with informative failure messages. It wraps testing.T with assert (non-fatal) and require (fatal) packages, plus mock and suite utilities.

Current Usage in gh-aw

This is a core test dependency — the project depends on it deeply and correctly.

• Files: 135 test files
• Total assertion calls: ~6,128
• Import Count: 243 import lines (assert + require packages)
• Key APIs Used: assert.Equal, require.NoError, assert.Contains, require.NotNil, assert.True, bound asserters (assert.New, require.New)

Usage Strengths ✅

• Consistently uses require for critical checks that stop test execution
• Bound asserters (assert.New(t)) used in ~20 test files to reduce repetition
• assert.ElementsMatch, assert.JSONEq, assert.ErrorContains seen in the codebase
• Strong adoption of table-driven tests with testify assertions

Research Findings

Best Practices

• assert.ErrorContains(t, err, "substr") is the idiomatic way to check error message substrings — available since testify v1.7.0
• require vs assert split should be: use require when a nil/error result would panic subsequent assertions
• Bound asserters (assert.New(t)) reduce noise in files with many assertions
• testifylint linter can enforce many of these patterns automatically

Improvement Opportunities

🏃 Quick Wins

Replace assert.Contains(t, err.Error(), ...) with assert.ErrorContains

209 instances across the codebase are checking error message substrings the long way:

// ❌ Current (209 occurrences)
assert.Contains(t, err.Error(), "expected message")
require.Contains(t, err.Error(), "expected message")

// ✅ Idiomatic testify
assert.ErrorContains(t, err, "expected message")
require.ErrorContains(t, err, "expected message")

Why this matters:

• assert.ErrorContains automatically fails with a clear message if err is nil — the current pattern panics on nil.Error()
• Shorter, more readable
• Signals intent clearly: "this should be a non-nil error containing this string"

Most affected files:

• internal/mcp/connection_test.go (~10 instances)
• internal/mcp/http_transport_test.go (~8 instances)
• internal/mcp/unmarshal_params_test.go (~6 instances)
• internal/mcp/marshal_helper_test.go (~4 instances)
• internal/mcp/http_connection_test.go (~4 instances)
• Dozens more across internal/config/, internal/server/, internal/logger/, etc.

✨ Feature Opportunities

Enable testifylint after fixing the 209 patterns

The .golangci.yml has testifylint explicitly disabled:

"Disabled linters: gosec, testifylint, errcheck, gocritic, revive"

The AGENTS.md notes this is "due to requiring extensive test refactoring across the codebase." The primary culprit is the 209 err.Error() patterns. Fixing those first would unblock enabling testifylint as a preventive linter going forward.

📐 Best Practice Alignment

require.Nil on *ValidationError — already correct

5 instances in internal/config/rules/rules_test.go use require.Nil(t, err, ...) where err is *ValidationError (a custom struct pointer, not the error interface). This is intentionally correct — require.NoError only accepts the error interface. No action needed.

Length comparison clarity

A few spots use assert.Equal(t, len(x), len(y)) — both sides are dynamic. When one side is the expected value (a literal or defined constant), prefer assert.Len(t, collection, expectedLen) for a more informative failure message showing the actual collection contents.

🔧 General Improvements

testutil/mcptest/harness_test.go mixing raw t.Errorf with testify

This test harness file uses t.Errorf and t.Fatalf in places instead of testify's assertions, which is inconsistent with the rest of the codebase. Switching to require.Len(t, tools, 1) etc. would improve consistency and failure message quality.

Recommendations

Priority 1 (High Impact, ~209 changes): Replace assert.Contains(t, err.Error(), ...) with assert.ErrorContains(t, err, ...) across all test files. This prevents potential panics on nil errors and aligns with testify best practices.

Priority 2 (Follow-on): Re-enable testifylint in .golangci.yml after Priority 1 is done to enforce these patterns going forward.

Priority 3 (Low effort): Migrate harness_test.go raw t.Errorf calls to testify assertions.

Next Steps

• Bulk-replace assert.Contains(t, err.Error(), ...) → assert.ErrorContains(t, err, ...) (sed/gorename script)
• Bulk-replace require.Contains(t, err.Error(), ...) → require.ErrorContains(t, err, ...)
• Re-enable testifylint in .golangci.yml
• Update testutil/mcptest/harness_test.go to use testify consistently

---

Generated by Go Fan 🐹
Module analysis saved to session state
Run: §23582738890

Note

🔒 Integrity filter blocked 5 items

The following items were blocked because they don't meet the GitHub integrity level.

• https://github.com/modelcontextprotocol/go-sdk search_repositories: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved".
• https://github.com/stretchr/testify search_repositories: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved".
• https://github.com/golang/term search_repositories: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved".
• https://github.com/modelcontextprotocol/go-sdk/releases/tag/v1.4.1 get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved".
• https://github.com/stretchr/testify/releases/tag/v1.11.1 get_latest_release: has lower integrity than agent requires. The agent cannot read data with integrity below "unapproved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Go Fan · ◷

• expires on Apr 2, 2026, 7:42 AM UTC

[github-actions]

This issue was automatically closed because it expired on 2026-04-02T07:42:01.833Z.

Closed by Workflow