4

This is an example of a typical function from a project I used to work on. I have troubling grasping unit testing because if I want to test this function properly, I would have to write about 50-100 unit tests to cover most of the possible code paths to ensure all the logic works correctly, and this is just a very small part of the application.

Almost every function with business logic is like this, and 90% of the codepaths for each function are critical, which means that if the codepath has a bug then the entire function is broken. While it's reasonable to refactor this function, the algorithm must be the same after refactoring, and that means you can't avoid writing the same amount of tests after refactoring, the only thing refactoring would change is that some of the test would be integration tests rather than unit tests. How do real world projects thoroughly test everything with automated without spending so much time writing tests to the point you would take more time writing tests than manually testing everything? Is there anything I'm missing?

    private async Task ValidateItem(Item item, IEnumerable<Item2> items2,
        ValidatorCache cache, CancellationToken cancellation)
    {
        (bool prop1IsValid, var propData) = await cache.ValidateProperty(item.Property1, cancellation);
        if (propData != null)
        {
            item.Property1 = propData.NAME;
        }
        Item2 Item2 = null;
        if (!string.IsNullOrWhiteSpace(item.Item2Id))
        {
            Item2 = items2.FirstOrDefault(a => a.Id == item.Item2Id && a.Code == item.Code);
        }
        PropData correctPropData = null, item2PropData = null;
        if (Item2 == null)
        {
            item.ItemState = ItemState.DoesNotExistInItems2;
            if (prop1IsValid)
            {
                FillFields(propData);
                correctPropData = propData;
            }
        }
        else
        {
            bool prop2Valid = false;
            item.Property2 = Item2.NAME?.ToUpperInvariant();
            if (string.IsNullOrWhiteSpace(item.Property3))
            {
                item.Property3 = Item2.Property3;
            }
            if (string.IsNullOrWhiteSpace(item.Property4))
            {
                item.Property4 = Item2.Property4;
            }
            if (string.IsNullOrWhiteSpace(item.Property5))
            {
                item.Property5 = Item2.Property5;
            }
            if (string.IsNullOrWhiteSpace(item.Property6))
            {
                item.Property6 = Item2.Property6;
            }
            (prop2Valid, item2PropData) = await cache.ValidateProperty(item2.Property2, cancellation);
            if (prop1IsValid && prop2Valid)
            {
                item.ItemState = ItemState.OK;
                item.ChooseProp = ChooseProp.Prop1;
                correctPropData = propData;
            }
            else
            {
                if (prop1IsValid)
                {
                    item.ItemState = ItemState.OK;
                    item.ChooseProp = ChooseProp.Prop1;
                    correctPropData = propData;
                }
                else if (prop2Valid)
                {
                    item.ItemState = ItemState.OK;
                    item.ChooseProp = ChooseProp.Prop2;
                    correctPropData = item2PropData;
                    item.Property1 = item.Property2;
                }
                else
                {
                    item.ItemState = ItemState.Property1Error;
                    return;
                }
            }
            FillFields(correctPropData);
        }
        if (Item2 == null && item.ItemState != ItemState.Property1Error && string.IsNullOrWhiteSpace(item.Property7) && item.Flag != true)
        {
            string prop = item.Item == ChooseProp.Prop2 ? item.Property2 : item.Property1;
            var validator = new Prop7Validator(await cache.GetProp7(prop, cancellation));
            item.Property7 = validator.GenerateProp7(item);
        }
        else
        {
            if (item.ItemState != ItemState.Property1Error && correctPropData != null)
            {
                string prop7 = item2 != null ? item2.Property7 : item.Property7;
                var validator = new Prop7Validator(await cache.GetProp7(correctPropData.NAME, cancellation));
                item.Property7 = validator.GetMissingData(prop7, item);
            }
        }
        if (correctPropData != null)
        {
            var prop7Data = await cache.GetProp7(correctPropData.NAME, cancellation);
            var validator = new Prop7Validator(prop7Data);
            if (!validator.ValidateProp7(item.Property7))
            {
                item.ItemState = ItemState.Property7Error;
            }
            else if (string.IsNullOrWhiteSpace(item.Property3))
            {
                item.ItemState = ItemState.MissingProperty3;
            }
            else if (item.Property8 != "2" && item.Property9 <= 0)
            {
                item.ItemState = ItemState.InvalidProperty9;
            }
            else
            {
                item.ItemState = ItemState.OK;
            }
        }

        void FillFields(PropData propData)
        {
            if (item.Property3 == "01" && item.Property9 > 0)
            {
                item.Property10 = item.Property9;
            }
            item.Property11 = propData.m012;
            if (item.Property11 == "2")
            {
                if (item.Property3 == "2" && item.Property9 > 0)
                {
                    item.Property12 = item.Property9;
                    item.Property10 = item.Property9;
                }
                else if (item.Property10 > 0)
                {
                    item.Property12 = item.Property10;
                }
                else
                {
                    item.Property10 = item.Property12;
                }
            }
            else if (item.Property3 == item.Property11)
            {
                item.Property12 = item.Property9;
            }
        }
    }
10
  • 4
    I find it very hard to believe that a function of that complexity actually has a single responsibility. (I'm hoping that you've just obfuscated the variable names for this example, but if not that's an even bigger problem). Nov 18 '21 at 23:00
  • 1
    Since "90% of the codepaths for each function are critical" and you don't have tests, you don't actually have much confidence that this works. So you might as well delete it, and start over by implementing small, focused functions - a function, or an object, for each rule, like your "validateProperty", and write tests for those. Then maybe write some functions/objects for simple combination of rules, test those. Maybe you pass a callback to be executed if the check passes. 1/2 Nov 19 '21 at 0:28
  • 4
    Well, it's no more insane than refactoring without tests. But, I didn't necessarily mean literally - refactoring is just deleting the old code slowly; what I'm saying is that you shouldn't leave this as is, it will come back to bite you (and is already biting you). Also, you completely missed my point about how changing the design can make testing more manageable. You say you want to know how people manage testing code with lots of codepaths - divide and conquer, and re-express your code, until you find a better design that removes and/or merges some codepaths, & removes test redundancies. Nov 19 '21 at 2:02
  • 2
    There is SO MUCH opportunity in this code to do better. Quite some duplication, or code that could be easily extracted into helper functions. For example, there's this code for filling in defaults from Item2, the code for choosing prop1 or prop2 (in which the "prop1 AND prop2" branch is redundant), the code for determining an ItemState if correctPropData is available. When faced with lots of steps, people typically test that each step is correct in all circumstances, and that the steps work together – but not that all paths through the entire code base are covered.
    – amon
    Nov 19 '21 at 7:56
  • 2
    If all the codepaths are critical .. you have to test all the codepaths. That's just how it is. You can reduce the workload somewhat by not trying to test each path entirely independently, but it does a lot of stuff so it needs a lot of tests.
    – pjc50
    Nov 19 '21 at 14:13
7

I agree with what VoiceOfUnreason said about refactoring to better support unit testing. For an extreme example, if you have 10 composed functions, each with 2 paths, it takes 20 unit tests to test them individually, but there are over 1000 path combinations if you glom them all together in one function.

That being said, I don't think 50 tests for 139 lines of relatively dense code is particularly excessive, especially if you're writing them TDD-style and get into a rhythm. There is usually one way for a function to go right, but several error cases to handle.

8
  • 3
    It is a little difficult to imagine that simply moving code to other objects reduces the number of code paths that need testing. It might become easier to see what tests need to be written, but an if-elseif-else is still 3 paths, no matter which class the logic belongs to. Nov 18 '21 at 23:32
  • I agree with Greg here, and that is the reason that I ask this question. Refactoring only splits one function into many, but the total number of codepaths is still the same if you actually refactor properly and don't end up changing the algorithm.
    – Magnus
    Nov 19 '21 at 0:53
  • 1
    @FilipMilovanović: refactoring doesn't mean you completely rewrite the internals. It could simply mean you split the code into multiple functions - essentially giving a name to a group of statements without actually changing the number of code paths. Nov 19 '21 at 12:22
  • 2
    @GregBurghardt: Let's say I flip a coin, roll a die, and pick one letter, all in one go. That's 2×6×26=312 possible outcomes. But if I have a separate coin flipper, that has 2 possible outcomes. A dice roller has 6 outcomes. A letter picker has 26 outcomes. So you now only write 2 coin picking tests, 6 dice rolling tests, and 26 letter picking tests, for a total of 34 tests. That's an almost 90% reduction in how many outcomes you need to test.
    – Flater
    Nov 21 '21 at 9:54
  • 1
    @Caleth: And the need for 100% coverage of the state space dissolves due to rigorous unit testing of individual components; a handful of integration tests can confirm the integration of the components. If you demand 100% coverage, sure, then you have to cover 100%. But then you inherently choose to work hard, not smart; rendering the basis of the question moot.
    – Flater
    Nov 22 '21 at 12:50
6

How to do unit testing with complex business logic with lots of different code paths?

Refactor the complex business logic so that it better supports unit testing.

"Design is what we do to get more of what we want than we would get by just doing it." -- Ruth Malan. In this case, what we want is easier testing of the complex logic, so we replace the design that puts all of the complexity into a single hard to test lump (unit), and instead distribute that logic over a number of smaller lumps that are each easier to test.

In other words, we make "the complicated bits need to be easy to test" a design constraint, and any design that doesn't satisfy this constraint is either rejected or forced to evolve into a design that does.

See also Integrated Tests are a Scam (JB Rainsberger, 2015).

4
  • 1
    How does refactoring reduce the total number of codepaths to be tested? This algorithm defines exactly how the functionality must work, if this function is split into many smaller functions and the functionality is identical then the total number of codepaths to be tested must be the same. Instead of writing 50 tests for one function you write 10 tests for each function. But you must write 50 tests to test everything. I want to know if there is a way to save time.
    – Magnus
    Nov 19 '21 at 1:04
  • 2
    Short answer: we're trying to detect mistakes. We're not trying to count how many code paths reach each mistake. Nov 19 '21 at 5:11
  • 1
    To detect mistakes it is needed to test most codepaths, otherwise you won't catch a lot of errors.
    – Magnus
    Nov 19 '21 at 21:55
  • 1
    @Magnus: Separation avoids exponential growth. Let's say I flip a coin, roll a die, and pick one letter, all in one go. That's 2×6×26=312 possible outcomes. But if I have a separate coin flipper, that has 2 possible outcomes. A dice roller has 6 outcomes. A letter picker has 26 outcomes. So you now only write 2 coin picking tests, 6 dice rolling tests, and 26 letter picking tests, for a total of 34 tests. That's an almost 90% reduction in how many outcomes you need to test.
    – Flater
    Nov 21 '21 at 9:50
3

Your code is complicated. It could be made a bit less complicated, but it’s mostly complicated because your business logic is.

There are two dozen places where you could hide a trivial bug. So yes, you need to have at least two dozen unit tests. Best to give a printed description of the business logic to someone who then writes the unit tests (you shouldn’t write unit tests base on your code). And then you want to make sure that your unit tests would find bugs. Spend 10 minutes: introduce a bug on the first line of code, run unit tests, something should fail, introduce a bug on the second line and so on. If a bug doesn’t fail, tell the unit test writer what went wrong and let them add a test.

5
  • Basically it sounds like you are advocating for black box unit testing. Not a bad approach here. Authors make assumptions about their code that complete strangers do not. Nov 22 '21 at 0:25
  • 1
    I wouldn't even call it black box testing. There's just too much risk that someone starts writing the unit tests by extracting bits from the implementation, and duplicates any bugs. So if the code contains "item.Property9" in some place instead of "item.Property8" there is too much of a risk that the unit test copies that bug. This code is just asking for trivial and stupid bugs.
    – gnasher729
    Nov 22 '21 at 17:40
  • This example was among the least complicated in my project. There are examples that are 100 times more complicated
    – Magnus
    Nov 23 '21 at 1:47
  • @Magnus Congratulations. I feel for you. There's the saying "better you than me". Oh well, I have some complicated code for parsing data from an API where the data is just what volunteers entered. Where "Date of Birth" might be "2nd century BC", for example.
    – gnasher729
    Nov 23 '21 at 8:54
  • @gnasher729 fortunately I've just started a new job, but I want to know how to test extremely complex code without just writing basic tests and calling it a day. Particularly at a business where everything is critical yet can't afford to spend 3x more time writing tests.
    – Magnus
    Nov 23 '21 at 17:56
2

Proper separation of logic leads to avoiding the exponential growth that leads to "lots of different code paths".

I suspect that you have not (or incorrectly) separated your logic, either mentally or in code, which is causing you to only consider tests that involve many variables; as opposed to individually testing each component of the (overall) complex business operation.

Keep in mind that you're talking about unit testing, but the tests you describe are of a complexity that is more related to integration tests, where more than one components is being tested at the same time.

Had this code been posted to CodeReview.SE, I would've strongly urged you to refactor this code. However, as your question is about testing, I'll limit my answer to the assumption that your business logic is fixed and we're only focused on the test strategy for this given business logic.

For what it's worth, when we're focusing on the amount of tests you need to write, it makes no difference whether your code is refactored into multiple classes, or if you write specific tests for specific parts of your existing code.


I would have to write about 50-100 unit tests to cover most of the possible code paths

Even if you did not refactor your code any further, which you IMHO should, it still wouldn't take this many tests to confirm that this code works as expected. By properly separating your concerns, which in this case refers to what your tests specifically assert, you can dramatically reduce the amount of test cases needed.

the only thing refactoring would change is that some of the test would be integration tests rather than unit tests

Au contraire. What a refactor would do is give you the ability to write simpler unit tests because you'd be dealing with simpler components. You shouldn't just integration test your complex business logic and leave it at that. Integration tests should only be an addition to a test suite that is already backed by unit tests for all individual components. Integration tests without unit tests give no indication as to which component failed where and why.

You're dealing with a bunch of if blocks, and you can (and should) write tests that only assert specific parts of the logic, not an entire output value. For the sake of example, I'll whittle your code down to 3 if blocks:

if (string.IsNullOrWhiteSpace(item.Property3))
{
    item.Property3 = Item2.Property3;
}
if (string.IsNullOrWhiteSpace(item.Property4))
{
    item.Property4 = Item2.Property4;
}
if (string.IsNullOrWhiteSpace(item.Property5))
{
    item.Property5 = Item2.Property5;
}

If you write tests that test a complete data set, you'd have to write 8 tests, because you need to combine three different binary states (property is/isnot null or whitespace). The test cases would be:

  • All are null or whitespace
  • 3 and 4 are null or whitespace, 5 is filled in
  • 3 and 5 are null or whitespace, 4 is filled in
  • 3 is null or whitespace, 4 and 5 are filled in
  • 3 is filled in, 4 and 5 are null or whitespace
  • 3 and 5 are filled in, 4 is null or whitespace
  • 3 and 4 are filled in, 5 is null or whitespace
  • All are filled in

This is every permutation possible for three properties that can either be filled in or whitespace. The math here is:

 (options for 3) x (options for 4) x (options for 5)
 = 2 x 2 x 2
 = 2³
 = 8

But this is not how you should write your tests. Instead, you should tests for these three properties separately. This reduces your total tests to 6 tests:

  • 3 is null or whitespace
  • 3 is filled in
  • 4 is null or whitespace
  • 4 is filled in
  • 5 is null or whitespace
  • 5 is filled in

The tests for 3 will not test nor assert anything about properties 4 and 5. They are only interested in property 3. The values of 4 and 5 do not meaningfully impact the behavior of property 3, and therefore it makes no sense to write additional test cases whose different values make no impact to the test's focus.

The math here is:

 (options for 3) + (options for 4) + (options for 5)
 = 2 + 2 + 2
 = 6

Mathematically, you've turned your multiplication into addition. The amount of tests that this saves you increases (pound for pound) as the amount of testable logical paths increases.

As we just established, for 3 binary states, this is a reduction from 8 to 6, which is 25% less.
But if we have 10 of these binary states to test, this is a reduction from 1024 (210) down to 20 (2+2+2+...), which is an 98% reduction.

Not all of your if blocks are binary. Sometimes you have more complicated evaluations which have more than two possible input permutations. But the principle remains the same.

Even when we're not discussing testing strategies but code maintenance instead, this is precisely why proper abstraction and separation of concerns is so powerful. By separating your logic, you can have a complex whole that is made up of individually simple components, which makes each component easy to maintain and test; while at the same time allowing your application to perform much more complex tasks that by themselves would not be as maintainable.

14
  • "The tests for 3 will not test nor assert anything about properties 4 and 5. They are only interested in property 3. The values of 4 and 5 do not meaningfully impact the behavior of property 3, and therefore it makes no sense to write additional test cases whose different values make no impact to the test's focus." In this example that is not true for the tests relating to filling props2 to 6 if Item2 is not null, determining which of Prop1 and Prop2 are correct, validating Prop7 and calling FillFields with the Prop1 or Prop2, determining ItemState. All those have dependencies with each other
    – Magnus
    Nov 23 '21 at 1:42
  • Refactoring does not magically eliminate data dependencies or ifs that are related. If I test properties separately then I can't detect inconsistencies between different properties that are related which are 99% of bugs that end up being detected. Your advice can't help about half of the ifs statements in this example because they have data dependencies with each other.
    – Magnus
    Nov 23 '21 at 2:28
  • Testing individual components can't catch errors introduced when these individual components are assembled together. You can mock dependencies but that is no substitute for actually testing all of the code with real data so you know that everything works.
    – Magnus
    Nov 23 '21 at 2:29
  • @Magnus: [In response to first comment] Many people have already told you that your code is too complex. I changed the example code to something more parseable for the sake of clarity of my explanation. I agree with you that your code is much more complex and not as easily split into separate tests. However, that doesn't invalidate my answer, it just adds further incentive to actually refactor your code.
    – Flater
    Nov 23 '21 at 9:40
  • 1
    @Magnus: [In response to second comment] Sufficient refactoring does elimininate contrived dependencies. You're either thinking about a level of refactoring that doesn't get you the whole way, or you're going about the refactoring the wrong way. Your code has clear sections that are contextually different. In the first comment you even pointed out one of those ("filling props2 to 6 if Item2 is not null"). These kinds of sections are ripe for abstraction, which in turn simplifies the overall testing. Again, your feedback just add more incentive to refactor, rather than invalidating the answer
    – Flater
    Nov 23 '21 at 9:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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