Unit tests : Make all concrete class mockable

[NicolasREY69330]

Hi,

I need to mock some methods and properties in Nest.IElasticClient interface that is used in my repo. As example I need to stub the Indices.Exists() method to return an ExistsResponse having an Exists property returning true.

The problem is that the concrete class has no interface implementation, nor setter on the Exists property, and is not declared virtual neither in Nest lib:

public class ExistsResponse : ResponseBase
{
    public ExistsResponse();
    public bool Exists { get; }
}

public ExistsResponse Exists(Indices index, Func<IndexExistsDescriptor, IIndexExistsRequest> selector = null);

So for the mocking I tried to set the property anyway on the concrete class, but it failed with all the following methods, I have no idea on how to do ...

/* Fail with exception :
System.NotSupportedException : Unsupported expression: x => x.Exists
    Non-overridable members (here: ExistsResponse.get_Exists) may not be used in setup / verification expressions.
*/
    var mock1 = new Mock<ExistsResponse>();
    obj.SetupGet(f => f.Exists).Returns(true);

/* Fail with exception :
 System.NotSupportedException : Unsupported expression: f => f.Exists
    Non-overridable members (here: ExistsResponse.get_Exists) may not be used in setup / verification expressions.
*/
    var mock2 = Mock.Of<ExistsResponse>(x => x.Exists == true);

/* Fail with exception :
System.ArgumentException : Property set method not found.
*/
    var mock3 = new ExistsResponse();
    var property = typeof(ExistsResponse).GetProperty("Exists", BindingFlags.Public | BindingFlags.Instance);
    property.SetValue(mock3, true);

/* Fail with exception :
System.NullReferenceException (setter is null)
*/
    var mock4 = new ExistsResponse();
    var setter = property.GetSetMethod(true);
    setter.Invoke(mock4, new object[] { true });


    // My Mock on the Indices.Exists method
    var elasticMock = new Mock<IElasticClient>();
    elasticMock
    .Setup(x => x.Indices.Exists(It.IsAny<string>(), null))
    .Returns(/*** my stubbed object here ***/); // <== how to stub the concrete class to return a ExistsResponse.Exists = true ?

var repositoryMock = new Mock<GeocodedCityElkRepository>(elasticMock.Object, new NullLogger<GeocodedCityElkRepository>());
            repositoryMock
                .Setup(x => x.Get(It.IsAny<int>(), It.IsAny<Func<QueryContainerDescriptor<GeocodedCity>, QueryContainer>>()))
                .Returns(Task.FromResult(new List<GeocodedCity>().AsReadOnly() as IReadOnlyCollection<GeocodedCity>));
     
            // Act
            var exception = await Assert.ThrowsAsync<GeocodingException>(() =>
                repositoryMock.Object.GetDensity("11111", "city", "FR"));

Well unless I missed something, I'm stuck ...

Describe the solution you'd like
Allow to mock concrete class (making property virtual) or at least provide interface for returned objects like ExistsResponse.

    public interface IExistsResponse
    {
        bool Exists { get; }
    }

    [DataContract]
    public class ExistsResponse : ResponseBase, IExistsResponse
    {
        public ExistsResponse();
        public bool Exists { get; }
    }

Describe alternatives you've considered
Do not test my repository ...

Additional context

My libs :

Nest 7.12.1
Moq 4.15.2
XUnit 2.4.1
.Net 5

Here is a subset of my repo

[stevejgordon]

Hi @NicolasREY69330.

The issue of testing classes that depend on IElasticClient has been discussed a few times (#3095, #4025, #4076, #4287) and I appreciate there are various opinions on the topic. Adding interfaces purely to support mocking where there is little need for multiple implementations doesn't sit well with me. In your example, the mock would also fail later as calling into Indices is also not virtual. I won't rehash the previous discussions here but I do want to add that I accept that we need to consider the story and guidance we provide for testing. I have that as a headline discussion for our work on the next major version once we progress further with that design.

The ExistsResponse is a little unusual in that it inspects the IApiCallDetails to determine the outcome from the status code.

From the options provided in the previous issues, two probably sit well with your use case.

1. Use the InMemoryConnection with the response bytes and status code configured appropriately.

var existsClient = new ElasticClient(new ConnectionSettings(new InMemoryConnection(Array.Empty<byte>())));
var doesNotExistClient = new ElasticClient(new ConnectionSettings(new InMemoryConnection(Array.Empty<byte>(), statusCode: 404)));

var responseOne = existsClient.Indices.Exists("test"); // Exists = true
var responseTwo = doesNotExistClient.Indices.Exists("test"); // Exists = false

2. If your testing is more extensive, consider adding your own abstraction to act as a facade over the client. This limits your testing to ensure the correct mapping of our types to your own. After that, since you own the types, you can design them and mock them in unit testing however you want. It also isolates the majority of your code from changes in our codebase in the future.

I've personally used both techniques in the past, as well as favouring more end-to-end integration tests for complex scenarios.

I hope these options help in the short term. I will review this further as part of the next release.

[NicolasREY69330]

Hi @stevejgordon I appreciate your detailed answer, and I finally get some help from SO pointing out the fact an InMemoryConnection should solve my problem (as explained in your doc), indeed it works perfectly well.

But to be honest it was not so obvious to me to use this kind of approach for unit tests, so you do you think it's an option to add some official guidelines about unit testing a class using IElasticClient in the docs, with some tiny examples ?

=> I have another unrelated question about using Elastic as primary database, I read a lot about it on https://discuss.elastic.co/ or here and I'm still confused, many cons seems undocumented, I would like to have, if possible, your opinion about it, but I don't know where to ask, the Github Issues seems a bit unrelated ... Where can I ask my question, beeing sure you and your core team can give your point of view ?

Many thanks again, you and your teammates do a great job on this lib!

[stevejgordon]

You're welcome @NicolasREY69330.

I 100% agree that we can and should review how to better document the use of InMemoryConnection for unit testing and more general guidance for consumers around testing. That is on my list already as something I picked up early on when joining Elastic. I appreciate that it's not easy to discover and you should not have to rely on a SO answer, or historical issues to guide you there.

As to your other question... the discussion forums are really the best place for ES question, although the answer in your case is likely to be, "it depends", as there are a lot of variables to consider and knowledge of your precise requirements is needed.

Here are two other links which may provide some insight when assessing this for your situation:

• Elasticsearch as a NoSQL Database
• Set up a cluster for high availability - Which provides guidance that will be important if you decide on ES as your core data store.

I can't offer specific guidance but I can share a couple of use cases I've been involved with...

Before joining Elastic I worked on a project where ES was the single store for metric data which we indexed at a rate of around 17 million docs per day. We needed a large cluster for the volume of data, querying needs and to ensure we had high availability. Before going to production, we made sure to test how the restore and snapshot process would work (although I was not personally involved in the specific details of that part of the project). With the project, we also accepted that a limited amount of data loss could be tolerated, not that we ever ran into that problem.

In a different project, we used an MS SQL database as the primary store for data, with ES used purely to index the fields relevant for searching. Our code used ES to search and refine results, and if required, pulled extra data from the SQL database. We chose this option since we could use a much smaller cluster for the small amount of searchable data and resiliency was less of a concern since we could rebuild an index quickly from the MS SQL data if needed. Our DBA was already experienced in ensuring that the SQL database was resilient. We also required some transactional functionality which SQL provided. One thing I will add is that while ES supports updating documents, the implementation is not ideally suited if the data changes often. For this project, MS SQL was a better fit for the frequently changing data, with the more static, searchable data being duplicated in ES. This avoided the need for frequent document updates and was a good balance.

Both approaches worked for their given situations.

While I want to avoid sounding like a salesperson, you could consider our consulting services to help you design and implement a solution that would fit your requirements. They could work closely with you to understand the relevant details needed to help you choose the best path and to implement it correctly.

I'm going to close this particular issue for now since I'm tracking the documentation improvements already and it's focused on the .NET client library.

[NicolasREY69330]

Thanks @stevejgordon for your time, I really appreciate it, and your feedback on previous experiences is precious.
I'll read the supplied articles carefully.

Well just to conclude:

• I will start using Elastic as main DB for non critical data, that won't change a lot (ie fallback for undistributed payload for future retry as the schema is really flexible) and with reasonable volume (3000 to 100 000 records a day) or for data that expires in time (based on any business rules)
• For critical data (ie Orders, Payments ...) I plan to use an event store DB as primary DB and project events into Elastic models to query them with high flexibility

If I still feel confuse with that I'll contact your consulting service, for sure.

Thank you!

[KPGDL]

@NicolasREY69330 Can you please share how you actually mocked the ExistsResponse built on top of InMemoryConnection? I'm still getting Non-overridable members (here: ExistsResponse.get_Exists) may not be used in setup / verification expressions.'

[NicolasREY69330]

@KPGDL
Steve Gordon gives the solution in this same topic :
#5714 (comment)

The key point is StatusCode (404 or 200 as default) depending of what you intend to mock.
I didn't want to test if the client.Indices.Exists("xxx") returned true or false, but as it is called from my ctor, I needed to always send true to get an instance of my object.

I personaly created an helper to use in unit tests

    public static class Helper
    {
        public static IElasticClient InMemoryElasticSearchClientFactory()
        {
            var response = new
            {
                took = 1,
                timed_out = false,
                _shards = new
                {
                    total = 2,
                    successful = 2,
                    failed = 0
                },
                hits = new
                {
                    total = new { value = 25 },
                    max_score = 1.0,
                    hits = Enumerable.Range(1, 25).Select(i => (object)new
                    {
                        _index = "project",
                        _type = "project",
                        _id = $"Project {i}",
                        _score = 1.0,
                        _source = new { name = $"Project {i}" }
                    }).ToArray()
                }
            };

            var responseBytes = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(response));
            var connection = new InMemoryConnection(responseBytes, 200);
            var connectionPool = new SingleNodeConnectionPool(new Uri("http://localhost:9999"));
            var settings = new ConnectionSettings(connectionPool, connection)
                .DefaultIndex("project")
                .DisableDirectStreaming();
            var client = new ElasticClient(settings);
            return client;
        }

and I use it this way :

        [Fact]
        public async Task When_NoGeocodedCityFound_Expect_GetDensityReturnsRural()
        {
            // Arrange
            var repositoryMock = new Mock<GeocodedCityElkRepository>(InMemoryElasticSearchClientFactory(), new NullLogger<GeocodedCityElkRepository>());
            repositoryMock
                .Setup(x => x.Get(It.IsAny<int>(), It.IsAny<Func<QueryContainerDescriptor<GeocodedCity>, QueryContainer>>()))
                .Returns(Task.FromResult(new List<GeocodedCity>().AsReadOnly() as IReadOnlyCollection<GeocodedCity>));

            // Act
            var density = await repositoryMock.Object.GetDensity("11111", "city", "FR");

            // Assert
            Assert.Equal(Domain.Enums.DensityCode.Rural, density);
        }

Hope it helps