Ecrire de bon tests unitaires
Le
Zouplaz
Bonjour, je suis en train de me faire violence et apprendre à utiliser un
outil de tests unitaires, Cactus en l'occurence.
Je compte mettre en oeuvre autant de tests qu'il y a de formulaires dans
mon application. Ca ne couvre pas tous les cas mais ça représente la
majeure partie des fonctionnalités, et qui plus est les plus sensibles
(l'ajout ou la modification de données).
Grâce à Cactus, je simule un post de formulaire et je vérifie ensuite si
les données présentes dans la base sont cohérentes.
Mais, ce faisant je commence à me poser quelques questions :
1) Est-il normal que j'ai à modifier un DAO pour permettre l'exécution
d'un test.
Par exemple, j'ai du créer cette méthode :
public Collection findWithCriteria(String criteria) throws
HibernateException
Alors que dans l'application je n'en n'ai (pour l'instant au moins) pas
besoin. D'accord c'est un cas bateau et avoir une méthode capable de
retourner une collection à partir d'un critère SQL fait parti du BA BA
des DAO mais quand même Sur le principe est acceptable ?
2) Comment faire pour que les tests eux-mêmes ne soit faillibles ?
Exemple avec un des tests :
public void testAddChildCategory() throws Exception
{
boolean found = false;
this.request.setRemoteUser("chris");
servletRun();
setupDAO();
Category parentCategory = categoryDAO.getCategory(new Long(1));
assertNotNull(parentCategory);
// Doit être trouvée dans les enfants de la catégorie parente
Collection child = parentCategory.getChildCategories();
Iterator iter = child.iterator();
while(iter.hasNext() && !found)
found = ((Category)iter.next()).getTitle().equalsIgnoreCase
("TESTCASE_CHILDCAT");
assertTrue(found);
// Doit avoir pour parent la catégorie ID 1
Collection cats = categoryDAO.findWithCriteria
("cat.title='TESTCASE_CHILDCAT'");
assertEquals(1,cats.size());
Category cat = (Category)cats.iterator().next();
assertTrue(cat.getParentCategory().getId().intValue() == 1);
// Ménage
categoryDAO.makeTransient(cat);
endDAO();
}
Dans le code ci-dessus, il y a plusieurs choses qui me dérangent :
- si le code échoue pour une raison Y (mise au point du test), il laisse
la base de donnée dans un état incohérent
- j'utilise (sans avoir d'autre alternative pour l'instant) une recherche
par chaine de caractères pour retrouver l'enregistrement crée. Une erreur
de typo est vite arrivée, générant une erreur de test alors que c'est le
test lui même qui est fautif, pas ce sur quoi il porte. Plus généralement
je fais référence à des données statiques (la catégorie parente dont l'ID
est 1 : FAUX si la base est vide, le titre de la catégorie doit être
'TESTCASE_CHILDCAT', etc
- pour des tests encore plus complexes, le code du test va devenir plus
délicat à mettre au point. Je risque d'avoir à examiner des collections,
des relations, ou que sais-je encore. Quelle technique/principe puis-je
mettre en oeuvre pour éviter d'écrire un test qui :
a) laisserait passer une erreur
b) ne produit pas d'erreurs par lui même
Ceux d'entre vous qui font du test unitaire sur des servlet devraient
pouvoir me donner quelques conseils qui seront les bienvenus
Merci d'avance !!
outil de tests unitaires, Cactus en l'occurence.
Je compte mettre en oeuvre autant de tests qu'il y a de formulaires dans
mon application. Ca ne couvre pas tous les cas mais ça représente la
majeure partie des fonctionnalités, et qui plus est les plus sensibles
(l'ajout ou la modification de données).
Grâce à Cactus, je simule un post de formulaire et je vérifie ensuite si
les données présentes dans la base sont cohérentes.
Mais, ce faisant je commence à me poser quelques questions :
1) Est-il normal que j'ai à modifier un DAO pour permettre l'exécution
d'un test.
Par exemple, j'ai du créer cette méthode :
public Collection findWithCriteria(String criteria) throws
HibernateException
Alors que dans l'application je n'en n'ai (pour l'instant au moins) pas
besoin. D'accord c'est un cas bateau et avoir une méthode capable de
retourner une collection à partir d'un critère SQL fait parti du BA BA
des DAO mais quand même Sur le principe est acceptable ?
2) Comment faire pour que les tests eux-mêmes ne soit faillibles ?
Exemple avec un des tests :
public void testAddChildCategory() throws Exception
{
boolean found = false;
this.request.setRemoteUser("chris");
servletRun();
setupDAO();
Category parentCategory = categoryDAO.getCategory(new Long(1));
assertNotNull(parentCategory);
// Doit être trouvée dans les enfants de la catégorie parente
Collection child = parentCategory.getChildCategories();
Iterator iter = child.iterator();
while(iter.hasNext() && !found)
found = ((Category)iter.next()).getTitle().equalsIgnoreCase
("TESTCASE_CHILDCAT");
assertTrue(found);
// Doit avoir pour parent la catégorie ID 1
Collection cats = categoryDAO.findWithCriteria
("cat.title='TESTCASE_CHILDCAT'");
assertEquals(1,cats.size());
Category cat = (Category)cats.iterator().next();
assertTrue(cat.getParentCategory().getId().intValue() == 1);
// Ménage
categoryDAO.makeTransient(cat);
endDAO();
}
Dans le code ci-dessus, il y a plusieurs choses qui me dérangent :
- si le code échoue pour une raison Y (mise au point du test), il laisse
la base de donnée dans un état incohérent
- j'utilise (sans avoir d'autre alternative pour l'instant) une recherche
par chaine de caractères pour retrouver l'enregistrement crée. Une erreur
de typo est vite arrivée, générant une erreur de test alors que c'est le
test lui même qui est fautif, pas ce sur quoi il porte. Plus généralement
je fais référence à des données statiques (la catégorie parente dont l'ID
est 1 : FAUX si la base est vide, le titre de la catégorie doit être
'TESTCASE_CHILDCAT', etc
- pour des tests encore plus complexes, le code du test va devenir plus
délicat à mettre au point. Je risque d'avoir à examiner des collections,
des relations, ou que sais-je encore. Quelle technique/principe puis-je
mettre en oeuvre pour éviter d'écrire un test qui :
a) laisserait passer une erreur
b) ne produit pas d'erreurs par lui même
Ceux d'entre vous qui font du test unitaire sur des servlet devraient
pouvoir me donner quelques conseils qui seront les bienvenus
Merci d'avance !!
Poser une question
Bravo !
Un principe qui peut aider à démarrer, c'est d'écrire d'abord des tests
pour les fonctionnalités dans lesquelles on a trouvé des bugs. En
particulier en intégrant le test dans le processus de correction: on me
signale un bug, j'écris un test qui échoue à cause du bug, je corrige le
bug, le test doit maintenant passer - s'il passe je sais que ma
correction était effective.
En règle générale, il ne faut pas s'étonner d'avoir à modifier du code
pour en améliorer la testabilité, si on n'a pas préalablement écrit le
code en ayant spécifiquement cherché à le rendre testable.
Sans en savoir plus, impossible de répondre à ta question sur le sujet
précis des DAO ou autres patterns particuliers.
Pourquoi pas... Si ça te gène, créée dans ta classe de test une classe
auxiliaire héritant de ton DAO et implémentant cette méthode:
class CategoryTest extends TestCase {
// bla bla
public void testAddChildCategory() {
// bla bla
class CategoryDAOForTest extends CategoryDAO {
public Collection findWithCriteria(String criteria) {
// etc
Si jamais tu en as besoin dans l'appli, un refactoring: Move Method. :)
Et c'est sans importance. Le "contrat" d'un test unitaire est le
suivant: chaque test doit instaurer un contexte connu et prévisible, de
telle sorte qu'on puisse prédire (et vérifier par des assert) ce que
devient ce contexte après le déroulement de la fonction qu'on teste. Si
tu testes sur la base de données, le test unitaire doit placer la base
dans un état connu, prévisible, avant de dérouler la fonction testée.
Chaque exécution d'un test est précédée et suivie respectivement d'un
appel aux méthodes setUp() et tearDown() de TestCase. C'est dans ces
méthodes que tu peux en fonction des besoins appeler le code qui va
replacer la base dans un état connu.
Cf:
http://www.codeproject.com/gen/design/autp5.asp (Rollback Pattern)
Une autre solution est d'utiliser une DB comme HypersonicSQL, qui permet
très facilement et rapidement de réinitialiser la base à un état connu.
Oui, et ça ne pose aucun problème dans la mesure où tu initialises la
base à un état connu avant chaque test.
La meilleure solution: écrire le test avant le code de la fonction
testée. (Donc, évidemment, au fil du développement, pas à la fin lorsque
l'appli est terminée.)
Dans ton exemple ci-dessus, imagine que le servlet ne fait rien - il est
totalement vide de toute implémentation. Tu écris d'abord le test -
celui que tu as posté me semble bien fichu. Tu exécutes le test:
forcément, il échoue, puisque le servlet est sans effet. Ensuite tu
implémentes la transaction dans le servlet. Si ton test est correct, et
que ton code est correct aussi, le test passe.
Il peut aussi arriver que le test soit incorrect, ou que le code soit
incorrect. Alors le test échoue. Tu ne sais pas faire la différence
entre les deux cas, à moins d'examiner les choses de plus près, mais tu
sais qu'une erreur a été détectée. Il est donc impossible d'écrire un
mauvais test si tu fais les choses dans cet ordre. (Pas impossible, en
fait, mais hautement improbable - il faudrait que l'erreur dans ton test
"annule" précisément l'erreur dans ton code.)
Laurent
Un détail encore, tu me parles des méthodes setUp() et tearDown()...
Si j'ai bien compris elles sont exécutées avant et après l'ensemble des
tests. Bien, mais est-ce qu'il n'y a pas un moyen d'avoir un setUpXXX et
tearDownXXX ?
Je m'explique : je viens à peine de comprendre que dans Cactus les méthodes
beginXXX et endXXX sont clientes et que la méthode textXXX est serveur.
Donc peine perdue si je veux faire passer des données des premières vers la
seconde puisque plusieurs instances sont crées par Cactus.
En clair, pourquoi est-il n'y a aucune possibilité d'avoir une méthode
appellée systématiquement à la fin de chaque test pour la partie serveur ?
Comme il n'y a que tearDown(), ça implique d'y mettre le code qui va faire
"le ménage" de l'ensemble des tests et non pas d'un test particulier...
Je suis surpris que les concepteurs de Cactus n'ai pas implémenté cette
fonctionnalité, ou alors je l'ai raté, ou alors ça ne sert à rien et il y a
quelque chose qui m'échappe... ;-)
L'ordre d'exécution est
beginXXX
setUp
testXXX
tearDown
endXXX
Actuellement, dans ma méthode beginXXX je dois passer comme paramètre
l'identifiant d'une catégorie à laquelle va être attaché un message (par
exemple).
Je pensais placer dans setUp() le code nécessaire qui crée cette catégorie
(qui sera détruite dans tearDown)
Or, beginXXX s'exécute AVANT setUp donc ça ne colle pas. Du coup je suis
obligé de placer le code en question (qui crée cette fameuse catégorie)
dans beginXXX, de récupérer l'identifiant (la clé primaire), de le stocker
dans une variable membre statique, identifiant qui sera exploité dans
testXXX (je dois vérifier que le message présent dans la base est bel et
bien attaché à cette catégorie).
D'une part ça me semble bizarre d'initialiser un objet dans beginXXX
puisque je suis du côté "client", et d'autre part d'être obligé de passer
par un membre statique (c'est le seul moyen que j'ai trouvé, vu que deux
instances de la classe de test sont créées par Cactus).
Bon, c'est peut être pas trop clair ce que je raconte ;-)
Si si, très clair.
Je connais JUnit mieux que je ne connais Cactus, mais je vais essayer
d'esquisser la solution... Ce n'est pas une astuce technique, c'est une
question de bien comprendre la logique du test unitaire.
Le problème est le suivant: setUp ne devrait pas avoir à "communiquer"
avec beginXXX.
Pour bien comprendre, il faut connaître un peu la philosophie de JUnit,
qui est l'ancètre de Cactus. Dans JUnit, il y a pour chaque classe de
test un seul setUp et un seul tearDown. Chaque classe de test doit
contenir des tests qui peuvent être déroulés dans le même contexte de
test (on dit aussi "fixture"). Donc, quand on veut des setUp()
différents, on va tout simplement créer une nouvelle classe de test,
parce que ça signifie qu'on est dans un contexte différent. Ce contexte,
on est censé le connaître.
C'est seulement parce que "l'astuce" de JUnit qui consiste à exécuter,
dans une même classe de test, toutes les méthodes testXXX() (via la
reflection de Java), rend la vie très (trop) simple qu'on a tendance à
faire l'équation "cas de test" = "méthode de test". En fait un cas de
test est une classe de test. Pour un seul cas de test tu peux avoir N
méthodes de test qui testent des comportements divers *dans un contexte
donné* et ce contexte est supposé invariable. JUnit est fait comme ça,
c'est peut-être un mauvais choix, mais c'est un choix qui fait partie de
sa conception et Cactus reprend ce choix.
La doc de Cactus n'est pas dans le vrai quand elle dit que beginXXX est
"l'équivalent côté client" de setUp(). Les méthodes beginXXX() *font
partie du test*, puisqu'elles précisent le comportement du client dans
un cas d'utilisation donné. Tu pourrais tout faire dans testXXX(). Alors
que les méthodes setUp() ne font jamais partie du test, tout ce qu'elles
font c'est établir un contexte pour le test.
Dans ton exemple, "le contexte" c'est la base de données dans un état
bien précis, avec des catégories dont les propriétés sont connues. Donc,
dans setUp() il faudrait recréer systématiquement les *mêmes* données en
base, de sorte que l'identifiant de la catégorie parente reste constant
d'un test à l'autre. Par exemple, utilise un INSERT qui spécifie la clé
primaire. Et du coup, tu vas coder "en dur" l'identifiant en question
dans tes beginXXX.
Cela dit, pragmatiquement, s'il est plus simple de créer tes catégories
à la volée et récupérer l'identifiant après... Cactus est open-source,
donc tu dois pouvoir modifier l'ordre d'appel en modifiant Cactus. Mais
la "portabilité" de cette astuce n'est pas garantie. :)
Laurent
Cf. plus bas
Bon, d'accord je comprends mieux : une partie de mes problèmes viennent
du fait que je place des tests dans la même classe alors que je devrais
créer plusieurs classes de test.
Bon, sur le principe oui mais dans la pratique l'objet http request (du
moins la simulation de l'objet request qui dans Cactus est un WebRequest)
n'est pas disponible lors de l'exécution de testXXX. Mais il est sans
doute possible d'altérer les paramètres par des moyens conventionnels (à
priori tout ça se sont des Map)
C'est ce que je me disais mais (sauf erreur de ma part), Cactus appelle
beginXXX avant setUp ! C'est une des choses que j'ai vraiment du mal à
comprendre. Comme tu le dis, je pourrais tenter de modifier Cactus mais
j'ai bien peur d'y passer beaucoup de temps vu mon niveau de pratique
général ;-)
Je reviens sur ce que tu dis plus haut : setUp et beginXXX ne devraient
pas communiquer entre eux. Voici plus précisement le problème auquel je
me heurte justement à ce sujet et pour lequel je ne vois aucune autre
solution que l'usage d'une variable membre statique pour faire transiter
une info entre les deux. Problème qui vient du fait que puisqu'on simule
un POST de formulaire je n'ai ni côté client, ni côté serveur (sous
Cactus j'entends bien) la possibilité de connaître la clé primaire
générée sans la faire transiter par un membre statique.
Le topo : chaque message du blog doit être associé à une catégorie.
Il y a des catégories racines et des catégories de niveau inférieur. J'ai
donc une relation parent/enfants entre les catégories.
Voici le test m'assurant que la modification d'une catégorie (pour
l'instant la seule information modifiable est le titre de la catégorie)
fonctionne correctement :
public void beginModifyCategory(WebRequest wr) throws Exception
{
setupAuthCommonParameters(wr);
beginDAO();
// On commence par créer et sauvegarder une catégorie
Category cat = new Category();
cat.setTitle(ROOTCAT_SIGNATURE);
categoryDAO.makePersistent(cat);
testCategoryID = cat.getId();
// On prépare le form post
wr.addParameter("action","modcat");
wr.addParameter("postback","true");
wr.addParameter("category.id",testCategoryID.toString());
wr.addParameter("category.title",EDITED);
endDAO();
}
public void testModifyCategory() throws Exception
{
this.request.setRemoteUser("chris");
servletRun();
beginDAO();
// Les champs de la catégorie doivent
Category cat = categoryDAO.getCategory(testCategoryID);
assertEquals(cat.getTitle(),EDITED);
categoryDAO.makeTransient(cat); // TODO trouver un moyen de faire
le ménage en cas d'exception : à mettre dans tearDown
endDAO();
}
1) J'ai besoin d'une catégorie à modifier, elle est créée pour l'occasion
2) dans begin j'ai besoin de l'identifiant de cette catégorie pour
effectuer le POST
3) dans test j'ai à nouveau besoin de l'identifiant pour vérifier que les
modifications sur la catégorie sont effectives
4) et aussi dans tearDown pour détruite le jeu de test (la catégorie)
Problèmes :
a) Si je crée la catégorie dans setUp, test y aura accès mais pas begin
(or j'en ai besoin dans begin)
b) de toutes façons setUp s'exécute après begin
c) et si je la crée dans begin, tearDown n'y a pas accès !
J'ai beau faire, je ne sais pas comment je peux me passer de la jonction
faite par le membre statique (ici testCategoryID) avec pour résultat :
# begin écrit testCategoryID
# test lit testCategoryID
# tearDown lit testCategoryID
La seule solution serait qu'il n'y ait plus rien dans begin et tout dans
setup et test (si c'est possible, à voir). Ce qui me semble bizarre
puisque ça va à l'encontre des fonctionnalités client/serveur de
Cactus...
Ouf, quel casse tête. C'est Cornélien ! ;-)