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)