[testify-expert] Improve Test Quality: pkg/errorutil/errors_test.go

[github-actions]

Overview

File: pkg/errorutil/errors_test.go
Source pair: pkg/errorutil/errors.go
Test functions: 3 (TestIsNotFoundError, TestIsForbiddenError, TestIsGoneError)
LOC: 92 (errors_test.go) + 131 (spec_test.go)

---

Strengths

• Good table-driven structure across all three test functions.
• Representative coverage: nil, wrapped, case-insensitive, and negative cases.
• Parallel spec tests in spec_test.go validate documented public API contracts.

---

Prioritized Improvements

1. Missing / High-Value Tests

The following edge cases are not currently exercised:

• IsNotFoundError with an error whose .Error() is exactly "404" (boundary: no surrounding text).
• IsNotFoundError with a partial-prefix match ("404abc") — behavior is unverified in tests.
• Double-wrapped errors (fmt.Errorf("%w", fmt.Errorf("%w", ...))) for all three classifiers — current tests wrap only one level deep.

Suggested additions for errors_test.go

// In TestIsNotFoundError cases:
{name: "exact 404 string", err: errors.New("404"), want: true},
{name: "partial 404 prefix", err: errors.New("404abc"), want: true},
{name: "double-wrapped not found",
    err: fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", errors.New("not found"))),
    want: true,
},

// In TestIsForbiddenError cases:
{name: "double-wrapped HTTP 403",
    err: fmt.Errorf("a: %w", fmt.Errorf("b: %w", errors.New("HTTP 403: Forbidden"))),
    want: true,
},

// In TestIsGoneError cases:
{name: "double-wrapped HTTP 410",
    err: fmt.Errorf("a: %w", fmt.Errorf("b: %w", errors.New("HTTP 410: Gone"))),
    want: true,
},

2. Testify Assertion Upgrade: Drop Redundant Failure Messages

Every subtest appends a format string that re-states the subtest name — already captured by t.Run:

// Before — redundant: subtest name is already in the t.Run label
assert.Equal(t, tt.want, got, "IsNotFoundError result mismatch for subtest %s", tt.name)

// After — cleaner
assert.Equal(t, tt.want, got)

Apply this to all three table loops in errors_test.go.

3. Organization: Clarify File Responsibility Split

Both errors_test.go and spec_test.go cover overlapping cases for all three exported functions:

Case	errors_test.go	spec_test.go
nil error	yes	yes
HTTP status match	yes	yes
wrapped error	yes	yes
non-matching error	yes	yes

This creates maintenance risk. Recommended fix: add a short comment block at the top of each file declaring its scope.

// errors_test.go — behavioral edge-case and boundary tests.
// Spec-contract tests (documented public API) live in spec_test.go.

// spec_test.go — public API contract tests tied to README documentation.
// Implementation edge cases live in errors_test.go.

---

Acceptance Checklist

• Add double-wrapped error cases to TestIsNotFoundError, TestIsForbiddenError, TestIsGoneError.
• Add boundary value cases (exact "404", partial "404abc") to TestIsNotFoundError.
• Remove redundant failure-message format strings from all three table loops in errors_test.go.
• Add file-header scope comments to errors_test.go and spec_test.go.
• Run make test-unit and confirm all tests pass.

Generated by 🧪 Daily Testify Uber Super Expert · 24.8 AIC · ⌖ 9.95 AIC · ⊞ 5.2K · ◷

• expires on Jul 19, 2026, 10:25 AM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · 3.13 AIC · ⌖ 18.6 AIC · ⊞ 6.8K · ◷