Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ajout de tests #3438

Open
wants to merge 3 commits into
base: Beta
Choose a base branch
from
Open

Ajout de tests #3438

wants to merge 3 commits into from

Conversation

detobel36
Copy link
Contributor

@detobel36 detobel36 commented Dec 29, 2021

Comme discuté dans ce ticket: #3432

Je propose de rajouter des tests (au moins pour vérifier le formatage).
Comme la tâche est "grosse", je propose de faire "fichier par fichier" 😉

Voici donc la "première étape" où j'ai "mis en place" la vérification pylint et adapté le fichier default.py pour qu'il passe les tests.

Exemple de rendu du test: https://github.com/detobel36/venom-xbmc-addons/actions/runs/1633774038



def setSetting(plugin_id, value):
def try_to_call_method(self, action_site_name, path):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cette méthode permet de remplacer tous les is... (isTrakt, isGui, isFav, ...)

@sizanic
Copy link
Collaborator

sizanic commented Dec 29, 2021

Ce qui est sûr, c'est qu'on va d'abord sortir une version avant de se lancer de telles modifications.
Pour tester, plutôt que des PR, à voir pour nous proposer une branche à la place 🤔

@detobel36
Copy link
Contributor Author

detobel36 commented Dec 30, 2021

Je suis d'accord que ça va prendre du temps et que c'est plus une "tâche de fond".
Cependant, je pense qu'il pourrait être intéressant de déjà faire en sorte que les fichiers qui sont créés ou "refactor" suivent déjà les "bonnes pratiques" (PEP). On peut rajouter les fichiers "un par un" dans le fichier "action" 😉

Mais je vous laisse gérer... Moi je propose juste 😜

@Arias800
Copy link
Contributor

C'est une très bonne idée.
J'avais commencé à faire une actualisation du code, suivre le PEP faisait partie des objectifs, mais c'est vrai que c'est long et fastidieux comme tache.

@yodidon
Copy link
Contributor

yodidon commented Dec 30, 2021

C'est une très bonne idée.
J'avais commencé à faire une actualisation du code, suivre le PEP faisait partie des objectifs, mais c'est vrai que c'est long et fastidieux comme tache.

Bon le boulet comme d’hab c’est quoi le pep ?

@yodidon
Copy link
Contributor

yodidon commented Dec 30, 2021

Python extension proposale ok oula de la lecture pour moi, je me réponds et vais de ce pas lire tranquillement toutes ces règles

@detobel36
Copy link
Contributor Author

PEP c'est la norme 😄
Mais on est "libre" de modifier nos règles si on "aime pas une règle" 😉

Pour "justifier" cette proposition (et pas juste dire "faut suivre la norme"), ça permet à une personne qui ne "connait pas le projet" de directement écrire "le même code" qu'une personne qui est la depuis "longtemps". Vu que (normalement) tous les projets python suivent cette norme...

En gros ça permet de rendre tous le codes "pareils" dans leur formatage 😜

@sizanic
Copy link
Collaborator

sizanic commented Dec 30, 2021

il pourrait être intéressant de déjà faire en sorte que les fichiers qui sont créés ou "refactor" suivent déjà les "bonnes pratiques" (PEP). On peut rajouter les fichiers "un par un" dans le fichier "action"

On est d'accord de suivre les mêmes principes de formatage et si une norme existe, cela donne un guide.
Peux-tu nous donner u lien vers une version light de la norme ?

Qu'est-ce que le fichier "action" ?

@TmpName
Copy link
Collaborator

TmpName commented Dec 30, 2021

Ben pour faire peur, il ya des sites qui valident un code par rapport a la norme pep8.

Essayez d'y mettre une page du code de Vstream http://pep8online.com/

@detobel36
Copy link
Contributor Author

detobel36 commented Dec 30, 2021

Peux-tu nous donner u lien vers une version light de la norme ?

C'est à nous de la définir la version "light" 😉
En gros PEP8 c'est la norme. Un peu comme tu pourrais voir "l'orthographe"... Il y a "une" bonne manière d'écrire (merci l'académie Française). Mais en vrai, on "accepte" que les gens fassent des fautes. Voir même on "inventes" des dialectes (moi par exemple, en tant que Belge, je dis "nonante" mais "officiellement" c'est pas "la norme").

Qu'est-ce que le fichier "action" ?

Sorry je suis parti du principe que les gens "connaissaient" les "Actions Github". Je vais reprendre depuis le début histoire que tout le monde suivent (sorry si ça fait des "redites").
Donc Github propose l'execution de commandes à chaque fois qu'il y a un push, une pull request ou qu'uon pousse sur un bouton (voir ici: https://github.com/Kodi-vStream/venom-xbmc-addons/actions ).
Ce processus permet de mettre en place de l'intégration continue (abrégé CI pour "continuous integration" en anglais). Cela permet de "tester" en permanance son code, voir même de le déployé automatiquement 😃

Dans cette PR je met en place Pylint qui est un "programme" python permettant de tester le code. Il test seulement "la forme". Donc il va voir si une variable est "mal écrite", si elle n'est pas référencée, si un import n'est pas au bon endroit, pas dans le bon sens, ...
Mais ce programme ne vous dira pas que votre code à une boucle infini par exemple.

Dans ma PR, je dit a Github de lancer la vérification seulement sur le fichier default.py (cela se passe ici:

pylint plugin.video.vstream/default.py
).
On peut donc rajouter "à la main" les fichiers. C'est ce que j'ai fais ici: https://github.com/detobel36/venom-xbmc-addons/blob/enhanceHosterTest/.github/workflows/pylint.yml#L26 (vous aurez l'occasion de voir ça dans une de mes prochaines PR (j'espère)).

Pour en revenir aux "règles" appliquées. Ici j'en ai désactivé. J'ai dit à Pylint de ne pas relever certains erreurs. Elles sont toutes listées ici: https://github.com/detobel36/venom-xbmc-addons/blob/enhanceHosterTest/.pylintrc#L21 (le fichier .pylintrc est le fichier de configuration de pylint).
Par exemple je lui ai dit de ne pas faire attention aux docstring (missing-docstring). En effet, dans un projet comme ça, je ne pse pas qu'il est utile d'avoir une description pour chaque méthode que nous avons (les fameux "docstring").

A "nous" (enfin "vous") de voir quels règles vous voulez appliquer 😉

Quelques liens utiles:

@sizanic
Copy link
Collaborator

sizanic commented Dec 30, 2021

je vote pour l'utilisation Pylint 👍🏻

@yodidon
Copy link
Contributor

yodidon commented Dec 31, 2021

je vote pour l'utilisation Pylint 👍🏻

Oui c’est une bonne idée, il vaut que je regarde si je comprends tout mais c’est plutôt cool comme méthode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants