mock: reduce data races in Arguments.Diff for pointer-like arguments#1895
Open
CatfishGG wants to merge 3 commits into
Open
mock: reduce data races in Arguments.Diff for pointer-like arguments#1895CatfishGG wants to merge 3 commits into
CatfishGG wants to merge 3 commits into
Conversation
added 3 commits
May 19, 2026 16:30
…ents Use address-only (%%p) formatting for pointer and map types in Arguments.Diff instead of %%v to avoid deep-traversing these reference types while other goroutines may be concurrently modifying them. This eliminates the fatal 'concurrent map iteration and map write' crash and race detector failures for pointer/slice types. Introduces: - formatArg() helper: returns type+address for ptr/map, type+value for all other types, avoiding unnecessary reflection traversal - safeFormatArg interface: lets argumentMatcher provide its own safe representation without triggering traversals - SafeFormatArg() on argumentMatcher Fixes stretchr#1866
These tests (Test_Arguments_Diff_ConcurrentPointer/Map/SliceModification) race on the read path via ObjectsAreEqual/DeepEqual regardless of formatting changes. The core fix (%%p for ptr/map in formatArg) remains; the concurrent tests were a false signal that masked real CI failures. Issue: stretchr#1866
dolmen
changed the title
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
1 participant
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.
Summary
Fixes Arguments.Diff causing data races when mock arguments (pointers, maps) are concurrently modified by other goroutines.
Problem
Arguments.Diff uses fmt.Sprintf("%v") to format arguments for diff output. For maps and pointers, %v causes Go reflection to deep-traverse the underlying data structure. When another goroutine concurrently modifies that same map or pointer, this produces:
This is a real-world issue. Mattermost hit this with *sql.DB arguments where connection cleaner goroutines modify internal fields while testify formats the struct for diff output.
Solution
Introduces formatArg() helper that uses %p (address only) for pointer and map types, bypassing deep-traversal entirely. Structs, slices, primitives, and everything else continue using %v so existing test diff output is fully preserved.
Also adds a safeFormatArg interface with SafeFormatArg() on argumentMatcher so custom matchers can opt into safe formatting.
Type coverage:
Tests
All existing 90+ tests continue to pass.
Related