I have created a class that implements behavior which is difficult to test without some intimate knowledge of internal state. I'd like not to clutter the class's public API with accessors for this internal state, and I'd also like to add some friction to other programmers writing code depending on this internal state. So would it be a good idea to do the following?
// SUT:
public class ComplicatedThing
{
// The important behavior
public void DoStuff() { ... }
// Things necessary for tests:
public interface ITestClient
{
int IntimateKnowledge { set; }
}
public void Accept(ITestClient client)
{
client.IntimateKnowledge = ...;
}
}
// Tests:
public class ComplicatedThingClass
{
class Client : ComplicatedThing.ITestClient
{
public int IntimateKnowledge { get; set; } = -1;
}
public class DoStuffMethodShould
{
[Fact]
public void DoComplicatedThings()
{
var thing = new ComplicatedThing();
thing.DoStuff(...);
var client = new Client();
thing.Accept(client);
Assert.Equal(123, client.IntimateKnowledge);
}
}
}
Details
Specifically, I have a class which can asynchronously detect finalization of an object:
public class LifetimeWatcher
{
private readonly ConditionalWeakTable<object, HashSet<...>> _weakTable = ConditionalWeakTable<object, HashSet<...>>();
private int ResidueLevel => _weakTable.Select(kvp => kvp.Value.Count + 1).Sum();
public async ValueTask WaitForFinalizationAsync(
WeakReference<object> weakTarget,
CancellationToken cancellationToken);
}
One of the things that I'd like to test is that no "residue" remains after a call to WaitForFinalizationAsync
completes. Specifically, any entries added to the private ConditionalWeakTable
need to be removed when the function returns. This is easy to quantify with the ResidueLevel
property.
But how do I make that information available to tests without cluttering the public API or making it easy for others to write code which begins depending on this implementation detail?
Not really knowing what else to do I have added the following to my class:
public class LifetimeWatcher
{
// ...
public interface ITestClient
{
int ResidueLevel { set; }
}
public Accept(ITestClient client)
{
client.ResidueLevel = this.ResidueLevel;
}
}
Normal tests are able to interact with instances of LifetimeWatcher
using its normal public API. But now the few tests that need to be concerned with "residue" are able to acquire that implementation detail by jumping through some hoops.
Question
Is this the right thing to do?
Lots of alarms are going off in my head when I do this:
- Tests that depend on implementation details are rigid
- I'm not writing unit tests: my tests are using this implementation detail in conjunction with calls to
GC.Collect()
- My software under test is doing extra stuff only for the benefit of tests. My
LifetimeWatcher
class has things called "test" within it, for crying out loud - I have to create a special class (implementing
LifetimeWatcher.ITestClient
) that will only live in the tests project
But at the same time:
- This class is doing some complicated things and is very sensitive to changes. I want it to be difficult to break this class in important ways, and leaving "residue" is an important thing to prevent
- "Residue" levels are an implementation detail. So if I want to test it then I have no choice but to test an implementation detail. At least I'm not calling
Thread.Sleep(...)
or making HTTP requests in the tests - Having to implement an interface and then having no guarantee that its members will even be populated when you pass it to the
Accept
method certainly makes it difficult for others to depend on implementation details - If tests in the future need additional intimate knowledge then it's easy to add additional setters to the interface
- I can further reduce the API footprint by making the
Accept
method an explicit implementation of some interface and moving theITestClient
interface out to a different file
What's the Better Way™?