1

Hi have read a lot about this but can't come to a conclusion about the best way to test a method that is dependent on other method call results to perform its actions.

Some of the questions I've read include:

Some of the answers sugest that we should only test the methods that perform only one action and then test the method that call this methods for conditional behaviuour (for example, verifying if a given method was called or not) and that's fine, I get it, but I'm struggling with other scenario.

I have a service with a REST api.

The controller has a create method that receives a DTO and calls the Service class create method with this argument (DTO).

I'm trying to practice TDD and for this I use this project I'm building without a database.

The code is as follows:

@Service
public class EntityService implements FilteringInterface {

    private MemoryDatabase db = MemoryDatabase.getInstance();

    //Create method called from controller: receives DTO to create a new 
    //Entity after validating that it's name and code parameters are unique
    public EntityDTO create(EntityDTO dto) throws Exception {

        validateUniqueFields(dto);
       
        Entity entity = Entity.toEntity(dto, "id1"); //maps DTO to Entity object
        db.add(entity);

        return new EntityDTO.Builder(entity);//maps entity to DTO
    }

    public void validateUniqueFields(EntityDTO dto) throws Exception {
        Set<Entity> foundEntities = filterEntityByNameOrCode(dto.getName(),
                dto.getCode(), db.getEntities());

        if (!foundEntities.isEmpty()) {
            throw new Exception("Already exists");
        }
    }
}

This is the interface with methods reused by other service classes:

public interface FilteringInterface {

    default Set<Entity> filterEntityByNameOrCode(String name, String code, Set<Entity> list) {
        return list.stream().filter(e -> e.getSiteId().equals(siteId)
                && (e.getName().equals(name)
                || e.getCode().equals(code))).collect(Collectors.toSet());
    }

    default Optional<Entity> filterEntityById(String id, Set<Entity> list) {
        return list.stream().filter(e -> e.getId().equals(id)).findAny();
    };
}

So, I'm testing this service class and I need to test the create() method because it can have different behaviors:

  1. If the received DTO has a name that already exists on the list of entities -> throws Exception
  2. If the received DTO has a code that already exists on the list of entities -> throws Exception
  3. If the received DTO has a name and a code that already exists on the list of entities -> throws Exception
  4. If name and code are different, than everything is ok, and creates the entity -> adds the entity to the existing list - > converts the entity to DTO and retrieves it.

Problem: To test any of the scenarios, suppose, scenario 1: I need to make the filterEntityByNameOrCode() method return a list with an Entity that has the same name as the Entity I'm trying to create. This method is called inside validateUniqueFields() method.

Problem is: I can't call mockito when() for any of this methods because, for that, I would have to mock the service class, which is the class that I'm testing and, thus, it's wrong approach. I've also read that using Spy for this is also wrong approach. So, where thus that leaves me?

Also: if this code is not the correct aprocah, and thats why it can't be correctly tested, than, whats should the correct approach be?

This service will have other methods (delete, update, etc.). All of this methods will make use of the FilteringInterface as well, so I will have the same problems. What is the correct way of testing a service class?

avaros
  • 177
  • 1
  • 1
  • 9
  • `Mockito.mock(EntityService.class, Mockito.CALLS_REAL_METHODS)` and then `Mockito.doReturn().when(service).filterEntityByNameOrCode();` – XtremeBaumer Jan 11 '23 at 10:48
  • @XtremeBaumer Thanks for your response. I get that can work, but by doing that I'm mocking the class that is under testing. I want to avoid doing so as I wonder if that is a bad practice? This is what I want to clarify: what is the right way of testing a scenario like this? Not just the solution that works. Thank you. – avaros Jan 11 '23 at 12:06
  • A partial mock is what you need, which is what `Mockito.mock(EntityService.class, Mockito.CALLS_REAL_METHODS)` does. The docs state `This implementation can be helpful when working with legacy code.When this implementation is used, unstubbed methods will delegate to the real implementation.This is a way to create a partial mock object that calls real methods by default.` You basically mock the class, but still call the actual implementation for the unstubbed methods – XtremeBaumer Jan 11 '23 at 12:15
  • You probably want to review Chapter 4 of _Working Effectively with Legacy Code_. – VoiceOfUnreason Jan 12 '23 at 04:58
  • Thanks for the responses but this is not legacy code. this is me trying to understand the principles of TDD and clean code and trying to learn how to do things the best way. I'm not looking for a solution that works. I'm trying to learn the correct way of doing things. – avaros Jan 12 '23 at 09:20

1 Answers1

1

I would apply an DI pattern in your service, in order to mock and control the db variable.

@Service
public class EntityService implements FilteringInterface {

    private Persistence db;

    public EntityService(Persistence db) {
        this.db = db;
    }
}

After that, you will be able to add entities to Set accordingly to your scenarios

@ExtendWith(MockitoExtension.class)
class EntityServiceTest {

    @Mock
    private Persistence persistence;

    @InjectMocks
    private EntityService entityService;

    @BeforeEach
    void before() {
        final Set<Entity> existentEntity = Set.of(new Entity(1L,1L, "name", "code"));
        when(persistence.getEntities()).thenReturn(existentEntity);
    }

    @Test
    void shouldThrowWhenNameAlreadyExists() {
        final EntityDTO dto = new EntityDTO(1L, "name", "anything");
        assertThrows(RuntimeException.class, () -> entityService.create(dto));
    }

    @Test
    void shouldThrowWhenCodeAlreadyExists() {
        final EntityDTO dto = new EntityDTO(1L, "anything", "code");
        assertThrows(RuntimeException.class, () -> entityService.create(dto));
    }

    @Test
    void shouldThrowWhenNameAndCodeAlreadyExists() {
        final EntityDTO dto = new EntityDTO(1L, "name", "code");
        assertThrows(RuntimeException.class, () -> entityService.create(dto));
    }

    @Test
    void shouldNotThrowWhenUnique() {
        final EntityDTO dto = new EntityDTO(1L, "diff", "diff");
        final EntityDTO entityDTO = entityService.create(dto);
        assertNotNull(entityDTO);
    }
}
  • 1
    I really like this. So the correct approach is to only mock the result of the persistence layer instead of trying to mock the result of the filter method? Also, in this case, does it make sense to test the scenarios of the filtering method separately and, then, test this possible scenarios on the create method (from the service class)? – avaros Jan 12 '23 at 09:17
  • Your analysis is correct @avaros. If you had a repository in your service, you would need a mock, so the same is applied to MemoryDatabase, right? You can create both tests depending on confidence in your tests and considering futures reusabilities of it. – Rodrigo Conceição Jan 12 '23 at 12:11