views:

627

answers:

4

So I have a factory class and I'm trying to work out what the unit tests should do. From this question I could verify that the interface returned is of a particular concrete type that I would expect.

What should I check for if the factory is returning concrete types (because there is no need - at the moment - for interfaces to be used)? Currently I'm doing something like the following:

[Test]
public void CreateSomeClassWithDependencies()
{
    // m_factory is instantiated in the SetUp method
    var someClass = m_factory.CreateSomeClassWithDependencies();

    Assert.IsNotNull(someClass);
}

The problem with this is that the Assert.IsNotNull seems somewhat redundant.

Also, my factory method might be setting up the dependencies of that particular class like so:

public SomeClass CreateSomeClassWithDependencies()
{
    return new SomeClass(CreateADependency(), CreateAnotherDependency(),
                         CreateAThirdDependency());
}

And I want to make sure that my factory method sets up all these dependencies correctly. Is there no other way to do this then to make those dependencies public/internal properties which I then check for in the unit test? (I'm not a big fan of modifying the test subjects to suit the testing)

Edit: In response to Robert Harvey's question, I'm using NUnit as my unit testing framework (but I wouldn't have thought that it would make too much of a difference)

+1  A: 

What we do is create the dependancies with factories, and we use a dependancy injection framework to substitute mock factories for the real ones when the test is run. Then we set up the appropriate expectations on those mock factories.

Robert
A: 

You can always check stuff with reflection. There is no need to expose something just for unit tests. I find it quite rare that I need to reach in with reflection and it may be a sign of bad design.

Looking at your sample code, yes the Assert not null seems redundant, depending on the way you designed your factory, some will return null objects from the factory as opposed to exceptioning out.

Sam Saffron
+3  A: 

If the factory is returning concrete types, and you're guaranteeing that your factory always returns a concrete type, and not null, then no, there isn't too much value in the test. It does allows you to make sure, over time that this expectation isn't violated, and things like exceptions aren't thrown.

This style of test simply makes sure that, as you make changes in the future, your factory behaviour won't change without you knowing.

If your language supports it, for your dependencies, you can use reflection. This isn't always the easiest to maintain, and couples your tests very tightly to your implementation. You have to decide if that's acceptable. This approach tends to be very brittle.

But you really seem to be trying to separate which classes are constructed, from how the constructors are called. You might just be better off with using a DI framework to get that kind of flexibility.

By new-ing up all your types as you need them, you don't give yourself many seams (a seam is a place where you can alter behaviour in your program without editing in that place) to work with.

With the example as you give it though, you could derive a class from the factory. Then override / mock CreateADependency(), CreateAnotherDependency() and CreateAThirdDependency(). Now when you call CreateSomeClassWithDependencies(), you are able to sense whether or not the correct dependencies were created.

Note: the definition of "seam" comes from Michael Feather's book, "Working Effectively with Legacy Code". It contains examples of many techniques to add testability to untested code. You may find it very useful.

Nader Shirazie
+1 I like this answer
Sam Saffron
+9  A: 

Often, there's nothing wrong with creating public properties that can be used for state-based testing. Yes: It's code you created to enable a test scenario, but does it hurt your API? Is it conceivable that other clients would find the same property useful later on?

There's a fine line between test-specific code and Test-Driven Design. We shouldn't introduce code that has no other potential than to satisfy a testing requirement, but it's quite alright to introduce new code that follow generally accepted design principles. We let the testing drive our design - that's why we call it TDD :)

Adding one or more properties to a class to give the user a better possibility of inspecting that class is, in my opinion, often a reasonable thing to do, so I don't think you should dismiss introducing such properties.

Apart from that, I second nader's answer :)

Mark Seemann
+1 for "let testing drive our design". Using properties to expose dependencies helps both configure the dependencies, and make them obvious. Both are good things.
Nader Shirazie
@nader: Well put - that was what I was trying to say :)
Mark Seemann
+1 So often I need to make something a public property so that I can unit test only to find that I needed it later in the code and other people also needed it.
uriDium
+1 had the same question about introducing properties to the API to enable UT.
Thomas Jaskula