Unit testing a class that tracks state

Question

I am abstracting the history tracking portion of a class of mine so that it looks like this:

private readonly Stack<MyObject> _pastHistory = new Stack<MyObject>();

internal virtual Boolean IsAnyHistory { get { return _pastHistory.Any(); } }

internal virtual void AddObjectToHistory(MyObject myObject)
{
  if (myObject == null) throw new ArgumentNullException("myObject");
  _pastHistory.Push(myObject);
}

internal virtual MyObject RemoveLastObject()
{
  if(!IsAnyHistory) throw new InvalidOperationException("There is no previous history.");
  return _pastHistory.Pop();
}

My problem is that I would like to unit test that Remove will return the last Added object.

• AddObjectToHistory
• RemoveObjectToHistory -> returns what was put in via AddObjectToHistory

However, it isn't really a unit test if I have to call Add first? But, the only way that I can see to do this in a true unit test way is to pass in the Stack object in the constructor OR mock out IsAnyHistory...but mocking my SUT is odd also. So, my question is, from a dogmatic view is this a unit test? If not, how do I clean it up...is constructor injection my only way? It just seems like a stretch to have to pass in a simple object? Is it ok to push even this simple object out to be injected?

Answer 1

There are two approaches to those scenarios:

1. Interfere into design, like making _pastHistory internal/protected or injecting stack
2. Use other (possibly unit tested) methods to perform verification

As always, there is no golden rule, although I'd say you generally should avoid situations where unit tests force design changes (as those changes will most likely introduce ambiguity/unnecessary questions to code consumers).

Nonetheless, in the end it is you who has to weigh how much you want unit test code interfere into design (first case) or bend the perfect unit test definition (second case).

Usually, I find second case much more appealing - it doesn't clutter original class code and you'll most likely have Add already tested - it's safe to rely on it.

Answer 2

I think it's still a unit test, assuming MyObject is a simple object. I often construct input parameters to unit test methods.

I use Michael Feather's unit test criteria:

A test is not a unit test if:

• It talks to the database
• It communicates across the network
• It touches the file system
• It can't run at the same time as any of your other unit tests
• You have to do special things to your environment (such as editing config files) to run it.

Tests that do these things aren't bad. Often they are worth writing, and they can be written in a unit test harness. However, it is important to be able to separate them from true unit tests so that we can keep a set of tests that we can run fast whenever we make our changes.

Answer 3

My 2 cents... how would the client know if remove worked or not ? How is a 'client' supposed to interact with this object? Are clients going to push in a stack to the history tracker? Treat the test as just another user/consumer/client of the test subject.. using exactly the same interaction as in real production. I haven't heard of any rule stating that you're not allowed to call multiple methods on the object under test.

To simulate, stack is not empty. I'd just call Add - 99% case. I'd refrain from destroying the encapsulation of that object.. Treat objects like people (I think I read that in Object Thinking). Tell them to do stuff.. don't break-in and enter.

e.g. If you want someone to have some money in their wallet,

• the simple way is to give them the money and let them internally put it into their wallet.
• throw their wallet away and stuff in a wallet in their pocket.

I like Option1. Also see how it frees you from implementation details (which induce brittleness in tests). Let's say tomorrow the person decides to use an online wallet. The latter approach will break your tests - they will need to be updated for pushing in an online wallet now - even though the object behavior is not broken.

Another example I've seen is for testing Repository.GetX() where people break-in to the DB to inject records with SQL now in the unit test.. where it would have be considerably cleaner and easier to call Repository.AddX(x) first. Isolation is desired but not to the extent that it overrides pragmatism.

I hope I didn't come on too strong here.. it just pains me to see object APIs being 'contorted for testability' to the point where it no longer resembles the 'simplest thing that could work'.