JUnit test to delete an entity by id fails-CodePudding

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.

CodePudding user response：

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

CodePudding user response：

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);
}