[FIXED] Mockito Object of willReturn() is a Parameter of the next given()

Issue

I don't know if I'm testing this method wrong or if the whole test is nonsense. (Example code is below)

I would like to test my ExampleService with Mockito. This has the client and the customerService as dependencies (both with @Mock). Both dependencies are used in exampleService.doYourThing().

client.findByName() returns a ResultPage, which in turn is used in customerService.createHitList(). I thought it would be a good idea to test that the page object does not change during the "doYourThing" method, after being returned by client.findByName(). Therefore, customerService.createHitList() should only return something if 'page' does not change.

The problem: If 'page' is returned in "doYourThing()" by client.findByName() and is changed with "page.setIsNew(true)" as in my example, then in my test method the 'pageForTest' object also changes. As a result, the condition I set with "given(this.customerService.createHitList(pageForTest))" is met and the customerList is returned.

I know that 'page' is a reference of 'pageForTest' and thus changes.

Anyway, the goal of my test is not fulfilled by this. How would you tackle the problem?

This is only sample code, there may be small syntax errors.

Test method:

ExampleService exampleService = new ExampleService(client, customerService);

ResultPage pageForTest = new ResultPage();
pageForTest.setIsFulltextSearch(true);
pageForTest.setIsNew(false);
pageForTest.setHits(hits);

given(this.client.findByName("Maria")).willReturn(pageForTest);

given(this.customerService.createHitList(pageForTest)).willReturn(customerList);

HitList hitList = this.exampleService.doYourThing("Maria");

ExampleService

public HitList doYourThing(String name) {
  ResultPage page = this.clientFindByName(name);
  page.setIsNew(true);
  HitList hitList = this.customerService.createHitList(page);
  return hitList; 
}

Solution

When writing unit tests it's important to actually define the unit. A common misconception is treating every class as a separate unit, while we use classes to separate responsibilities, but a certain unit of work can be represented by more than one class (so it could include more than one responsibility as a whole). You can read more about it on the Internet, for example on the Martin Fowler's page.

Once you define a unit you want to test, you will need to make some assumptions regarding the dependencies used by the test. I will point you towards the Martin Fowler's page for more information, but in your case this could be a mock of the repository retrieving some data from the database or a service representing another unit, which has it's own tests and it's behavior can be assumed and reproduced using a mock. The important thing is that we do not say here that all the dependencies should be mocked, because maybe testing given method while mocking all the dependencies only tests Mockito and not actually your code.

Now it's time to actually understand what you want to test - what is the actual logic of the code. When developing using test-driven development, you write the tests first as the requirements are pre-defined and the tests can point you towards the best API that should be exposed by the class. When you have tests ready, you implement the code as designed during writing the tests. In your case you have your logic ready and let's assume it's doing what it's supposed to do - that's what you want to test. You are not actually testing the code, but the desired behavior of the code. After the test is written, you can change the code in many ways including extracting some parts to a method or even a separate class, but the behavior should be well defined and changing the internals of a method or the class (refactoring) should not require changing the tests, unless API or the expected behavior changes.

The main problem with trying to help you understand how to test your code is actually the lack of context. Method named doYourThing does not describe the expected behavior of the method, but this is actually the most important thing when writing a test - that thing should be done. If you strictly stick to how it's done internally in your test, the code will be hard to modify in the future and the tests may be unreliable. If the important behavior includes setting the isNew value, maybe the object returned from the mock should be a spy or it should be verified with assertions? If the actual logic lies in the customerService, then maybe it shouldn't be mocked, but it should be the part of the unit? It requires context, but hopefully I explained some things regarding testing as a concept. I recommend reading up on testing online as multiple articles like those by Martin Fowler can be more helpful in understanding it.

Answered By - Jonasz
Answer Checked By - Mildred Charles (JavaFixing Admin)