This is the third posting in the WEWLC-series.
Other posts are:
I Can't Get This Class into a Test Harness
I don't have much time and I have to change it
Assumptions on the reader
"Getting tests in place to make changes can be a bit of a problem. If you can instantiate your class separately in a test harness, consider yourself lucky."
Instantiating a class should not be hard, if you do have issues have a look at I Can't Get This Class into a Test Harness. That should take care of most of your problems.
But instantiating is often just the first part of all the hurdles to take. The second part is writing a test for the methods inside.
Fortunately, in most cases, the amount of work that we have to do to write tests for methods isn't as drastic. Here are some of the problems that we can run into.
The method may be private
The method may be hard to call because it has parameters that are hard to construct
The method might have bad side effects (modifying a database, writing to the file system etc)
The Case of the Hidden Method
If we need to write a test for a private method we need to make it public. There are some tricks in getting visibility on the method in your tests but I don't recommend it. First of all, if there is a need for private method odds are that what you need is a another collaborator.
This is of course a rather fundamental discussion. If you do care about keeping the method invisible to callers you can make it protected. Once it is protected you can derive a test class and add a public method that will simply call the protected method in the base class.
In most cases I would recommend moving the method to a separate class and inject it into the constructor of the class that interacts with it. Once there is a need to alter the behavior of a private method it is obviously doing something important, hence, it should be under test.
The Case of the "Helpful" Language Feature
Michael Feather is referring to sealed classes that don't have constructor. How do you test interaction code that takes such a class as a parameter in a method? Let's have a look at an example:
public class StreamHelper { const int MIN_LEN = 4; public IList GetStreams(HttpFileCollection files) { var list = new List<Stream>(); foreach (string name in files) { HttpPostedFile file = files[name]; if ((file.FileName.EndsWith(".txt") && file.ContentLength > MIN_LEN)) { list.Add(file.InputStream); } } return list; } }
According to MSDN, the HttpFileCollection provides access to and organizes files uploaded by a client. Since this class is sealed and only has an internal constructor, how can you test this method?
Turns out that HttpFileCollection derives from NameObjectCollectionBase which has a parameterless constructor which is protected. If we thus derive from NameObjectCollectionBase, implement the indexer required and change the signature of the GetStreams method a bit to take our custom class instead of the HttpFileCollection we should be able to get going.
Please note, this is going to be a hefty refactoring. Right, lets get down to it. First let's create a fixture for our tests:
[TestFixture] public class HttpCollectionTests { [Test] public void can_instantiate_a_derived_class_from_NameObjectCollectionBase() { var customHttpFileCollection = new CustomHttpFileCollection(); var collectionBase = customHttpFileCollection as NameObjectCollectionBase; collectionBase.ShouldNotBeNull(); } }
This means deriving from NameObjectCollectionBase
public class CustomHttpFileCollection : NameObjectCollectionBase { }
The next test is to be able to call GetStreams() in StreamHelper. Rather than changing the existing method we are going to create a new method with the same name, but different signature so that we can call it with our new CustomerHttpFileCollection:
[Test] public void can_use_the_custom_class() { var collection = new CustomHttpFileCollection(); new StreamHelper().GetStreams(collection); }
public IList<Stream> GetStreams(CustomHttpFileCollection files) { var list = new List<Stream>(); foreach (string name in files) { HttpPostedFile file = files[name]; if ((file.FileName.EndsWith(".txt") && file.ContentLength > MIN_LEN)) { list.Add(file.InputStream); } } return list; }
This means implementing the indexer used in GetStreams() in our custom class. Luckily Resharper knows what to do and we can use ALT-ENTER to generate the code. Next to the indexer we will also need an IList to store the files and a method to add them to the internal collection:
public class CustomHttpFileCollection : NameObjectCollectionBase { public HttpPostedFile this[string name] { get { return httpPostedFiles.Where(s => s.FileName == name).FirstOrDefault(); } } }
This poses a bit of an issue; we can instantiate a HttpPostedFile so how are we going to populate an internal collection? Since this one is sealed, does not have a suitable base class and has no constructors we need to be creative. We need a custom implementation of the HttpPostedFile. First we create an interface, then define the same members on the interface as are in the HttpPostedFile class. Once that is done we can change the signature of the indexer to IHttpPostedFile and add some logic to store and add the files to the collection class, like so:
public class CustomHttpFileCollection : NameObjectCollectionBase { readonly IList<IHttpPostedFile> httpPostedFiles = new List<IHttpPostedFile>(); public IHttpPostedFile this[string name] { get { return httpPostedFiles.Where(s => s.FileName == name).FirstOrDefault(); } } public void AddHttpPostedFile(IHttpPostedFile file) { httpPostedFiles.Add(file); } }
public interface IHttpPostedFile { int ContentLength { get; } string ContentType { get; } string FileName { get; } Stream InputStream { get; } }
Change the HttpPostedFile to the new IHttpPostedFile in GetStreams():
public IList<Stream> GetStreams(CustomHttpFileCollection files) { var list = new List<Stream>(); foreach (string name in files) { IHttpPostedFile file = files[name]; if ((file.FileName.EndsWith(".txt") && file.ContentLength > MIN_LEN)) { list.Add(file.InputStream); } } return list; }
Now that we have set up the collection, let's write a test to see if it all works
Stream GetStream() { const string s = "hello world"; var memoryStream = new MemoryStream(System.Text.Encoding.Default.GetBytes(s)); return memoryStream; }
[Test] public void can_use_the_customcollection_to_test_the_StreamHelper_class() { var collection = new CustomHttpFileCollection(); var httpPostedFile = MockRepository.GenerateMock<IHttpPostedFile>(); httpPostedFile.Stub(s => s.InputStream).Return(GetStream()); httpPostedFile.Stub(s => s.FileName).Return("somefilename.txt"); httpPostedFile.Stub(s => s.ContentLength).Return(5); collection.AddHttpPostedFile(httpPostedFile); IList<Stream> streams = new StreamHelper().GetStreams(collection); streams.Count.ShouldEqual(1); }
When we execute this test a lot is happening. First I create a mock of the IHttpPostedFile interface so that I can stub the properties. This allows me to test the method without having an actual implementation of the interface. Not that it would have complicated things tremendously, but it's a nice technique.
I also use a convenient helper method to get me a stream. This will come in handy later when I want to verify the outcome of the GetStreams() method.
Let's execute and:
------ Test started: Assembly: Koning.WEWLC.Tests.dll ------
TestCase 'Koning.WEWLC.Tests.HttpCollectionTests.can_use_the_customcollection_to_test_the_StreamHelper_class' failed: Expected: 1 But was: 0 at Machine.Specifications.NUnitShouldExtensionMethods.ShouldEqual(Object actual, Object expected) C:\Dev\Personal\WEWLC\Koning.WEWLC\Koning.WEWLC.Tests\Tests\HttpCollectionTests.cs(59,0): at Koning.WEWLC.Tests.HttpCollectionTests.can_use_the_customcollection_to_test_the_StreamHelper_class()
0 passed, 1 failed, 0 skipped, took 1.37 seconds.
Why is this happening? My bad, I didn't implement GetEnumerator() on the custom collection class. This is easily fixed however:
public class CustomHttpFileCollection : NameObjectCollectionBase, IEnumerable { readonly IList<IHttpPostedFile> httpPostedFiles = new List<IHttpPostedFile>(); public IHttpPostedFile this[string name] { get { return httpPostedFiles.Where(s => s.FileName == name).FirstOrDefault(); } } public new IEnumerator<string> GetEnumerator() { return httpPostedFiles.Select(s => s.FileName).GetEnumerator(); } IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } public void AddHttpPostedFile(IHttpPostedFile file) { httpPostedFiles.Add(file); } }
I'm using some Linq goodness to get to the filenames and iterate over that. Next up is refining our test and run it again:
[Test] public void can_use_the_customcollection_to_test_the_StreamHelper_class() { var collection = new CustomHttpFileCollection(); var httpPostedFile = MockRepository.GenerateMock<IHttpPostedFile>(); httpPostedFile.Stub(s => s.InputStream).Return(GetStream()); httpPostedFile.Stub(s => s.FileName).Return("somefilename.txt"); httpPostedFile.Stub(s => s.ContentLength).Return(5); collection.AddHttpPostedFile(httpPostedFile); IList<Stream> streams = new StreamHelper().GetStreams(collection); streams.Count.ShouldEqual(1); streams.First().Length.ShouldBeGreaterThan(0); new StreamReader(streams.First()).ReadLine().ShouldContain("hello world"); }
1 passed, 0 failed, 0 skipped, took 1.48 seconds.
Now note that when using the GetStreams() method one would have to transform the items in HttpPostedFileCollection to the IHttpPostedFile interface before using it.
The Case of the Undetectable Side Effect
Every so often you encounter code that just makes you mumble WTF way more than is healthy for any sane developer. In most cases things just happen everywhere. When the code is performing a query, it's also updating some other pieces, altering the file system a bit and lastly setting a whole bunch of other objects in an invalid state.
I wouldn't recommend getting the debugger up and running to see what's going on. Most of the time it consumes a lot of time and keeps the frustration really high.
What this kind of code is lacking is proper Command Query Separation (CQS). From the WEWLC book "It's a is a design principle first described by Bertrand Meyer. Simply put, it is this: A method should be a command or a query, but not both. A command is a method that can modify the state of the object but that doesn't return a value. A query is a method that returns a value but that does not modify the object."
You should always strive to adhere to this principle. It will make your code easier to read and more importantly, easier to skip when reading. When I encounter code that does not adhere to CQS I always split it up in several smaller methods, break out objects etc etc. Once that is done, testing immediately becomes easier on the class.
It's a matter of subclassing and overriding any behavior that is unwanted so that the method you like to test can be tested in isolation.
And with the Undetectable side effect covered we end the I can't get this class into a test harness chapter of WEWLC. Hope you enjoyed reading. Drop a line in the comments for feedback, or ideas.