Produits Entr'ouvert » Hobo

En fait on pourrait aller jusqu'à tester si les URL à déployer ne répondent pas 404 : dans ce cas, message d'erreur et arrêt. (si c'est pas 404 c'est que le logiciel qui doit faire le déploiement n'est pas là)

Christophe Siraut a écrit :

• ajouté une condition pour les actions sans url.

(c'est le même patch)

Typo : s/resovable/resolvable/

Et ça serait mieux de raiser un CommandError plutôt qu'un sys.exit.

Et c'est juste moi, mais je préfère cette forme pour gérer l'absence d'url dans l'action:

   if 'url' not in action_args:
       continue
   ...

Mais pour moi, ça marche pas avec des déclarations comme "url": "https://${combo}/" qu'on pose dans nos recipe.json. Il faut gérer les variables d'abord.

Christophe Siraut a écrit :

Il faut gérer les variables d'abord.

Oui c'est juste, ajouté une méthode get_steps() pour compiler les arguments.

Je trouve pas ça super joli (une méthode appelée une seule fois, je doute toujours ; et le patch serait plus lisible sans). Je sais, je suis pénible, mais j'ai pas envie que cette partie du code devienne trop touffue...

J'ai adapté le patch en suivant la remarque de Benjamin.

Il me semble qu'on a aucun test sur cook...

Déplacé les tests vers le bon dossier. (Et je ne suis pas parvenu à concilier pytest-django et django-tenant-schema)

Christophe Siraut a écrit :

Déplacé les tests vers le bon dossier. (Et je ne suis pas parvenu à concilier pytest-django et django-tenant-schema)

Sans charger debian_config_common c'est difficile effectivement, il suffirait de l'ajouter à tests/settings.py comme on le fait pour multitenant (le fait est que hobo le charge aussi dans debian/server/debian_config.py).

C'est un truc que je disais à Nicolas par rapport à l'intégration Matomo je préfère garder le maximum hobo indépendant du côté multitenants (comme les autres applications).

Tester un bout de code vérifiant une série de noms de domaine, ça me semble pouvoir tenir sans faire appel à du multitenant.

(qu'il y ait, par ailleurs, de nouveaux tests appelant toute l'infrastructure multitenant pour un gros test de cook, très bien).

Dans la pratique, ça m'ennuie de voir un changement comme :

-def test_response(app, admin_user, services, monkeypatch):
+def test_response(tenant_schema, fake_notify, app, admin_user, services, monkeypatch):

qui me fait dire que tous les tests devraient désormais dépendre de deux fixtures supplémentaires.

Je confirme mon commentaire précédent, je préfère garder le maximum indépendant du multitenant. (et ça inclut les tests).

Je n'ai pas d'avis sur les remarques de Fred, de mon coté ça m'intéressait juste de ne pas mélanger les tests de hobo avec les tests de l'infra multitenant; si seul la commande hook demande le mélange des deux, je ne pense pas que ça pose problème que seul ses tests dépendent d'une fixture avec multitenant.

J'ai relu.
Peut-être ajouter les 2 tests ci-dessous pour couvrir tout le code de la fonction ajoutée.

def test_cook_invalid_certificat(db, fake_notify, monkeypatch):
    monkeypatch.setattr(ServiceBase, 'is_resolvable', lambda x: True)
    monkeypatch.setattr(ServiceBase, 'has_valid_certificate', lambda x: False)
    with pytest.raises(CommandError) as e_info:
        call_command('cook', 'tests_schemas/recipe.json')
        assert 'has no valid certificate' in str(e_info.value)

def test_cook_unknown_action(db, fake_notify, monkeypatch):
    RECEIPE=""" 
{
  "steps": [
    {"create-Bobo": {
      "url": "https://dev.entrouvert.org/" 
    }}
  ]
}
""" 
    recipe_file = '/tmp/recipe_having_wrong_action.json'
    with open(recipe_file, 'w') as handler:
        handler.write(RECEIPE)
    with pytest.raises(CommandError) as e_info:
        call_command('cook', recipe_file)
        assert 'Unknown action Bobo' in str(e_info.value)