Produits Entr'ouvert » w.c.s.

Pour information quand même, sur l'ensemble des conditions (page pré et post, champs, workflows) des wcs de prod, je trouve :

• 0 addition
• 0 soustraction
• 0 multiplication
• 0 division

D'une discussion jabber avec Pierre, on parle ici plutôt de calculs, par exemple de coût à envoyer dans un lingo.

Yes les calculs dans les données de traitement par exemple.

Attention dans le patch subtract plutôt que subStract.

• dans le test, y'a un "import qommon.errors" en trop au début
• on s'autorise des lignes de 100 caractères de large, donc les with pytest.raises(Exception, match=r'.*Could not find variable at start.*'): doivent pouvoir tenir sur une ligne, ça sera plus joli
• j'ai tendance à mettre les cas non-passants (ces with pytest.raises) en fin du test... Globalement j'organiserais les tests ainsi, idem dans les sections plus bas, par exemple sur divide ou tu testes d'abord ce qui ne marche pas. Même si c'est pas toujours vrai, les tests sont également un peu une façon de documenter un truc, donc autant commencer par documenter "comment ça marche" avant de montrer comment ça plante.
• comme on dit en Python, "explicit is better than implicit" : return v1 if v1 is not None else '' c'est pas très lisible, ajoute plutôt des «if term1 is None: term1 = 0» après le parsing. On comprendra qu'il s'agit de valeur par défaut en cas de crash du parsing.
• on ne nomme par des variables "v1" ou "v2" en Python, c'est trop court... utilise plutôt term1 et term2

Aussi : les tests sur |decimal sont inutiles ici, déjà fait ailleurs (dans test_templates.py). Tu peux les virer, plus un patch est petit mieux c'est :)

Enfin, dans les tests il faudra ajouter des calculs avec l'usage de variables qui viennent du formulaire (formdata). On peut voir ça ensemble tout à l'heure.

Il faut essayer d'être plus systématique dans tes tests, car sinon tu ne va pas bien couvrir le code (va sur le lien du jenkins qui s'affiche ici, clique sur "Cobertura Coverage Report", puis sur "qommon.templatetags" puis "qommon.py" : tu verras que tes tests ne sont pas passés par le "ZeroDivisionError")

• test avec deux décimaux
• test avec un nombre sous forme de chaine à gauche
• test avec un nombre sous forme de chaine à droite
• test avec deux nombres sous forme de chaine
• test avec une string-qui-n-est-pas-un-nombre à gauche
• test avec une string-qui-n-est-pas-un-nombre à droite
• test avec deux string-qui-ne-sont-pas-un-nombre

• ... et la division par zéro.

• gère la dé-lazyfication dans parse_decimal
• ajoute une valeur par défaut de retour sur parse_decimal(value, default=None)
  ... comme ça tu pourras faire « value = parse_decimal(value, Decimal(0)) » pour l'addition, par exemple.

Aussi, plutôt que value et arg, j'aurai eu tendance à utiliser "term1" et "term2", mais bon, c'est totalement perso et ton choix de value,arg est bien aussi (c'est la notation classique dans un templatetags, pourrait-on dire)

Pour suivre la logique add et substract qui essayent de renvoyer un decimal même quand un des termes est foireux, sur multiply je renverrai bien Decimal(0) quand l'un des deux termes est None.

• renvoyer '' si term2 est None (vu comme une division par zéro)
• renvoyer Decimal(0) si term1 est None

Truc que je ne vois que maintenant : ton test "test_mathematics_filters(pub, variable_test_data):" ne fait en fait appel à aucune variable de formulaire. Tu peux en supprimer les 6 premières lignes... mais surtout il devrait plutôt être dans tests/test_templates.py, à la fin, après test_decimal_templatetag

J'ajouterai des tests qui montrent que le résultat est bien un decimal, parce que c'est ce qui sera attendu et utilisé dans les conditions, pour faire des comparaisons. Et donc au niveau de test_lazy_formdata_mathematics_filters, ajouter des lignes du genre :

        condition = Condition({'type': 'django', 'value': 'form_var_term1|add:form_var_term2 == 7'})
        assert condition.evaluate() is True
        condition = Condition({'type': 'django', 'value': 'form_var_term1|add:form_var_term2 == 7.0'})
        assert condition.evaluate() is True
        condition = Condition({'type': 'django', 'value': 'form_var_term1|add:form_var_term2 == "7"|decimal'})
        assert condition.evaluate() is True
        condition = Condition({'type': 'django', 'value': 'form_var_term1|add:form_var_term2 > 6'})
        assert condition.evaluate() is True
        condition = Condition({'type': 'django', 'value': 'form_var_term1|add:form_var_term2 > 8'})
        assert condition.evaluate() is False

et le même genre de truc pour les autres opérateurs.

Sounds good, mais on va pas pousser ça cette semaine, ça sera pour la release prochaine (mi mars).

A noter un mini-détail :

           '9': '2018-07-31',
+          '10': '3',
+          '11': '4'
       }

Il faut penser ici à l'avenir et pour que les prochains patches n'aient pas à modifier ta dernière ligne, termine-la par une virgule : '11': '4', (comme pour la ligne du '9': ...)

Tiens, truc auquel je pense maintenant : vérifier que {{form_var_foo_foo|decimal}} ne plante pas, et renvoie bien le nombre 0 (idem avec form_var_date, form_var_itemsfield et les autres). C'est toujours ça qu'on aura de moins comme trace au cas où :)

Nicolas Roche a écrit :

Questions :
- True|decimal renvoit '0', c'est bon ?

Mouais... Bon, on va dire que oui, c'est comme ça :)

- pourquoi 'from wcs.conditions import Condition' dans test_templates.py ?

Un import qui traine... Ca arrive, on en a par-ci par-là, on fait pas toujours assez attention (surtout sur les tests qui ne sont même pas pylintés). On peut les nettoyer de temps à autre, mais pas "en cachette" -- je veux dire, par exemple, ne pas nettoyer cet import dans ton patch qui n'a rien à voir. Plutôt faire un autre mini-patch genre «tests: remove unused import (trivial)», même sans ticket tellement c'est bateau.