aerospike/aerospike-client-java

Cannot mock query results

daniel-siegmann-aol opened this issue · 8 comments

I am trying to write unit tests for some code of mine which is calling AerospikeClient.queryAggregate. I have mocked the AerospikeClient via the IAerospikeClient interface using Mockito. So far, so good.

Where I run into trouble is that I cannot mock the result. Mockito cannot mock ResultSet because that class is final. Its dependency, QueryAggregateExecutor is also final, which would lead me to mock Cluster. That is possible, but leads down a very complicated rabbit hole.

RecordSet might be more practical to mock since it depends on a QueryExecutor, which is the abstract parent class. However, many of QueryExecutor's method are final, so it may still be difficult.

I think the most expedient solution would be to make RecordSet and ResultSet non-final. I understand making classes final is good practice, but I believe making the code mock-able so it can be unit tested is considerably more valuable. I am certainly open to other suggestions.

By the way, the ExecutionTask returned from AerospikeClient.execute has a similar problem, so you may wish to apply the same solution. I was able to work around that case like so:

ExecuteTask task = client.execute(...); // When client is a mock, will return null
if (task != null) task.waitTillComplete();

I have never been keen on changing optimized code due to requirements of a testing tool. It seems intrusive to me. Would adding IResultSet and IRecordSet solve your problem?

I'm curious though. Does Mockito really have advantages over more lenient test frameworks like JMockit (http://jmockit.org/about.html) that can mock final classes and static methods without source code modifications?

Is it possible to use JMockit when testing AerospikeClient?

I understand and agree with your reluctance to compromise good practices for the requirements of any particular tool. However, testability of code is something I place a very high value on, and for me the value of Mockito in testing exceeds the drawback of not making classes final (even if it makes me feel a bit dirty).

I do believe IResultSet and IRecordSet would solve my problem. Unfortunately, I don't think the use of these can be implemented in a backward-compatible manner.

Now that said, I don't have any experience with JMockit. I'm going to look at that now.

I have confirmed that JMockit can test these classes. I don't like the JMockit API, but fortunately I was able to confirm that PowerMock + Mockito works as well.

I am a bit leery of using such tools because the bytecode manipulation they use may be fragile. PowerMockito exploded in the presence of Java 8's default methods until I upgraded its dependency on javassist from 3.19.0-GA to 3.20.0-GA - which was released just a few hours ago.

I think it would be better if there were interfaces for ResultSet and RecordSet, but since that would break compatibility I don't think it's an option, at least in the short term.

@Mustaine thanks for the input, I had not heard of that principle before. This isn't really the place for a discussion on testing philosophies, so I'll simply say that after reading the article you linked I was unconvinced. That said I do have integration tests of my queries.

@daniel-siegmann-aol no problem. Here another article
http://www.markhneedham.com/blog/2009/12/13/tdd-only-mock-types-you-own

But I would recommend read the book mentioned.

@maverickgautam thanks for the info. I'm not working with Aerospike at the moment though, so don't rush on my account.

@daniel-siegmann-aol You can use Mockito2 now to mock final classes and methods, so I think this issue could be closed now.

@Mustaine I agree with don't mock what you don't own, but nonetheless, it is still useful and great in some cases. I explain:

I implemented a RxJava wrapper for aerospike-client-java in Kotlin, and I just wanted to reuse the whole of the AsyncClient class implementation (without having to rewrite all the methods, or test them, for interfaces IAsyncClient and IAerospikeClient) + extend/add new methods using RxJava return values. This is something very nice, clean and easy to do if you use class delegation, but I had no other choice than to use Mockito 2 to make my tests work. I am not interested in testing any other methods (that would be the responsibility of aerospike-client-java module) and I need to mock the AsyncClient class to test the return values are wrapped correctly. As I said, I could still implement all the interfaces, but (to me personally) it makes no sense, as I just want to extend/wrap the AsyncClient class functionality.