3

What would be the best practice for unit testing a method that only calls 2 other methods and no logic?

internal ICollection<ObjA> FunctionA()
{
    IEnumerable<ObjB> b = _factory.FunctionB();
    return FunctionC(b);
}

_factory.FunctionB() and FunctionC(b) both have unit tests for it, so if those methods broke we would know.

I'm considering faking both functions and just have a unit test that makes sure the methods are called from FunctionA(). But then if FunctionB() or FunctionC() changes, FunctionA() could become a false positive.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
Leo Reyes
  • 101
  • 3
  • Mock out both functions, as you mentioned. It is not a false positive if either of the mocked functions changes; it is a correct test to see that functionA calls both functions. The testing of the two mocked functions is, correctly, covered in other tests. – DwB Dec 02 '14 at 20:34
  • 1
    I assume this is C#? – Codeman Dec 02 '14 at 20:44

1 Answers1

1

I would tend not to unit test private or internal methods as they should be executed when unit testing the public API.

Assuming this was a public method I would have tests that check that the return value you expect when calling FunctionA() based on the state of the private member _factory. Assuming you can control the output of _factory (via it being a mock-able interface, subclass-able base class, or just plain instantiated with a known value and passed into the constructor for the class) you can ensure that you get the expected output of FunctionA based on that initial input.

If you find that you are getting false positives every time either FunctionB or FunctionC are updated I would question why FunctionA exists at all. The more likely scenario is that unit tests that break on FunctionA will cause you to either change the internal implementation of FunctionA as the changes to FunctionB and FunctionC no longer make them a suitable implementation for FunctionA, or they will cause you to add unit tests for those functions to fix the indirect breakage in FunctionA.

In this specific example it appears that FunctionA is just a wrapper to convert ObjB's to ObjA's with a data source already specified by an internal factory class. For this specific scenario I would set up my tests for FunctionA so that if upper level requirements change (because new rules are implemented as to how to convert ObjB to ObjA) so that I could either put that new logic in FunctionA or FunctionC depending on which I think is more appropriate.

Example:

internal ICollection<ObjA> FunctionA()
{
    IEnumerable<ObjB> b = _factory.FunctionB();
    // in the future unit test may drive that a different implemetation is used 
    // based on some data about b.
    if(b.SomeCondition())
        return FunctionC(b);
    else
        return FunctionD(b);
}

Or it could be that C needs to change

internal ICollection<ObjA> FunctionC(IEnumerable<ObjB> objBEnumerable)
{
    ICollection<ObjA> objACollection = initializeCollection();
    foreach(var objB in objBEnumerable)
    {
        //updated logic to create objA from objB goes here
        objACollection.Add(createdObjA);
    }
    return objACollection;
}

Basically the answer to your question is "it depends", but I would err on the side of writing a potentially meaningless test if the coverage of functions B and C are already good in case A needs to divert in the first code example in the future.

Phil Patterson
  • 1,242
  • 15
  • 25
  • 1
    If you don't test private or internal methods, couldn't your test cases become more complex if those methods were nested inside a few levels of abstraction? You would have to setup data for all dependencies. Let's say FunctionA returns obj with values greater than 10 and FunctionB returns obj with values greater than 3. If you made changes to FunctionB and you mock FunctionB in the unit test, causing your FunctionA unit test to still pass, couldn't that end up being a false positive? – Leo Reyes Dec 02 '14 at 21:31
  • It's not that I don't test them, it's that I test them indirectly through the public API (these methods are the ones invoking the functions). I probably wouldn't mock function B (_factory would be a live implemtation, not a mock) and FuncitonA would now be a failure in this scenario. This failing test can now lead you to the thought process of how FunctionB and A should interact with each other. It's a bit difficult to talk this abstractly, so I will update my answer with something more concrete this evening when I return home. – Phil Patterson Dec 02 '14 at 21:56
  • But yes mocking the internal objects can lead to false positives if you do not test all the potential scenarios of the mocked object. So ideally you would have FunctionA tests that check many various return values from FunctionB. – Phil Patterson Dec 02 '14 at 21:57
  • 1
    More info on the philosophy behind not unit testing private methods directly: http://stackoverflow.com/questions/5601730/should-private-protected-methods-be-under-unit-test – Phil Patterson Dec 03 '14 at 19:40
  • 1
    internal <> private. Often internal is used for classes that need to be unit tested but would otherwise also be exposed to any and all users of the assembly. Using internal you can expose them to the unit test project using InternalsVisibleTo, and still keep them an implementation detail of the assembly. Much better than having to "integration test" those classes from the public API. Internal _methods_ I do not "get" by the way :) – Marjan Venema Dec 06 '14 at 17:25
  • This is true. That being said I tend to only use that attribute if the entire class itself is internal. This is just a guideline however and I will use this if I feel moving the internal logic to a public class (that the internal methods delegate to) is not worth the effort/exposes something that really shouldn't be publically available. – Phil Patterson Dec 08 '14 at 00:43
  • So I would claim this is a strong opinion held weakly. – Phil Patterson Dec 08 '14 at 00:44