Maxi ContieriIf you work on unit testing, sooner or later you will face this dilemma
TL;DR: Don't test your private methods.
Problems
Breaking Encapsulation
Code Duplication
Solutions
If your method is simple, you don't need to test it.
If your method is complicated, you need to convert it into a Method Object.
Do not make your methods public for testing.
Do not use metaprogramming to avoid protection.
Do not move the private computation to helpers.
Do not use static methods for computations.
Context
We test our classes and methods.
At some point, we rely on auxiliary computations and we need to test them in a white-box way.
Sample Code
Wrong
<?
final class Star {
private $distanceInParsecs;
public function timeToReachLightToUs() {
return $this->convertDistanceInParsecsToLightYears($this->distanceInParsecs);
}
private function convertDistanceInParsecsToLightYears($distanceInParsecs) {
return 3.26 * $distanceInParsecs;
//function is using an argument which is already available.
//since it has private access to $distanceInParsecs
//this is another smell indicator.
//We cannot test this function since it is private
}
}
Right
<?
final class Star {
private $distanceInParsecs;
public function timeToReachLightToUs() {
return new ParsecsToLightYearsConverter($this->distanceInParsecs);
}
}
final class ParsecsToLightYearsConverter {
public function convert($distanceInParsecs) {
return 3.26 * $distanceInParsecs;
}
}
final class ParsecsToLightYearsConverterTest extends TestCase {
public function testConvert0ParsecsReturns0LightYears() {
$this->assertEquals(0, (new ParsecsToLightYearsConverter)->convert(0));
}
//we can add lots of tests and rely on this object
//So we don't need to test Star conversions.
//We can yet test Star public timeToReachLightToUs()
//This is a simplified scenario
}
Detection
[X] SemiAutomatic
This is a semantic smell.
We can only find metaprogramming abuse on some unit frameworks.
Tags
- Test Smells
Conclusion
With this guide, we should always choose the method object solution.
Relations
More Info
Credits
Photo by Dan Nelson on Unsplash
Just as it is a good practice to make all fields private unless they need greater visibility, it is a good practice to make all fields final unless they need to be mutable.
Brian Goetz
This article is part of the CodeSmell Series.
