How would you reduce the redundancy in tests of a class where a couple methods are just a wrapper around a few other methods of same class.
For example if I were to test a class which validates the account status of a user based on some conditions verified by other methods. This class for example:
public function validateProfile(UserInterface $user)
{
// check if profile is completed
}
public function validatePurchasedProducts(UserInterface $user)
{
// check if user has purchased products
}
public function validateAssociatedCard(UserInterface $user)
{
// check if user has a card associated with account
}
public function validateLoginStatus(UserInterface $user)
{
return $this->validateProfile($user)
and $this->validatePurchasedProducts($user)
and $this->validateAssociatedCard($user);
}
I can write tests for first 3 methods but when it comes to the last method I have to repeat the exact same thing that I did in last 3 methods and combine that together.
It make tests too redundant:
public function testUserHasValidProfileDetails()
{
// arrange mocks // act // assert
}
public function testUserHasPurchasedProduct()
{
// arrange mocks // act // assert
}
public function testUserHasCardAssociated()
{
// arrange mocks // act // assert
}
public function testUserCanLogInToDashboard()
{
// arrange mocks // act // assert - for profile validation
// arrange mocks // act // assert - for products validation
// arrange mocks // act // assert - for card validation
}
Is there a way (or feature in PHPUnit) that allows this kind of behaviour ? I know I can annotate the tests with @depends but that is not quite this thing.
I have used the @depends to pass items from one test to another, quite similar to the array in the example. However, internally, I would change my code (and then my tests) based on what you are trying. I perform the validation by having each set an internal value for being valid. This allows me to test each function and have the internal state of the object set, so that I can hard set it for future tests.
private $ProfileValid;
private $PurchasedProducts;
private $CardAssociated;
public function validateProfile(UserInterface $user)
{
// check if profile is completed
$this->ProfileValid = true;
}
public function validatePurchasedProducts(UserInterface $user)
{
// check if user has purchased products
$this->PurchasedProducts = true;
}
public function validateAssociatedCard(UserInterface $user)
{
// check if user has a card associated with account
$this->CardAssociated = true;
}
public function validateLoginStatus(UserInterface $user)
{
if(is_null( $this->ProfileValid) )
{
$this->validateProfile($user);
}
if(is_null( $this->PurchasedProducts) )
{
$this->validatePurchasedProducts($user)
}
if(is_null( $this->CardAssociated) )
{
$this->validateAssociatedCard($user);
}
return $this->ProfileValid && $this->PurchasedProducts && $this->CardAssociated;
}
Then, I can create an object and run each test individually (with or without mock objects) and use reflection to see if the internal variable is set correctly.
The final test then creates the object and sets the value (again with reflection) and calls the final validateLoginStatus(). As I can control the object, I can set one or many of the variables to be null to have the test called. Again, with a mock if needed. Also, the setting of the Mock can be an internal function in the test code that accepts parameters.
Here is an example of something similar for testing my own internal iterator works from an Abstract Class.
class ABSTRACT_FOO extends FOO
{
public function CreateFoo() { }
public function CloseFoo() { }
public function AddTestElement($String)
{
$this->Data[] = $String;
}
}
class FOO_Test extends \PHPUnit_Framework_TestCase
{
protected $FOOObject;
protected function setUp()
{
$this->FOOObject = new ABSTRACT_FOO();
}
protected function tearDown()
{
}
/**
* Create the data array to have 3 items for test iteration
*/
public function testCreateData()
{
$this->FOOObject->AddTestElement('Record 1');
$this->FOOObject->AddTestElement('Record 2');
$this->FOOObject->AddTestElement('Record 3');
$ReflectionObject = new \ReflectionObject($this->FOOObject);
$PrivateConnection = $ReflectionObject->getProperty('Data');
$PrivateConnection->setAccessible(TRUE);
$DataArray = $PrivateConnection->getValue($this->FOOObject);
$this->assertEquals(3, sizeof($DataArray));
return $this->FOOObject; // Return Object for next test. Will have the 3 records
}
/**
* @covers lib\FOO::rewind
* @depends testCreateData
*/
public function testRewind($DataArray)
{
$DataArray->Next();
$this->assertGreaterThan(0, $DataArray->Key(), 'Ensure the iterator is not on the first record of the data.');
$DataArray->Rewind();
$this->assertEquals(0, $DataArray->Key());
}
/**
* @covers lib\FOO::current
* @depends testCreateData
*/
public function testCurrent($DataArray)
{
$DataArray->Rewind();
$Element = $DataArray->Current();
$this->assertInternalType('string', $Element);
$this->assertEquals('Record 1', $Element);
}
/**
* @covers lib\FOO::key
* @depends testCreateData
*/
public function testKey($DataArray)
{
$DataArray->Rewind();
$this->assertEquals(0, $DataArray->Key());
}
/**
* @covers lib\FOO::next
* @depends testCreateData
*/
public function testNext($DataArray)
{
$DataArray->Rewind();
$this->assertEquals(0, $DataArray->Key(), 'Ensure the iterator is at a known position to test Next() move on');
$DataArray->Next();
$this->assertEquals(1, $DataArray->Key());
$Element = $DataArray->Current();
$this->assertInternalType('string', $Element);
$this->assertEquals('Record 2', $Element);
}
/**
* @covers lib\FOO::valid
* @depends testCreateData
*/
public function testValid($DataArray)
{
$DataArray->Rewind();
for($i = 0; $i < 3; ++ $i) // Move through all 3 entries which are valid
{
$this->assertTrue($DataArray->Valid(), 'Testing iteration ' . $i);
$DataArray->Next();
}
$this->assertFalse($DataArray->Valid());
}
}
This has allowed me to check the class without repeating a lot of functionality on loading data. Using the individual tests you have in the first place, can also check that each function is working correctly. If you structure your tests to have a test of the validateLoginStatus() can then be done with a mock with only values you want set to ensure all the combinations work as you may want to continue if not all 3 are present in the future. I would even use the testing of all 3 options to be tried using the dataProvider functionality.
The purpose of the validateLoginStatus method is call the other ones. So for testing that method, you don't need to test if the other methods work as expected (you do that on each method test). You just need to be sure that the other methods are called, maybe in the correct order. For that you can use a partial mock of the class, and mock the methods being called.
$object = $this->getMock(
'Class',
array(
'validateProfile',
'validatePurchasedProducts',
'validateLoginStatus'
)
);
// configure method call expectations...
$object->validateLoginStatus($user);
Another reason for doing this is that when some method stops working as expected, only a single test will fail.
You should end up repeating yourself. Since all the methods are part of the same class, you don't necessarily know that the other methods are being called. The class could have been written to have the logic in the 3 validation methods copied into the last method and the tests should pass (not that this is a good thing). But it is possible that this was the original case and refactoring to expose the partial validations in your class should not result in any tests failing.
Generally speaking, if something is difficult to test it is a code smell that you should rethink your design. In your case, I would break the partial validations into their own classes that would get passed in and these can get mocked.
IMO, mocking the system being tested is a bad practice as you now are specifying the implementation details of the system. This will make refactoring the class more difficult later on.