Des vérifications automatisées pour alléger vos revues de code

La revue de code automatisée, dans sa forme déterministe, est un ensemble de vérifications CI qui font échouer une pull request lorsque le code enfreint une règle. Les relecteurs consacrent alors leur temps à la conception, à l’intention et à l’adéquation au produit, plutôt qu’aux erreurs mécaniques.

Les agents de code ouvrent désormais des pull requests plus vite que quiconque ne peut les lire ligne par ligne. Si vous êtes lead, vous finissez soit par valider les diffs les yeux fermés, soit par devenir le goulot d’étranglement, et aucune de ces deux situations n’est tenable.

La solution consiste à confier aux machines la détection des problèmes mécaniques et à réserver l’attention humaine pour tout le reste. Ces vérifications valent la peine d’être exécutées, quel que soit l’auteur du code. Cet article en présente trois, y ajoute deux compléments peu coûteux, montre comment intégrer le tout dans la CI et se termine par ce qu’aucune d’entre elles ne peut détecter. Vous exécutez déjà le lint, le formatage et les tests unitaires. Si ce n’est pas le cas, commencez par cette configuration ESLint et Prettier.

Générer votre client d’API à partir de la spécification OpenAPI

Si le client TypeScript est généré à partir d’une spécification OpenAPI, lire un champ que l’API ne renvoie jamais, comme user.fullName, provoque une erreur de compilation plutôt qu’un commentaire de revue. Une interface écrite à la main peut diverger du backend sans que personne ne s’en aperçoive. Une interface générée, non.

Un type écrit à la main compile même lorsqu’il est faux :

// Hand-written: compiles, but the backend never sends fullName
interface User { id: string; fullName: string }
declare const user: User;

const label = user.fullName;

Générez plutôt le type à partir de la spécification, et la même lecture échoue à la compilation :

// Generated from the spec
type User = { id: string; firstName: string; lastName: string };
declare const user: User;

const label = user.fullName;
// error TS2339: Property 'fullName' does not exist on type 'User'.
const fixed = `${user.firstName} ${user.lastName}`;

Des outils comme Hey API openapi-ts lisent un document OpenAPI et produisent des types ainsi qu’un client. Une fois le client généré, c’est la spécification qu’il faut relire. Traitez le code généré comme un artefact de build : ne le modifiez pas à la main et ne le relisez pas ligne par ligne.

Les types disparaissent à la compilation : ajoutez donc un validateur à l’exécution, comme Zod, pour les réponses auxquelles vous ne pouvez pas vous permettre de faire confiance. Rien de tout cela ne fonctionne sans une spécification de référence que le backend maintient à jour. La configuration de bout en bout est détaillée dans la création de clients d’API typés avec OpenAPI et TypeScript.

Faire respecter les frontières entre couches avec dependency-cruiser

dependency-cruiser vérifie chaque import d’un projet JavaScript ou TypeScript par rapport aux règles forbidden de son fichier de configuration. Les violations s’affichent dans un format très proche de la sortie d’ESLint, et toute violation d’une règle définie à error fait sortir la commande avec un code non nul, ce qui fait échouer le job CI. La question « ce service doit-il vraiment importer un composant ? » disparaît ainsi de la liste du relecteur.

npm install --save-dev dependency-cruiser
npx dependency-cruiser --init

--init génère .dependency-cruiser.js, ou .dependency-cruiser.cjs lorsque package.json définit "type": "module". Le fichier créé contient un ensemble de règles par défaut. Elles signalent les cycles d’imports, les modules orphelins, les paquets importés mais absents de package.json, ainsi que le code de production qui embarque des dépendances de développement. Ajoutez votre règle de couche à leurs côtés :

module.exports = {
  forbidden: [
    {
      name: "services-not-to-components",
      comment:
        "Services must not import UI. Move shared types to src/types or next to the service.",
      severity: "error",
      from: { path: "^src/services" },
      to: { path: "^src/components" },
    },
  ],
};

Rédigez le comment comme une consigne, pas comme une simple étiquette. Le reporter par défaut err l’omet, mais le reporter err-long l’affiche avec chaque violation. Le développeur ou l’agent à l’origine de l’échec lit alors la correction, et pas seulement l’erreur :

npx dependency-cruiser --output-type err-long src

eslint-plugin-boundaries remplit le même rôle au sein d’ESLint, si vous préférez garder cette vérification dans votre linter. oxlint ne dispose pas de règle intégrée pour les couches architecturales, mais sa règle no-restricted-imports permet de bloquer les imports correspondant à un motif.

Utiliser les tests de mutation pour vérifier que les tests détectent les régressions

Les tests de mutation évaluent la qualité des tests : la couverture de lignes montre que le code a été exécuté, tandis que les tests de mutation montrent si un test échoue lorsque ce code est volontairement modifié. Une suite peut atteindre 100 % de couverture tout en ne vérifiant presque rien, et les tests de mutation permettent de le découvrir.

Stryker apporte de petites modifications à votre code source ; chaque version modifiée est appelée un mutant. Il exécute ensuite vos tests sur chaque mutant. Un mutant est tué lorsqu’au moins un test échoue face à lui, et il survit lorsque toute la suite passe encore.

L’exemple fourni par Stryker mute return user.age >= 18 de quatre façons : > 18, < 18, return false et return true.

export function isUserOldEnough(user: { age: number }): boolean {
  return user.age >= 18;
}

// Weak: 100% line coverage, two survivors
it("lets adults in", () => {
  expect(isUserOldEnough({ age: 25 })).toBe(true);
});

// Added: boundary cases
it("lets in someone who is exactly 18", () => {
  expect(isUserOldEnough({ age: 18 })).toBe(true);
});
it("keeps out someone who is 17", () => {
  expect(isUserOldEnough({ age: 17 })).toBe(false);
});

Un test qui ne vérifie qu’une personne de 25 ans passe aussi bien face au mutant > 18 que face au mutant return true : les deux survivent alors même que la couverture de lignes est de 100 %. Ce test ne tue que < 18 et return false. Le cas des 18 ans tue > 18, et celui des 17 ans tue return true. Avec les trois tests, tous les mutants sont tués.

Les mutants survivants proviennent généralement de quelques schémas de tests faibles facilement identifiables :

• Vérifier qu’une fonction a été appelée sans vérifier avec quels arguments.
• Vérifier qu’un résultat existe, ou quelle est sa longueur, sans examiner son contenu.
• Commiter un snapshot que personne n’a comparé à la sortie attendue.
• Calculer la valeur attendue avec la même logique que le code testé.
• Vérifier ce que renvoie un mock plutôt que ce que le code en fait.

Un mutant survivant signale un comportement qu’aucun test ne verrouille. Certains mutants sont toutefois équivalents au code d’origine : le score de mutation fonctionne donc mieux comme détecteur que comme seuil de réussite ou d’échec. Un seuil strict récompense aussi les tests écrits pour tuer des mutants plutôt que pour vérifier un comportement. L’option thresholds.break de Stryker fait échouer le build en dessous d’un score donné. Sa valeur par défaut est null, et il vaut mieux la laisser ainsi. Examinez plutôt les survivants sur la PR.

Pour garder des exécutions courtes, ne mutez que les fichiers modifiés par une PR avec npx stryker run --mutate <comma-separated files>. Le mode incrémental (--incremental) conserve les résultats de la dernière exécution dans reports/stryker-incremental.json et réutilise ce qu’il peut, mais en CI, cela ne fait gagner du temps que si le fichier est mis en cache entre les exécutions. Installez également le plugin de test runner correspondant à votre framework (@stryker-mutator/vitest-runner ou @stryker-mutator/jest-runner). Sans lui, Stryker se rabat sur son command runner, qui exécute toute la suite pour chaque mutant. Pour l’aspect jugement de la qualité des tests, c’est-à-dire décider de ce qui mérite un test, consultez notre article sur les tests à l’ère de l’IA.

Exécuter chaque vérification comme un job CI obligatoire distinct

Une vérification que l’on peut contourner n’est pas appliquée. Attribuez à chaque vérification son propre job afin qu’un échec saute aux yeux, puis marquez chaque job comme status check obligatoire dans votre protection de branche ou votre ruleset. Si vous découvrez GitHub Actions, commencez par ce guide pratique de GitHub Actions.

on: pull_request
jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
      - uses: actions/setup-node@v7
        with: { node-version: 22, cache: npm }
      - run: npm ci
      - run: npx eslint . && npx prettier --check .
      - run: npx dependency-cruiser --output-type err-long src
  typecheck:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
      - uses: actions/setup-node@v7
        with: { node-version: 22, cache: npm }
      - run: npm ci
      - run: npx tsc --noEmit
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
      - uses: actions/setup-node@v7
        with: { node-version: 22, cache: npm }
      - run: npm ci
      - run: npm test
  mutation:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
        with: { fetch-depth: 0 }
      - uses: actions/setup-node@v7
        with: { node-version: 22, cache: npm }
      - run: npm ci
      - name: Mutate changed files only
        run: |
          FILES=$(git diff --name-only --diff-filter=AMR "origin/${{ github.base_ref }}...HEAD" -- ':(glob)src/**/*.ts' ':(glob)src/**/*.tsx' \
            | grep -vE '\.(test|spec)\.tsx?$' | paste -sd, -)
          if [ -z "$FILES" ]; then echo "No source changes"; exit 0; fi
          npx stryker run --mutate "$FILES"
  build:
    needs: [lint, typecheck, test, mutation]
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v7
      - uses: actions/setup-node@v7
        with: { node-version: 22, cache: npm }
      - run: npm ci
      - run: npm run build

Le job de mutation récupère l’historique complet (fetch-depth: 0) afin de pouvoir calculer le diff par rapport à la branche de base. Il ne transmet ensuite à Stryker que les fichiers source modifiés, en excluant les fichiers de test.

Déployez les nouvelles règles progressivement :

1. Ajoutez chaque nouvelle règle sous forme d’avertissement. Dans dependency-cruiser, severity: "warn" signale les violations sans faire échouer le job. Seul error le fait.
2. N’exécutez les vérifications que sur les fichiers modifiés.
3. Passez un avertissement en erreur dès qu’il détecte un vrai bug, et supprimez les règles qui ne produisent que du bruit.
4. Ne rendez un job obligatoire qu’une fois qu’il s’est stabilisé.

Deux autres vérifications automatisées peu coûteuses méritent d’être ajoutées : Knip pour le code mort et jscpd pour la duplication. Knip signale les fichiers que rien n’importe, les exports que personne n’utilise et les paquets dont vous n’avez plus besoin. jscpd repère les blocs de code dupliqués.

Ce que les vérifications automatisées ne peuvent pas vous dire

Les vérifications déterministes ne peuvent pas vous dire si la fonctionnalité est la bonne, si une abstraction a été introduite trop tôt ou si quelqu’un dans l’équipe comprend le code. Ce sont des questions de jugement, et elles nécessitent un humain.

Une pull request qui passe toutes les vérifications déterministes a seulement démontré qu’elle est exempte des défauts que ces vérifications recherchent. Pour l’aspect humain de la revue, consultez notre article sur les tests à l’ère de l’IA.

Commencer par le contrat

Chaque question mécanique confiée à une machine libère du temps de revue humaine pour les questions auxquelles seuls les humains peuvent répondre. Si vous ne devez ajouter qu’une seule vérification ce sprint, générez votre client d’API à partir de la spécification : cela élimine toute une catégorie de défauts avant même que quiconque ne les écrive. Ajoutez ensuite les tests de mutation sur le diff de la PR, dans un job en mode avertissement uniquement, et examinez ce qui survit.

FAQ

Comment ajouter des règles dependency-cruiser à une base de code existante qui compte déjà des centaines de violations ?

Enregistrez les violations existantes comme référence (baseline) et ne faites échouer que les nouvelles. Depuis dependency-cruiser 18.3.0, la méthode recommandée est l'option --baseline. Par défaut, elle écrit .dependency-cruiser-known-violations.json à la racine du projet, et l'exécution avec --ignore-known abaisse chaque violation qui y figure au niveau ignore. Tout nouvel import qui enfreint la même règle échoue toujours. Comme la correspondance ne tient pas compte de la sévérité, modifier la sévérité d'une règle laisse ses entrées de baseline en place.

Les tests de mutation avec Stryker fonctionnent-ils avec Vitest ?

Oui. StrykerJS a ajouté la prise en charge de Vitest dans la version 7.0 via le plugin @stryker-mutator/vitest-runner, et les projets Jest utilisent @stryker-mutator/jest-runner. Le runner Vitest utilise toujours l'analyse de couverture perTest et ignore tout paramètre coverageAnalysis que vous configurez. Le Browser Mode de Vitest n'est pas pris en charge. Stryker exécute les tests dans un seul thread par worker et parallélise plutôt sur ses propres workers.

Qu'est-ce qu'un mutant équivalent dans les tests de mutation ?

Un mutant équivalent modifie le code source sans changer le comportement du programme, si bien qu'aucun test ne peut jamais le tuer. Un exemple classique consiste à remplacer i < length par i != length dans une boucle qui compte de un en un à partir de zéro : les deux s'arrêtent au même endroit. Comme les mutants équivalents survivent toujours, un score de mutation de 100 % est souvent inatteignable, et les survivants doivent être examinés par un humain.

Que se passe-t-il lorsque Stryker s'exécute sur des fichiers modifiés qui n'ont pas de tests ?

Par défaut, Stryker s'arrête avec une erreur No tests found lorsque son exécution initiale à blanc (dry run) n'exécute aucun test, ce qui fait échouer un job CI qui ne mute que le diff de la PR. Depuis StrykerJS 7.0, l'option --allowEmpty (ou allowEmpty: true dans le fichier de configuration) évite que cela soit considéré comme une erreur. Les mutants situés dans du code qu'aucun test n'exécute sont signalés comme sans couverture, ce qui pointe directement vers les tests manquants.