[TECH] Prévenir les faux négatifs dans les tests.

[octo-topi]

:unicorn: Problème

Certaines assertions chai sont toujours vraies, car leur syntaxe est incorrecte.

Cela peut entraîner des faux négatifs:

• une régression est introduite dans l'implémentation:
• le test passe toujours.

:robot: Solution

Utiliser le plugin chai-expect

:rainbow: Remarques

Levée d'erreur

La vérification qu'une fonction (SUT) ne lance pas d'erreur est effectuée automatiquement par Mocha. Si le catchErr est utilisé, une erreur est lancée par catchErr si le SUT ne lance pas d'erreur.

Il est probablement possible d'utiliser le matcher throw, mais lorsque le SUT renvoie une promesse, je n'ai pas réussi à le faire fonctionner.

Cas à résoudre

Le custom matcher deepEqualArray comporte un test que je ne comprend pas, aussi j'ai ajouté une exception

:100: Pour tester

Muter l'implémentation et vérifier que le test ne passe plus. Ajouter un expect(true).to.be.true et vérifier qu'une erreur de lint apparaît.

[pix-service]

I'm deploying this PR to these urls:

• App (.fr): https://app-pr4683.review.pix.fr
• App (.org): https://app-pr4683.review.pix.org
• Orga: https://orga-pr4683.review.pix.fr
• Certif: https://certif-pr4683.review.pix.fr
• Admin: https://admin-pr4683.review.pix.fr
• API: https://api-pr4683.review.pix.fr/api/

Please check it out!

[ajoanny]

Certaines assertions chai sont toujours vraies, car leur syntaxe est incorrecte.

Cela peut entraîner des faux négatifs:

☝🏽 L'assertion est toujours vrai même quand c'est faux => C'est des faux positfs du coup non ?

Coucou !

Je suis curieux, je regardais dans le code et je trouve peu de code qui ne respecte pas les règles de la lib. Je trouve assez peu ces erreurs dans le code, c'est quoi la plus value du coup ? On met en place un outil qui vérifie des erreurs qu'on ne fait pas trop. Par contre oui il y a des faux positifs dans le code, mais c'est lié aux erreurs qui sont recherchées par la lib.

Par exemple un des avantages à faire du TDD (correctement) c'est de ne pas avoir de faux positifs. (Test rouge, puis vert) Je pense que c'est plus enrichissant de pousser une pratique qui invite à réfléchir à ce qu'on teste et comment on teste. Automatiser ces vérifications ce n'est pas nous aider à être plus attentif à nos tests, ce n'est pas nous aider à écrire de meilleurs tests. Automatiser des vérifications c'est pour ne pas discuter => Ça ne passe pas ça ne part pas en prod. Les tests ce n'est pas quelques dont on ne pas parler comme l'indentation, les points virgules ou d'autres choses comme ça.

Je prend un exemple :

  context('when the headers are found', function () {
      it('should not throw a UnprocessableEntityError', async function () {
        // given
        odsBuffer = await readFile(DEFAULT_ODS_FILE_PATH);

        // when
        await validateOdsHeaders({
          odsBuffer,
          headers: VALID_HEADERS,
        });

        // then
        expect(true).to.be.true;
      });
    });

Ce test à des defauts, mais ça reste un tests acceptable. On veut vérifier les validations, si les validations passent c'est ok => expect(true).to.be.true; Si le test ne passent pas les validations cassent et une exception est levée le test échoue. L'intention n'est pas forcément explicite, mais le test va échouer pour les bonnes raisons.

context('when the headers are found', function () {
      it('should not throw a UnprocessableEntityError', async function () {
        // given
        odsBuffer = await readFile(DEFAULT_ODS_FILE_PATH);

        // when
        // then
        await validateOdsHeaders({
          odsBuffer,
          headers: VALID_HEADERS,
        });
      });
    });

Ce test la n'est pas mieux, il a les mêmes problème, mais l'outil dit ok, alors bah c'est ok. Ici l'outil n'aide pas à améliorer le test, on se contente de faire passer les vérifications.

PS: Une meilleure approche serait d'utiliser throw et not

Bref je dis pas ça pour que ça soit pas mergé, mais ça va pas régler les problèmes et ça va pas aider les gens à écrire de meilleurs tests. Pour ça faut changer de pratique de tests. Je pense pas que les faux positifs seront pas moins présent, parce qu'on va continuer de tester de la même manière...

---

Pour vérifier si certains trucs sont dans le code (je sais pas si les 'ExpReg sont parfaitement correctes)

no-inner-compare - Prevent using comparisons in the expect() argument

C'est des trucs comme ça non ? : expect(a, 12), expect(a, "12"), expect(a, b) grep -R -E "expect(['"][a-zA-Z0-9]+['"], ['"][a-zA-Z0-9]+['"]*)" .

no-inner-literal - Prevent using literals in the expect() argument (undefined, null, NaN, (+|-)Infinity, this, booleans, numbers, strings, and BigInt or regex literals)

grep -R -E "expect((['"]+[a-zA-Z0-9]+['"]+|null|NaN|undefined|[0-9]+|true|false|[+-]Infinity|this|\.\.))" .

missing-assertion - Prevent calling expect(...) without an assertion like .to.be.ok

grep -R -E "expect(.+)$"

terminating-properties - Prevent calling to.be.ok and other assertion properties as functions

Je savais pas comment chercher ces lignes.