The simplest answer to this question is of course “Make them public”, but this is probably also the worst answer.
This answer also won’t work if…
- You don’t want to overload the user of your class with ridiculous numbers of irrelevant methods.
- The code is legacy and you don’t know what sort of crazy reflection is happening inside the code that might break when the private method becomes public.
- You want to hide away the implementation details from the consumer
Another answer you might get is: “Use reflection to execute the method”.
The important reason both of these answers is wrong is that trying to test private methods is a code smell.
It usually indicates that your class is doing too much.
When might you want to test a private method?
Here is an example of where you might think you need to test a private method.
public class AccountCreator {
public bool CreateBossAccount(string password){
var isValid = isPasswordValid(password);
if(!isValid) return false;
//Create the boss account
}
public bool CreateEmployeeAccount(string password){
var isValid = isPasswordValid(password);
if(!isValid) return false;
//Create the employee account
}
private bool isPasswordValid(string password){
//Do some sort of complex validation
}
}
So you want to test the CreateBossAccount
and the CreateEmployeeAccount
methods.
Your unit test might look something like this
public class AccountCreatorTests {
[Test]
public void DoesNotCreateBossAccountWithEmptyPassword(){
var createResult = creator.CreateBossAccount("");
Assert.IsFalse(createResult);
}
//You may have a few tests to check the password validation
public void DoesNotCreateBossAccountWithShortPassword()
public void DoesNotCreateBossAccountWithSimplePassword()
public void DoesNotCreateBossAccountWithNumbersOnlyPassword()
}
Now the problem is clear. You want to test that the same password rules apply to both CreateBossAccount
and CreateEmployeeAccount
without duplicating the tests.
The wrong answer
If we try to apply the wrong answer to our test, a secret bug hole appears.
With the wrong answer applied, the isPasswordValid
method has now been changed to public.
And the unit test looks something like this
public class AccountCreatorTests {
[Test]
public void EmptyPasswordIsNotValid(){
var createResult = creator.IsPasswordValid("");
Assert.IsFalse(createResult);
}
//You may have a few tests to check the password validation
public void ShortPasswordIsNotValid()
public void SimplePasswordIsNotValid()
public void SimplePasswordIsNotValid()
}
Now you might think “Great I don’t have to worry about testing the password validation now”.
Well you’d be wrong. A few months down the line a new programmer is hired who is a bit of an idiot.
Somehow this idiot is refactoring and removes the call to IsPasswordValid
from the CreateBossAccount method.
public class AccountCreator {
public bool CreateBossAccount(string password){
//TODO: Is this even required? - The idiot
//var isValid = isPasswordValid(password);
//if(!isValid) return false;
//Create the boss account
}
...
public bool IsPasswordValid(string password){
//Do some sort of complex validation
}
}
The unit tests pass and the secret bug hole is revealed.
You know for sure that your IsPasswordValid
method validates passwords brilliantly.
But at no point is the system tested to prove that the result is used or that the method is even called.
The right answer
There is a very simple solution to this sort of problem that allows you to remove all the duplication, and close this secret bug hole.
The first step is to abstract away the act of validating passwords into a new interface.
public interface PasswordValidator {
public bool IsPasswordValid(string password);
}
Now in our unit test we can test test the code that deals with a bad password in isolation.
public class AccountCreatorTests {
private AccountCreator Creator;
private Mock<PasswordValidator> MockPasswordValidator;
[SetUp]
public void SetUp(){
MockPasswordValidator = new Mock<PasswordValidator>();
Creator = new AccountCreator(MockPasswordValidator.Object);
}
[Test]
public void CanCreateBossAccount(){
MockPasswordValidator.SetUp(v => v.IsPasswordValid("pwd"))
.Returns(true);
Creator.CreateBossAccount("pwd");
//Check it created successfully
}
[Test]
public void DoesNotCreateBossAccountWithInvalidPassword(){
MockPasswordValidator.SetUp(v => v.IsPasswordValid("pwd"))
.Returns(false);
var createResult = creator.CreateBossAccount("pwd");
Assert.IsFalse(createResult);
}
}
With this new pattern the password validator is injected into the constructor of the account creator.
Now the account creator doesn’t know anything about password validation as it’s left to the abstraction.
To complete the design you’ll need an implementation of the PasswordValidator
public class LengthOnlyPasswordValidator : PasswordValidator {
public bool IsPasswordValid(string password){
return password.Length > 5;
}
}
Now you are perfectly able to test the LengthOnlyPasswordValidator
in isolation, and there are no more secret bug holes.
This design also gives you the ability to swap out the validation rules by just creating a new implementation.