uk flag

Exemple de refactorisation - Partie 2 - Créer les tests unitaires

Dans la partie 1, j'ai effectué les refactorisations nécessaires pour rendre la classe UserManager testable. Dans cette partie 2, je vais réaliser les tests unitaires. Voici la classe UserManager:

// UserManager.php
class UserManager
{
    private $databaseRepository;

    private $emailClient;

    public function __construct(DatabaseRepository $databaseRepository, EmailClient $emailClient)
    {
        $this->databaseRepository = $databaseRepository;
        $this->emailClient = $emailClient;
    }

    public function createUser($data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        $this->databaseRepository->saveUser($data);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }
}

Analysons tout d'abord ce que fait la méthode createUser:

  1. Elle formate des données reçues en entrée
  2. Elle enregistre les données dans une base
  3. Elle envoie un email à l'administrateur pour lui indiquer qu'un utilisateur a été crée

Je vais vérifier que les deux derniers points sont bien exécutés en testant le comportement des mocks de DatabaseRepository et EmailClient: je vais m'assurer que la méthode d'enregistrement des données et la méthode d'envoi d'email sont bien utilisées dans la méthode createUser:

// UserManagerTest.php
class UserManagerTest extends \PHPUnit\Framework\TestCase
{
    public function testCreateUserMustCallMethodsSaveUserAndSendEmail()
    {
        $data = [
           'firstName' => 'John',
           'lastName' => 'Doe',
           'title' => 'Mr',
           'birthYear' => '1981',
           'birthMonth' => '06',
           'birthDay' => '04',
        ];

        $databaseRepositoryMock = $this->getMockBuilder(DatabaseRepository::class)->getMock();
        $databaseRepositoryMock->expects($this->once())->method('saveUser');
        $emailClientMock = $this->getMockBuilder(EmailClient::class)->getMock();
        $emailClientMock->expects($this->once())->method('sendEmail');
        $userManager = new UserManager($databaseRepository, $emailClient);
    }
}
OK (1 test, 2 assertions)

Comment ensuite tester que les données reçues en entrée par createUser sont formatées correctement? Dans l'état actuel du code ce n'est pas possible car les lignes qui effectuent le formatage ne sont pas séparées du reste de la méthode:

// UserManager.php
[...]
    public function createUser($data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        $this->databaseRepository->saveUser($data);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }

je dois extraire les lignes 5 à 10 dans une méthode qui aura comme valeur de retour les données formatées, je pourrai alors effectuer des assertions sur cette valeur. Pour atteindre cet objectif j'ai deux possiblités qui s'offrent à moi:

  1. Extraire ces lignes dans une méthode d'une nouvelle classe
  2. Extraire ces lignes dans une nouvelle méthode de la classe existante

Avant d'effectuer ce choix, il faut avoir en tête que pour avoir un code plus robuste, une classe ne doit avoir qu'une seule raison d’être modifiée c'est le principe de responsabilité unique. Quelles sont les différentes responsabilités de la classe UserManager? La classe UserManager formate les données reçues et gère le processus d'enregistrement (enregistrement dans la base de donnée et envoi d'email à l'administrateur). Cela fait 2 responsabilités.

Je pourrais donc extraire les lignes 5 à 10 dans une autre classe. Ceci aurait comme avantage de rendre le code de formatage des données facilement testable via la méthode public de cette nouvelle classe. Toutefois, ce serait Overkill vu le contexte: la classe UserManager est courte, je ne souhaite pas fragmenter mon code à cause de quelques lignes. Si les lignes concernant le formatage des données étaient beaucoup plus nombreuses et si la classe UserManager avait plus de méthodes, j'aurai sans aucun doute extrait ces lignes dans une nouvelle classe.

Je vais donc extraire les lignes 5 à 10 dans une nouvelle méthode. La visibilité de la méthode sera privée, j'expliquerai ce choix plus tard. Je vais tout d'abord créer le test unitaire de la future méthode _formatUserData et comme celle-ci est privée je vais devoir utiliser la reflexion pour pouvoir la tester:

// UserManagerTest.php
class UserManagerTest extends \PHPUnit\Framework\TestCase
{
    public function testFormatUserData()
    {
        $data = [
           'firstName' => 'John',
           'lastName' => 'Doe',
           'title' => 'Mr',
           'birthYear' => '1981',
           'birthMonth' => '06',
           'birthDay' => '04',
        ];
        $expectedResult = [
            'firstName' => 'john',
            'lastName' => 'doe',
            'title' => 'Mr',
            'birthdate' => '1981-06-04'
        ];

        $databaseRepositoryStub = $this->getMockBuilder(DatabaseRepository::class)->getMock();
        $emailClientStub = $this->getMockBuilder(EmailClient::class)->getMock();
        $userManager = new UserManager($databaseRepository, $emailClient);
        $result = $this->_invokePrivateMethod($userManager, '_formatUserData', [$data]);
        $this->assertEquals($expectedResult, $result, 'the transformed data is not like expected');
    }

    private function _invokePrivateMethod($object, $method, array $params = [])
    {
        $reflectionClass = new \ReflectionClass($object);
        $method = $reflectionClass->getMethod($method);
        $method->setAccessible(true);
        return $method->invokeArgs($object, $params);
    }
}

Lorsque j'éxécute les tests j'ai le résultat suivant:

ReflectionException : Method _formatUserData does not exist
ERRORS!
Tests: 2, Assertions: 2, Errors: 1.

Le test est en place, je vais donc modifier le code:

// UserManager.php
class UserManager
{
    private $databaseRepository;

    private $emailClient;

    public function __construct(DatabaseRepository $databaseRepository, EmailClient $emailClient)
    {
        $this->databaseRepository = $databaseRepository;
        $this->emailClient = $emailClient;
    }

    public function createUser($data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        $formattedUserData = $this->_formatUserData($data);
        $this->databaseRepository->saveUser($formattedUserData);
        $msg = 'a new user has been created';
        $this->emailClient->sendEmail('admin@example.com','A new user has been created', $msg);
    }

    private function _formatUserData(array $data)
    {
        $data['firstName'] = mb_strtolower($data['firstName']);
        $data['lastName'] = mb_strtolower($data['lastName']);
        $data['birthdate'] = $data['birthYear'] .'-'. $data['birthMonth'] .'-'. $data['birthDay'];
        unset($data['birthYear']);
        unset($data['birthMonth']);
        unset($data['birthDay']);
        return $data;
    }
}

Lorsque j'éxécute à nouveau les tests:

OK (2 tests, 3 assertions)

J'ai fait précédemment le choix de rendre la visibilité de la méthode _formatUserData privé, pourquoi ne pas avoir crée une méthode public?

Elle aurait été plus facilement testable car je n'aurai pas eu besoin d'utiliser la reflexion via la méthode _invokePrivateMethod. De plus, même si formatUserData est utilisée seulement par UserManager, cela n'aurai pas eu de mauvaises conséquences. C'est un argument recevable, toutefois, je trouve que cela rend moins lisible le fonctionnement de la classe: lors de lecture du code je pourrai me poser la question de savoir pourquoi cette méthode est public alors qu'elle n'est jamais utilisée directement dans l'application. J'ai donc fait ce choix pour ne pas introduire d'ambiguité et rendre le code plus lisible.

Les méthodes privées sont généralement considérées comme un détail d'implémentation. Elles doivent être testées à travers l'appel à une méthode public. Cela permet d'avoir des tests unitaires qui ne sont pas liés aux détails d'implémentation de la classe et donc de pouvoir effectuer des changements plus facilement : si je modifie le comportement interne d'une classe, un bon test unitaire doit m'assurer que l'interface public retourne toujours les données voulues. Avec de mauvais tests unitaires, si je modifie le comportement interne d'un classe tout en conservant les mêmes valeurs de retour des interfaces public, je devrais alors tout de même modifier ces tests unitaires car ils retourneront des erreurs.

Les principes expliqués précédemment ne peuvent pas s'appliquer dans cet exemple car tester la méthode createUser ne permet pas de savoir si la méthode _formatUserData fonctionne correctement. C'est pour cette raison que j'ai testé directement la methode _formatUserData.

J'ai fini la création des tests unitaires de la classe UserManager. Dans le prochain article je vais pouvoir effectuer quelques refactorisations de façon sûr car j'ai dorénavant des tests en place :D

Exemple de refactorisation - Partie 3 - Améliorer la lisibilité du code