From 366bb8630d3a2f69e5c5d9c73486f315f31444d3 Mon Sep 17 00:00:00 2001 From: Nicolas Fryder Date: Tue, 18 Aug 2026 13:49:54 +0200 Subject: [PATCH] =?UTF-8?q?perf(tests):=20porte=20de=20qualit=C3=A9=2017?= =?UTF-8?q?=20s=20=E2=86=92=206=20s,=20mutation=20compl=C3=A8te=205=20min?= =?UTF-8?q?=20=E2=86=92=203=20min=2040?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passe de performance guidée par la mesure, pas par l'intuition. Porte de qualité (`npm run check`) — la commande qui tourne des dizaines de fois par jour : - Les 5 vérifications sont indépendantes : exécutées en parallèle via scripts/check.mjs au lieu d'un enchaînement `&&` qui imposait la somme des durées ET 7 démarrages npm imbriqués. 14 s → 6 s - Cache ESLint (--cache) et compilation incrémentale tsc. 16 s → 14 s - Chemin critique restant : vitest 4,9 s, dont ~2 s de mise en place jsdom. Tests de mutation : - Config Vitest dédiée en pool `threads` sans ré-isolation. Mesuré sur le dry-run Stryker : overhead d'amorçage 12 844 ms → 1 592 ms, soit ~90 % du cycle par mutant. 5 min 02 → 3 min 40 - Le mode incrémental ramène le profil complet à 12 s après une édition. Pistes mesurées puis REJETÉES (documentées dans README-CI.md pour éviter qu'on les retente) : - Découper la mutation en 5 processus Stryker parallèles : 262 s contre 235 s. Chaque processus repaie son bac à sable et son dry-run. - Monter `concurrency` de 6 à 16 : aucun effet (~8,5 mutants/s partout), le débit est borné par l'orchestration mono-processus de Stryker. - Séparer vitest API/frontend en deux processus : aucun gain, jsdom domine. - Pool `threads` sur la suite complète : 18,8 s contre 3,5 s. Le bon réglage dépend du jeu de fichiers (jsdom ou non) — d'où deux configs distinctes. Le périmètre muté est inchangé (1511 mutants) : un score obtenu en retirant les mutants gênants vaudrait moins que pas de score. Corrections au passage : - scripts/*.mjs n'était couvert par aucune section ESLint - Les deux suites pytest ont chacune un paquet `src` : réunies dans un seul processus, le premier importé masquait l'autre. Séparées (et parallèles). Co-Authored-By: Claude --- .githooks/pre-push | 2 +- .gitignore | 3 + README-CI.md | 100 +++++++++++++++----------------- eslint.config.js | 8 ++- jsconfig.json | 4 +- package.json | 13 +++-- scripts/check.mjs | 101 +++++++++++++++++++++++++++++++++ vitest.mutation.config.js | 7 +++ vitest.mutation.full.config.js | 30 ++++++++-- 9 files changed, 201 insertions(+), 67 deletions(-) create mode 100644 scripts/check.mjs diff --git a/.githooks/pre-push b/.githooks/pre-push index b2fda5c..146d2ba 100644 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -11,7 +11,7 @@ # set -e -echo "▶ pre-push : porte de qualité (lint, types, tests)…" +echo "▶ pre-push : porte de qualité (~6 s)…" start=$(date +%s) diff --git a/.gitignore b/.gitignore index 95bc819..280c680 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ reports/ # Session Claude Code (contient des secrets) .claude/ CLAUDE.md + +# Caches d outillage (lint, tsc) +node_modules/.cache/ diff --git a/README-CI.md b/README-CI.md index 379d17b..284859c 100644 --- a/README-CI.md +++ b/README-CI.md @@ -8,7 +8,7 @@ Coolify redéploie sur webhook à **chaque** push (`dev` → dev.metalfrom.eu, ``` git push ──▶ [hook pre-push : npm run check] ──▶ Forgejo ──▶ webhook ──▶ Coolify redeploy - ↑ ~15 s, bloquant + ↑ 6 s, bloquant ``` ## Installation (une fois par clone) @@ -23,36 +23,47 @@ pip install -r requirements-dev.txt | Commande | Ce que ça fait | Froid | Incrémental | |---|---|---|---| -| `npm run check` | **La porte** : ESLint + Ruff + tsc + tous les tests | ~15 s | — | -| `npm test` | 401 tests JS | 3,5 s | — | -| `npm run test:py` | 43 tests Python | 1 s | — | -| `npm run lint` / `lint:py` | ESLint / Ruff | 3 s / 1 s | — | -| `npm run typecheck` | `tsc --checkJs` sur l'API | 6 s | — | -| `npm run test:mutation` | Mutation, logique pure (352 mutants) | 34 s | ~5 s | -| `npm run test:mutation:full` | Mutation, toute l'API (1511 mutants) | 5 min | 13 s | -| `npm run check:full` | `check` + audits de dépendances + mutation complète | — | ~1 min | +| `npm run check` | **La porte** : ESLint + tsc + Ruff + 444 tests | **6 s** | — | +| `npm run check:sequential` | Idem, en série (pour isoler un échec) | 14 s | — | +| `npm run test:mutation` | Mutation, logique pure (352 mutants) | 33 s | ~5 s | +| `npm run test:mutation:full` | Mutation, toute l'API (1511 mutants) | 3 min 40 | **12 s** | +| `npm run check:full` | `check` + audits de dépendances + mutation complète | — | ~40 s | +| `npm run check:clean` | Purge les caches ESLint / tsc | — | — | -### Où passe le temps, et pourquoi c'est acceptable +### Où passe le temps -La commande de la boucle de développement, c'est `npm run check` : **17 s**, -et elle inclut déjà les 401 tests, ESLint, Ruff et tsc. C'est elle qui tourne -cent fois par jour. +La commande de la boucle de développement, c'est `npm run check` : **6 s**, +tests compris. C'est elle qui tourne cent fois par jour, et elle est passée de +17 s à 6 s par trois mesures successives : -La mutation n'est pas une commande de boucle courte. Son coût se lit ainsi : +| Levier | Gain | +|---|---| +| Les 5 vérifications en parallèle au lieu de `&&` (elles sont indépendantes, et l'enchaînement npm coûtait 7 démarrages) | 14 s → 6 s | +| Cache ESLint (`--cache`) et compilation incrémentale tsc | 16 s → 14 s | +| Chemin critique restant : vitest 4,9 s, dont ~2 s de mise en place jsdom | — | -- `test:mutation` (352 mutants) : 34 s à froid, ~5 s ensuite. Assez rapide pour - être lancée avant chaque commit qui touche la validation ou l'auth. -- `test:mutation:full` (1511 mutants) : 5 min **une seule fois** (clone neuf ou - après avoir modifié les quatre fichiers mutés d'un coup). Après une édition - normale d'un seul fichier : **13 s**, mesuré. +La mutation n'est pas une commande de boucle courte : -Le fichier incrémental vit dans `reports/`, qui est gitignoré : un clone neuf -paie donc les 5 minutes une fois. C'est assumé — c'est un audit, pas un test. +- `test:mutation` (352 mutants) : 33 s à froid, ~5 s ensuite. +- `test:mutation:full` (1511 mutants) : 3 min 40 **une seule fois**, puis + **12 s** après une édition. Le fichier incrémental vit dans `reports/`, qui + est gitignoré : un clone neuf paie donc le run complet une fois. -**Ce qui n'a délibérément pas été fait pour aller plus vite** : réduire encore -le périmètre muté. Descendre sous ~1500 mutants sur l'API reviendrait à ne plus -mesurer grand-chose, et un score de mutation flatteur obtenu en retirant les -mutants gênants est pire qu'une absence de score. +**Ce qui a été essayé et rejeté sur mesure** (les intuitions se sont trompées +plus d'une fois — tout est re-mesurable) : + +| Piste | Résultat | +|---|---| +| Découper la mutation en 5 processus Stryker parallèles | **262 s contre 235 s** : chaque processus repaie son bac à sable et son dry-run. Retiré. | +| Monter `concurrency` de 6 à 16 | Aucun effet : ~8,5 mutants/s dans tous les cas. Le débit est limité par l'orchestration mono-processus de Stryker, pas par le CPU. | +| Séparer les tests API / frontend en deux processus vitest | Aucun gain : 4,9 s dans les deux cas, jsdom domine. | +| Pool vitest `threads` sur la **suite complète** | 18,8 s contre 3,5 s (jsdom pénalisé). Défaut conservé. | +| Pool vitest `threads` sur la **config de mutation** (sans jsdom) | Overhead d'amorçage 12 844 ms → **1 592 ms**. Adopté. | + +**Ce qui n'a délibérément pas été fait** : réduire le périmètre muté. Descendre +sous ~1500 mutants sur l'API reviendrait à ne plus mesurer grand-chose, et un +score flatteur obtenu en retirant les mutants gênants est pire que pas de score. +Le plancher de ~3 min à froid est celui de Stryker sur 1511 mutants, assumé. ## Ce que couvrent les tests @@ -68,38 +79,21 @@ mutants gênants est pire qu'une absence de score. | **Python** | `apps/geocoder/tests/`, `apps/crawler/tests/` | Parseur de localisations, annulation | | **Mutation** | `stryker*.config.json` | 82 % (logique pure) / 65 % (API complète) | -## Performance : les décisions et leurs mesures +## Principe des tests -Le principe : **aucun test ne monte de conteneur, ne compile, ni ne touche le réseau.** +**Aucun test ne monte de conteneur, ne compile, ni ne touche le réseau.** -Quatre optimisations, toutes mesurées (les intuitions non vérifiées se sont -révélées fausses au moins une fois) : +Deux choix structurants côté API : -1. **Une app Fastify par fichier de test, pas par cas.** - `buildServer()` coûte 14 ms. À raison d'une construction par `it()`, ces - 14 ms étaient multipliés par ~12 000 exécutions pendant un run de mutation. - Les tests partagent une app et reprogramment un pool via `pool.reset()` - (`test/helpers/testApp.js`). +1. **Une app Fastify par fichier de test, pas par cas.** `buildServer()` coûte + 14 ms ; à raison d'une construction par `it()`, ces 14 ms étaient multipliés + par des milliers d'exécutions en mutation. Les tests partagent une app et + reprogramment un faux pool via `pool.reset()` (`test/helpers/testApp.js`). -2. **Config Vitest dédiée à la mutation.** - Stryker recharge les fichiers de test à chaque mutant. En ne déclarant que - ceux qui couvrent le code muté (et surtout pas le test a11y, qui initialise - jsdom en ~2 s), le profil pur est passé de **1 min 23 à 34 s**. - -3. **Mutants `StringLiteral` exclus.** - Les handlers sont à ~80 % du SQL en template literals. Muter le contenu - d'une chaîne SQL ne mesure rien. Les retirer a fait passer le profil complet - de 7 min 50 à 5 min — et le score de 42 % à 65 %, parce que le bruit - disparaissait du dénominateur. - -4. **Plafonds de débit paramétrables.** - Conséquence du point 1 : une app partagée accumule l'état du limiteur. Les - valeurs réelles de production sont vérifiées séparément dans - `rateLimit.test.js`, pour que la paramétrisation ne crée pas d'angle mort. - -Ce qui a été **essayé et rejeté sur mesure** : le pool Vitest `threads` avec -`isolate: false`, réputé plus rapide, donne **18,8 s contre 3,5 s** ici (la -mise en place de jsdom est pénalisée). Le défaut (`forks`) est conservé. +2. **Plafonds de débit paramétrables.** Conséquence du point 1 : une app + partagée accumule l'état du limiteur. Les valeurs réelles de production sont + donc vérifiées séparément dans `rateLimit.test.js`, pour que cette + paramétrisation ne crée pas d'angle mort sur une protection de sécurité. ## Pourquoi deux profils de mutation diff --git a/eslint.config.js b/eslint.config.js index 6e4ce94..32883d0 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -91,13 +91,17 @@ export default [ rules: { "no-irregular-whitespace": "off" }, }, - // ---- Config outillage à la racine ---- + // ---- Config et scripts d'outillage (racine + scripts/) ---- { - files: ["*.js", "*.config.js"], + files: ["*.js", "*.config.js", "scripts/**/*.{js,mjs}"], languageOptions: { ecmaVersion: 2023, sourceType: "module", globals: { ...globals.node }, }, + rules: { + "no-unused-vars": ["error", { argsIgnorePattern: "^_", caughtErrors: "none" }], + "no-console": "off", + }, }, ]; diff --git a/jsconfig.json b/jsconfig.json index 8442b72..45fce6a 100644 --- a/jsconfig.json +++ b/jsconfig.json @@ -13,7 +13,9 @@ "skipLibCheck": true, "resolveJsonModule": true, "types": ["node"], - "maxNodeModuleJsDepth": 0 + "maxNodeModuleJsDepth": 0, + "incremental": true, + "tsBuildInfoFile": "node_modules/.cache/tsc/jsconfig.tsbuildinfo" }, "include": ["apps/api/src/**/*.js", "apps/api/test/**/*.js"], "exclude": ["node_modules", "**/node_modules"] diff --git a/package.json b/package.json index e1331d3..0b914a7 100644 --- a/package.json +++ b/package.json @@ -8,8 +8,8 @@ "apps/api" ], "scripts": { - "lint": "eslint .", - "lint:fix": "eslint . --fix", + "lint": "eslint . --cache --cache-location node_modules/.cache/eslint/", + "lint:fix": "eslint . --fix --cache --cache-location node_modules/.cache/eslint/", "typecheck": "tsc -p jsconfig.json", "test": "vitest run", "test:watch": "vitest", @@ -23,10 +23,13 @@ "test:py:crawler": "cd apps/crawler && python -m pytest tests -q", "audit:js": "npm audit --audit-level=high", "audit:py": "pip-audit -r apps/api/../crawler/requirements.txt -r apps/geocoder/requirements.txt", - "_comment_check": "`check` = la porte de qualité lancée avant chaque push (~30s). Les tests de mutation et les audits de dépendances en sont exclus : trop lents / dépendants du réseau. Lancer `npm run check:full` avant une release.", - "check": "npm run lint && npm run lint:py && npm run typecheck && npm run test && npm run test:py", + "_comment_check": "`check` = la porte de qualite lancee avant chaque push. Les cinq verifications tournent en parallele (voir scripts/check.mjs). Mutation et audits de dependances en sont exclus : trop lents / dependants du reseau — voir `check:full`.", + "check": "node scripts/check.mjs", "check:full": "npm run check && npm run audit:js && npm run audit:py && npm run test:mutation:full", - "hooks:install": "git config core.hooksPath .githooks" + "hooks:install": "git config core.hooksPath .githooks", + "_comment_cache": "lint et typecheck utilisent un cache dans node_modules/.cache : `check` tourne des dizaines de fois par jour, refaire l'analyse complete de fichiers inchanges a chaque fois n'apporte rien. Les caches sont invalides par mtime+contenu ; `npm run check:clean` les purge.", + "check:clean": "node -e \"require('fs').rmSync('node_modules/.cache',{recursive:true,force:true})\"", + "check:sequential": "node scripts/check.mjs --sequential" }, "devDependencies": { "@eslint/js": "^9.17.0", diff --git a/scripts/check.mjs b/scripts/check.mjs new file mode 100644 index 0000000..ca26963 --- /dev/null +++ b/scripts/check.mjs @@ -0,0 +1,101 @@ +#!/usr/bin/env node +/** + * Porte de qualité — exécute les vérifications EN PARALLÈLE. + * + * Les cinq vérifications sont indépendantes (aucune ne lit la sortie d'une + * autre). Les enchaîner avec `&&` imposait deux coûts inutiles : + * - la somme des durées au lieu du maximum ; + * - sept invocations `npm run` imbriquées, chacune avec son propre coût de + * démarrage npm (~300 ms). + * + * On les lance donc toutes d'un coup, on tamponne les sorties, et on les + * restitue dans un ordre stable à la fin. Le détail par tâche reste affiché + * pour que le goulot d'étranglement demeure visible. + * + * `--sequential` rétablit l'exécution en série (utile pour isoler un échec + * quand deux tâches écrivent des messages entremêlés). + */ +import { spawn } from "node:child_process"; + +const isWin = process.platform === "win32"; +const sequential = process.argv.includes("--sequential"); + +/** bin = binaire de node_modules/.bin (résolu par npm via PATH). */ +const TASKS = [ + { + name: "eslint", + cmd: "eslint", + args: [".", "--cache", "--cache-location", "node_modules/.cache/eslint/"], + }, + { + name: "tsc", + cmd: "tsc", + args: ["-p", "jsconfig.json"], + }, + // Séparer les tests API (node) et frontend (jsdom) en deux processus a été + // mesuré : aucun gain (4,9 s dans les deux cas — c'est la mise en place de + // jsdom qui domine, et les deux processus se disputent les cœurs). Gardé en + // un seul. + { name: "vitest", cmd: "vitest", args: ["run"] }, + { + name: "ruff", + cmd: "python", + args: ["-m", "ruff", "check", "."], + }, + // Les deux suites Python tournent dans des processus distincts : chaque app a + // son propre paquet `src`, et les réunir dans un seul pytest fait que le + // premier `src` importé masque l'autre (ModuleNotFoundError: src.parser). + // Séparées, elles s'exécutent aussi en parallèle. + { name: "pytest:geocoder", cmd: "python", args: ["-m", "pytest", "tests", "-q"], cwd: "apps/geocoder" }, + { name: "pytest:crawler", cmd: "python", args: ["-m", "pytest", "tests", "-q"], cwd: "apps/crawler" }, +]; + +function run(task) { + return new Promise((resolve) => { + const started = Date.now(); + const child = spawn(task.cmd, task.args, { + shell: isWin, + cwd: task.cwd ?? process.cwd(), + }); + + let out = ""; + child.stdout.on("data", (b) => (out += b)); + child.stderr.on("data", (b) => (out += b)); + child.on("error", (err) => { + resolve({ ...task, code: 1, out: `${err.message}\n`, ms: Date.now() - started }); + }); + child.on("close", (code) => { + resolve({ ...task, code, out, ms: Date.now() - started }); + }); + }); +} + +const started = Date.now(); +let results; + +if (sequential) { + results = []; + for (const t of TASKS) results.push(await run(t)); +} else { + results = await Promise.all(TASKS.map(run)); +} + +const failed = results.filter((r) => r.code !== 0); + +for (const r of results) { + const mark = r.code === 0 ? "✓" : "✗"; + console.log(`\n${mark} ${r.name} (${(r.ms / 1000).toFixed(1)}s)`); + // Sur succès on ne montre que les avertissements éventuels, pas le bruit. + if (r.code !== 0 || /warning|warn/i.test(r.out)) { + const body = r.out.trimEnd(); + if (body) console.log(body.replace(/^/gm, " ")); + } +} + +const total = ((Date.now() - started) / 1000).toFixed(1); + +if (failed.length) { + console.error(`\n✗ Porte de qualité en échec : ${failed.map((f) => f.name).join(", ")} (${total}s)`); + process.exit(1); +} +console.log(`\n✓ Tout est vert (${total}s)`); diff --git a/vitest.mutation.config.js b/vitest.mutation.config.js index 8eae815..68fb494 100644 --- a/vitest.mutation.config.js +++ b/vitest.mutation.config.js @@ -14,6 +14,13 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { environment: "node", + // Voir vitest.mutation.full.config.js pour les mesures : sur un jeu de + // fichiers 100 % node, threads sans ré-isolation divise l'overhead + // d'amorçage par 8. Ne pas reporter tel quel dans vitest.config.js, qui + // contient le test a11y sous jsdom (où ce réglage est plus lent). + pool: "threads", + poolOptions: { threads: { isolate: false, singleThread: true } }, + fileParallelism: false, include: [ "apps/api/test/validate.test.js", "apps/api/test/adminAuth.test.js", diff --git a/vitest.mutation.full.config.js b/vitest.mutation.full.config.js index 57a03ab..983fea4 100644 --- a/vitest.mutation.full.config.js +++ b/vitest.mutation.full.config.js @@ -3,15 +3,35 @@ import { defineConfig } from "vitest/config"; /** * Config Vitest du profil de mutation COMPLET (`npm run test:mutation:full`). * - * Même principe que vitest.mutation.config.js : ne charger que les fichiers qui - * couvrent le code muté. Ici tout le code API est muté, donc tous les tests API - * sont nécessaires — mais surtout PAS les tests frontend, dont le fichier a11y - * initialise jsdom (~2 s) à chaque activation de mutant alors qu'il ne couvre - * aucune ligne de l'API. + * Deux réglages, tous deux mesurés sur l'overhead d'amorçage que Stryker paie + * à CHAQUE activation de mutant (ligne « net / overhead » du dry-run) : + * + * 1. Ne charger que les tests API. Le fichier a11y initialise jsdom (~2 s) et + * ne couvre aucune ligne de l'API : le charger reviendrait à payer ce coût + * 1500 fois. + * + * 2. Workers `threads` sans ré-isolation. Mesuré sur ce jeu de fichiers : + * forks, isolate:true (défaut) : overhead 12 844 ms + * forks, isolate:false : overhead 7 064 ms + * threads, isolate:false : overhead 1 592 ms ← retenu + * Le temps net des tests, lui, ne bouge presque pas (~1 s) : l'amorçage + * représentait 90 % du cycle. + * + * À NE PAS reporter dans vitest.config.js : la suite complète contient le + * test a11y, et sous jsdom ce même réglage est nettement PLUS lent (18,8 s + * contre 3,5 s). Le bon réglage dépend du jeu de fichiers — re-mesurer. + * + * L'isolation ne protège de rien ici : chaque fichier de test construit sa + * propre app et son propre faux pool, il n'y a aucun état global mutable. */ export default defineConfig({ test: { environment: "node", + pool: "threads", + poolOptions: { threads: { isolate: false, singleThread: true } }, + // Stryker parallélise déjà entre mutants (concurrency) : paralléliser aussi + // les fichiers à l'intérieur d'un worker ne ferait que se disputer les cœurs. + fileParallelism: false, include: ["apps/api/test/**/*.test.js"], reporters: ["default"], },