Don't use the class under test to create the expected result
From time to time I come across a mistake in unit tests that makes them useless. The mistake is that we use the class and method that we are testing to create the expected result of the test:
<?php
declare(strict_types=1);
class BadTest extends TestCase
{
public function testThatPasswordIsHashed(): void
{
$hasher = new PasswordHasher();
$actualResult = $hasher->hash('super secret');
$expectedResult = $hasher->hash('super secret');
$this->assertSame($expectedResult, $actualResult);
}
}
If we were to break the behavior of the hash method, the test would pass. Returning an empty string is enough to break it but leave the tests green.
The SortOfFixedTest example shows an improvement over the BadTest. The problem here is that we repeat the underlying algorithm in the test. Sometimes we can’t avoid this, but those are rare cases.
<?php
declare(strict_types=1);
class SortOfFixedTest extends TestCase
{
public function testThatPasswordIsHashed(): void
{
$hasher = new PasswordHasher();
$actualResult = $hasher->hash('super secret');
$expectedResult = password_hash('super secret', PASSWORD_DEFAULT);
$this->assertSame($expectedResult, $actualResult);
}
}
Repeating algorithms like this will slow down the test suite. We are lazy, we would copy/paste the algorithm from the actual method. If we made a bug in the original algorithm, we would have it in our test as well.
We have to set the expected result to the result that we are expecting:
<?php
declare(strict_types=1);
class FixedTest extends TestCase
{
public function testThatPasswordIsHashed(): void
{
$hasher = new PasswordHasher();
$actualResult = $hasher->hash('super secret');
$expectedResult = '$2y$10$PfAO94tkT3whsYZRpzAmG.aNb9HOVUP9j92zn2Nfc8Qi8bxv5rx8O';
$this->assertSame($expectedResult, $actualResult);
}
}
If the hash method changes, the test will fail. We are not repeating the underlying algorithm in our test code, so there is no chance of repeating a bug from the tested method.
Happy hackin’!
