how to unit test method using "Spring Data JPA" Specifications

Question

I was playing with org.springframework.data.jpa.domain.Specifications, it's just a basic search :

 public Optional<List<Article>> rechercheArticle(String code, String libelle) {
    List<Article> result = null;

    if(StringUtils.isNotEmpty(code) && StringUtils.isNotEmpty(libelle)){
        result = articleRepository.findAll(Specifications.where(ArticleSpecifications.egaliteCode(code)).and(ArticleSpecifications.egaliteLibelle(libelle)));
    }else{
        if(StringUtils.isNotEmpty(code)){
            result= articleRepository.findAll(Specifications.where(ArticleSpecifications.egaliteCode(code)));
        }else{
            result = articleRepository.findAll(Specifications.where(ArticleSpecifications.egaliteLibelle(libelle)));
        }
    }

    if(result.isEmpty()){
        return Optional.empty();
    }else{
        return Optional.of(result);
    }
}

And that's actually working fine but I'd like to write unit tests for this method and I can't figure out how to check specifications passed to my articleRepository.findAll()

At the moment my unit test looks like :

@Test
public void rechercheArticle_okTousCriteres() throws FacturationServiceException {
    String code = "code";
    String libelle = "libelle";
    List<Article> articles = new ArrayList<>();
    Article a1 = new Article();
    articles.add(a1);
    Mockito.when(articleRepository.findAll(Mockito.any(Specifications.class))).thenReturn(articles);


    Optional<List<Article>> result = articleManager.rechercheArticle(code, libelle);

    Assert.assertTrue(result.isPresent());
    //ArgumentCaptor<Specifications> argument = ArgumentCaptor.forClass(Specifications.class);
    Mockito.verify(articleRepository).findAll(Specifications.where(ArticleSpecifications.egaliteCode(code)).and(ArticleSpecifications.egaliteLibelle(libelle)));
    //argument.getValue().toPredicate(root, query, builder);


}

Any idea?

Solution 1

I was having almost the same problems as you had, and I changed my class that contains Specifications to be an object instead of just one class with static methods. This way I can easily mock it, use dependency injection to pass it, and test which methods were called (without using PowerMockito to mock static methods).

If you wanna do like I did, I recommend you to test the correctness of specifications with integration tests, and for the rest, just if the right method was called.

For example:

public class CdrSpecs {

public Specification<Cdr> calledBetween(LocalDateTime start, LocalDateTime end) {
    return (root, query, cb) -> cb.between(root.get(Cdr_.callDate), start, end);
}
}

Then you have an integration test for this method, which will test whether the method is right or not:

@RunWith(SpringRunner.class)
@DataJpaTest
@Sql("/cdr-test-data.sql")
public class CdrIntegrationTest {

@Autowired
private CdrRepository cdrRepository;

private CdrSpecs specs = new CdrSpecs();

@Test
public void findByPeriod() throws Exception {
    LocalDateTime today = LocalDateTime.now();
    LocalDateTime firstDayOfMonth = today.with(TemporalAdjusters.firstDayOfMonth());
    LocalDateTime lastDayOfMonth = today.with(TemporalAdjusters.lastDayOfMonth());
    List<Cdr> cdrList = cdrRepository.findAll(specs.calledBetween(firstDayOfMonth, lastDayOfMonth));
    assertThat(cdrList).isNotEmpty().hasSize(2);
}

And now when you wanna unit test other components, you can test like this, for example:

@RunWith(JUnit4.class)
public class CdrSearchServiceTest {

@Mock
private CdrSpecs specs;
@Mock
private CdrRepository repo;

private CdrSearchService searchService;

@Before
public void setUp() throws Exception {
    initMocks(this);
    searchService = new CdrSearchService(repo, specs);
}

@Test
public void testSearch() throws Exception {

    // some code here that interact with searchService

    verify(specs).calledBetween(any(LocalDateTime.class), any(LocalDateTime.class));
   // and you can verify any other method of specs that should have been called
}

And of course, inside the Service you can still use the where and and static methods of Specifications class.

I hope this can help you.

Solution 2

If you are writing Unit Tests then you should probably mock the call to findAll() method of articleRepository Class using a mocking framework like Mockito or PowerMock.

There is a method verify() using which you can check if the mock is invoked for the particular parameters.

For Example, if you are mocking the findAll() method of articleRepository Class and want to know if this method is called with particular arguments then you can do something like:

Mokito.verify(mymock, Mockito.times(1)).findAll(/* Provide Arguments */);

This will fail the test if mock has not been called for the arguments that you provided.

Comments

[GhostCat]

Side note: why suggest to use PowerMock? If you can't test your code with EasyMock; then you should change your code; instead of turning to PowerMock.

[Seb]

Yes but if I check something like that : Mockito.verify(articleRepository).findAll(Specifications.whe‌​re(ArticleSpecificat‌​ions.egaliteCode(cod‌​e)).and(ArticleSpeci‌​fications.egaliteLib‌​elle(libelle))); I get a different instance of Specification than the one the was actually passed to my findAll, and the test fail

[user2004685]

I'm not suggesting him to start using PowerMock. I'm just saying to start using one, whatever he is comfortable with.

[Seb]

I'm actually trying to avoid having three differents methods, what if I have 10 criteria and not just two ?

[Seb]

I'm using Mockito by the way

[GhostCat]

Then you have 10 paths in your code. Ignoring that wont help. It is even more a clear indication that you might re-design your whole thing. Complexity doesn't vanish by pushing everything into one block.

[GhostCat]

The only point I have is: when suggesting a mocking framework to somebody, I would never include PowerMock in a list of options.

[Seb]

I'm not trying to make only one test, I'll indeed need 5 of them.

[Seb]

I edited my question, you can now see the test I'm trying to implement

[Seb]

Still I totally agree with what you said.

[user2004685]

Mockito.verify(articleRepository) This should be Mockito.verify(articleRepository, Mockito.times(1)) right? The mock will only be called 1 time. Also, can you share what JUnit says when the test is executed.

[Seb]

I don't want to check if it's executed only one time I want to be sure of which Specifications are used : Specifications.where(ArticleSpecifications.egaliteCode(code)‌​).and(ArticleSpecifi‌​cations.egaliteLibel‌​le(libelle)) or Specifications.where(ArticleSpecifications.egaliteCode(code)‌​) or Specifications.where(ArticleSpecifications.egaliteLibelle(li‌​belle))

[user2004685]

Understood. Could you share the JUnit output. What is the expected value and what is the actual?

[Seb]

Argument(s) are different! Wanted: articleRepository.findAll( org.springframework.data.jpa.domain.Specifications@275b772f ); -> at com.mondialrelay.facturation.business.manager.ArticleManager‌​Test.rechercheArticl‌​e_okTousCriteres(Art‌​icleManagerTest.java‌​:1410) Actual invocation has different arguments: articleRepository.findAll( org.springframework.data.jpa.domain.Specifications@2e8291d3 );

[user2004685]

JUnit will say this even if the header of the Specifications object is different and the value is same. Do this: Do not say any(Specifications.class), make the object yourself and pass the same object to your mock as well as your verify.

[Seb]

That would say I have to pass my Specifications as a method parameter ... and I don't want to do that

[GhostCat]

That is the point of putting stuff into different methods! Don't place the specifications as literals. Instead: write a method that generates the specification. Test that method. Then mock that the result of the method; and just check that your method applies the result of that spec-generation. That is the whole idea of unit testing: you have to decouple things. If you want to check that a certain thing is used in a certain way; well, then this thing has to come "from outside"; and you are able to provide a "mock" to control the internal behavior.

[user2004685]

I completely agree with @Jägermeister.

[Seb]

As I said in the other answer, I'm can't be anything but ok with that. But the point of using Specifications/Specification is to avoid having a method per search and let's imagine a search form with 10 criterias how many methods I would have ? The main purpose of this question was to explore alternative way of doing, basically be able to keep number of method low but still be able to write meaningfull test cases.

[Nick Foote]

I think the method itself is fine, its simple example of what Specifications are designed for; dynamic queries. As OP says, if there are many optional search criteria having multiple methods is simply impossible as you'd need one for every single possible combination of query params. A single method to dynamically build a Specification is correct. Using a mock to attempt to inspect the Specification was built correctly in a unit test is also correct. The problem is that the Specification API does not seem to facilitate inspection of the Specification once it is built.

[Nick Foote]

As above I agree with OP. This answer is OK in that it has suggested use of mocks, which OP has updated into the question. However the question still remains; how to assert the Specification created by the method under test and passed to the mock is as expected. I like Specifications however once built they are frustratingly opaque. Any resolution to this question @Seb would be amazing.

[Seb]

I still haven't found any nice way to test it :/