1

We are using OpenCover for our solution test coverage and I noticed that

public async Task Build(ProcessorContext context)
{
    context.ResultBetSlip.Bets.Clear();

    // Here OpenCover tells me that I need to cover two branches
    // so I assume I need to verify that GetAvailablePlugins can be empty and
    // no Build method was called.
    // How do I do that?
    foreach (var plugin in _pluginFactory.GetAvailablePlugins())
    {
        await plugin.Build(context);
    }
}

Now my test would look like that

[Test]
public async Task Build_ShouldntEnterForeachWhenThereAreNoPluginsRegistered()
{
    // Arrange
    var pluginFactoryMock = new Mock<IBetSlipProcessorServicePluginFactory>();
    var sut = new BetSlipProcessorService(pluginFactoryMock.Object);
    pluginFactoryMock
        .Setup(pf => pf.GetAvailablePlugins())
        .Returns(new List<IBetSlipProcessorServicePlugin>());

    // Act
    await sut.Build(AutoFixtureSimplified.Create<ProcessorContext>());

    // Assert
    ???
}

Should I even consider testing such case if it is possible?

EDIT:

As requested this is the test coverage report:

enter image description here

And here you can find gist of all the tests that I do in order to achieve such coverage. https://gist.github.com/kuskmen/df3f112b2b6d880741ee6ab559d64d53

kuskmen
  • 3,648
  • 4
  • 27
  • 54
  • Have you already tested when it has items? – Nkosi Sep 05 '17 at 13:57
  • Yes, but `OpenCover` tells me I need to cover one more branch there. – kuskmen Sep 05 '17 at 13:57
  • Well then just verify that `GetAvailablePlugins` was called. – Nkosi Sep 05 '17 at 13:58
  • Yes, but verifying `GetAvailablePlugins` was called doesn't necessary means that `plugin.Build` was not... its kinda dumb to me to even consider such case but I was wondering whats the idea behind that branch splitting of `OpenCover` – kuskmen Sep 05 '17 at 14:00
  • The test code you shared doesn't cover the line inside the foreach loop. That's why you might be seeing that warning or message. You need to have different test case where you setup the mock to return some populated list so that it will cover the entire code. This again is to satisfy your code coverage tool. But otherwise you don't need to have test with empty list. – Chetan Sep 05 '17 at 14:00
  • @Chetan you are talking about line coverage I am talking about branch coverage - there is a difference. – kuskmen Sep 05 '17 at 14:02
  • 1
    In your case they are similar, as the line `plugin.Build` is within a branch. Imagine the method `GetAvailablePlugins` doesn´t return any element. Then of coursethe line within the loop won´t execute, so that´s a further branch you have to test. However usually when you know such scenarios won´t happen you should just ignore the warning, just test the *usual* scenarios, achieving 100% coverage is unrealistic. – MakePeaceGreatAgain Sep 05 '17 at 14:07
  • @HimBromBeere I guess it doesn't worth writing redundant code only for branch coverage, but I asked only because of curiosity :) – kuskmen Sep 05 '17 at 14:08
  • Well... Then for branch coverage you need to have this test case. Now whether you should have it or not that depends on you. If your code quality matrix requires this to be considered you should have it. Else you can leave it. – Chetan Sep 05 '17 at 14:09
  • Is it possible? Sure. Is it needed? Depends on your conditions within your team. We can´t know your teams conventions for unit-testings. If this is your actual question it´s clearly not answerable here and will be closed as opinion-based. – MakePeaceGreatAgain Sep 05 '17 at 14:11

1 Answers1

1

I am assuming you are using the Moq framework for mocking. If this is the case you can do one of two things.

  1. You can create your mock in strict mode
  2. You can expect that when the plugin.Build(..) method is called that an exception is thrown.

A similar question was asked here: How to verify that method was NOT called in Moq?

edit: I simulated the exact scenario that you are seeing and I have narrowed it down to the data type that you are iterating over. Due to the fact that you are using a list I would guess there is some internal workings of the list that are causing this problem. I changed all the list references to arrays and the branch coverage returned as expected.

Aaron Roberts
  • 1,342
  • 10
  • 21
  • This is what I was thinking of, but isn't it stupid .. making mock, not using it anywhere and then verifying its methods were not called... kinda feels redundant, but I guess this is the only way – kuskmen Sep 05 '17 at 14:07
  • I'm not familiar with the branch coverage that OpenCover is telling you, but yes it does feel redundant. It doesn't feel like it would be necessary to test that path as it is inherent that `foreach` will not execute the loop block when the collection is empty. – Aaron Roberts Sep 05 '17 at 14:12
  • Could it be that the test you are missing is for when the .GetAvailablePlugins() returns null? – Aaron Roberts Sep 05 '17 at 14:19
  • I was just testing this test, cause it seems that returning empty list was *NOT* the case.. – kuskmen Sep 05 '17 at 14:21
  • nope `.GetAvailablePlugins()` returning null still doesn't cover the branch, I am taking this question to `OpenCover` project as I clearly don't understand the branching I guess, thanks for the help tho. – kuskmen Sep 05 '17 at 14:27
  • Can you update the question with the results from the coverage report and the full set of tests? – Aaron Roberts Sep 05 '17 at 14:33
  • Sure, I edited my question, feel free to check it :) – kuskmen Sep 05 '17 at 14:41
  • Great thanks! I wonder if the issue is that the GetAvailablePlugins() method is being called inside the loop. Since I don't have a setup to test opencover what if you try this - var plugins = _pluginFactory.GetAvailablePlugins; if (plugins == null) { return; } then do the foreach loop on the plugins variable. – Aaron Roberts Sep 05 '17 at 14:58
  • The test that you added simulated that the GetAvailablePlugins() method throws a NullReferenceException but not that it returns an actual null. – Aaron Roberts Sep 05 '17 at 14:59
  • nope still the same with `pluginFactoryMock .Setup(pf => pf.GetAvailablePlugins()) .Returns((IEnumerable)null);` – kuskmen Sep 05 '17 at 15:10
  • Interesting... I would love to see the answer from `OpenCover` author, thanks for the support! – kuskmen Sep 05 '17 at 20:43