tests de workflow (#83593) #945

Merged
vdeniaud merged 8 commits from wip/83593-testdef-socle-de-base-pour-les-w into main 2024-02-13 11:27:12 +01:00
Owner
No description provided.
vdeniaud added 1 commit 2023-12-18 17:26:50 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
5fb2b19be0
wip
vdeniaud changed title from WIP: tests de workflow (#83593( to WIP: tests de workflow (#83593) 2023-12-18 17:26:56 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 5fb2b19be0 to c0ca879cbf 2023-12-18 17:39:15 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from c0ca879cbf to febf111b6c 2023-12-18 17:58:29 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from febf111b6c to afe27a29b5 2023-12-18 18:06:07 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from afe27a29b5 to 30aa717c61 2023-12-19 09:46:59 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 30aa717c61 to dbb5d28b40 2023-12-19 10:35:44 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from dbb5d28b40 to f37d8ad125 2023-12-20 13:22:57 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from f37d8ad125 to 4c4dc4b201 2023-12-20 14:30:37 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 4c4dc4b201 to 9a153ef905 2023-12-20 14:35:03 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 9a153ef905 to ca989b1ada 2023-12-20 14:42:12 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from ca989b1ada to fdb98df353 2023-12-20 15:31:18 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from fdb98df353 to 85c991a9dd 2023-12-20 16:01:54 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 85c991a9dd to 3d0403f9ae 2023-12-20 17:56:03 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 3d0403f9ae to d2a14001db 2023-12-21 13:29:21 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from d2a14001db to 9c3624731b 2023-12-21 13:41:00 +01:00 Compare
vdeniaud added 1 commit 2023-12-21 14:07:21 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
4be86cf637
fixup! wip filter items
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 4be86cf637 to 6113f76539 2023-12-21 14:15:18 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 6113f76539 to 22c65a72a1 2023-12-21 15:49:02 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 22c65a72a1 to dfae18fe09 2023-12-21 16:07:40 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from dfae18fe09 to e16517fdaf 2023-12-21 16:18:00 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from e16517fdaf to 437c1c19b7 2023-12-21 16:25:40 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 437c1c19b7 to 70056daad3 2024-01-03 15:00:50 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 70056daad3 to cf81a6fda3 2024-01-03 18:33:48 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from cf81a6fda3 to f9a4992a04 2024-01-04 10:12:17 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from f9a4992a04 to 93472cf035 2024-01-04 10:23:58 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 93472cf035 to d2b7c6880e 2024-01-04 14:20:16 +01:00 Compare
vdeniaud changed target branch from main to wip/84500-fausse-exception-None-dans-les-t 2024-01-04 15:32:57 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from d2b7c6880e to a584e8d614 2024-01-04 15:33:20 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from a584e8d614 to a1fbbb7f21 2024-01-04 17:14:44 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from a1fbbb7f21 to befef4270b 2024-01-09 11:54:56 +01:00 Compare
vdeniaud reviewed 2024-01-09 14:34:57 +01:00
@ -160,6 +171,8 @@ class TestPage(FormBackOfficeStatusPage):
r += htmltext('<h2>%s</h2>') % self.testdef
r += htmltext('<span class="actions">')
r += htmltext('<a href="edit-data/">%s</a>') % _('Edit data')
if get_publisher().has_site_option('enable-workflow-tests'):
Author
Owner

Tout ça est derrière feature flag.

Tout ça est derrière feature flag.
vdeniaud marked this conversation as resolved
@ -0,0 +105,4 @@
return items
class WorkflowTestItem(XmlStorableObject):
Author
Owner

En regardant ce que ça donne niveau trad je me rends compte qu'on va plutôt parler « d'actions de tests », donc cette classe pourrait s'appeler WorkflowTestAction (ça différencierait plus de WorkflowStatusItem dont héritent les actions de wf également). En attente d'avis pour/contre je laisse comme ça

En regardant ce que ça donne niveau trad je me rends compte qu'on va plutôt parler « d'actions de tests », donc cette classe pourrait s'appeler WorkflowTestAction (ça différencierait plus de WorkflowStatusItem dont héritent les actions de wf également). En attente d'avis pour/contre je laisse comme ça
fpeters marked this conversation as resolved
@ -0,0 +237,4 @@
raise WorkflowTestError(_('No email was sent.'))
def fill_admin_form(self, form, formdef):
pass
Author
Owner

La suite ce sera de permettre de vérifier le contenu d'un mail (pas évident donc je laisse pour un ticket dédié). En l'état ici on va afficher une popup vide moche pour l'édition de cette action, mais je ne passe pas de temps à améliorer ça puisque c'est transitoire

La suite ce sera de permettre de vérifier le contenu d'un mail (pas évident donc je laisse pour un ticket dédié). En l'état ici on va afficher une popup vide moche pour l'édition de cette action, mais je ne passe pas de temps à améliorer ça puisque c'est transitoire
vdeniaud marked this conversation as resolved
vdeniaud changed title from WIP: tests de workflow (#83593) to tests de workflow (#83593) 2024-01-09 14:35:20 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from befef4270b to 42bbacd7fd 2024-01-10 14:23:58 +01:00 Compare
vdeniaud added 1 commit 2024-01-10 17:17:47 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 598477b387 to 42bbacd7fd 2024-01-10 17:20:14 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 42bbacd7fd to 4738f64c4d 2024-01-15 15:24:17 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 4738f64c4d to 3a4745c71b 2024-01-15 15:26:43 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 3a4745c71b to f1efd6838b 2024-01-15 15:48:13 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from f1efd6838b to 3853420a6d 2024-01-15 15:57:37 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 3853420a6d to c9c0ec4fcc 2024-01-18 17:41:57 +01:00 Compare
fpeters reviewed 2024-01-23 16:09:59 +01:00
@ -0,0 +34,4 @@
self.testdef = testdef
self.formdef = formdef
try:
self.item = [x for x in testdef.workflow_tests.items if x.id == component][0]
Owner

Je serais pour appeler ça "action" plutôt que "item" (il y a bien sûr un tas d'existant où ça s'appelle item mais je serais pour les renommer petit à petit). Je le note ici parce que c'est la première apparition mais ça concerne surtout le modèle, item_uuid → action_uuid, etc.

Je serais pour appeler ça "action" plutôt que "item" (il y a bien sûr un tas d'existant où ça s'appelle item mais je serais pour les renommer petit à petit). Je le note ici parce que c'est la première apparition mais ça concerne surtout le modèle, item_uuid → action_uuid, etc.
vdeniaud marked this conversation as resolved
@ -0,0 +50,4 @@
return redirect('.')
if not form.is_submitted() or form.has_errors():
get_response().breadcrumb.append(('edit', _('Edit item')))
Owner

C'est sans doute le cas à d'autres endroits mais il manque un get_response().set_titlte(...) pour fixer le <title> de la page; éventuellement il peut être fait dans un _q_lookup ou _q_traverse, pour contenir une valeur un peu générique,

C'est sans doute le cas à d'autres endroits mais il manque un `get_response().set_titlte(...)` pour fixer le `<title>` de la page; éventuellement il peut être fait dans un _q_lookup ou _q_traverse, pour contenir une valeur un peu générique,
vdeniaud marked this conversation as resolved
wcs/testdef.py Outdated
@ -26,3 +26,4 @@
from wcs.qommon.form import FileWithPreviewWidget, Form, get_selection_error_text
from wcs.qommon.storage import Equal as XmlEqual
from wcs.qommon.template import TemplateError
from wcs.sql_criterias import Equal
Owner

Tu peux juste importer Equal de wcs.qommon.storage et supprimer l'autre, et ça doit s'adapter correctement.

Tu peux juste importer Equal de wcs.qommon.storage et supprimer l'autre, et ça doit s'adapter correctement.
vdeniaud marked this conversation as resolved
@ -0,0 +57,4 @@
def run(self, formdata, agent_user):
formdata.record_workflow_event = lambda *args, **kwargs: None
formdata.record_workflow_action = lambda *args, **kwargs: None
formdata.store = lambda *args, **kwargs: None
Owner

J'ajouterais un commentaire, type # mock methods so nothing is stored.

J'ajouterais un commentaire, type `# mock methods so nothing is stored`.
vdeniaud marked this conversation as resolved
@ -0,0 +200,4 @@
status = formdata.get_status()
if status.name != self.status_name:
raise WorkflowTestError(
'Form should be in status "%s" but is in status "%s".' % (self.status_name, status.name)
Owner

Pas utile de marquer la chaine pour traduction ?

Pas utile de marquer la chaine pour traduction ?
vdeniaud marked this conversation as resolved
wcs/workflows.py Outdated
@ -80,2 +80,4 @@
if depth == 0: # prevents infinite loops
return
if filter_func:
items = [good_item for item in items if (good_item := filter_func(item))]
Owner

Je préférerais une réécriture dans le := ; et ça doit être sur deux lignes so be it.

Aussi, "filter_func" mais en fait ça fait filtre + substitution, si je comprends bien, je renommerais.

In fine donc je trouverais plus maintenable,

items = [replace_func(item) for item in items]
items = [x for x in items if x is not None]
Je préférerais une réécriture dans le := ; et ça doit être sur deux lignes so be it. Aussi, "filter_func" mais en fait ça fait filtre + substitution, si je comprends bien, je renommerais. In fine donc je trouverais plus maintenable, ``` items = [replace_func(item) for item in items] items = [x for x in items if x is not None] ```
Author
Owner

Je réécris sur deux lignes mais perso je trouve l'usage de := assez élégant dans les listes (et je n'invente rien, c'est un usage recommandé dans la PEP https://peps.python.org/pep-0572/#simplifying-list-comprehensions)

Je réécris sur deux lignes mais perso je trouve l'usage de := assez élégant dans les listes (et je n'invente rien, c'est un usage recommandé dans la PEP https://peps.python.org/pep-0572/#simplifying-list-comprehensions)
vdeniaud marked this conversation as resolved
wcs/workflows.py Outdated
@ -3375,2 +3377,4 @@
return destinations
def collect_for_tests(self, *args, **kwargs):
return None
Owner

Sur la classe de base j'ajouterais un commentaire avec le rôle de la méthode, type

# get action to be used in workflow tests, None for skipping the action

Et arrivé ici, ça me ferait remonter à plus haut, et plutôt qu'un filter_func se voulant générique démarrer avec quelque chose de plus orienté, perform_items(..., workflow_test=False), de la sorte,

if workflow_test:
    items = [self.get_workflow_test_action(item) for item in items]
    items = [x for x in items if x is not None]
...

Ça réduira le nombre d'indirections, qui complexifient la compréhension.

Sur la classe de base j'ajouterais un commentaire avec le rôle de la méthode, type ``` # get action to be used in workflow tests, None for skipping the action ``` Et arrivé ici, ça me ferait remonter à plus haut, et plutôt qu'un filter_func se voulant générique démarrer avec quelque chose de plus orienté, perform_items(..., workflow_test=False), de la sorte, ``` if workflow_test: items = [self.get_workflow_test_action(item) for item in items] items = [x for x in items if x is not None] ... ``` Ça réduira le nombre d'indirections, qui complexifient la compréhension.
Author
Owner

J'ai fait ça sauf pour la partie self.get_workflow_test_action(item) où je pense que tu t'es un peu mélangé les pinceaux, je te laisse me dire si tu imaginais autre chose

J'ai fait ça sauf pour la partie `self.get_workflow_test_action(item)` où je pense que tu t'es un peu mélangé les pinceaux, je te laisse me dire si tu imaginais autre chose
Owner

Ah oui je voulais juste dire self.get_workflow_test_action(), ça n'était pas explicite c'était parce qu'après avoir écrit un commentiaire sur la méthode collect_for_tests je me disais qu'il y avait tout à gagner à la renommer.

Ah oui je voulais juste dire self.get_workflow_test_action(), ça n'était pas explicite c'était parce qu'après avoir écrit un commentiaire sur la méthode collect_for_tests je me disais qu'il y avait tout à gagner à la renommer.
Author
Owner

OK c'est fait (et ça donne bien item.get_workflow_test_action() (c'est aussi le self. qui me perturbait))

OK c'est fait (et ça donne bien `item.get_workflow_test_action()` (c'est aussi le `self.` qui me perturbait))
vdeniaud marked this conversation as resolved
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from c9c0ec4fcc to b5898c60de 2024-01-23 16:52:18 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from b5898c60de to 324694a30c 2024-01-23 17:33:40 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 324694a30c to 1b318fd076 2024-01-23 17:48:09 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 1b318fd076 to e75801b395 2024-01-23 18:11:00 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from e75801b395 to 65855f0200 2024-01-23 18:11:24 +01:00 Compare
Author
Owner

Merci pour les remarques, c'est pris en compte

Merci pour les remarques, c'est pris en compte
vdeniaud requested review from fpeters 2024-01-23 18:24:12 +01:00
fpeters reviewed 2024-01-24 12:49:33 +01:00
wcs/workflows.py Outdated
@ -81,1 +81,4 @@
return
if workflow_test:
items = [item.collect_for_tests(formdata) for item in items]
items = [x for x in items if x is not None]
Owner

Pour continuer à m'intéresser ici (encore un peu), je pense que je n'aurais simplement pas fait de list comprehension ici.

J'aurais juste ajouté à la boucle,

    for item in items or []:
+       if workflow_test:
+           item = self.collect_for_tests(formdata)
+           if not item:
+               continue
        if getattr(item.perform, 'noop', False):
            continue
        if not item.check_condition(formdata):
            continue
Pour continuer à m'intéresser ici (encore un peu), je pense que je n'aurais simplement pas fait de list comprehension ici. J'aurais juste ajouté à la boucle, ``` for item in items or []: + if workflow_test: + item = self.collect_for_tests(formdata) + if not item: + continue if getattr(item.perform, 'noop', False): continue if not item.check_condition(formdata): continue ```
Author
Owner

En effet c'est mieux, appliqué !

En effet c'est mieux, appliqué !
vdeniaud marked this conversation as resolved
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 65855f0200 to 88d1a8ef94 2024-01-24 13:55:50 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 88d1a8ef94 to ea9dfaa7bf 2024-01-24 14:05:45 +01:00 Compare
fpeters closed this pull request 2024-01-26 13:49:22 +01:00
fpeters reopened this pull request 2024-01-26 15:23:26 +01:00
fpeters changed target branch from wip/84500-fausse-exception-None-dans-les-t to main 2024-01-26 15:23:43 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from ea9dfaa7bf to 03b8e8f20d 2024-01-29 09:58:14 +01:00 Compare
Author
Owner

(rebasé)

(rebasé)
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 03b8e8f20d to 47ed12f5db 2024-01-31 14:33:27 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 47ed12f5db to b7b61c7565 2024-02-01 10:49:57 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from b7b61c7565 to 47527803ba 2024-02-07 10:41:16 +01:00 Compare
vdeniaud changed target branch from main to wip/86587-testdef-inclure-les-tests-dans-l 2024-02-07 10:41:38 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 47527803ba to 35ff30ac8e 2024-02-07 15:07:06 +01:00 Compare
vdeniaud changed target branch from wip/86587-testdef-inclure-les-tests-dans-l to wip/86587-testdef-inclure-les-tests-dans-l-old 2024-02-07 15:13:01 +01:00
fpeters requested changes 2024-02-13 10:10:07 +01:00
wcs/workflows.py Outdated
@ -3509,6 +3513,30 @@ class WorkflowStatusItem(XmlSerialisable):
markers_stack.append({'status_id': formdata.status[3:]})
formdata.update_workflow_data({'_markers_stack': markers_stack})
def get_possible_target_options(self):
Owner

Cette méthode n'apparait plus utilisée.

Cette méthode n'apparait plus utilisée.
Author
Owner

En effet j'ai foiré un rebase à un moment, c'est corrigé

En effet j'ai foiré un rebase à un moment, c'est corrigé
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 35ff30ac8e to 91510c0498 2024-02-13 10:17:45 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 91510c0498 to 5db419dba3 2024-02-13 10:33:10 +01:00 Compare
vdeniaud changed target branch from wip/86587-testdef-inclure-les-tests-dans-l-old to main 2024-02-13 10:34:02 +01:00
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 5db419dba3 to 0305702b39 2024-02-13 11:01:38 +01:00 Compare
vdeniaud force-pushed wip/83593-testdef-socle-de-base-pour-les-w from 0305702b39 to 3a3ed59748 2024-02-13 11:08:09 +01:00 Compare
Author
Owner

Voilà c'est rebasé sur main en prenant en compte les changements format de date

Voilà c'est rebasé sur main en prenant en compte les changements format de date
vdeniaud requested review from fpeters 2024-02-13 11:18:39 +01:00
fpeters approved these changes 2024-02-13 11:21:33 +01:00
vdeniaud merged commit 3a3ed59748 into main 2024-02-13 11:27:12 +01:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/wcs#945
No description provided.