Can I write any meaningful unit test in this scenario?

Question

I have this entity -

public class MerchantConfig {
    @Id
    @GeneratedValue(strategy = GenerationType.AUTO)
    private Long id;
    private Long merchantId;
    private String configKey;
    private String configValue;
    ..
}

It stores configurations (key and value) at merchant level presently.

We need to store configs at user level (User to Merchant One to many mapping) as well now.

We added these two columns to the entity -

private Long entityId;
private String entityType;

entityId will now store merchantId/UserId and entityType will indicate the same with string value "merchantid" or "user_id".

Here is an existing service layer method -

public MerchantConfig enableSync(boolean isActive, Long merchantId,boolean isSync){
        MerchantConfig merchantConfig= merchantConfigRespository.findByMerchantIdAndConfigKey(merchantId,Constants.MerchantConfigKeys.SYNC_ENABLED.key);
        if(merchantConfig==null){
            merchantConfig = new MerchantConfig();
            merchantConfig.setMerchantId(merchantId);
            merchantConfig.setConfigKey(Constants.MerchantConfigKeys.SYNC_ENABLED.key);
        }
        if(isSync)
            merchantConfig.setConfigValue("1");
        else
            merchantConfig.setConfigValue("0");
        merchantConfig.setIsActive(isActive);
        return merchantConfigRespository.save(merchantConfig);
    }

And here is the controller above it -

@PostMapping("/admin/enableSync")
public ResponseEntity<Object> enableSync(HttpServletRequest request, HttpServletResponse response,
                                                  @RequestParam("merchantId") Long merchantId,
                                                  @RequestParam("isValid") boolean isValid,
                                                  @RequestParam("isSync") boolean isSync) {
    if (isValid && isSync)
        LOGGER.info("enabling sync for merchant {}", merchantId);
    else
        LOGGER.info("disabling sync for merchant {}", merchantId);

    MerchantConfig merchantConfig = merchantConfigService.enableSync(isValid, merchantId, isSync);
    if(merchantConfig!=null)
        return new ResponseEntity<>(new ResponseDTO(Constants.API_RESPONSE_SUCCESS, merchantConfig, "success"),
                HttpStatus.OK);
    return new ResponseEntity<>(new ResponseDTO(Constants.API_RESPONSE_FAILURE, "unable to enable sync"),
            HttpStatus.OK);
}

We decided to change only service and downwards, keeping the controller layer unchanged.

Replaced repository method call findByMerchantIdAndConfigKey() by findByEntityIdAndEntityTypeAndConfigKey() inside service method and made this change -

public MerchantConfig enableSync(boolean isActive, Long merchantId,boolean isSync){
    MerchantConfig merchantConfig= merchantConfigRespository.findByEntityIdAndEntityTypeAndConfigKey(merchantId,Enum.MerchantConfigType.MERCHANT_ID.v,Constants.MerchantConfigKeys.SYNC_ENABLED.key);
    if(merchantConfig==null){
        merchantConfig = new MerchantConfig();
        merchantConfig.setEntityId(merchantId);
        merchantConfig.setEntityType(Enum.MerchantConfigType.MERCHANT_ID.v);
        merchantConfig.setConfigKey(Constants.MerchantConfigKeys.SYNC_ENABLED.key);
    }
    if(isSync)
        merchantConfig.setConfigValue("1");
    else
        merchantConfig.setConfigValue("0");
    merchantConfig.setIsActive(isActive);
    return merchantConfigRespository.save(merchantConfig);
}

What useful unit test case do I put here at the service layer?

If I were to mock the repository layer response and verify that the servive method is returning the MerchantConfig entity same as the mocked response, won't it be an overkill?

Instead, I feel it would be more useful to test the db values, by making some known entries to db. E.g., in this case, when there already is some merchant level config in db and a user level config is to be entered, the user level entry should overwrite, because the configurer is trying to unset all merchant level configs.

It would have been useful to test if given a merchant level config in db, when the save config API end point is called to save a user level config, the merchant level configs are marked inactive.

By the way, what are these tests be called? Please give some solution as I have been struggling with this debate over quite some time. Every time I sit down writing the traditional unit test cases, I spend too much time and still something breaks, which could not have been covered by unit tests. Please give some comprehensive guide to write such tests as well.

Answer 1

First of all, thanks for asking! This is a good question, that made me thinking. Now about the answer...

If there's any scientifically proven the only right way to write tests, I am not aware of it. You, as a professional, will have to make a call. Here're things that I would consider if I were you:

• Writing unit test for a couple of lines change may indeed look like overkill. It is easy to imagine how one ends up with 30-lines unit test with mocking to test just a couple of line changes. Let it be option 1.
• On the other hand, writing an integrated test with DB (which, for the test sake, maybe in-memory) may look like a simple solution now. Let it be option 2.

Option 1 The rule of thumb I use is that if something can be tested by unit-tests, it should be tested by unit-test unless I have good reasons to think otherwise. That is my personal choice, and the reasons for it:

• Speed. Unit tests always faster to run and debug (even if they are longer to write)
• Granularity. With a unit test, I almost always know where the problem is.

Option 2 Now, the only legitimate reason to use integrated test instead of unit test, in my opinion, is that something is extremely difficult/impossible to test otherwise. I would probably have chosen this in your case only if I were completely sure that this part of logic would never be changed anymore, but I don't think it is the case.

And now we can come up with option 3: it does look like enableSync does to many things (i.e. creates/looks for object and toggles sync flag)

If you extract the code below into a separate method (let's say, build merchantConfig):

 MerchantConfig merchantConfig= 
    if(merchantConfig==null){
        merchantConfig = new MerchantConfig();
        merchantConfig.setEntityId(merchantId);
        merchantConfig.setEntityType(Enum.MerchantConfigType.MERCHANT_ID.v);
        merchantConfig.setConfigKey(Constants.MerchantConfigKeys.SYNC_ENABLED.key);
    }

This should be pretty easy to unit-test, and no DB interaction or mocking will be necessary.