Skip to main content

Should I test a method that calls a method that is already tested? [Resolved]

I have a method that does something like this

public void addFunds(Account account, int price) {
   int credits = account.getCredits()
   account.setCredits(credits + price)
   saveOrUpdate(account)
}

the method saveOrUpdate is already tested, so my question is: should I test the method add funds? I'm trying to follow the best practices.


Question Credit: Wissam Goghrod
Question Reference
Asked October 4, 2018
Posted Under: Programming
35 views
3 Answers

When you (unit) test addFunds, you should not be testing whether saveOrUpdate works. You should be testing whether saveOrUpdate gets called (in the right circumstances) and leave it at that.

If you're going to unit test, any lower dependencies (such as the saveOrUpdate method) should be mocked. You are, after all, only trying to test the addFunds method. What you're suggesting in your question is that you test both addFunds and saveOrUpdate, which means you're no longer unit testing.

Never lose track of what you're testing.

Whenever you want to write a test, first ask yourself what you seek to prove with the test's results. If you can't think of anything meaningful, then you shouldn't be writing a test.

I don't think you need to test this method. As it is, all you're really going to be able to test is if the orchestration of the underlying methods (getCredits, setCredits, saveOrUpdate) is being called as it should.

But this test is superfluous. The orchestration of the three methods is trivially readable from simply looking at the code. The method doesn't have diverging logical paths (e.g. an if else structure) that causes the logical flow to be less than obvious.

Some counterexamples of methods that do warrant testing:

public void addFunds1(Account account, int price) {
    if(!account.IsLocked)
    {
        int credits = account.getCredits();
        account.setCredits(credits + positivePrice);
        saveOrUpdate(account);
    }
}

Here, there is a meaningful test: does the method correctly refuse to update accounts that are locked?

public void addFunds2(Account account, int price, Currency currency) {
    if(currency != account.Currency)
    {
         price = ConvertCurrency(currency, account.Currency, price);
    }

    int credits = account.getCredits();
    account.setCredits(credits + positivePrice);
    saveOrUpdate(account);
}

Here, there is a meaningful test: are we able to add any kind of currency to an account?

Take note that we are NOT testing:

  • If the currency conversion is correct.
  • If the equality check between currencies is implemented correctly.
  • If the account update logic works.

What we ARE testing:

  • If the method converts to the correct currency when the price currency is different from the account currency.


public void addFunds3(Account account, int price) {
    bool shouldUpdate = false;        

    if(!account.IsLocked)
    {
        shouldUpdate = account.Currency.Code.StartsWith("Z");
    }
    else
    {
        int foo = account.AgeInDays + account.getCredits() % 97;
        string fooResult = externalRepo.CalculateResult(foo);
        shouldUpdate = foo.Length > 3;
    }

    if(shouldUpdate)
    {
        int credits = account.getCredits();
        account.setCredits(credits + price);
        saveOrUpdate(account);
    }
}

I know this is a silly example, but it's intended as an example of complex and not at all obvious logic. There are many valid things to test here. A generalized test intention could be described as:

Since there are reasons to only update the account in certain situations, does the method correctly update the account in all needed circumstances where an update is required, and not update the account in all needed circumstances where an update is expressly prohibited? (This will be many different tests)


credit: Flater
Answered October 4, 2018
Your Answer
D:\Adnan\Candoerz\CandoProject\vQA