Exposing implementation details to tests

Question

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 the ITestClient interface out to a different file

What's the Better Way™?

Answer 1

Unit tests don't care about internal state

I have created a class that implements behavior which is difficult to test without some intimate knowledge of internal state.

This is a contradiction. When talking about testing, behavior is specifically defined as public behavior, i.e. what is externally visible. Internal state is pretty much the exact opposite.
This doesn't nullify the question, but it does reframe both your expectations and your question.

If your public behavior can only be observed using internal state, then that which is internal state should be part of the public behavior.

To give a simple example, suppose we create a simple addition tool:

public class Adder
{
    public int Add(int a, int b)
    {
        // magic
    }
}

I say magic to prove the point. You're only made aware of the public behavior of this class, not the internal behavior. How do we test that this adder works? A simple test would be:

public void TestAdder()
{
    var adder = new Adder();

    Assert.AreEqual(adder.Add(1,1), 2);
    Assert.AreEqual(adder.Add(1,2), 3);
    Assert.AreEqual(adder.Add(2,1), 3);
}

This is provably a working test, yet you did not know how the adder actually works internally. Maybe it does simple + addition, maybe it uses a library, maybe it asks Google, maybe it posts a question to a math forum and parses the answer, maybe it emails me personally to ask for the answer, maybe...

The purpose of unit testing is so that you are able to completely change the internal working of your class, while being able to run the same test to confirm that your changes did not break the class.

If your test relies on the internal state (or knowledge thereof), it wouldn't be a useful unit test as it would inherently break when the (affected) internals change.

---

How to test the outcome

public class ComplicatedThing
{
    // The important behavior
    public void DoStuff() { ... }
}

The lasting effects of having called DoStuff() (i.e. the changes that DoStuff() caused) fit into one of two categories:

• Internal state
• Public behavior, which falls into one of two subcategories:
  • Returning a value
  • Interacting with dependencies

Since you didnt show us exactly what it is you intend to test and where this effect can be found, I'll list how to approach either possibility.

Internal state, for example:

public class ComplicatedThing
{
    private string _privateField = "joke";

    public void DoStuff()
    {
        _privateField = "secret";
    }
}

Very simply put, this should not be tested. Knowing whether this private value is "joke" or secret" is not part of the public behavior, and therefore it doesn't matter for a unit test.

However, it is possible (and indeed likely) that this internal state in some way impacts other external behavior, for example:

public class ComplicatedThing
{
    private string _privateField = "joke";

    public void DoStuff()
    {
        _privateField = "secret";
    }

    public string TellMeSomething()
    {
        return $"I know a {_privateField}";
    }
}

This public behavior (TellMeSomething()) can and should be tested, as it is part of the public contract. You'd expect a test along the lines of:

public void Tells_me_it_knows_a_joke_without_doing_stuff()
{
    // Arrange
    var thing = new ComplicatedThing();

    // Act
    var result = thing.TellMeSomething();

    // Assert
    Assert.AreEqual(result, "I know a joke");
}

public void Tells_me_it_knows_a_secret_after_doing_stuff()
{
    // Arrange
    var thing = new ComplicatedThing();

    // Act
    thing.DoStuff();
    var result = thing.TellMeSomething();

    // Assert
    Assert.AreEqual(result, "I know a secret");
}

But I do need to stress here that you should not expand the external behavior just because you want to test something. The justification of a behavior's existence should be that you need it for the codebase's purpose, not just so you can test it.

Test that which is public behavior. Don't write public behavior so you can test.

Returning a value

The test for TellMeSomething() that I just showed you actually already answers this. You simply observe the returned result and assert that it conforms to your expectations.

Interacting with dependencies, for example:

public class ComplicatedThing
{
    private Foo _foo;

    public ComplicatedThing(Foo foo)
    {
        _foo = foo;
    }

    public void DoStuff()
    {
        _foo.PostMessage("secret");
    }
}

This is a dependency that's injected in the constructor, but you can also have dependencies that are passed as method parameters:

public class ComplicatedThing
{
    public void DoStuff(Foo foo)
    {
        foo.PostMessage("secret");
    }
}

For the current topic, these are equally valid dependencies. The rest of the answer applies in either case.

In a unit test, all dependencies should be mocked. The nice thing about this is that your unit test can therefore create a mocked object that secretly records what happens to it, so you can find out afterwards if certain methods were called with certain parameters.

I tend to use NSubstitute for this, but other mocking frameworks exist, and you could also do this by yourself but it'd take more time.

You would test this (using NSubstitute syntax) as follows:

public void Posts_secret_message_to_foo()
{
    // Arrange
    var mockedFoo = Substitute.For<Foo>();        // this is a FAKE foo!
    var thing = new ComplicatedThing(mockedFoo);  // we make thing use the fake foo

    // Act
    thing.DoStuff();                              // thing doesn't know it's using a fake foo

    // Assert                                     // our fake foo secretly recorded how it was being handled by thing
    mockedFoo.Received(1).PostMessage("secret");
}

The assert tests two things:

• It ensures that PostMessage was called exactly once
• It ensures that the method parameter passed to PostMessage was equal to "secret"

Obviously, these asserts can be changed to what you actually need to assert.

---

Conclusion

In this answer are all the possible effects that calling a certain method can have, and how to test each of them.

Combined with the knowledge that unit tests should never care about internal state (as this violates the concept of what a unit test tries to achieve), then we've eliminated all possibilities.

The approach you suggest taking is not correct. Depending on where the mistake/misconception lies, one of the given solutions is the appropriate one. To summarize:

• This should indeed be internal state => then don't test it.
• This should have been a return value => test the return value
• This should have caused changes to take place in dependencies => mock the dependencies and assert that they were interacted with in the way you expect them to be

Answer 2

Decorate your class under test with the InternalsVisibleToAttribute, citing your test assembly. Your tests will have access to the class's internal members, and you won't have to pollute your API with irrelevant details.

Answer 3

I'd first and foremost wager even as a starting point if this is the right way to go about it.

It feels to me that you are trying to "force yourself" to unit test a flow that probably isn't directly linked to any "unit" (read, function) and thus, should not be unit tested to start with.

If it'd be me, I'd find a way to re-structure the design so you can more easily stub sensible pieces of code for example, or that you can somehow ensure it's correctness at every level by ensuring correctness of the surrounding code. As in: if I unit test this and it passes, and, that other thing and it also passes, then surely means that this internal logic works, or else, "that other thing" would say A instead of B. Something like this, kind of assurance by proxy.

Also, I don't know exactly how true this holds: but it feels like a smell to me if you need to change production code to support something in the tests.

Answer 4

If it's possible split ComplicatedThing into smaller Thing's which will expose these states for tests.