Point stabilité, mise à jour des dépendance & qualité de code

Salut à tous !

Les deux gros développements du moment: Multi-utilisateurs & présence bluetooth dans Gladys 4 venant d’être mergée, je pense qu’il est temps de faire une petite pause niveau fonctionnalité et d’investir un peu de temps sur la stabilité, les mise à jour majeures des dépendances et la qualité du code.

Je pense qu’il est sain de faire ça régulièrement pour être sur que le projet continue d’être d’aussi bonne qualité qu’au début !

Dépendances:

  • J’en parlais hier, il devient critique de revoir le process de build du frontend, encore bloqué sous preact-cli@2.0.0: Mise à jour du build du front (preact-cli@2 actuellement)

  • Au niveau DB, on est sous Sequelize 4 actuellement, alors que Sequelize 6.5.0 est sorti. Je pense qu’il y a beaucoup de petites optimisations/bugfix qui ont du sortir entre ces deux versions, et ça pourrait faire un grand bien au projet de mettre à jour !

  • On est toujours en Node 12.xx, alors que Node 14 est la version LTS actuelle la plus récente.

  • Côté frontend, il va falloir retirer momentjs qui est super lourd, et deprecated ! datejs que l’on utilise dans le frontend fait tout autant le travail, en plus léger.

  • vis-data et vis-network prend une place folle pour pas grand chose, ça vaut vraiment le coup de le retirer.

  • unistore, la librarie de state managment qu’on utilise, a été mise à jour et je pense qu’il faudrait vraiment y passer ! D’ailleurs, de manière générale, je pense qu’il faut qu’on arrête de l’utiliser pour les données de composants, et se restreindre au variable globale (httpClient, user, c’est à peut près tout :stuck_out_tongue: ). On a pas mal de petit bug qui sont du au fait qu’on utilise ce store global pour stocker des états locaux au composants. C’est moi qui avait mis ça en place, à tord :slight_smile:

Qu’en pensez-vous ?

1 Like

Il y a une confusion entre frontend et backend, ou momentjs ET datejs sont utilisés en parallèle dans le frontend ?

Prends une folle ? C’est à dire :sweat_smile:

Pour le reste, je suis tout à fait d’accord, même si je peux pas encore donner un coup de main dessus… La stabilité c’est super important et une réécriture est souvent nécessaire, tu l’as montré régulièrement en 4 versions de Gladys …! Chapeau :slight_smile:

momentjs et datejs sont utilisé côté frontend. En fait on a de la chance, momentjs n’est utilisé qu’à un seul endroit: le calendrier :slight_smile: C’est de ma faute, j’ai laissé ça passé dans la review de la PR, mais clairement il faut se débarrasser de moment.

prendre une *place folle :slight_smile: Pardon j’ai loupé un mot !

Top!

Effectivement, je pense que dans tout projet informatique il faut avoir l’habitude de dédier au moins 20% du temps à de la réduction de dette technique. ça permet d’avoir un projet à jour, maintenable, et clean :slight_smile:

1 Like

@AlexTrovato @cicoub13 @VonOx Je ne sais pas si vous avez vu mon post, vous en pensez quoi ?

preact-cli@3

Oui, mais je pense que les changements Front majeurs sont difficiles aujourd’hui car 0 tests. Même remarque pour unistore un peu plus bas.

Est-ce que l’idée, c’est de réinitialiser un projet vide avec la version 3 et de copier le code Gladys dans ce nouveau projet à jour ?

Sequelize 6.5.0

Si on garde SQLite, go pour cette version. Je peux aider

Node 14

Je vois déjà la MR et ça devrait être assez facile

retirer momentjs pour remplacer par datejs

Yes, pareil : pas mal de tests manuels à faire côté Front

vis-data et vis-network

Je ne sais pas à quoi ça sert, je pense qu’on peut le virer.

unistore

Tu voudrais utiliser quoi à la place pour le store Front ?

@pierre-gilles je pense en effet que c’est nécessaire, et que c’est même quelque chose qu’il faut faire « au jour le jour » et éviter d’attendre 6 mois pour le faire.

Comme le dit @cicoub13 il y a pas de sujets côté front. On devrait peut-être passer du temps sur des tests du front avant migrations / changements histoire de s’assurer d’une certaines campagnes de non régression, même si cela ne remplacera pas les tests faits par un humain.
On en avait déjà parlé, et si je me souviens bien, on avait mentionné https://www.cypress.io/

Je pense qu’on va devoir y passer.

Le top serait donc qu’on se dispatch les tâches, et rien n’empêche de commencer les migrations sans les tests cypress.

Qui prend quoi ? (je prends nodejs v14 :stuck_out_tongue: )
Perso moyen chaud pour la DB,
mais je veux bien me lancer sur cypress, au moins histoire de poser les bases et introduire les tests sur le « core » (il ne faudra pas être pressé, c’est une découverte pour moi) .

1 Like

Allez y moi je vous regarde :sweat_smile:

Moment ça doit être dans mes cordes

EDIT: Le component react ne supporte pas dayjs :confused:

3 Likes

Et autre sujet mais assez lié, nous avons des dépendances avec des failles de sécurité de niveau high (12 front et 4 back). Je sais que la plupart des installations ne sont pas ouvertes sur le web mais je pense que c’est mieux de mettre à jour ces dépendances.

Je pense que ça vaut le coup d’activer DependaBot : Enabling and disabling version updates - GitHub Docs

1 Like

Lol tu trouves tjs une excuse pour ne pas faire !! :stuck_out_tongue:

1 Like

10000% pour !
Automatiser à minima la recherche de vulns dans le code et limite le configurer en bloquant sur les PR c’est une bonne idée pour s’assurer d’un niveau de sécurité minimum.

Il faut les tests avant je pense

Je suis d’accord pour la maintenance générale, c’est au jour le jour :slight_smile: (version de node, upgrade mineurs/patch des dépendances, qualité de code).

Après pour les changements majeures qui n’apportent rien à l’utilisateur/au projet, je ne suis pas forcément pour la course au dépendances.

On a quand même des moyens très limité sur le projet, et franchement si tu suis absolument toutes les dépendances en mettant à jour dès qu’une nouvelle sort (je ne parle pas des mises à jour de sécurité), bah tu ne développe plus.

Sequelize/tabler/etc… ils ont des rythmes de mise à jour breaking très supérieur à ce qu’on serait capable d’absorber au quotidien. Pour ça que pour ces mises à jour, il faut faire le pour et le contre et évaluer si les pour sont supérieur au temps qu’on pourrait passer sur d’autres fonctionnalités « interne » à Gladys… :slight_smile:

Je suis d’accord!

Je suis chaud pour qu’on se mette à Cypress :slight_smile:

Je pense que personne ici n’a jamais fais de Cypress, donc limite il faudrait avancer ensemble. Si tu veux regarder pour faire un POC, et ensuite on factorisera tous ensemble ? Je peux regarder aussi de mon côté :slight_smile:

ah merde? C’est sur à 100% ? On pourrait soit proposer une PR, soit trouver un hack pour que ça marche… sinon globalize ou date-fns c’est moins pire que moment

J’ai vu, c’est aussi le but de ce point stabilité :slight_smile:

J’ai déjà dependabot activé sur le repo pour la partie « notifications », après pour la partie « PR automatisée » je suis plutôt contre.

Sur un projet de l’échelle de Gladys, ça fait des dizaines/centaines de PR par jour, ça remplit complètement les PRs et le CI, et en fait c’est juste impossible d’aller aussi vite que lui :stuck_out_tongue:

ça je suis pour :slight_smile: il faut mettre un npm audit dans la CI et bloquer uniquement sur les failles « high/critical » :slight_smile:

Ok j’ai déjà commencé le POC pour cypress, je pense que je vais faire quelques pages (login, dashboard, inscription) pour voir les différents aspects, et l’intégration ci github action certainement.
Je vais tenter de tenir un rapport pour les questions ouvertes et autres points de difficulté rencontrés.
Dès que j’ai un petit quelque chose, je partagerai pour qu’on en discute.

3 Likes

Bonsoir à tous, si certains veulent jouer ou commenter la mise en place de cypress :

La doc (qui évolue avec l’expérimentation) :

Et la je joue avec les actions Github pour automatiser la chose, c’est plutot pas mal, mais biensur, ça va rallonger le temps de build :

2 Likes

Salut @AlexTrovato ! Super boulot, je mets ici mes réponses à tes questions ouvertes :

  1. Choix du nommage des fichiers de tests :
  2. je pense qu’il ne faut pas nommer tous nos fichier index.js mais tenter de découpage les cas fonctionnels par fichier
  3. 1 fichier par cas ? (login_success.js / login_error.js)

Un fichier par cas ça me semble énorme, tu trouves pas ? Côté serveur on fait un fichier de test par fichier dans Gladys, en général un fichier par fonction. Et ensuite dans le fichier on fait un cas par cas à tester, ça me parait plus clean, sinon ça va faire beaucoup de fichiers non ?

  1. snake_case vs camelCase ?

Côté front c’est du CamelCase avec la première lettre en majuscule, je pense qu’il faut garder la même règle pour les fichiers de tests.

  1. Démarrer le front en développement ou production ?

J’ai pas l’expérience de Cypress pour savoir quelle est la best practice la dessus ^^

  1. Démarrer également le serveur ? Ou mocker les réponses
  2. Si on démarre le serveur, on ne peut pas tester les services / intégrations
  3. Si on mock les réponses, plus de travail, maintenance plus lourde

Bonne question ça. Pour moi l’avantage de mocker ça serait effectivement pour les services, après l’inconvénient c’est que potentiellement si une route d’API change dans le code backend, et que les mocks sont pas mise à jour, bah les tests du front vont passer alors qu’en fait dans la vrai vie ça pète. C’est moins end-to-end quoi

Peut-être qu’une approche « hybride » serait possible ?

On mock tout ce qui est services, et l’API de Gladys core (login, etc…) n’est pas mockée comme ça on est certain que les routes critiques (login, forgot password, signup) fonctionnent bien en E2E.

  1. Comment détecter que le front est prêt pour démarrer les tests automatiquement ?

Là dessus j’ai pas l’expérience de Cypress, il va falloir enquêter ^^

  1. Comment gérer “proprement” la langue du navigateur ?

Il doit y avoir un moyen dans cypress non?