Patterns or practices for unit testing methods that call a static method
As of late, I have been pondering heavily about the best way to "Mock" a static method that is called from a class that I am trying to test. Take the following code for example:
using (FileStream fStream = File.Create(@"C:\test.txt"))
{
string text = MyUtilities.GetFormattedText("hello world");
MyUtilities.WriteTextToFile(text, fStream);
}
I understand that this is a rather bad example, but it has three static method calls that are all different slightly. The File.Create function access the file system and I don't own that function. The MyUtilities.GetFormattedText is a function that I own and it is purely stateless. Finally, the MyUtilities.WriteTextToFile is a function I own and it accesses the file system.
What I have been pondering lately is if this were legacy code, how could I refactor it to make it more unit-testable. I have heard several arguments that static functions should not be used because they are hard to test. I disagree with this idea because static functions are useful and I don't think that a useful tool should be discarded just because the test framework that is being used can't handle it very well.
After much searching and deliberation, I have come to the conclusion that there are basically 4 patterns or practices that can be used in order to make functions that call static functions unit-testable. These include the following:
- Don't mock the static function at all and just let the unit test call it.
- Wrap the static method in an instance class that implements an interface with the function that you need on it and then use dependency injection to use it in your class. I'll refer to this as interface dependency injection.
- Use Moles (or TypeMock) to hijack the function call.
- Use dependeny injection for the function. I'll refer to this as function dependency injection.
I've heard quite a lot of discussion about the first three practices, but as I was thinking about solutions to this problem, the forth idea came to me of function dependency injection. This is similar to hiding a static function behind an interface, but without actually needing to create an interface and wrapper class. An example of this would be the following:
public class MyInstanceClass
{
private Action<string, FileStream> writeFunction = delegate { };
public MyInstanceClass(Action<string, FileStream> functionDependency)
{
writeFunction = functionDependency;
}
public void DoSomething2()
{
using (FileStream fStream = File.Create(@"C:\test.txt"))
{
string text = MyUtilities.GetFormattedText("hello world");
writeFunction(text, fStream);
}
}
}
Sometimes, creating an interface and wrapper class for a static function call can be cumbersome and it can pollute your solution with a lot of small classes whose sole purpose is to call a static function. I am all for writing code that is easily testable, but this practice seems to be a workaround for a bad testing framework.
As I was thinking about these different solutions, I came to an understanding that all of the 4 practices mentioned above can be applied in different situations. Here is what I am thinking is the correct cicumstances to apply the above practices:
- Don't mock the static function if it is purely stateless and does not access system resources (such as the filesystem or a database). Of course, the argument can be made that if system resources are being accessed then this introduces state into the static function anyway.
- Use interface dependency injection when there are several static functions that you are using that can all logically be added to a single interface. The key here is that there are several static functions being used. I think that in most cases this will not be the case. There will probably only be one or two static functions being called in a function.
- Use Moles when you are mocking up external libraries such as UI libraries or database libraries (such as linq to sql). My opinion is that if Moles (or TypeMock) is used to hijack the CLR in order to mock your own code, then this is an indicator that some refactoring needs to be done to decouple the objects.
- Use function dependency injection when there is a small number of static function calls in the code that is being tested. This is开发者_JS百科 the pattern that I am leaning towards in most cases in order to test functions that are calling static functions in my own utility classes.
These are my thoughts, but I would really appreciate some feedback on this. What is the best way to test code where an external static function is being called?
Using dependency injection (either option 2 or 4) is definitely my preferred method of attacking this. Not only does it make testing easier it helps to separate concerns and keep classes from getting bloated.
A clarification I need to make though is it is not true that static methods are hard to test. The problem with static methods occurs when they are used in another method. This makes the method that is calling the static method hard to test as the static method can not be mocked. The usual example of this is with I/O. In your example you are writing text to a file (WriteTextToFile). What if something should fail during this method? Since the method is static and it can't be mocked then you can't on demand create cases such as failure cases. If you create an interface then you can mock the call to WriteTextToFile and have it mock errors. Yes you'll have a few more interfaces and classes but normally you can group similar functions together logically in one class.
Without Dependency Injection: This is pretty much option 1 where nothing is mocked. I don't see this as a solid strategy because it does not allow you to thoroughly test.
public void WriteMyFile(){
try{
using (FileStream fStream = File.Create(@"C:\test.txt")){
string text = MyUtilities.GetFormattedText("hello world");
MyUtilities.WriteTextToFile(text, fStream);
}
}
catch(Exception e){
//How do you test the code in here?
}
}
With Dependency Injection:
public void WriteMyFile(IFileRepository aRepository){
try{
using (FileStream fStream = aRepository.Create(@"C:\test.txt")){
string text = MyUtilities.GetFormattedText("hello world");
aRepository.WriteTextToFile(text, fStream);
}
}
catch(Exception e){
//You can now mock Create or WriteTextToFile and have it throw an exception to test this code.
}
}
On the flip side of this is do you want your business logic tests to fail if the file system/database can't be read/written to? If we're testing that the math is correct in our salary calculation we don't want IO errors to cause the test to fail.
Without Dependency Injection:
This is a bit of a strange example/method but I am only using it to illustrate my point.
public int GetNewSalary(int aRaiseAmount){
//Do you really want the test of this method to fail because the database couldn't be queried?
int oldSalary = DBUtilities.GetSalary();
return oldSalary + aRaiseAmount;
}
With Dependency Injection:
public int GetNewSalary(IDBRepository aRepository,int aRaiseAmount){
//This call can now be mocked to always return something.
int oldSalary = aRepository.GetSalary();
return oldSalary + aRaiseAmount;
}
Increased speed is an additional perk of mocking. IO is costly and reduction in IO will increase the speed of your tests. Not having to wait for a database transaction or file system function will improve your tests performance.
I've never used TypeMock so I can't speak much about it. My impression though is the same as yours that if you have to use it then there is probably some refactoring that could be done.
Welcome to the evils of static state.
I think your guidelines are OK, on the whole. Here are my thoughts:
Unit-testing any "pure function", which does not produce side effects, is fine regardless of the visibility and scope of the function. So, unit-testing static extension methods like "Linq helpers" and inline string formatting (like wrappers for String.IsNullOrEmpty or String.Format) and other stateless utility functions is all good.
Singletons are the enemy of good unit-testing. Instead of implementing the singleton pattern directly, consider registering the classes you want restricted to a single instance with an IoC container and injecting them to dependent classes. Same benefits, with the added benefit that IoC can be set up to return a mock in your testing projects.
If you simply must implement a true singleton, consider making the default constructor protected instead of fully private, and define a "test proxy" that derives from your singleton instance and allows for the creation of the object in instance scope. This allows for the generation of a "partial mock" for any methods that incur side effects.
If your code references built-in statics (such as ConfigurationManager) which are not fundamental to the operation of the class, either extract the static calls into a separate dependency which you can mock, or look for an instance-based solution. Obviously, any built-in statics are un-unit-testable, but there's no harm in using your unit-testing framework (MS, NUnit, etc) to build integration tests, just keep them separate so you can run unit tests without needing a custom environment.
Wherever code references statics (or has other side effects) and it is infeasible to refactor into a completely separate class, extract the static call into a method, and test all other class functionality using a "partial mock" of that class that overrides the method.
Just create a unit test for static method and feel free to call it inside methods to test without mock it.
For the File.Create
and MyUtilities.WriteTextToFile
, I'd create my own wrapper and inject it with dependency injection. Since it touchs the FileSystem, this test could slow down because of the I/O and maybe even throw up some unexpected exception from the FileSystem which would lead you to think that your class is wrong, but it's now.
As for the MyUtilities.GetFormattedText
function, I suppose this function only does some changes with the string, nothing to worry about here.
Choice #1 is the best. Don't mock, and just use the static method as it exists. This is the simplest route and does exactly what you need it to do. Both of your 'injection' scenarios are still calling the static method, so you aren't gaining anything through all of the extra wrapping.
精彩评论