Best practice for Unit Testing class which is mostly responsible to call methods of dependencies, but contains logic as well

Question

Let's assume I have StartCommandHandler which has responsibility to create some file with required files. But for doing this I have to give him a set of sub-responsibilities, like:

• Checks whether file exists in the FTP
• If Not downloads files from multiple sources to the temp folder
• Then executes some script in the folder
• Then read generated file after script execution
• Then create zip from that folder
• Then remove that folder
• Then updates database

As a result of that Command Handler, we are creating folder with all required files. And now that folder is ready for another operations.

I have just read "Art of the Unit testing". And started to add unit tests. I have followed SOLID principles as well. Especially, SRP and DIP, which are in my opinion prerequisites for Unit Testing. So, most of that things which I stated above are done with specific interfaces. So, 90% job of that Command Handler is to call methods of dependencies. And 10% is the logic like this:

if(!_dependency1.IsAnySomething())
{
     _dependency2.Download();

      var isScriptNeeded = _dependency2.IsScriptNeeded();

      if(isScriptNeeded)
      {
          var res = _dependency3.ExecuteScript();
         _dependency4.SetScriptResult(res.Info, res.Date, res.State);
      }

     _dependency3.Archive();

     _dependency5.DeleteTemp();
}

I already tested all dependencies of that command handler. But, hat command handler also includes some small logics like, is download file needed, or temp files are deleted or not and so on...

I have so many question in my mind like:

1. May be Unit Testing doesn't make sense for such units? Integration Test to the rescue? Because, it seems wrong to test whether to check all calls, like whether DeleteTemp is called after download, or script is executed or not, or script result is passed in a right way to SetScriptResult method. Is it GOOD Unit Test?
2. Is there any way to refactor that class for making it testable?

Answer 1

Unit tests should test the behavior of your code, and not the implementation of the code.

It is helpful to consider how unit tests add value: they communicate the intended behavior of your code, and they verify that the intended behavior is generated by the implementation. They add value two times in your project lifecycle: first when the code is initially implemented, and second when the code is refactored.

But unit tests can't add value when refactoring if they are closely tied to a particular implementation.

It's never a perfect science, but one way to know if you're testing the behavior or the implementation is to ask "will this unit test break if I refactor?" If refactoring will break the test, then it's not a good unit test.

It is often not helpful to write a unit test to simply ensures that method A is called, then method B, then method C (or whatever). That's just going to test that your implementation is your implementation, and it's likely to hinder rather than help the next developer who wants to refactor the code.

Instead, think about the behaviors and how your code interacts with other objects. Try to tease each of those behaviors into separate objects, and test those objects individually.

For example, you might break the above code up into three different behaviors:

1. a cache object that checks if a value doesn't exist then call a factory to create it,
2. a factory object that creates an empty directory, calls a builder object to populate it, and then zips and deletes it
3. a builder object that downloads files to a directory and runs scripts it finds there.

Each of those objects has individually testable behavior:

class Cache {
    Cache(ValueStore store, ValueFactory factory) { ... }

    object GetValue(object key) {
        if (!store.HasValue(key))
            factory.CreateValue(key);
        return store.GetValue(key);
    }
}

class CacheTest {
   void GetValue_CallsFactory_WhenValueNotInStore() {
      // arrange
      var store = Mock.Of<VaueStore>(_ => _.HasValue() == false);
      var factory = Mock.Of<ValueFactory>();
      var cache = new Cache(store, factory);

      // act
      cache.getValue();

      // assert
      Mock.Get(factory).Verify(_ => _.CreateValue(), Times.Once());
   }
}

You can do a similar breakdown of the factory and builder, and test their behaviors individually.

Answer 2

Is it GOOD Unit Test?

I fear you'll have to make your own mind on this topic.

As far as I'm aware of, not everybody agree on what should be covered by UT. I would say it also depends on your team, your company and what you really are trying to achieve.

• Some are blindly testing everything, no matter what, because you never know what the next bug will be. And they probably are right. In your case they'll do multiple mocks for each dependency in order to test passing and non-passing cases.

• Some argue that it is too expensive and prove nothing to unitary test high level coordination classes, like you are pointing it here. And they probably are right too.

I'm pretty confident that the only argument for not doing it is the cost. Or at least I am not aware of any other. If your high level UT maintenance is cost-less, there is no reason not to do it.

Unfortunately there are many cases (in my opinion, but I also agree it doesn't apply everywhere) were mocking is hard to maintain and makes the UT worthless.

For instance (as I don't want to be misunderstood), if your dependencies are using low-level objects likes sockets or files, that induce race-conditions and/or unexpected latency (in a multi-threaded environment), you cannot mock them in a way they help you detect a bug : you should emulate those race-conditions and latency.

Would you try to emulate them, you'll spend so much effort in mock development and maintenance, that you'd better had increase your integration tests in a way to detect bugs in real environment.

And If you don't emulate them, your UT doesn't provide any safety, as it's pretty sure the bugs will come from those race-conditions you are not emulating.

Finally, I would point out, there is a 'teaching' aspect in this question too. You may want to implement those high-level UT, using mocks even if they do not prevent anything... just because you want your coworkers to learn how the general rules for UT works, before allowing themselves to break it.

I wish I had more clue. Good luck.

Answer 3

You can use unit testing for this method as well. This can be done by mocking the dependencies. The principle is as follows:

You create mocks of your dependencies (usally by implementing the interface of the dependency) with a predefined result. For example, you provide a implementation for dependency2 that always returns true for IsScriptNeeded().

And then you monitor the dependency3 and check if it is called (it should be!) and if the parameters of dependency4.SetScriptResult(...) match with the result.

For C#, there are libraries like FakeItEasy and Moq, which makes mocking a breeze. Example code snippet using xUnit and FakeItEasy:

using System;
using FakeItEasy;
using Xunit;

public class MyTest
{
    [Fact]
    public void Test() 
    {
        // Create the mock
        var mock = A.Fake<IMyDependency>();
        A.CallTo(() => mock.DoSomething()).Returns(true);

        // Create the class to test
        var sut = new SystemUnderTest(mock);
        sut.DoSomethingElse();

        // Assert that mock.DoSomething() was called
        A.CallTo(() => mock.DoSomething()).MustHaveHappened();
    }
}

Answer 4

Update;

Since the responsibility of this to call and orchestrate other stuff, You may muck all dependencies and verify they are being called in the right sequence and as many times as they required. Something like what Moq offers with Verify method on the mock classes.

        // Explicitly verify each expectation...
        mockSomeClass.Verify(s => s. SetScriptResult(It.IsAny<YourType>()/* or whatever value parameters you may setup snd ecpect it to be called*/), Times.Once());

https://stackoverflow.com/a/9136721/2663492

I believe what you have done and mentioned here is enough for unit testing, because the responsibility of unit testing is to test a unit assuming other interconnected units are working properly, besides unit tests should be completed in a fraction of second, so that when you or your teammates have made some changes, it can be quickly verified whether this change is not breaking existing functionalities, so be quick and be gone is a serious requirement for a UT.

Having said that, I think what you wanna do is to make sure all this units are going to gel and work fine together. This hints you may need some integration tests to check whether this feature is going to work.

Depends on the system complexity, number of parallel users the requirements and so on and so forth you may decide whether you need sort of load testing or not.

I should emphasis I’m talking about unit testing as a technique, you may write integration and load tests using unit testing tools as a tool/medium. I would suggest to have a look at BDD frameworks like Specflow which IMHO which would help to better picture and demonstrate the problems and solution in action. (Again BDD frameworks as media not the process.)

Answer 5

You have two if statements. You need to test that if your code calls correct dependency methods or not, based on the if statements.

It may seem wrong you to test the sequential method calls. For that case, I would recommend you to cover worse cases. For instance what if _dependency2.Download(); throws an exception? Then

_dependency3.Archive();
_dependency5.DeleteTemp();

methods should not be called right? This is a case that you need to test.

And what are the rollback actions for those exceptions? If you have rollback actions then you need to test that if they are called or not in case an exception is thrown.

Regarding to paramaters, you should not test them here. You need to test _dependency3.ExecuteScript() method if it returns correct paramaters or not.