feat: run phpunits

[pifou25]

Description

J'ai passé un peu de temps à essayer de faire marcher les TU existants, mais je n'avance plus, alors je propose déjà le projet actuel, avec quelques questions... :)

Explications : 2 types de tests:

• intégration: tester jeedom en fonctionnement, utilise docker pour démarrer la bdd et le container jeedom = github workflow docker-test.yml (existe déjà mais j'ajoute juste l'exécution des TU)
• unitaire: tester une classe seule sans dépendance = github workflow phpunit.yml

les questions

A priori, les tests existants sont à classer dans la 1ere catégorie, mais, ils sont tous en erreur... on est d'accord qu'ils n'ont pas été maintenu depuis longtemps, est-ce que ça vaut le coup de les corriger ou bien on supprime tout pour repartir sur une base propre?

Pour la 2e catégorie, en principe la plus simple, on ne devrait inclure qu'une seule classe, celle à tester. Le problème c'est que toutes les classes ont le require_once __DIR__ . "/../php/core.inc.php"; et donc on se prend toutes les dépendances. C'était peut-être utile avant, mais avec l'autoloader maintenant on n'a plus besoin de répéter cet include dans toutes les classes ( ... vrai ?) Je l'ai donc enlevé. Mais ça reste compliqué de faire des TU simples car il y a un couplage fort avec certaines classes (DB, utils, en particulier) donc il faudra soit faire un mock de ces classes indispensables (j'ai commencé à le faire dans bootstrap.php), soit refactorer pour diminuer ce couplage.

(ps: j'ai aussi des changes d'une autre PR sur celle ci, mais sera mise à jour au fil de l'eau. C'est un brouillon)

Suggested changelog entry

exécution automatique des tests unitaires

[Hotfirenet]

Salut @pifou25,
Je constate également que les tests unitaires ne sont plus utilisés depuis un certain temps, et personnellement, je pense que c’est une évolution positive.
Je vais soumettre ta proposition à l'équipe pour discussion. Par ailleurs, je partage ton avis : repartir de zéro avec une nouvelle documentation pourrait être une excellente initiative.

Cordialement,

[kwizer15]

Merci pour cette initiative sur les tests. Je pense pouvoir apporter quelques éléments de réponse basés sur mon expérience précédente sur ce sujet :

Concernant les tests d'intégration existants : je confirme qu'il est préférable de les corriger plutôt que repartir de zéro. J'ai déjà fait ce travail dans une PR précédente #2902 et ils sont maintenant fonctionnels.

Pour le couplage fort avec les classes comme DB et utils :

• Garder le require_once de core.inc.php pour ne pas modifier la structure existante
• Pour DB spécifiquement, j'utilise une base de test dédiée qui fonctionne bien
• Pour gérer les autres dépendances fortement couplées, voici l'approche que je suggère :

  class MaClasse {
      // Au lieu d'appeler Utils directement
      protected static function getUtilsValue($param) {
          return utils::getValue($param);
      }
  }
  
  class MaClasseTest extends MaClasse {
      // Dans les tests, on peut surcharger la méthode
      protected static function getUtilsValue($param) {
          return 'valeur de test';
      }
  }

Cette approche d'encapsulation permet de :

• Tester la classe isolément sans modifier sa structure
• Garder le code de production propre et lisible

Je propose de collaborer pour partager la configuration de la base de test, échanger sur les patterns d'encapsulation qui fonctionnent bien et éviter les duplications d'effort.

N'hésitez pas si vous souhaitez qu'on en discute plus en détail !

[kwizer15]

Pourquoi ne pas faire directement $filename = __DIR__.'/core/config/common.config.php'); ?

Mais je trouve quand même risqué de pouvoir lancer le tests d'intégration avec la configuration courante de la bdd.
J'avais proposé le .env, mais on peut aussi

• réécrire ce fichier avec une config adaptée en gardant l'original en copie (pas fan, je trouve ca bancal, mais c'est une solution)
• intégrer un nouveau fichier common.config.test.php par exemple mais ca necessite le meme type de changement qu'avec le .env, chose que tu sembles vouloir éviter.
• surement d'autres solutions mais il faut analyser les impacts.

(Désolé pour le pavé)

[pifou25]

Oui ne me souviens plus pourquoi j'ai pondu un truc aussi compliqué, mais aucune raison de faire ça 👍
Il n'y a aucun risque à lancer les tests sur la config courante, on sera toujours dans un contexte spécifique (workflow, ou bien configuration de dev locale) je ne vois vraiment pas le besoin de rajouter on fichier spécifique common.config.test.php
On ne pourra en aucun cas lancer les TU sur un jeedom de prod puisque la dépendance phpunit ne sera pas installée, c'est une "dev-dependance" dans composer.json.

[kwizer15]

Je ne comprend pas pourquoi tu veux mocké l'autoloader ici. Quelle était le problème que tu as rencontré qui t'as amener a faire ca ?

[kwizer15]

En fait, je voudrais expliquer plus clairement mon point de vue sur les risques. Prenons un exemple concret de développeur : tu travailles sur Jeedom en local, tu navigues dans l'interface pour tester tes modifications, tout en voulant lancer des tests unitaires pour vérifier ton code. Le problème c'est que les tests actuels peuvent interférer avec ton travail - par exemple, ils vont automatiquement installer le plugin "virtual" s'il n'est pas là. Du coup, ton environnement de dev se retrouve modifié sans que tu l'aies voulu. Il y a aussi un autre souci : pendant que tu développes, tu fais plein de manipulations dans la base de données via l'interface. Mais pour que des tests d'intégration soient fiables, ils doivent partir d'un état connu et contrôlé - soit une base vierge, soit des données de test bien définies. Là, on se retrouve coincé : soit les tests écrasent tes modifications en cours, soit ils tournent sur une base dans un état qu'on ne maîtrise pas. C'est pour ça que je propose d'ajouter un fichier de config spécifique pour les tests - ça nous permettrait d'avoir un environnement dédié et contrôlé, sans risquer d'impact sur le travail de développement en cours. Effectivement, comme tu le dis, ce n'est pas un problème en prod ou en CI puisque ces environnements sont dédiés. C'est vraiment pour faciliter la vie des développeurs au quotidien.