[llvm] r211080 - Expose ValueMap's mutex type as a typedef instead of a sys::Mutex.

[Rafael Espíndola]

On 17 June 2014 14:31, Zachary Turner <zturner at google.com> wrote:
> True, but isn't that the point of a unit test?

Sometimes :-)

For llvm the main value of unit tests IMHO is checking code that is
not easily visible from an existing tool. So it is testing code as is
used by the rest of the system, it is just taking a closer look at
what that code is doing.

Sometimes we do use them to test functionally that is not used by any
in tree code, but that is rare. This is why I am suspicious that in
this case the unit test is just testing a dead functionality and
should be changed to use the same mutex as the rest of the code.

> A unit test shouldn't depend
> on, or have any knowledge of how a class is used, only of how a class *can
> be* used.  We don't know what code will be added in the future, and we
> shouldn't require someone who uses the class in a different way to go write
> a new unit test.  The unit test should provide coverage of every way the
> class supports being used.
>
> That being said, I don't think any unit tests should even use a mutex to
> begin with, because thread safety is inherently not unit-testable IMO, and
> needs different types of testing.

Cheers,
Rafael