Claude commited on
Commit
6724f94
·
unverified ·
1 Parent(s): d641f6e

fix(report): corrections d'audit du sprint de découpage de generator.py

Browse files

Suite à un auto-audit du commit d641f6e, deux problèmes corrigés :

1. **5 ré-exports rétrocompat inutilisés (ruff F401)**.
J'avais ajouté 9 alias ``_*`` au top de generator.py par excès
de rétrocompat. Audit cross-référencé des tests : seuls 2 sont
réellement consommés depuis ``picarones.report.generator`` :
- ``_build_report_data`` (importé par 14 tests)
- ``_cer_color`` (importé par tests/report/test_report.py)
Plus 3 utilisés en interne (``_load_vendor_js``,
``_externalize_images_to_dir``, ``_encode_images_b64_from_result``).
Les 4 autres (``_pct``, ``_safe``, ``_cer_bg``, ``_encode_image_b64``)
ne sont importés par personne — supprimés. Les 2 ré-exports
conservés sont marqués ``# noqa: F401`` avec commentaire
explicatif documentant l'intention de ré-export.

2. **test_generator_imports_three_views trop laxiste**.
Mon assouplissement précédent acceptait la chaîne
``"economics_view_html"`` n'importe où dans generator.py
(y compris dans une docstring). Faux positif possible : si
on supprime le splat ``**section_html`` mais qu'une chaîne
subsiste dans un commentaire, le test passe à tort.
Remplacé par un vrai test runtime qui :
- instancie ``ReportGenerator``
- appelle ``_build_section_html`` avec un report_data minimal
- vérifie que le dict retourné contient les 3 clés attendues
- vérifie que chaque valeur est bien une chaîne (adaptive : "")
Garantit le contrat fonctionnel, pas la forme du code.

Vérifications post-fix :
- ruff check picarones/ tests/architecture/ → All checks passed!
- pytest tests/ → 3830 passed, 2 skipped (parité stricte)
- mypy : 30 erreurs (vs 48 avant le découpage — réduction nette,
toutes restantes sont des [type-arg] sur ``dict`` désactivés
par la config projet [tool.mypy] disallow_any_generics=false).

picarones/report/generator.py CHANGED
@@ -23,11 +23,20 @@ découpées en sous-modules :
23
  passé au template (engines, documents, statistiques, Pareto, etc.).
24
  - :mod:`picarones.report.render_helpers` — couleurs / SVG mutualisés.
25
 
26
- Les noms ``_build_report_data``, ``_cer_color``, ``_cer_bg``,
27
- ``_externalize_images_to_dir``, ``_encode_image_b64``,
28
- ``_encode_images_b64_from_result``, ``_load_vendor_js``, ``_pct``,
29
- ``_safe`` sont conservés en alias rétrocompat — plusieurs tests les
30
- importent directement.
 
 
 
 
 
 
 
 
 
31
  """
32
 
33
  from __future__ import annotations
@@ -40,20 +49,16 @@ from typing import Any, Optional
40
  from picarones.core.results import BenchmarkResult
41
  from picarones.measurements.statistics import build_critical_difference_svg
42
  from picarones.report.assets import (
43
- encode_image_b64 as _encode_image_b64,
44
  encode_images_b64_from_result as _encode_images_b64_from_result,
45
  externalize_images_to_dir as _externalize_images_to_dir,
46
  load_vendor_js as _load_vendor_js,
47
  )
48
- from picarones.report.render_helpers import (
49
- cer_step_bg as _cer_bg,
50
- cer_step_color as _cer_color,
51
- )
52
- from picarones.report.report_data import build_report_data as _build_report_data
53
- from picarones.report.report_data._helpers import (
54
- percent_string as _pct,
55
- safe_round as _safe,
56
- )
57
 
58
  logger = logging.getLogger(__name__)
59
 
 
23
  passé au template (engines, documents, statistiques, Pareto, etc.).
24
  - :mod:`picarones.report.render_helpers` — couleurs / SVG mutualisés.
25
 
26
+ Rétrocompat
27
+ -----------
28
+ Deux noms historiques sont **encore importés par des tests** sous
29
+ leur préfixe ``_`` et doivent être préservés :
30
+
31
+ - ``_build_report_data`` (importé par 14 fichiers de tests).
32
+ - ``_cer_color`` (importé par ``tests/report/test_report.py``).
33
+
34
+ Les autres noms ``_pct``, ``_safe``, ``_cer_bg``, ``_encode_image_b64``,
35
+ ``_encode_images_b64_from_result``, ``_externalize_images_to_dir``,
36
+ ``_load_vendor_js`` sont soit utilisés en interne (les 3 derniers,
37
+ voir :meth:`ReportGenerator.generate`), soit accessibles via leur
38
+ nom canonique dans :mod:`picarones.report.assets` ou
39
+ :mod:`picarones.report.render_helpers`.
40
  """
41
 
42
  from __future__ import annotations
 
49
  from picarones.core.results import BenchmarkResult
50
  from picarones.measurements.statistics import build_critical_difference_svg
51
  from picarones.report.assets import (
 
52
  encode_images_b64_from_result as _encode_images_b64_from_result,
53
  externalize_images_to_dir as _externalize_images_to_dir,
54
  load_vendor_js as _load_vendor_js,
55
  )
56
+
57
+ # Ré-exports rétrocompat consommés par les tests externes (cf. docstring
58
+ # de module). La directive de fin de ligne documente l'intention de
59
+ # ré-export et empêche ruff de marquer l'import comme inutilisé.
60
+ from picarones.report.render_helpers import cer_step_color as _cer_color # noqa: F401
61
+ from picarones.report.report_data import build_report_data as _build_report_data # noqa: F401
 
 
 
62
 
63
  logger = logging.getLogger(__name__)
64
 
tests/report/test_views.py CHANGED
@@ -332,36 +332,48 @@ class TestDetailsShell:
332
 
333
  class TestGeneratorWiring:
334
  def test_generator_imports_three_views(self):
335
- """generator.py doit importer les 3 vues automatiques (economics,
336
- advanced_taxonomy, diagnostics) pour les passer au template.
337
 
338
- Tolère les deux conventions de câblage : argument nommé
339
- ``economics_view_html=...`` ou clé de dict ``"economics_view_html"``
340
- splatée via ``**section_html`` (cf. ``_build_section_html``).
 
 
 
 
 
341
  """
342
- from pathlib import Path
 
343
 
344
- gen_src = (
345
- Path(__file__).parent.parent.parent / "picarones" / "report" / "generator.py"
346
- ).read_text(encoding="utf-8")
347
- # Les 3 imports doivent être présents
348
- assert "build_economics_view_html" in gen_src
349
- assert "build_advanced_taxonomy_view_html" in gen_src
350
- assert "build_diagnostics_view_html" in gen_src
351
- # Et les 3 variables doivent être câblées vers le template, soit
352
- # par argument explicite (``var=...``), soit par clé de dict
353
- # splatée (``"var": ...``).
 
 
354
  for name in (
355
  "economics_view_html",
356
  "advanced_taxonomy_view_html",
357
  "diagnostics_view_html",
358
  ):
359
- assert (
360
- f"{name}=" in gen_src
361
- or f'"{name}"' in gen_src
362
- ), (
363
- f"variable {name!r} ni argument nommé ni clé de dict "
364
- "dans generator.py"
 
 
 
 
365
  )
366
 
367
  def test_template_uses_three_views(self):
 
332
 
333
  class TestGeneratorWiring:
334
  def test_generator_imports_three_views(self):
335
+ """Test runtime du câblage des 3 vues automatiques (economics,
336
+ advanced_taxonomy, diagnostics).
337
 
338
+ Vérifie que la méthode ``ReportGenerator._build_section_html``
339
+ retourne un dict contenant les 3 clés attendues, ce qui
340
+ garantit qu'elles seront splatées vers le template Jinja.
341
+
342
+ Cette version remplace l'ancien test qui scannait textuellement
343
+ ``generator.py`` à la recherche de ``var=`` ou ``"var"`` —
344
+ approche fragile (passait sur n'importe quelle occurrence dans
345
+ une docstring) et trop liée à la forme du code.
346
  """
347
+ from picarones.fixtures import generate_sample_benchmark
348
+ from picarones.report.generator import ReportGenerator
349
 
350
+ bench = generate_sample_benchmark()
351
+ gen = ReportGenerator(bench, lang="fr")
352
+ from picarones.i18n import get_labels
353
+
354
+ report_data = {
355
+ "engines": [],
356
+ "inter_engine_analysis": None,
357
+ "stratified_ranking": None,
358
+ "available_strata": [],
359
+ "corpus_homogeneity": None,
360
+ }
361
+ section_html = gen._build_section_html(report_data, get_labels("fr"))
362
  for name in (
363
  "economics_view_html",
364
  "advanced_taxonomy_view_html",
365
  "diagnostics_view_html",
366
  ):
367
+ assert name in section_html, (
368
+ f"clé {name!r} absente du dict retourné par "
369
+ "ReportGenerator._build_section_html la vue ne sera "
370
+ "pas câblée vers le template."
371
+ )
372
+ # Adaptive : avec un report_data vide, chaque vue retourne ""
373
+ # (rapport adaptatif). On vérifie le type, pas le contenu.
374
+ assert isinstance(section_html[name], str), (
375
+ f"section {name!r} doit être une chaîne, "
376
+ f"pas {type(section_html[name]).__name__}"
377
  )
378
 
379
  def test_template_uses_three_views(self):