Produits Entr'ouvert » w.c.s.

Je ne comprends pas la matrice.

Par exemple, j'y lis que si sumitter=anonymous et accesser=user1, alors deny ? Mais non, il faut au contraire accepter... non ?

Nicolas Roche a écrit :

En fait si, c'est bon :

Oui, j'avais lu trop vite croyant que tu respectais le sujet du ticket ;-) En fait la matrice concerne "Direct access to ressources", rien à voir avec le code de suivi...

Il m'a l'air d'y avoir mille choses dans ce test, genre vraiment besoin de six champs dans la démarche pour tester l'accès, vraiment besoin d'un workflow particulier, etc. ? je serais pour drastiquement réduire, et ne pas mélanger d'histoire de préremplissage ici.

• directs (via une URL)
• et indirects (via code de suivi)

La problématique des accès directs a été ajouté pour couvrir les éventuelles régression liées à la résolution de #37808. Je dirais qu'elle reste secondaires ici, car ce qui a motivé ces tests c'est d'abord #38239 (qui vient solutionner #37095).

La problématique des accès via code de suivi est liée à celle de l'appartenance du formulaire (formdata_user) comme le suggère #37095. Dit autrement, ces tests sont proposés afin de pouvoir mesurer les régressions des futures corrections relatives aux champs pré-remplis avec les données de l’utilisateur.

Ci-dessous, voici ce que vérifient les tests qui couvrent cette seconde problématique :

• commentaire : pour voir que {{form_user_display_name}} évolue dans le commentaire ; et que c'est la valeur la plus proche de formdata_user (qui fait référence), sans l'être complètement.

• prefill-string-page1 : pré-remplissage avec {{form_user_display_name}}
• prefill-user-page1 : pré-remplissage avec les variables de sessions (solution mise en avant par l'IHM)

On constate que ces 2 pré-remplissages sont cohérents : ils ont été figés lors du pré-remplissage et ne suivent donc pas l'évolution de la valeur formdata_user.

• prefill-string-page2 : pré-remplissage avec {{form_user_display_name}}
• prefill-user-page2 : pré-remplissage avec les variables de sessions

On constate que ces 2 champs qui suivent la valeur du champ commentaire respectent la règle : "Taper le code de suivi = devenir/être la personne à l'origine de la demande." ; ce que décrit #37095.

• prefill-string-workflow : pré-remplissage avec {{form_user_display_name}}
• prefill-user-workflow : pré-remplissage avec les variables de sessions

On constate que la règle citée ci-dessus ne s'applique globalement pas, mais suffisamment quand même pour confirmer les doutes évoqués en réunion concernant l'élargissement du périmètre couvert par cette problématique.

Tout ça pour dire que tant qu'on ne sait pas trop vers où on va, je ne saurais pas quoi supprimer ici.

Ce test est inmaintenable; dans ton commentaire tu décris les multiples aspects qui sont testés; mon souhait c'est que tous les aspects ne soient pas testés dans un unique test, qui se trouve définir ses propres fixtures, fonctions, dans les fonctions 'autres fonctions, context manager, etc.

À noter que ce qui me heurte vraiment est la taille et la structure de l'affaire, davantage que l'idée de tester plusieurs trucs à la fois (même si je ne suis pas fan et que ça fera un truc beaucoup trop pollué lors d'un futur debug).

À noter (bis) que déjà simplement la docstring est inmaintenable.

À reprendre, je dirais que si on veut des tests englobant tout avec plein de redondance, il faut plutôt multiplier les fixtures paramétriques. (ex: les tests sur l'API dans Chrono, ils sont courts, mais ils sont répétés dans différents fuseaux horaires, à différentes dates).

Faut laisser à l'ordinateur le soin de faire les combinaisons.

i.e.

Avoir is_draft, submitTer (deux T), is_front, assessor (?), owner, être chacuns des fixtures paramétriques.

J'ai mis "assessor" pour l'utilisateur qui accède à la demande (par opposition à "submitter")
C'est pas terrible, ni comme mot, ni comme traduction, mais je n'ai pas trouvé mieux.
https://fr.wiktionary.org/wiki/accesseur
http://www.granddictionnaire.com/ficheOqlf.aspx?Id_Fiche=17051753

nested pytest.mark.parametrize

Je ne l'ai pas fait sur le premier lot car les résultats attendus ne me semblent pas factorisables en fonction des paramètres.

J'ai mis "assessor" pour l'utilisateur qui accède à la demande (par opposition à "submitter")

Ailleurs dans les tests on met agent.

Je ne l'ai pas fait sur le premier lot car les résultats attendus ne me semblent pas factorisables en fonction des paramètres.

Ça reste illisible, il faut vraiment trouver autre chose. Si ça simplifie tu peux te dire qu'on se fiche du cas particulier des admins. Tu peux aussi pointer des trucs qui sont incohérents, et ne pas faire un test compliqué pour des trucs incohérents.

Mais déjà, vu comment l'intérieur du test c'est juste quasi directement deux branches d'un "if is_draft", ça semble facilement pouvoir se couper en deux, un test pour quand c'est brouillon, un test pour quand c'est pas brouillon.

_~~

Par ailleurs il reste des mélanges de bout de fonctions dans les fonctions, et j'ai l'impression que les tests en eux-mêmes donneraient l'impression d'être bien moins long si ce n'était pas le cas.

"assesssor" : l'utilisateur qui accède à la demande

En fait je voulait dire l'utilisateur qui y ré-accède à la demande pour la compléter ou pour la suivre, via le code de suivi... mais ok pour agent.

Ok alors tout simplement user.

_~~

Vraiment il faut trouver un truc pour cette longue liste; j'ai toujours du mal à comprendre ce que tout ça teste et j'espère enfin comprendre une fois arrivé au bout des simplifications.

Si je prends test_direct_access_to_draft

On a un mode login/deny/deny/deny/deny les trois fois, SAUF le dernier où on a login/FRONT/deny/deny/deny.

Je serais ainsi à dégager expected_access des fixtures, à avoir dans le code du test :

result = 'login' if user == 'anonymous' else 'deny'
if submitter == 'user1' and user == 'user1':
    result = 'front'  # user is allowed access to its draft

Réfléchir à un truc similaire pour test_direct_access_to_demand.

j'ai toujours du mal à comprendre ce que tout ça teste

Ces tests sont dans un premier temps un outils pour mesurer les changement apportés par les patchs qui concerneront l'accès via code de suivi, et dans un second temps ils valideront (ou donneront les limites) de règles sur lesquelles on pourra s'appuyer, comme celle actuellement en vigueur : "Taper le code de suivi = devenir/être la personne à l'origine de la demande."

je trouvais ça intéressant d'avoir tout au même endroit : ça rejoignait l'idée d'avoir une matrice

J'ai vraiment en tête de proposer des tests dont les valeurs attendues varieraient dans le temps, en fonctions de l'applications des patchs liés au ticket chapeau. Donc, mon idée c'est que ces valeurs (ou la matrice par abus de langage) soient faciles à faire évoluer. Or, si je factorise les valeurs attendues dans le tests, d'une certaine façon je les fige, ce qui est l'inverse de l'effet que je recherche. Une fois que ces valeurs attendues deviendront cohérentes alors, oui, cela prendra tout son sens de les factoriser.

Ce patch ne passera pas ainsi.

Vraiment je ne sais pas trop comment l'expliquer une nouvelle fois différemment, cette approche ne me convient pas; on ne fait comme ça nulle part ailleurs dans w.c.s.; faire un truc compliqué sans savoir où ça va.

Et peut-être prendre de là alors. Savoir/décider où aller. Faire les tests qui correspondent à ça. Et marquer en xfail là où la réalité ne correspond pas encore.

Ok, compris.
C'est juste un outil dont j'ai besoin pour comprendre où je met les pieds et pour pouvoir communiquer sur les patchs proposés (parce que je n'ai pas de vision d'ensemble sur ces mécanismes).
Je continuerai à utiliser ma branche locale pour rebaser les tickets, et donc s'il n'y a que moi qui en voit l'utilité, autant ne pas les pousser dans master.
Après, comme tu dis, on pourra (ou pas) les intégrer quand on saura vers où aller.