This is a class I'm a bit concerned about. My goal is to unit test the addresses list:
public class LabelPrinter
{
private readonly IEnumerable<Address> _addresses;
public LabelPrinter(IEnumerable<Address> addresses)
{
_addresses = addresses;
}
public Document Create()
{
// ... Generate PDF, etc ...
}
}
So what is best:
- Use reflection to inspect the private property, or
- Since the original IEnumerable can be modified from outside anyway, make a public getter and test it instead?
In general, private members shouldn't be unit tested, since anything the class is doing with it's private members should somehow be reflected in the externally testable behavior of the object. In other words, who cares what's going on in there, as long as its external behavior is what it should be.
Unit testing private members also couples your tests to the internals of a class, making them more brittle. If you decide to use a more efficient collection later down the road, your tests will break, even though the behavior of the object hasn't changed. You especially want to avoid reflection, since looking up properties by name means your tests break if the property name ever changes.
In other words - if you need to test the Address
class, do it from its own unit tests, rather than from the LabelPrinter
's tests. If you must use one of your two methods, use the second one, rather than reflection.
What are you trying to test about the addresses
list here? In the example code you've provided above your job is actually rather easy because you can inject the list through your constructor. So in your tests you can access the list as such and therefore don't necessarily need to expose it again:
[Test]
public void Test()
{
IEnumerable<Address> addresses = new List<Address>();
LabelPrinter printer = new LabelPrinter(addresses);
... // execute your test here
Assert.AreEqual(blah, addresses.get(0));
// or any other assertion you want to make on the addresses list
// created up above.
}
Particularly while learning unit testing, concerns about keeping fields private are trumped by easy testing and better coverage. Option 2.
Test Create
and not the setter (which is effectively what you have here). I find testing setters/getters to be a bit of a waste of time. Esp. as most of the time the setter will have to be executed for some other test to work. They are also for the most part too simple to fail.
So rather than validate that LabelPrinter
has a _addresses
and it is Y, check that the output of Create
includes the relevant details.