ungdev / etuutt-api

Users, classes, assos : An awesome API to rule them all
MIT License
2 stars 2 forks source link

Feat/tests #37

Open TeddyRoncin opened 1 year ago

TeddyRoncin commented 1 year ago

Description

Checklist

Test

Implementation

Tools

Documentation

ThomasRitaine commented 1 year ago

Salut @TeddyRoncin ! Super travail. Je vois que tu prends le projet à cœur et que as atteint un super niveau en dev, félicitations ! ✨

Par contre, une PR de 4 000 lignes ajoutées et 200 supprimés sans explications c'est hors d'atteinte pour un humain normal. Est-ce que tu pourrais diviser ta PR en de multiples PR organisées par thème ? Par exemple, je vois que tu as ajouté des espaces dans les annotations PHP des entités, tu devrais créer une PR distincte des Tests pour cela. De plus, est-ce que tu peux faire une PR nommée test/assos pour les tests sur la classe Assos, et ainsi de suite pour le reste ?

En tout cas, excellent boulot, et hésite pas à me contacter si t'as besoin d'aide ou de conseils !

TeddyRoncin commented 1 year ago

Hello, ouais, pas de problème. Le truc des espaces c'était pour le linting et respecter ce qu'il fallait faire avant de merge une pr, mais ok, je vais en faire une autre a côté pour ça

TeddyRoncin commented 1 year ago

Et au fait je sais pas si tu as vu le gform passer, mais le projet devrait beaucoup avancer ce semestre, on a relancé le dev, on est en train de recruter 10ish nouveaux ;)

TeddyRoncin commented 1 year ago

Ok, j'ai fini le review de ta PR ! J'ai mis plein de commentaires de review, j'espère qu'ils te seront utiles.

Tu étais vachement déter mdr, merci beaucoup ! :)

Pour php-cs-fixer, quand je le lance sur ta branche, ça me fait les modifs dont on a parlé, mais quand je le lance sur la branche dev, ça ne les fait pas. Est-ce que tu peux merge dev dans ta branche pour voir si ça change quelque chose ? Attention, merge dev dans ta branche, pas l'inverse

C'est bizarre, je viens de refaire un php-cs-fixer sur le projet (normalement tout devrait être comme sur cette branche, donc sans avoir déjà linté le code), mais il me trouve quasiment rien (genre 4 fichiers).

Par contre, je ne peux pas tester toutes les merveilleuses fonctionnalités que tu as développées, j'ai cette erreur au lancement des tests :

J'ai pas ça de mon côté. Essaie de lancer un php -i | grep "php.ini", puis récupère le nom du fichier après Loaded Configuration File, puis dans ce fichier regarde la valeur de memory_limit. Pour moi elle est set à -1 (https://stackoverflow.com/questions/51152078/allowed-memory-size-of-134217728-bytes-exhausted-tried-to-allocate-20480-bytes#answer-51153007) Btw je viens de relancer les tests de mon côté, on a encore du temps avant de merge, y'en a beaucoup qui ont crash/fail (superrrrr.... ça marchait il y a 2 semaines :angry: )

ThomasRitaine commented 1 year ago

Salut beau gosse 👋 Je viens de trouver pourquoi nos php-cs-fixer ne fixaient pas de la même manière. C'est parce que tu utilises la dernière version PHP CS Fixer 3.13.2, et la branche dev utilisait une version mineure avant PHP CS Fixer 3.14.2. Donc la prochaine fois, indique dans la description de ta PR que tu as fais un compser update

ThomasRitaine commented 1 year ago

Pour la limite de mémoire, maintenant nous utilisons Docker pour que tout le monde ait la même configuration. Donc on a deux solutions :

Dans les deux cas, il faut absolument que tu merge Dev dans ta branche pour avoir les dernières grosses modifications, et que tu utilises le projet avec Docker au lieu de ta config perso. N'hésite pas à m'appeler si tu as des problèmes avec l'installation

TeddyRoncin commented 1 year ago

Salut beau gosse wave Je viens de trouver pourquoi nos php-cs-fixer ne fixaient pas de la même manière. C'est parce que tu utilises la dernière version PHP CS Fixer 3.13.2, et la branche dev utilisait une version mineure avant PHP CS Fixer 3.14.2. Donc la prochaine fois, indique dans la description de ta PR que tu as fais un compser update

Ah ok, bien vu ! Désolé, je me souviens plus du tout de l'avoir fait

Soit tu arrives à gérer dans le code pour libérer la mémoire au fur et à mesure des tests pour rester dans les 128MB permis par la config du Docker (Meilleure option trophy)

Je peux essayer de voir si ça se fait, mais je suis pas sûr qu'on puisse maîtriser la RAM utilisée dans les tests (je ferai quand même des recherches là-dessus, parce que ouais c'est chiant, surtout si la quantité de RAM utilisée est proportionnelle aux nombre de tests)

Dans les deux cas, il faut absolument que tu merge Dev dans ta branche pour avoir les dernières grosses modifications, et que tu utilises le projet avec Docker au lieu de ta config perso. N'hésite pas à m'appeler si tu as des problèmes avec l'installation

Ok, je vais essayer avec Docker du coup, j'avais commencé à tester la dernière fois mais j'avais eu un peu la flemme sur le coup :)

TeddyRoncin commented 1 year ago

Rebase fait sur la dev

larueli commented 1 year ago

Pour vos commandes, vous pouvez spécifier directement la mémoire souhaitée : php -d memory_limit=-1 .... Est-ce que ça résoudrait vos problèmes ?

TeddyRoncin commented 1 year ago

Pour vos commandes, vous pouvez spécifier directement la mémoire souhaitée : php -d memory_limit=-1 .... Est-ce que ça résoudrait vos problèmes ?

Oui et non j'imagine. Oui dans le sens où il n'y aurait plus d'erreur, non au sens où si la quantité de mémoire utilisée est proportionnelle au nombre de tests, ça risque de vite partir en sucette. Mais ça reste utile de savoir ça (surtout si c'est pas possible de réduire la mémoire utilisée), merci !