Best way to unit test methods that call other methods inside same class

  • I was recently discussing with some friends which of the following 2 methods is best to stub return results or calls to methods inside same class from methods inside same class.

    This is a very simplified example. In reality the functions are much more complex.

    Example:

    public class MyClass
    {
         public bool FunctionA()
         {
             return FunctionB() % 2 == 0;
         }
    
         protected int FunctionB()
         {
             return new Random().Next();
         }
    }
    

    So to test this we have 2 methods.

    Method 1: Use Functions and Actions to replace functionality of the methods. Example:

    public class MyClass
    {
         public Func<int> FunctionB { get; set; }
    
         public MyClass()
         {
             FunctionB = FunctionBImpl;
         }
    
         public bool FunctionA()
         {
             return FunctionB() % 2 == 0;
         }
    
         protected int FunctionBImpl()
         {
             return new Random().Next();
         }
    }
    
    [TestClass]
    public class MyClassTests
    {
        private MyClass _subject;
    
        [TestInitialize]
        public void Initialize()
        {
            _subject = new MyClass();
        }
    
        [TestMethod]
        public void FunctionA_WhenNumberIsOdd_ReturnsTrue()
        {
            _subject.FunctionB = () => 1;
    
            var result = _subject.FunctionA();
    
            Assert.IsFalse(result);
        }
    }
    

    Method 2: Make members virtual, derive class and in derived class use Functions and Actions to replace functionality Example:

    public class MyClass
    {     
         public bool FunctionA()
         {
             return FunctionB() % 2 == 0;
         }
    
         protected virtual int FunctionB()
         {
             return new Random().Next();
         }
    }
    
    public class TestableMyClass
    {
         public Func<int> FunctionBFunc { get; set; }
    
         public MyClass()
         {
             FunctionBFunc = base.FunctionB;
         }
    
         protected override int FunctionB()
         {
             return FunctionBFunc();
         }
    }
    
    [TestClass]
    public class MyClassTests
    {
        private TestableMyClass _subject;
    
        [TestInitialize]
        public void Initialize()
        {
            _subject = new TestableMyClass();
        }
    
        [TestMethod]
        public void FunctionA_WhenNumberIsOdd_ReturnsTrue()
        {
            _subject.FunctionBFunc = () => 1;
    
            var result = _subject.FunctionA();
    
            Assert.IsFalse(result);
        }
    }
    

    I want to know wich is better and also WHY ?

    Update: NOTE: FunctionB can also be public

    Your example is simple, but not exactly correct. `FunctionA` returns a bool but only sets a local variable `x` and doesn't return anything.

    In this particular example, FunctionB can be `public static` but in a different class.

    For Code Review, you are expected to post actual code not a simplified version of it. See the FAQ. As its stands, you are asking a specific question not looking for a code review.

    `FunctionB` is broken-by-design. `new Random().Next()` is almost always wrong. You should inject the instance of `Random`. (`Random` is also a badly designed class, which can cause a few additional problems)

    on a more general note DI via delegates is absolutely fine imho

    @jk DI via delegates works fine when you do it manually, but pretty much every DI container is keyed on type.

  • Martin

    Martin Correct answer

    8 years ago

    Edited following original poster update.

    Disclaimer : not a C# programmer (mostly Java or Ruby). My answer would be : I would not test it at all, and I do not think you should.

    The longer version is : private/protected methods are not parts of the API, they are basically implementation choices, that you can decide to review, update or throw away completely without any impact on the outside.

    I suppose you have a test on FunctionA(), which is the part of the class that is visible from the external world. It should be the only one that has a contract to implement (and that could be tested). Your private/protected method has no contract to fulfil and/or test.

    See a related discussion there : https://stackoverflow.com/questions/105007/should-i-test-private-methods-or-only-public-ones

    Following the comment, if FunctionB is public, I'll simply test both using unit test. You may think that the test of FunctionA is not totally "unit" (as it call FunctionB), but I would not be too worried by that : if FunctionB test works but not FunctionA test, it means clearly that the problem is not in the subdomain of FunctionB, which is good enough for me as a discriminator.

    If you really want to be able to totally separate the two tests, I would use some kind of mocking technique to mock FunctionB when testing FunctionA (typically, return a fixed known correct value). I lack the C# ecosystem knowledge to advice a specific mocking library, but you may look at this question.

    Completely agree with @Martin answer. When you write unit tests for class you should not test *methods*. What you're testing is a **class** behavior, that the contract (the declaration what class is supposed to do) is satisfied. So, your unit tests should cover all the requirements exposed for this class (using public methods/properties), including exceptional cases

    Hello, thanks for the response, but it didn't answer my question. I doesn't matter if FunctionB was private / protected. It can also be public and still be called from FunctionA.

    The most common way to handle this problem, without redesigning the base class, is to subclass `MyClass` and override the method with the functionality you want to stub. It might also be a good idea to update your question to include that `FunctionB` could be public.

    When it comes to unit testing and TDD you would write your test first, make your test pass all inside your public method. Once you get it passing you can either move on or refactor. Refactoring gives you private methods.. Now In this particular case you would would want to look into dependency injection. Find a way that you put `FunctionB` into your parameter. This way you can force feed `FunctionA` with any known values to and assert certain values back.

    `protected` methods are part of the public surface of a class, unless you ensure that there can be no implementations of your class in different assemblies.

    The fact that FunctionA calls FunctionB is an irrelevant detail from a unit testing perspective. If the tests for FunctionA are written correctly, it is an implementation detail that could be refactored-out later without breaking tests (so long as the overall behavior of FunctionA is left unchanged). The real issue is that FunctionB's retrieval of a random number needs to be done with an injected object, so that you can use a mock during testing to ensure that a well-known number is returned. This allows you test well-known inputs/outputs.

    To add to @CodesInChaos's very important addendum: `protected`/`protected internal` methods defined within non-sealed classes are indeed part of your public API. The client is free to extend your class and call these methods. Some classes are even intended to be used this way: the `ApiController`, for example, has 32 well documented protected methods. Private methods are implementation details - protected methods are **not**.

License under CC-BY-SA with attribution


Content dated before 6/26/2020 9:53 AM