No tengas miedo a refactorizar un test legacy en PHP

26-09-2023

Por Rubén Zamora

Wuola amiguis 👋🏻!

Llevo esta semana trabajando en un nuevo desarrollo, que me ha dado la oportunidad de poder enseñaros mis tips o consejitos 🤓 del día, esos que me ayudan a mejorar la legibilidad de los 🧪test en PHP que encuentro. Porque ya que paso por ahí, vamos a ir mejorando el código poquito a poquito.

PD: Aclaro que aunque se vea más de un test, en el que más me voy a centrar es en el primero, porque el principio que aplico es lo mismo en todos más o menos 🤪

Dejo por aquí cosas que veremos:


COMENZAMOS (Música épica suena)

Refactor-time

Imaginemos que andas un día, como otro cualquiera super feliz con un desarrollo entre manos, no sé tu, pero yo me veo tal que así

dad-coraline

Y de repente QUÉ ES ESO ?? UN TEST LEGACY HA APARECIDO

Yo siempre me imagino esta canción cuando digo eso.

🧪 El test legacy en cuestión ``` public function testValidDataForClassicContractShouldReturnValidJson(): void { $contract = (new ContractMockBuilder( $this->prophesize(ClassicContract::class), $this->prophesize(Address::class) ))->build();
    $invoice = (new InvoiceMockBuilder($this->prophesize(Invoice::class)))->build();

    $billingRepository = (new BillingRepositoryMockBuilder(
        $this->prophesize(BillingRepositoryInterface::class)
    ))->build();

    $clientApiDto = (new ClientApiDtoMockBuilder($this->prophesize(ClientApiDto::class)))
        ->build();

    $clientCommunication = (new ClientCommunicationMockBuilder(
        $this->prophesize(ClientCommunicationInterface::class)
    ))
        ->shouldCallByEkonId(
            ContractMockBuilder::DEFAULT_CONTRACT_EKON_ID,
            $clientApiDto
        )->build();

    $userDto = (new UserDtoMockBuilder($this->prophesize(UserDto::class)))->build();

    $userCommunication = (new UserCommunicationMockBuilder(
        $this->prophesize(UserCommunicationInterface::class)
    ))
        ->shouldCallByUserId(
            ContractMockBuilder::DEFAULT_CLIENT_USER_ID,
            $userDto
        )->build();

    $applicationLogger = (new ApplicationLoggerMockBuilder(
        $this->prophesize(ApplicationLogger::class)
    ))
        ->build();

    $createInvoiceEmailDTO = new CreateInvoiceEmailDTO(
        $billingConceptRepository,
        $clientCommunication,
        $userCommunication,
        $applicationLogger
    );
    $invoiceEmailDTO = $createInvoiceEmailDTO->execute($contract, $invoice);
    $jsonSerializedInvoiceEmail = json_encode($invoiceEmailDTO);
    $this->assertJsonStringEqualsJsonString(
        '{"invoiceId":"2015G00000001","contractId":"2015000001","erpId":"' .
        ClientApiDtoMockBuilder::CLIENT_API_DTO . '","cups":"ES021701005100000PL","clientName":"' .
        ClientApiDtoMockBuilder::CLIENT_API_DTO_BUSINESS_NAME . '","clientPersonId":"4610000A","clientAddress":"' .
        'full address","holderName":"Paquito","holderPersonId":"46140000X",' .
        '"holderAddress":"Calle P\u00e0dua n\u00fam. 106, EN-1","contactName":"Paquito",' .
        '"contactEmails":["paquito@test.com"],"dueDate":"08\/11\/2015","dateStart":"1\/10\/2015",' .
        '"dateEnd":"31\/10\/2015","issueDate":"1\/11\/2015","totalAmount":"19.29","voidedInvoice":null,' .
        '"language":"ca","rateCode":"3.2","billedDays":"31","consumptionKwh":"187.36",' .
        '"consumptionCubicMeters":"16.00","conversionFactor":"11.710000",' .
        '"bankAccount":"ES8014650120341700537356","vatPercentage":"21.0","vatAmount":"3.35",' .
        '"invoiceConcepts":[],"readingRanges":[],' .
        '"comment":"\u00a1Gracias por ser como eres!",' .
        '"hash":"0a812fac182cda772c851c761db35ed3477573d3fe8d0f2694539bc651e87f9a",' .
        '"fileName":"0a812fac182cda772c851c761db35ed3477573d3fe8d0f2694539bc651e87f9a",' .
        '"taxableOtherCostsAmount":"0","nonTaxableOtherCostsAmount":"0","totalInvoice":"3.35","fee":"19.95",' .
        '"regularizationAmount": null,"tags":[], "product": "classic"}',
        $jsonSerializedInvoiceEmail
    );
}

public function testValidInvoiceConceptShouldCreateInvoiceConceptsEmailDTO(): void { $invoiceConcept = (new InvoiceConceptMockBuilder($this->prophesize(InvoiceConcept::class))) ->build();

    $billingConceptRepository = (new BillingConceptRepositoryMockBuilder(
        $this->prophesize(BillingConceptRepositoryInterface::class)
    ))->build();

    $clientCommunication = (new ClientCommunicationMockBuilder(
        $this->prophesize(ClientCommunicationInterface::class)
    ))
        ->build();

    $userCommunication = (new UserCommunicationMockBuilder(
        $this->prophesize(UserCommunicationInterface::class)
    ))
        ->build();

    $applicationLogger = (new ApplicationLoggerMockBuilder(
        $this->prophesize(ApplicationLogger::class)
    ))
        ->build();

    $createInvoiceEmailDTO = new CreateInvoiceEmailDTO(
        $billingConceptRepository,
        $clientCommunication,
        $userCommunication,
        $applicationLogger
    );
    $invoiceCocnept = $createInvoiceEmailDTO->createInvoiceConceptDto($invoiceConcept);

    $this->assertNotEmpty($invoiceCocnept);
}
</details>

Voy a empezar haciendo más legibles los test. Hay muchas veces que se suele poner el prefijo "test" para que PHPUnit los pueda leer, pero esto es bastante incómodo la verdad, por no hablar de cuando tenemos que leer algo super largo en camelCase. Por lo que utilizaremos la anotación `@test` que pa' eso está y pasaremos el enunciado a snake_case

Nos apoyaremos en el plugin [string manipulation](https://plugins.jetbrains.com/plugin/2162-string-manipulation) para cambiar el texto directamente a snake_case

>💡 **ProTIP:** Shortcut en Mac ⇧⌥M

<img src="/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/refactor-test-enunciado.gif" alt="refactor test enunciado" width="690" height="424">

Aquí de paso, vemos que usan una librería de mock que se llama [Prophecy](https://github.com/phpspec/prophecy) hasta dentro de la cocina 🤯. *Esto no es de por sí malo, pues hay que entender que lo mismo en su momento era la mejor manera en la que podían hacer los test, ahora mismo tenemos más cultura de ellos, pero lo mismo mañana todo cambia*.

Dicho esto, simplemente **nos limitaremos a mejorar lo que podamos**.

## GIVEN
----

Vemos que el bloque GIVEN es bastante complicado de identificar a simple vista, aparte de que se repite en los 2 test que vemos. Entonces, lo que podemos ir haciendo es extraerlo a métodos de inicialización que sean parte del GIVEN. Aquí identifico dos bloques, uno que necesita el groso de contratos y facturas, pero hay otro bloque que sólo se usa para construir el `CreateInvoiceEmailDTO`

![code_blocks](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/code_blocks.webp)

Ahora el bloque nos queda mucho más simplificado, desestructurando el retorno de la función para devolvernos múltiples valores. [Más info aqui](https://www.php.net/manual/en/functions.returning-values.php#example-168)

El test que ya nos trae una instancia de `createInvoiceEmailDTO`

![code_1](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/code-1.webp)

Y el test que solo necesita los parámetros para crear `createInvoiceEmailDTO`

![code_2](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/code-2.webp)

## WHEN
----
Vamos simplemente a dejar claro donde está el **when** separándolo del **given** y del **assert**

![assert_block](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/assert-block.webp)

😮‍💨UFFF.... madre mía, eso ha sido tan duro de hacer que tengo que levantarme a desconectar 5 min.

![two-hours-later](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/two-hours-later.webp)

Bueno, seguimos al tajo para el siguiente y último bloque

## ASSERT
----
![asser_json_block](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/assert-json-block.webp)

Para el momento estaba bien, pero es bastante complicado o incomodo de trabajar así un json, aunque usemos `inject language` , por lo que aquí he optado por usar la función de PHP [file_get_contents](https://www.php.net/manual/en/function.file-get-contents.php), que nos ayuda a leer un fichero pasándolo a string, quedando finalmente tal que así:

![assert_refactorized](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/assert-refactorized.webp)

Así mejor en mi opinión

![flirt-millhouse](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/flirt-millhouse.gif)


Si necesitamos inyectarle variables al JSON, podemos ayudarnos con la función [sprintf](https://www.php.net/manual/es/function.sprintf.php) poniendo en el fichero JSON `%s` en los valores que queremos sustituir:

![json_y_sprintf](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/json-and-sprintf.webp)


Si vemos el bloque del test, ahora creo que queda más clara la división del **given**, **when** y **assert**

![final_result](/images/articles/no-tengas-miedo-a-refactorizar-un-test-legacy-en-PHP/final-result.webp)


## Resultado final
----
🥸 Antes
<details>
<summary>Ver código antes del proceso de refactorización</summary>

public function testValidDataForClassicContractShouldReturnValidJson(): void { $contract = (new ContractMockBuilder( $this->prophesize(ClassicContract::class), $this->prophesize(Address::class) ))->build();

    $invoice = (new InvoiceMockBuilder($this->prophesize(Invoice::class)))->build();

    $billingRepository = (new BillingRepositoryMockBuilder(
        $this->prophesize(BillingRepositoryInterface::class)
    ))->build();

    $clientApiDto = (new ClientApiDtoMockBuilder($this->prophesize(ClientApiDto::class)))
        ->build();

    $clientCommunication = (new ClientCommunicationMockBuilder(
        $this->prophesize(ClientCommunicationInterface::class)
    ))
        ->shouldCallByEkonId(
            ContractMockBuilder::DEFAULT_CONTRACT_EKON_ID,
            $clientApiDto
        )->build();

    $userDto = (new UserDtoMockBuilder($this->prophesize(UserDto::class)))->build();

    $userCommunication = (new UserCommunicationMockBuilder(
        $this->prophesize(UserCommunicationInterface::class)
    ))
        ->shouldCallByUserId(
            ContractMockBuilder::DEFAULT_CLIENT_USER_ID,
            $userDto
        )->build();

    $applicationLogger = (new ApplicationLoggerMockBuilder(
        $this->prophesize(ApplicationLogger::class)
    ))
        ->build();

    $createInvoiceEmailDTO = new CreateInvoiceEmailDTO(
        $billingConceptRepository,
        $clientCommunication,
        $userCommunication,
        $applicationLogger
    );
    $invoiceEmailDTO = $createInvoiceEmailDTO->execute($contract, $invoice);
    $jsonSerializedInvoiceEmail = json_encode($invoiceEmailDTO);
    $this->assertJsonStringEqualsJsonString(
        '{"invoiceId":"2015G00000001","contractId":"2015000001","erpId":"' .
        ClientApiDtoMockBuilder::CLIENT_API_DTO . '","cups":"ES021701005100000PL","clientName":"' .
        ClientApiDtoMockBuilder::CLIENT_API_DTO_BUSINESS_NAME . '","clientPersonId":"4610000A","clientAddress":"' .
        'full address","holderName":"Paquito","holderPersonId":"46140000X",' .
        '"holderAddress":"Calle P\u00e0dua n\u00fam. 106, EN-1","contactName":"Paquito",' .
        '"contactEmails":["paquito@test.com"],"dueDate":"08\/11\/2015","dateStart":"1\/10\/2015",' .
        '"dateEnd":"31\/10\/2015","issueDate":"1\/11\/2015","totalAmount":"19.29","voidedInvoice":null,' .
        '"language":"ca","rateCode":"3.2","billedDays":"31","consumptionKwh":"187.36",' .
        '"consumptionCubicMeters":"16.00","conversionFactor":"11.710000",' .
        '"bankAccount":"ES8014650120341700537356","vatPercentage":"21.0","vatAmount":"3.35",' .
        '"invoiceConcepts":[],"readingRanges":[],' .
        '"comment":"\u00a1Gracias por ser como eres!",' .
        '"hash":"0a812fac182cda772c851c761db35ed3477573d3fe8d0f2694539bc651e87f9a",' .
        '"fileName":"0a812fac182cda772c851c761db35ed3477573d3fe8d0f2694539bc651e87f9a",' .
        '"taxableOtherCostsAmount":"0","nonTaxableOtherCostsAmount":"0","totalInvoice":"3.35","fee":"19.95",' .
        '"regularizationAmount": null,"tags":[], "product": "classic"}',
        $jsonSerializedInvoiceEmail
    );
}

public function testValidInvoiceConceptShouldCreateInvoiceConceptsEmailDTO(): void { $invoiceConcept = (new InvoiceConceptMockBuilder($this->prophesize(InvoiceConcept::class))) ->build();

    $billingConceptRepository = (new BillingConceptRepositoryMockBuilder(
        $this->prophesize(BillingConceptRepositoryInterface::class)
    ))->build();

    $clientCommunication = (new ClientCommunicationMockBuilder(
        $this->prophesize(ClientCommunicationInterface::class)
    ))
        ->build();

    $userCommunication = (new UserCommunicationMockBuilder(
        $this->prophesize(UserCommunicationInterface::class)
    ))
        ->build();

    $applicationLogger = (new ApplicationLoggerMockBuilder(
        $this->prophesize(ApplicationLogger::class)
    ))
        ->build();

    $createInvoiceEmailDTO = new CreateInvoiceEmailDTO(
        $billingConceptRepository,
        $clientCommunication,
        $userCommunication,
        $applicationLogger
    );
    $invoiceCocnept = $createInvoiceEmailDTO->createInvoiceConceptDto($invoiceConcept);

    $this->assertNotEmpty($invoiceCocnept);
}

</details>

🤩 Después
<details>
<summary>Ver código después del proceso de refactorización</summary>
/** @test */
public function valid_data_for_classic_contract_should_return_valid_json(): void
{
    [$contract, $invoice, $createInvoiceEmailDTO] = $this->initializeSceneryTo(self::CLASSIC_CONTRACT);

    $invoiceEmailDTO = $createInvoiceEmailDTO->execute($contract, $invoice);

    $jsonExpected = file_get_contents('tests/unit/[...]/JSON/emailDtoFromClassicContract.json');
    $jsonSerializedInvoiceEmail = json_encode($invoiceEmailDTO);
    $this->assertJsonStringEqualsJsonString(
        sprintf($jsonExpected, ClientApiDtoMockBuilder::CLIENT_API_DTO, ClientApiDtoMockBuilder::CLIENT_API_DTO_BUSINESS_NAME),
        $jsonSerializedInvoiceEmail
    );
}
</details>

## Conclusión

Creo que el `$this->` de este lenguaje es lo que peor llevo, pero igualmente se muestra bastante flexible para poder refactorizar un test, tenga las líneas que tenga. Y me gustaría hacer especial énfasis en que **el código perfecto no existe, pero sí que podemos hacer que tenga menos imperfecciones, dándole amor cuando más cultura de mejorarlos tenemos 🥳.**

Y si has llegado hasta aquí, espero que hayas disfrutado del camino y de la lectura 👏🏼 Espero debates de cosas que harían ustedes para mejorarlo incluso .