stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
22.5k stars 1.56k forks source link

mock: avoid data races in Arguments.Diff #1598

Open petergardfjall opened 1 month ago

petergardfjall commented 1 month ago

Fixes a concurrency issue that would lead to testify mocks producing data races detected by go test -race. These data races would occur whenever a mock pointer argument was concurrently modified. The reason being that Arguments.Diff uses the %v format specifier to get a presentable string for the argument. This also traverses the pointed-to data structure, which would lead to the data race.

It should be noted that it is heavily inspired by https://github.com/stretchr/testify/pull/1229, but since that PR hasn't seen any updates in over a year I decided to start fresh.

Summary

Avoids testify mocks causing data races for (concurrently accessed) pointer arguments when running go test -race.

Changes

Arguments.Diff now uses %p to produce a "presentable string" for pointers rather than %v, since the latter traverses the pointed to data structure.

Motivation

We need to test our code for race conditions with go test -race, and we cannot have testify be the offender of data races.

Related issues

https://github.com/stretchr/testify/issues/1597

petergardfjall commented 1 month ago

Friendly reminder. :slightly_smiling_face:

petergardfjall commented 2 weeks ago

Any chance of this being accepted?