Ajouter des outils d'assurance qualité à un projet existant

Dylan Ballandras

Cover Image for Ajouter des outils d'assurance qualité à un projet existant

Dylan Ballandras

06/02/2020

L'histoire

Il est souvent difficile de mettre les autres d'accord pour supprimer du code inutilisé, améliorer la qualité du code ou simplement ajouter des outils ou une méthodologie. Voici une histoire adaptée d'un cas réel.

J'ai rejoint un projet que je devais maintenir. Plusieurs personnes y travaillaient. Sans maintenir les tests unitaires, sans linter de style de code ou d'analyseur statique. Ils n'utilisaient pas beaucoup de type hinting ou ne mettaient pas à jour PHPDoc sur un projet PHP 7.1 (PHP 7.4 étant déjà dispo...). C'était le chaos total. 😱

J'ai trouvé qu'il était vraiment difficile de déboguer, de tracer les erreurs et d'ajouter des fonctionnalités sans tout casser. Alors, j'ai d'abord essayé de réécrire beaucoup de choses, j'ai ajouté PHPStan, plus de tests PHPUnit, des scénarios Behat et j'ai ouvert une PR. Elle n'a pas bien été accueillie et a été fermée directement.

Les autres développeurs ne voulaient pas améliorer le code de la version actuelle, mais seulement négocier une réécriture totale de l'application. Ma proposition n'était donc pas utile pour eux car elle prouvait que l'application pouvait être maintenue et améliorée.

Mais tout le monde dans l'entreprise sait que la direction n'aurait jamais voulu une réécriture. Le projet fonctionne toujours. Il y a une chose qu'ils ne voient pas, en tant que manager : Ils perdent beaucoup d'argent à chaque fois que quelqu'un passe du temps à déboguer manuellement, à tester manuellement ou à faire des aller-retour avec des collègues PO/Chef de projet pour valider qu'une correction de bug n'ajoute pas de régression. Une autre chose que l'équipe ne veut pas comprendre, c'est qu'elle passerait trop de temps et ferait perdre beaucoup d'argent à son entreprise si elle adoptait la voie de la réécriture complète.

Alors, de mon point de vue, la meilleure façon de mettre les deux parties d'accord était de procéder à une mise à jour incrémentale. Cette fois, pas à la main ! Il existe un tas d'outils développés en Open Source et nous avons un environnement Gitlab CI prêt. Il était temps de l'utiliser pour prouver ce qu'il fallait améliorer, réécrire, supprimer.

Pour l'équipe de direction, vous avez besoin de chiffres, d'informations sur les coûts : Il faut faire l'effort de tout rapporter aux 💲.

Combien cela coûterait-il ? 💸
Combien allons-nous gagner ? 💰
De combien de temps aurez-vous besoin ? ⏲

Il y a déjà quelques bonnes listes pour l'AQ donc je ne vais pas être exhaustif mais seulement parler des outils actuels avec lesquels je travaille.

Ajouter des outils via Composer

Nous utiliserons différents types d'outils :

Normes de codage
Analyse statique
Un détecteur de copier/coller (CPD) pour le code PHP.
Un "mess"-detector
Un outil pour mesurer rapidement la taille d'un projet PHP.

Ajoutez-les via composer :

$ composer require --dev sylius-labs/coding-standard phpstan/phpstan sebastian/phpcpd phpmd/phpmd phploc/phploc

Ajoutez ensuite les scripts à votre composer.json :

{
  "scripts": {
    "code-style": [
      "vendor/bin/ecs check $(git diff --name-only --diff-filter=ACMRTUXB HEAD~..HEAD -- '*.php')"
    ],
    "code-style-fix": ["@code-style --fix"],
    "static-analysis": [
      "vendor/bin/phpstan analyse $(git diff --name-only --diff-filter=ACMRTUXB HEAD~..HEAD -- '*.php') -c phpstan.neon"
    ],
    "mess-detector": [
      "vendor/bin/phpmd $(git diff --numstat --name-only --diff-filter=ACMRTUXB HEAD~..HEAD -- '*.php' | awk -vORS=, '{ print $1 }' | sed 's/,$//') text unusedcode,cleancode,controversial,design"
    ]
  }
}

Commencez par des exemples

Dans chacun des prochains commits, vous utiliserez ces scripts pour faire des ajustements. Grâce à git diff --name-only --diff-filter=ACMRTUXB HEAD~..HEAD -- '*.php, vous ne verrez que les erreurs des fichiers que vous avez modifiés. modifiés.

Travaillez comme vous en avez l'habitude et jouez ces commandes :

$ composer code-style
$ composer code-style-fix
$ composer static-analysis -- --level max # start from 0 then upgrade the level toward 8.

Vous ne ferez que de petits changements. Votre équipe ne sera pas en mesure de s'opposer à ces améliorations. Ils verront les bénéfices de l'utilisation de vos nouveaux outils.

Faites attention à une chose lorsque vous utilisez les règles EasyCodingStandard de Sylius : Il ajoutera declare(strict_types=1); à vos fichiers. Selon la qualité initiale du projet, cette règle peut créer des changements de comportements importants qu'il ne faudrait pas laisser passer en production sans que ce soit validé !

Vous pouvez supprimer cette règle facilement :

imports:
  - {resource: 'vendor/sylius-labs/coding-standard/easy-coding-standard.yml'}

parameters:
  skip:
    # Will need to be remove when the entire codebase will be fully fixed with phpstan and ecs
    # @see https://stackoverflow.com/questions/48723637/what-does-strict-types-do-in-php
    PhpCsFixer\Fixer\Strict\DeclareStrictTypesFixer:
      - '*.php'

Impliquer vos collègues

Ajouter de l'Intégration Continue

Ensuite, vous devez informer votre équipe de ce qui ne va pas dans le projet et dans chacun de leurs commits. La seule condition qui est vraiment difficile à faire respecter est que vous ne pouvez pas fusionner si la pipeline échoue. Ce serait plus facile s'ils sont impressionnés par les améliorations précédentes.

Les gens ne changeront pas leurs habitudes sans y être forcés.

Voici un sous-ensemble d'un fichier CI de Gitlab. Comme vous pouvez le voir, certaines étapes permettent des échecs. Ceci permet des mises à jour incrémentales de la qualité du projet.

codequality:
  stage: pre-tests
  allow_failure: false
  script:
    - composer code-style
    - composer static-analysis -- --level max --error-format gitlab > phpstan-report.json
  artifacts:
    when: always
    reports:
      codequality: phpstan-report.json
    expire_in: 1 week

mess-detector:
  stage: pre-tests
  allow_failure: true
  script:
    - vendor/bin/phpmd src html unusedcode,cleancode,controversial,design --reportfile=phpmd.html --ignore-violations-on-exit
  artifacts:
    paths:
      - phpmd.html
    expire_in: 1 week

copy-paste-detector:
  stage: pre-tests
  allow_failure: true
  script:
    - vendor/bin/phpcpd src --fuzzy --log-pmd=phpcpd.xml || true # there is no flag to ignore violations on exit
  artifacts:
    paths:
      - phpcpd.xml
    expire_in: 1 week

phploc:
  stage: pre-tests
  allow_failure: true
  script:
    - vendor/bin/phploc src --log-xml=phploc.xml
  artifacts:
    paths:
      - phploc.xml
    expire_in: 1 week

Ajouter une revue de code

Après la création d'un ticket, chaque changement devrait être lu et discuté avec au moins un collègue développeur. Cet échange permettrait de s'entraider, de valider que le code correspond aux attentes du métier et que les tests ne semblent pas avoir de failles.

Quel sera votre workflow ?

A partir de maintenant, vous aurez les informations nécessaires pour savoir ce qui ne va pas. Localement, tout en travaillant, utilisez les scripts Composer que nous avons ajoutés pour faire des ajustements. PHPStan et PHP-CS-Fixer seront tes meilleurs amis A travers la CI, vous ferez respecter les nouvelles règles.

In fine, vous pourrez supprimer les lignes de code superflues, les classes inutiles, ajouterez plus de types, rendrez la base de code plus facile à travailler. N'oubliez pas de supprimer les classes, méthodes et variables inutilisées.

Les lignes de code commentées sont également des erreurs potentielles.

Une dernière chose, vous pouvez également supprimer toutes les lignes de code commentées si vous utilisez un système de contrôle de version comme Git. Creez une nouvelle branche. A partir de là, supprimez toutes les CLOC sur tout le projet. Create a new branch. From there, remove all the CLOC on all the project. Ce sont des informations inutiles. Pourquoi ? Parce qu'elles cachent ce qui est important.

Enfin, si vous vouliez vraiment réécrire votre projet, ce sera plus facile à partir de là. Il ne reste plus qu'à choisir des métriques pour évaluer la réalisation de cette correction. Elles peuvent prendre la forme d'une couverture minimale de tests sur les points critiques d'une application, par exemple (ce n'est pas une métrique que je recommenderais).

Même si votre projet n'est pas truffé de nombreux bugs, si vous venez de démarrer votre projet, commencez par la bonne méthodologie ! Je n'ai pas parlé des tests dans cet article de blog mais ils sont vraiment le meilleur moyen d'avoir un produit de bonne qualité. Ils sont bien plus importants à ajouter à votre flux de travail que l'analyse statique.

Pour moi, les personnes qui ne travaillent pas avec ces outils ou équivalents et qui ne font pas de tests sur leurs projets ne sont pas des professionnels, ce sont des hobbyistes. 🤜 🖐️ 🎤