Code coverage for ElasticClient using Moq

Question

I am attempting to analyze code coverage via unit testing, I am currently using Moq library to perform unit testing, somehow I am heading into a wrong path, would like to know if the below scenario is applicable using Moq

Below is a snippet of code

public interface ISearchWorker
{
    void DeleteIndex(string indexName);

    T GetClient<T>();
}

public class ElasticSearchWorker : ISearchWorker
{
    public void DeleteIndex(string indexName)
    {
        IElasticClient elasticClient = GetClient<IElasticClient>();

        if (elasticClient.IndexExists(indexName).Exists)
        {
            _ = elasticClient.DeleteIndex(indexName);
        }
    }

    public T GetClient<T>()
    {        
        string nodeList = "http://localhost:9200/";

        List<Node> nodes = nodeList.Split(',').Select(uri => new Node(new Uri(uri))).ToList();

        IConnectionPool sniffingConnectionPool = new SniffingConnectionPool(nodes);

        IConnectionSettingsValues connectionSettings = new ConnectionSettings(sniffingConnectionPool);

        return (T)(IElasticClient)new ElasticClient(connectionSettings);        
    }
}

Below is the code snippet for the unit test

    [TestClass]
    public class SearchTestClass
    {
        private ISearchWorker searchWorker;

        private Mock<ISearchWorker> searchWorkerMoq;

        private readonly string indexName = "testIndex";

        [TestInitialize]
        public void SetupElasticClient()
        {
            searchWorkerMoq = new Mock<ISearchWorker>();

            var elasticClient = new Mock<IElasticClient>();

            searchWorkerMoq.Setup(c => c.GetClient<IElasticClient>()).Returns(elasticClient.Object).Verifiable();

            searchWorker = searchWorkerMoq.Object;
        }

        [TestMethod]
        public void DeleteIndexTest()
        {
            try
            {
                searchWorker.DeleteIndex(indexName);

                searchWorkerMoq.Verify(c => c.GetClient<IElasticClient>(), Times.Once()); 
            }
            catch (System.Exception)
            {
                throw;
            }
        }
    }

The line

searchWorkerMoq.Verify(c => c.GetClient<IElasticClient>(), Times.Once());

Throws the following exception

(Moq.MockException: '
Expected invocation on the mock once, but was 0 times: c => c.GetClient<IElasticClient>())

Reading through most Moq related information it seems this is not the appropriate way to perform a Moq test, the IElasticClient object should be supplied externally to the ElasticSearchWorker class

The reason for not supplying the IElasticClient object from external injection is that we plan to implement ISearchWorker for another search provider (Azure Search) so would like to keep the client entity enclosed within the class implementing the ISearchWorker interface

Would like to know if there is a better way to perform this test, Also how would we achieve code coverage for this scenario.

Answer 1

So I assume you understand why this is not working and you are merely asking for "a good way" to fix this.

Now I am not saying this is the best way to do this, but it is probably the quickest while still being "clean".

Apply Interface Segregation ("I" from SOLID). Make two interfaces instead of one and then implement them both at a later stage.

// Don't have a C# IDE with me, so sorry if I leave some syntax errors. 
public interface ISearchClientProvider
{
    T GetClient<T>();
}

public interface ISearchWorker
{
    void DeleteIndex(string indexName);
}

public class ElasticSearchWorker : ISearchWorker{

    private readonly ISearchClientProvider _clientProvider;

    public ElasticSearchWorker(ISearchClientProvider clientProvider){
        _clientProvider = clientProvider;
    }

    public void DeleteIndex(string indexName)
    {
        var elasticClient = _clientProvider.GetClient<IElasticClient>();

        if (elasticClient.IndexExists(indexName).Exists)
        {
            _ = elasticClient.DeleteIndex(indexName);
        }
    }
}

public class ElasticSearchClientProvider : ISearchClientProvider{/*some implementation*/}
public class AzureSearchWorker : ISearchWorker{/*some implementation*/}
public class AzureSearchClientProvider : ISearchClientProvider{/*some implementation*/}

Then test code should be something like:

// would actually prefer to name it ElasticSearchWorkerTests
[TestClass]
    public class SearchTestClass
    {
        private readonly ElasticSearchWorker _searchWorker;
        private readonly ISearchClientProvider _elasticClientProvider;

        private readonly string indexName = "testIndex";

        // would prefer to name it SetupElasticSearchWorker
        [TestInitialize]
        public void SetupElasticClient()
        {
            var elasticClient = new Mock<IElasticClient>();
            // Setup for IElasticClient.IndexExists() function:
            // I don't know what is the return type of IndexExists, 
            // so I am assuming here that it is some dynamic Object
            elasticClient.Setup(c => c.IndexExists(indexName)).Returns(new {Exists = true});
            // Setup for IElasticCleint.DeleteIndex might also be necessary here.

            _elasticClientProvider = new Mock<ISearchClientProvider>();
            _elasticClientProvider.Setup(c => c.GetClient<IElasticClient>()).Returns(elasticClient.Object).Verifiable();

            _searchWorker = new ElasticSearchWorker(_elasticClientProvider);
        }

        // would prefer to name it DeleteIndexTest_GetsSearchClient, 
        // because the function does more than is checked here, e.g., Checks index, deletes index.
        [TestMethod]
        public void DeleteIndexTest()
        {
            try
            {
                searchWorker.DeleteIndex(indexName);

                searchWorkerMoq.Verify(c => c.GetClient<IElasticClient>(), Times.Once()); 
            }
            catch (System.Exception)
            {
                throw;
            }
        }
    }

This way there will be no http requests in this test.

In general (the arguably opinionated part) if you want your code to be more unit testable:

• Use Ports and Adapters (Hexagon, or however you want to call it) architecture
• Rule of thumb. Do not call public methods inside of other public methods. Use composition to fix this. I used composition in this example.
• Rule of thumb. Do not create your objects, that have behavior, inside your class. Let your composition root create them and inject them into your class as a dependency, an implementation of an interface. If it is not possible to create them in your composition root (maybe you know only at run time what kind of object you need), then use factory pattern.
• Disclaimer. You don't need a DI container in order to use composition root pattern. You can start with poor man's DI.
• There will always be classes that are not unit testable, because some dependencies from 3rd parties will not be mockable. Your goal is to make those classes very small and then do some integration testing with them. (which, I believe, test coverage tools will also pickup)
• Disclaimer. I have seen people mocking out HttpClient successfully, via HttpClientHandler substition. But I am yet to see a successful EF db context mock - they all fail at some point.
• Note. There is a common belief that you should not mock interfaces you do not own. So, in this sense, my solution above is also not clean, and may get you into trouble at some point in the future.