Unit Tests are fun

Unit Tests are something that helps us breathe. Save hours for QA's so that they can focus on validating solution against their deepest fetishes, crazy ideas and bussiness requirements. Without them testing any implementation begins with confirming that the super hacky programmer didn't screw up the fluently working scenarion that were implemented last time.

Some programmers don't know how to write tests, some don't like doing that. But there are people like Uncle Bob, who can take your breath away speaking about testing. They make you believe, they make you wish to have that sweet automatic coverage, so you can push your commits as soon as all tests are green. That gives courage and will to refactor. Unit Tests are (as for me) the ultimate mean for continuous improvement, and that is fun!

So what are those wet dreams about? Recently I came back to my my Movie Organizer project. I craved for ad free subtitle downloader that I can run from console, so having all the subsystems in place I decided to go for it. This won't be about implementation, but improvements. When the SubtitleDownloader and it's Unit Tests were ready I got back there to mess around a little. For the sake of this article I'll focus only on the last test that was verifying the outcome. It required quite a lot of setting up (making this more like an Integration Testing) and some test data on side.

SubtitleDownloaderTests.cs

Looks messy, doesnt't it? So what do we have here?

• Test data
• Test scenario(Fact)
• Lots of nasty, Moq specific setup code
• Even more of validation code

That's not exactly object oriented approach, even though all the methods are meaningful, they're just colorful packages full of nasty laziness.
So how we improve?

First thing I looked into was my SpyStream. It's only used by mock of IFileHelper to prevent it from failure while writing subtitle contents. That gave me the idea of breaking test class apart and instead of using Mock directly with generics, configuring it like it's object from outer space, I started building on top of it.
After happily deriving from Mock I realized that that's not the game I'm aiming for. Entire Mock's interface was exposed, leaving me with the mess I've had.
Next idea yielded something more interesting. FileHelperMock became an adapter.

FileHelperMock.cs

That separated me from vast amount of possibilities, that intellisense was flooding me with, every time I typed a dot after _fileHelperMock. This mock now contains only what's important for me to verify, encapsulates trivial setup and maintains life of the stream. It's not much in context of this single mock, but after refactoring all of them - there was some space for light to come through.
Below is how the Mock became bussiness related object

RpcClientMock.cs

Test class after this refactor.

SubtitleDownloaderTests.cs

Not bad. But still test scenario is polluted with all the responses and constant data. I also wanted to move out that Func that's responsible for verification of the outcome. For now I just extracted it all to separate data class to keep it in one place.

Test class without the test data.

SubtitleDownloaderTests.cs

That solved the problem of overfilled test class, but data provider class still gives me itches. My guess at this point is further extraction. I remember Uncle Bob doing 'extract till you drop' thing in one of his videos. This made me laugh, but also realize that there's huge benefit in making all the methods/classes as small as possible. After year in huge legacy project I understood that readible and self explainatory code is a gem that we should care about each time the opportunity is given. But that's something for the next session.

Unit Testing and NullArgumentException Assertions

One of the coolest things about TDD is that you don't have to write any code before you know the reason, and some reasons can be quite trivial... Like the NullArgumentException checks on constructor. I really like the idea of Fail-fast design so I can watch debugger less often so I always do those assertions. How does it look for average middle layer class?

Part of RpcClientTest.cs

And this is alright, you can clearly see what those tests are for and they serve a good purpose. But wait. Can we do it shorter? Can it take less time? When I modify parameters on ctor I'll have to change all of them.

Now don't be sad. Wanna do it that way?

Part of RpcClientTest.cs after change

Hold on. How does it work?

Well quite simply. Method ThrowsOnAnyNullArgument takes class Type as generic parameter and array of valid, ordered params that will let constructor be invoked correctly. Behind the scenes appropriate constructor of given Type is selected, based on number and Type of parameters. After that it is being invoked for each given parameter to check if replacing current one with null will throw NullArgumentException.

That replaces three tests with just one still doing its job properly. Here's first version of it's code with covering tests.

ConstructorAssertTest.cs

ConstructorAssert.cs

Switching on Type

One of the first things I've learned long time before my journey with C# was a conditional statement. "If-else" really makes life simple. When I faced more complex scenarios, swich kicked it, great thing for simple factories or in my case ViewTemplateSelector. A class that contains all the views and provides DataTemplate to display current view model in.

The problem was that view is selected by view models type and switch built in C# doesn't support switching on types. What to do, what to do...Lets do a type switch!

Looking on original switch statement gives pretty clear idea how it should work.

• First of all there's an object to switch on, however I'll have it last, supplying with Execute method after everything is set.
• Set? Right there are the cases. So what to do when Type is matched.
• What if is not? Oh, the default case.
• Do I need to fall through the cases? I can't see a reason to do it here.

And thats what I came up with. default case is called fallback because it felt more like fallback value used when binding in XAML than actually default action, it's kind of last stand where you usually don't want to go in scenario I described. So I made it optional. If that ViewTemplateSelector won't be able to find view for my view model than I consider it as a mistake and don't want to let it go unnoticed. So by default, when there's no match exception will be thrown. If exception is not what you want, constructor allows you to set case for fallback.
Each case is set in dictionary and simply retrieved by key when executing switch, then it returns result of invoked Func. When case is not present, fallback is attempted and if that doesn't exist, we have exception. And thats it. All the code below.

TypeSwitch.cs

TypeSwitchTest.cs

ViewSelector.cs

MVVM - Improving RelayCommand

One of the simpliest and ingenious things separating views from view models is ICommand. Three simple methods this interface provide enough functionality to satisfy most of user input scenarios, on top of that we receive full control during unit testing. RelayCommand is simple implementation of that interface. Usually taking Action to execute and Predicate for CanExecute check. Those two probably won't change because there's nothing specific about them. What already changed is the way that CanExecuteChanged is raised.

With WPF applications we have CommandManager.RequerySuggested event inside System.Windows.Input namespace. That one is being raised when user interacts with UI.

If you set the breakpoint within your condition and debug it, you'll see that it's being hit almost instantly after you get focus on UI. Brutal but easy to use.

Now when I was trying to do the same in Windows Phone 8.1 app, surprise came at once. CommandManager doesn't exist anymore. Vanished. Oh no, what now. I did some quick search on the net and the most obvious solution appeared. Right now you can simply expose invocator from your command and call it from your ViewModel. Wow!

Yea, what a mess. Alright, I don't want it. So I hooked up on what I already have. My ViewModel already have INotifyPropertyChanged implemented on it. Why not. I derieved from RelayCommand and created WatchingRelayCommand. As the name says - it's watching. Watching the properties that can affect its execution. All I need to do is to pass context and the property names. It's attaching itself to INotifyPropertyChanged on context and each time when some property changes on the ViewModel, command checks if it's interested in that property and if so raises CanExecuteChanged.

On top of that small test ViewModel and couple of tests. When the RelayCommandTestViewModel is instantiated condition is not satisfied (Id as integer by default sets to 0). Constructor creates command with condition that will check if value passed in constructor is matched with changed Id. You can see that if condition is still not satisfied event won't trigger. Thats actually because my implementation of RelayCommand that checks previous state of condition.

Web Image Downloader - WebsiteConstructor

Some exciting stuff coming up today. It's finally time for Website and WebsiteImage models. Like a cherry on a top of big whiped cream, pancake, icecream pie there will be the Constructor, which I'll create!
So first of all - models. Don't need no more right now.
When I already have some containers, now it'll be good to know what to expect from them. I don't need anymore to simulate any web operations. Right now I care just about returning valid models from my WebsiteConstructor and WebsiteImageConstructor. I was wondering what should be input for such a constructor and decided to go for my HtmlParser as it already takes page content and returns its elements and url as a second parameter so I don't have to play with modifying the model it'll return. That results in a need to create one more stub (I already tested my parser so the test doesn't have to go through that again). I've extracted interface for its vital methods and created stub that returns elements I want.
At this point I exactly now what's the result I want to have. Now I just have to make sure that WebsiteConstructor returns it.
Now Url and Name are directly available from IHtmlParser but it doesn't return any WebsiteImage, just string urls which it can dig from the content. Thats good, it's not supposed to do that, for that purpose I'll use another constructor that will return classes I need. Lets take a second to test that first. That WebsiteImageCreator could be made static, but for now I don't need it anywhere else so I'll stick to instantiating it. It'll take just string url that parser spits out. As a matter of fact I could use entire collection so thats what I'll go for. HtmlParser already throws ImagesNotFoundException when I've got no images found, so I'm not sure if I should do the same if passed collection will contain no elements. Perhaps yes, because this class is still public. It could be private but then while testing it would depend on WebsiteCreator and I don't want that. Lets throw some exceptions instead. I've had an idea that I could just return null but it wouldn't be meaningful enough. Maybe I'm overdoing that but it seems to work so far, lets stick to that idea for now.
The final step for todays session is to make the test for WebsiteConstructor pass. I still wonder for that exceptions I keep throwing, but thats how I see it: to create website I use IHtmlParser that means that the webpage exists and I was able to get content from it, parser gives me title and images. When it won't find any images exception will be thrown and I'll know that I can discard page from further steps (that will be downloading images), but if some other implementation of parser would be used I'd lost that behaviour and I don't want that. Theres one thing I noticed now. In my parser I also throw exception if no title is found, it's time to handle that exception here. It just feels right for WebsiteConstructor to take care of assigning default name for unknown website, not the IHtmlParser. So one more case to test.

Web Image Downloader - HtmlParser

Before I started today, I decided to go back for a moment and look at my tests so far. Everything seems fine, except for the WebRetriever class I've extracted yesterday. The tests for it were part of HeaderRetrieverTest and there was also one case missing. Let me quickly show you the changes.
First thing was to merge WebRequestCreatorForContentStub with WebRequestCreatorForHeaderStub. You can see that I return three stub requests. Two last of them are related to test my actual implementations of HeaderRetriever and ContentRetriever which I have already tested. Now the one with error simulates situation when response comes back with an error code, lets say NotFound. That was the case I was missing. The other test just makes sure that if StatusCode is alright the abstract method is able to return.
To complete those two tests another stubs were required
Alright. Enough enhancements for now. Lets get back to the main quest. Today I'll take care of following step or second part of it as the first one is complete - content is being read from the website and strings matching img pattern extracted.
To accomplish this I'll decided to go easy way and use HtmlAgilityPack library instead of scratching up anything on my own.
There are just four cases for now that I want to check so here they are:
And making that pass will assure me that I'm really close to start building models. There will be one more step before that. You can notice the _link2 is relative. Some webpages provide with full urls while others will show just relative paths. To download anything I need to have absolute path to the resource, but it's nothing to worry this time. Just make sure it comes back from my parser.
Thanks to HtmlAgilityPack implementing this class is piece of cake. Both SelectSingleNode and SelectNodes take XPath expression to look for the nodes, you can read about it here. It was helpful to take a look into comments on those methods. I discovered that if no nodes are found then they return just null that made check at GetImageLinks() easier as I didn't have to verify if the collection is empty or not. Handling those exceptions will allow me later to apply default title for website or just discard if from further process notifying user that there's nothing to download there...And that made me realise that it will come in handy if I derive from HtmlWebException and make two new ones that will tell me directly whats missing, so I can avoid any further checks when this occurs.
With those two wonderful exceptions and four more green lights on my path I can say that todays goal is accomplished.

Web Image Downloader - ContentRetriever

At this point I'm already able to determine if website exists, so I finally have a need to get page content to read elements I'm interested in. So thats the task for today. First of all, I'll be using urls that have been already checked, so I believe I can skip tests that check for url related exceptions. My class that I'll test will be ContentRetriever which I'll introduce in the moment. There's only one test I want to complete here, as the class has only one responsibility - return page content as string. That means that I have to deal with some stream, to be more specific Stream that will come from my CustomWebResponse.

Test to accomplish my goal: Now you may remember or lookup at previous post about HeaderRetriever. I decided to refactor it, extracting base class so it can be used in the way I need it. So the base class WebRetriever consists almost entirely of the class that I extracted it from. So right now what I need to do is derive from it, pass my method string to the constructor, instance of IWebRequestCreator and override abstract method HandleResponse which simply allows me to pick and return any part of the response I'll get.
Now to make test go green again, I simply did the steps I just mentioned and as the result got quite slim and even easier to read HeadRetriever:
At this point ContentRetriever was already piece of cake. One thing I've had to do in addition was the Stream property on my IHttpWebResponse.
And thats pretty much it for the production code. The test would pass, but again, I don't want to rely on the internet connection while testing. There's a functional test to check if it also works with real connection, but I need to mock it again. You probably already noticed that in the test I used some static members of WebRequestCreatorForContentStub class. That will probably need more refactoring as it can easily be connected with my creator for headers, but for now I'll just created another stub for IWebRequestCreator. The most important reason here was that I wanted to have some constant strings to rely on.
The last thing to make it work was a IHttpWebRequest stub that will allow me to get some made up stream from. Notice that for StreamWriter is enough to flush its buffer, disposing it would cause us to lose that precious stream.
Now the test passes without access to the internet and another step is complete. There's always some effort to put in those tests but I cannot overrate the feeling of having all the code under control by just looking on the list of green lights. Furthermore that implementation of ContentRetriever is safe for memory as it never will keep any stream alive more than it's necessary to read it and return content as string to process in the next step.