refactor(tests): adopt idiomatic testify assertion methods across test suite#5102
Merged
Conversation
…ic equivalents - Replace assert/require.Contains(t, err.Error(), ...) with assert/require.ErrorContains(t, err, ...) across ~392 sites - Replace assert.Equal(t, 0, len(x)) with assert.Empty(t, x) in launcher tests (6 sites) - Replace assert.Equal(t, N, len(x)) with assert.Len(t, x, N) in multiple test files (11 sites) - Replace assert.True(t, len(url) > 0, ...) with assert.NotEmpty(t, url, ...) (1 site) - Also handles bound asserter style (assert.Contains(err.Error(), ...) and a.Contains(err.Error(), ...)) - Preserves assert.Equal(t, 0, len(rawPayload)%4) modulo checks in oidc tests" Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/5a683381-1ecf-4481-8042-e9c63dbfb267 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Review stretchr/testify Go module for usage and effectiveness
refactor(tests): adopt idiomatic testify assertion methods across test suite
May 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a broad mechanical cleanup of the Go test suite by replacing non-idiomatic testify assertions with the dedicated helpers that the codebase already depends on. It stays within tests and aims to improve nil-safety and failure diagnostics without changing production behavior.
Changes:
- Replaced
Contains(err.Error(), ...)patterns withErrorContains(...)across many test files. - Replaced some
assert.Equal(..., len(x))checks withassert.Len(...)orassert.Empty(...). - Replaced one
len(url) > 0assertion withassert.NotEmpty(...).
Show a summary per file
| File | Description |
|---|---|
| test/integration/proxy_container_test.go | Swaps certificate-error assertion to ErrorContains. |
| internal/sys/docker_test.go | Uses ErrorContains for container ID validation errors. |
| internal/strutil/random_hex_test.go | Uses ErrorContains for invalid size error text. |
| internal/server/write_sink_guard_test.go | Updates bound require error substring assertion. |
| internal/server/system_tools_test.go | Modernizes several tool dispatch error assertions. |
| internal/server/routed_test.go | Replaces map length checks with Len. |
| internal/server/require_guard_policy_test.go | Uses ErrorContains for policy validation error. |
| internal/server/register_tools_from_backend_test.go | Updates backend registration error assertions. |
| internal/server/register_guard_test.go | Updates guard creation error assertions. |
| internal/server/gateway_tls_test.go | Uses ErrorContains for TLS load failures. |
| internal/server/ensure_guard_initialized_test.go | Updates guard init error assertions. |
| internal/server/difc_log_test.go | Uses ErrorContains for filtered error content checks. |
| internal/server/collaborator_permission_test.go | Modernizes collaborator permission error assertions. |
| internal/server/circuit_breaker_test.go | Uses ErrorContains on ErrCircuitOpen. |
| internal/server/call_backend_tool_difc_test.go | Updates DIFC/backend tool error assertions. |
| internal/proxy/tls_test.go | Uses ErrorContains for TLS handshake failure. |
| internal/proxy/rest_backend_caller_tool_test.go | Updates REST backend tool error assertions. |
| internal/proxy/init_guard_policy_test.go | Modernizes guard policy init error assertions. |
| internal/proxy/collaborator_permission_test.go | Uses ErrorContains for collaborator permission errors. |
| internal/oidc/provider_test.go | Updates OIDC provider error assertions. |
| internal/oidc/jwt_expiry_test.go | Replaces length/error checks with Len and ErrorContains. |
| internal/oidc/errors_test.go | Uses ErrorContains for composed OIDC env error. |
| internal/middleware/jqschema_test.go | Updates jq schema timeout/cancel error assertions. |
| internal/middleware/jqschema_coverage_test.go | Uses ErrorContains for file operation failures. |
| internal/mcp/unmarshal_params_test.go | Modernizes param unmarshal error assertions. |
| internal/mcp/tool_result_test.go | Uses ErrorContains for parse/marshal failures. |
| internal/mcp/sdk_method_dispatch_test.go | Updates SDK dispatch/session error assertions. |
| internal/mcp/marshal_helper_test.go | Uses ErrorContains for marshal failures. |
| internal/mcp/http_transport_test.go | Modernizes SSE/HTTP/OIDC transport error assertions. |
| internal/mcp/http_connection_test.go | Updates HTTP connection error assertions. |
| internal/mcp/connection_test.go | Uses ErrorContains across connection/pagination tests. |
| internal/mcp/connection_stderr_test.go | Updates unsupported-method assertion. |
| internal/mcp/collaborator_permission_test.go | Uses ErrorContains for arg validation errors. |
| internal/logger/tools_logger_test.go | Updates file write/rename error assertions. |
| internal/logger/jsonl_logger_test.go | Uses ErrorContains for logger failure paths. |
| internal/logger/helper_functions_test.go | Replaces line count equality with Len. |
| internal/logger/global_helpers_test.go | Uses ErrorContains for closer ordering assertion. |
| internal/logger/common_test.go | Modernizes log init error assertions. |
| internal/launcher/launcher_test.go | Updates launcher errors and one connection count assertion. |
| internal/launcher/getorlaunchforsession_test.go | Uses ErrorContains for launch failure. |
| internal/launcher/getorlaunch_timeout_test.go | Replaces timeout/count assertions with idiomatic helpers. |
| internal/launcher/getorlaunch_stdio_test.go | Modernizes launch error and empty-map assertions. |
| internal/guard/wasm_validate_test.go | Uses ErrorContains for validation failures. |
| internal/guard/wasm_test.go | Updates many WASM/policy parsing error assertions. |
| internal/guard/wasm_response_parse_test.go | Uses ErrorContains in parse error cases. |
| internal/guard/wasm_parse_test.go | Updates label-agent parse error assertions. |
| internal/guard/policy_helpers_test.go | Uses ErrorContains for policy helper errors. |
| internal/guard/init_test.go | Modernizes label-agent init error assertions. |
| internal/guard/guard_test.go | Updates guard/payload validation error assertions. |
| internal/difc/path_labels_coverage_test.go | Uses ErrorContains for path-label error cases. |
| internal/config/validation_test.go | Modernizes auth config validation error checks. |
| internal/config/validation_string_patterns_test.go | Updates string-pattern validation error assertions. |
| internal/config/validation_schema_test.go | Uses ErrorContains for schema validation errors. |
| internal/config/validation_schema_fetch_test.go | Modernizes schema fetch/parse/count assertions. |
| internal/config/validation_gateway_coverage_test.go | Updates gateway config validation error assertions. |
| internal/config/validation_env_test.go | Uses ErrorContains on EnvValidationResult and related errors. |
| internal/config/validate_against_custom_schema_test.go | Modernizes custom-schema validation errors. |
| internal/config/rules/rules_test.go | Uses ErrorContains for rule validation errors. |
| internal/config/guard_policy_unmarshal_coverage_test.go | Updates guard policy parse/normalize error assertions. |
| internal/config/guard_policy_test.go | Modernizes guard policy normalization/unmarshal assertions. |
| internal/config/guard_policy_parse_test.go | Uses ErrorContains for parse/build failures. |
| internal/config/fetch_and_fix_schema_test.go | Updates invalid JSON schema assertion. |
| internal/config/expand_raw_json_test.go | Modernizes JSON/env/mount validation error checks. |
| internal/config/docker_helpers_and_env_test.go | Uses ErrorContains for env helper failures. |
| internal/config/custom_types_test.go | Updates custom type validation error assertions. |
| internal/config/config_tracing_test.go | Uses ErrorContains for tracing validation failures. |
| internal/config/config_test.go | Modernizes config load/validation error assertions. |
| internal/config/config_stdin_test.go | Updates stdin conversion/validation error assertions. |
| internal/config/config_guardpolicies_test.go | Uses ErrorContains for write-sink validation errors. |
| internal/config/config_difc_test.go | Modernizes DIFC/guard policy parse error assertions. |
| internal/config/config_core_test.go | Updates TOML/config error assertions. |
| internal/cmd/stdout_config_test.go | Uses ErrorContains and NotEmpty in stdout config tests. |
| internal/cmd/resolve_guard_policy_test.go | Modernizes CLI guard policy error assertions. |
| internal/cmd/proxy_test.go | Uses ErrorContains for newline rejection. |
| internal/cmd/flags_difc_test.go | Updates DIFC mode flag validation assertion. |
| internal/cmd/completion_test.go | Uses ErrorContains for unsupported shell errors. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 76/76 changed files
- Comments generated: 0
This was referenced May 4, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test suite had widespread use of assertion anti-patterns identified in the Go Fan report for
stretchr/testify. ~400 mechanical replacements across 76 test files to align with idiomatic testify usage.Changes
ErrorContains(~392 sites):assert/require.Contains(t, err.Error(), "msg")→assert/require.ErrorContains(t, err, "msg"). Covers both t-passing and bound asserter styles (assert.Contains(err.Error(), ...),a.Contains(err.Error(), ...)).ErrorContainsis nil-safe and produces better failure output.assert.Empty(6 sites):assert.Equal(t, 0, len(x))→assert.Empty(t, x). Modulo checks (len(x)%4) are intentionally preserved.assert.Len(11 sites):assert.Equal(t, N, len(x))→assert.Len(t, x, N). Shows actual contents on failure.assert.NotEmpty(1 site):assert.True(t, len(url) > 0, "...")→assert.NotEmpty(t, url, "...").Example
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build809957896/b509/launcher.test /tmp/go-build809957896/b509/launcher.test -test.testlogfile=/tmp/go-build809957896/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas olang.org/protob-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 6855�� rg/x/net@v0.52.0-errorsas cfg 64/pkg/tool/linu-nilfunc --gdwarf-5 g/protobuf/refle/usr/bin/runc -o 64/pkg/tool/linu-tests(dns block)/tmp/go-build3608143029/b513/launcher.test /tmp/go-build3608143029/b513/launcher.test -test.testlogfile=/tmp/go-build3608143029/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s sPat�� /opt/hostedtoolcache/go/1.25.9/x64/src/net -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -uns�� -unreachable=fal-errorsas /tmp/go-build809-ifaceassert x_amd64/vet g_.a 6855492/b151/ /usr/local/.ghcu-test.paniconexit0 x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build809957896/b491/config.test /tmp/go-build809957896/b491/config.test -test.testlogfile=/tmp/go-build809957896/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true pare.go mat.go x_amd64/vet 6855492/b151/ base -o x_amd64/vet -qui�� gy1CzlV8t /tmp/go-build299-ifaceassert .13/x64/bin/as . .io/otel/metric x86_64-linux-gnu-bool ortcfg(dns block)/tmp/go-build3608143029/b495/config.test /tmp/go-build3608143029/b495/config.test -test.testlogfile=/tmp/go-build3608143029/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3155913750/b520/vet.cfg /opt/hostedtoolcache/go/1.25.9/x64/src/net tmain.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -I 957896/b491/config.test -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)nonexistent.local/tmp/go-build809957896/b509/launcher.test /tmp/go-build809957896/b509/launcher.test -test.testlogfile=/tmp/go-build809957896/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas olang.org/protob-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 6855�� rg/x/net@v0.52.0-errorsas cfg 64/pkg/tool/linu-nilfunc --gdwarf-5 g/protobuf/refle/usr/bin/runc -o 64/pkg/tool/linu-tests(dns block)/tmp/go-build3608143029/b513/launcher.test /tmp/go-build3608143029/b513/launcher.test -test.testlogfile=/tmp/go-build3608143029/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s sPat�� /opt/hostedtoolcache/go/1.25.9/x64/src/net -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -uns�� -unreachable=fal-errorsas /tmp/go-build809-ifaceassert x_amd64/vet g_.a 6855492/b151/ /usr/local/.ghcu-test.paniconexit0 x_amd64/vet(dns block)slow.example.com/tmp/go-build809957896/b509/launcher.test /tmp/go-build809957896/b509/launcher.test -test.testlogfile=/tmp/go-build809957896/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/c-errorsas olang.org/protob-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet 6855�� rg/x/net@v0.52.0-errorsas cfg 64/pkg/tool/linu-nilfunc --gdwarf-5 g/protobuf/refle/usr/bin/runc -o 64/pkg/tool/linu-tests(dns block)/tmp/go-build3608143029/b513/launcher.test /tmp/go-build3608143029/b513/launcher.test -test.testlogfile=/tmp/go-build3608143029/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s sPat�� /opt/hostedtoolcache/go/1.25.9/x64/src/net -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -uns�� -unreachable=fal-errorsas /tmp/go-build809-ifaceassert x_amd64/vet g_.a 6855492/b151/ /usr/local/.ghcu-test.paniconexit0 x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build809957896/b518/mcp.test /tmp/go-build809957896/b518/mcp.test -test.testlogfile=/tmp/go-build809957896/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rg/x/net@v0.52.0-errorsas 64/src/runtime/c-ifaceassert x_amd64/vet --gdwarf-5 g/protobuf/inter-qE -o x_amd64/vet -o /go-build -trimpath x_amd64/vet -p 64/src/runtime/c/usr/bin/runc -lang=go1.24 x_amd64/vet(dns block)/tmp/go-build3608143029/b522/mcp.test /tmp/go-build3608143029/b522/mcp.test -test.testlogfile=/tmp/go-build3608143029/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet -ato�� -bool -buildtags x_amd64/vet -errorsas -ifaceassert -nilfunc x_amd64/vet(dns block)If you need me to access, download, or install something from one of these locations, you can either: