1

I'm struggling to come to a consensus on the right approach that can be used somewhat consistently to balance integration and unit testing.
Take the following method, that is extremely common to find in a C# .NET Core application.

public interface IBusinessLogicService 
{
    CalculationResult CalculateBusinessRuleResult(
        string businessRuleName, 
        int input
    );
}
public class BusinessLogicService: IBusinessLogicService
{
    private readonly ICache _cache;
    private readonly DbContext _context;
    private readonly BusinessValidator _validator;

    public BusinessLogicService(
            ICache cache, 
            DbContext context, 
            BusinessValidator validator //injected
    ) {
        _cache = cache;
        _context = context;
        _validator = validator;
    } 
    public CalculationResult CalculateBusinessRuleResult(
            string businessRuleName, 
            int input
    ) {
        var hasRule = _cache.TryGet(businessRuleName, out BusinessRule rule);
        if (!hasRule) 
        {
            rule = _context.BusinessRules.First(f => f.Name == businessRuleName);
            _cache.Add(businessRuleName, rule);
        }
        
        return ApplyRule(rule, intermediateValue);
    }

    internal CalculationResult ApplyRule(rule, input)
    {
        // Logic Line 1
        // Logic Line 2
        // Logic Line 3
        return new CalculationResult();
    }
}

In my mind, I identify CalculateBusinessRuleResult as an integration of parts (cache, database, and ApplyRule), and I view ApplyRule as a pure function. To me, this means that I should be unit testing ApplyRule and integration testing CalculatebusinessRuleResult.

My primary conflict is that I've seen it said many times that in this situation, ApplyRule is an implementation detail and should not be unit tested, as it is not directly exposed via the API Surface Area intended by the interface implementation. Furthermore there is an argument that if you have to mock dependencies, you are not unit testing, it is an integration test -- clearly visible by my dependency on the cache and database.

Is there really no unit-testable code here? To me, there is -- ApplyRule. A pure, functional business logic method.
This is the approach I've taken to a lot of code recently - declare dependency usage at the top of the public method, and then utilize those dependencies in units of business logic further down. Integration test this method, and then unit-test the bits of business logic below. This also lines up with the idea that an application should be produced of X number of end-to-end tests, +X number of integration tests, and ++X number of unit tests.

2
  • You can't have two different things called _context in the same class. _validator doesn't exist. Where are you getting your ExternalDependency instance from? Commented Sep 2, 2020 at 7:18
  • 1
    Maybe you could see this as an opportunity to introduce a new, explicit concept to replace what's currently implicit; e.g., instead of having a dependency on ICache and DbContext and doing some logic with them inside some function, replace the two with a ICachedRepository that just has GetBussinessRule(name) (and maybe a couple of other app-logic specific functions that you need - you're not building a generic repository), and put the retrieval logic in there. Then you can fake/mock that. And your code becomes just: return ApplyRule(_cachedRepo.GetBussinesRule(name), input);. Commented Sep 2, 2020 at 8:04

2 Answers 2

2

First, to your misunderstanding. You wrote

"If you have to mock dependencies, you are not unit testing, it is an integration test "

which is - in my understanding of those terms - incorrect. By injecting mocks, one does unit testing - the unit BusinessLogicService is tested in perfect isolation. Opposed to this, by not injecting mocks, but making use of "real" dependencies, this would become an integration test.

But now to your main question: the real issue I see with the code in this example (and which is IMHO the root cause why unit testing may be hard) is that it mixes up two different responsibilities:

  • it integrates three different components (ICache, DbContext, BusinessValidator)

  • but it also contains some logic which appears to be complex enough to make it worth writing unit test on its own: the rule evaluating logic in ApplyRule, which seems to be a "generic rule evaluator".

Now this little bit of technical debt may be acceptable in this specific case, but if that really bothers you, and you think unit testing by mocks is overcomplicating things, you can resolve this by extracting the code in ApplyRule into a new class (which is then injected into BusinessLogicService as well). By separating the responsibilities this way, ApplyRule is not an "implementation detail any more" and you can (and should) write unit tests for it.

Let me add that in the real code (outside of this contrived example), it may be not be necessary to create a new class for ApplyRule at all. It may be perfectly possible that there is already a class where it fits well. Maybe one can move it into the BusinessRule class instead, or the class of intermediateValue, which we currently don't know. But to make such a decision, one needs to see the real thing, know the real names and responsibilites of the sourrounding classes.

7
  • Why does a class encapsulating the logic mean it is no longer an impl detail, but a method encapsulating the logic is? Certainly you don't extract every pure method into a class. Commented Sep 2, 2020 at 15:15
  • I do understand that the intention of mocking dependencies is to unit test. My confusion is that if you are testing something that has dependencies you are testing an integration of parts, not a unit. Commented Sep 2, 2020 at 15:26
  • 1st comment: when you split up two components into separate classes, you make it more clear in your code that something is not "an implementation detail" and you want it to be unit testable, that's a popular convention, not a law. If you don't like the extra code, you can surely keep the ApplyRule function as an internal function where it is, and use the possibility of testing internals (or make the function public just for testing). Several people don't like that, it is unpopular, but not "forbidden". Regardless of how you do it ...
    – Doc Brown
    Commented Sep 2, 2020 at 15:41
  • .. when you start calling a function like ApplyRule from a test, or from somewhere else, and not just internally in the class where it is, you make it harder to change its signature later. That's what "being an implementation detail" really means: the ability of changing signatures or internal structure with the least possible effort, without having to change any external code.
    – Doc Brown
    Commented Sep 2, 2020 at 15:43
  • 1
    To the 2nd comment: if you are testing something that has dependencies, but you mock out all dependencies, you are unit testing - that is my understanding of the term. An integration test is a test where you don't replace all of the dependencies by mocks, but use at least one "real" dependency. You are testing if two "real" components work together as intended. You can't do this when all dependencies are mocked out.
    – Doc Brown
    Commented Sep 2, 2020 at 15:47
-1

Unit Testing

Is about testing a unit of code.

Pure functions are pretty trivial, you test the function.

But what is CalculateBusinessRuleResult? Its not a unit, the object its embedded in is the unit.

Which still means you should unit test it. Pass appropriate stubs/mocks into the constructor. Then call CalculateBusinessRuleResult and if that had collaborators (pass in mocks/stubs for those too).

Integration Tests

These are for just those pieces of code that you cannot pass a mock/stub into. They must have an external collaborator/service/database/filesystem.

They should only be applied to the smallest sliver of code on your end, and as close to where the system 'integrates' with the collaborator.

End 2 End tests

These are the test where the whole system is strung together with as few mocks/stubs as possible. These shouldn't test work, but the stringing together of components.

Not the answer you're looking for? Browse other questions tagged or ask your own question.