JUnit test to delete an entity by id fails

Question

I am trying to make a test for the method to delete an entity by id. I've written the test for the case when an entity with the given id doesn't exist, but in case when the entity exists, I'm apparently doing something wrong. This is the delete method from the service. I'm using the deleteById() method of the JpaRepository.

  public void deleteById(Integer id) {
        Optional<Agency> optionalAgency = repo.findAll().stream()
                .filter(agency -> agency.getId().equals(id))
                .findFirst();

        if (optionalAgency.isPresent()) {
            repo.deleteById(optionalAgency.get().getId());
        } else {
            throw new AgencyNotFoundException("Agency not found");
        }
    }

And this is my test:

    @Mock
    private AgencyRepository agencyRepository;

    @Mock
    private AgencyMapper mapper;

    @InjectMocks
    private AgencyService agencyService;

    @Test
    void delete() {
        Agency agency = new Agency();

        when(agencyRepository.findById(1)).thenReturn(Optional.of(agency));

        agencyService.deleteById(1);

        verify(agencyRepository).deleteById(1);

    }

What assertion should I make in order to verify that the deletion was successful? The way I've tried it it doesn't work, the result of the test is that an exception is thrown. I'm guessing that the exception is thrown because agencyRepository.findById((int) 1L); basically doesn't exist anymore, but I thought maybe there's a better way to verify the deletion, without searching for the deleted object.

why do you make a float, just to parse it to an int? just remove the (int) and the L. Either way, you know the code of your agencyRepository better than we do, so you should check whether or not that method is there — Stultuske, Dec 22 '21 at 14:11
The only code in my AgencyRepository is `@Repository public interface AgencyRepository extends JpaRepository {}` and I have to pass a parameter to the delete method. I've tried passing directly an int (for ex. 1), but the test fails with the same error — Alessia, Dec 22 '21 at 14:14

Mureinik · Answer 1 · 2021-12-22T14:50:24.140

1

repo is a mock, so calling deleteById on it won't actually delete anything. Instead, you could verify that calling the AgencyService's deleteById actually calls AgencyRepository's deleteById.

EDIT:
The repository's code uses findAll, not findById, so you need to make sure you mock that:

@Test
void delete() {
    Agency agency = new Agency();
    agenct.setId((int) 1L);

    when(agencyRepository.findAll()).thenReturn(Collections.singletonList(agency));

    agencyService.deleteById((int) 1L);

    verify(agencyRepository).delteById((int) 1L);
}

edited Dec 22 '21 at 14:50

answered Dec 22 '21 at 14:14

Mureinik

297,002
52
306
350

Ok, I understand that, so everything that I am able to do here is verify the correct flow. I've changed my code the way you suggested, but it still throws an exception. – Alessia Dec 22 '21 at 14:21
1

@Alessia can you post the output of the exception or error – ata Dec 22 '21 at 14:28
@Alessia I think that the mocking was wrong - see my edited answer – Mureinik Dec 22 '21 at 14:34
Hmm, ok, but how can I return `Optional.of(agency)` from `agencyRepository.findAll()` which returns a list? This causes an error. This is the error that I got before switching the method to `findAll()`: `com.project.exceptions.AgencyNotFoundException: Agency not found at com.project.services.AgencyService.deleteById(AgencyService.java:39) at com.project.AgencyServiceTest.delete(AgencyServiceTest.java:64) at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)` – Alessia Dec 22 '21 at 14:48
@Alessia sorry, that was a careless partial edit. You should mock `findAll()` and return a list containing the mocked `Agency` object. See my further edit. – Mureinik Dec 22 '21 at 14:51
Ok, now it works, thank you! But why do we need to verify the `deleteById()` method from the repository, rather than the one from the service? After all, the method from the repository is contained in the JpaRepository. – Alessia Dec 22 '21 at 15:13
@Alessia you explicitly call the method from the service, so you know it's being called and there's no point in verifying it. Verifying the method from the repository tests that when you call the service's method with an ID, the same repository's method (which performs the actual work on the database) gets called with the same ID. – Mureinik Dec 22 '21 at 15:17
I got it, thanks a lot! – Alessia Dec 22 '21 at 15:31

score 1 · Answer 2 · answered Dec 22 '21 at 14:23

Not the answer, but your method could be written something like this:

 public void deleteById(Integer id) {
     Agency agency = repo.findById(id).orElseThrow(() -> throw new AgencyNotFoundException("Agency not found"));
     repo.deleteById(agency.getId());
 }

Because:

Optional<Agency> optionalAgency = repo.findAll().stream().filter(agency -> agency.getId().equals(id)).findFirst(); is not efficient. What if you have thousands of agencies? Will you fetch all and filter through all of them just to find by id. You have that method in your repository already.
if (optionalAgency.isPresent()) {} Optional.isPresent is not good practise. Read here and this answer

Oh ok, I got it, makes sense, thank you! – Alessia Dec 22 '21 at 14:26 — Alessia, Dec 22 '21 at 14:26

JUnit test to delete an entity by id fails

2 Answers2