Claude commited on
Commit
e071a2c
·
unverified ·
1 Parent(s): dbf17e9

fix(tests): 2 bugs CI précis identifiés via logs partagés

Browse files

Diagnostic chirurgical grâce aux logs CI complets fournis par
l'utilisateur. Deux bugs distincts dans deux fichiers différents.

1. macOS 3.12 — test_some_docs_succeed_others_timeout flaky
-----------------------------------------------------------
Log :

FAILED tests/pipeline/test_sprint_a14_s8_timeout.py::test_some_docs_succeed_others_timeout
assert 2 == 3
where 2 = CorpusRunResult(...n_succeeded=2, n_timed_out=4...)

Le test S8 (mon sprint) testait :
- 6 docs : 3 pairs sleep 0.01s, 3 impairs sleep 0.5s
- timeout 0.1s
- attendu : 3 succeeded + 3 timed_out

Sur macOS GitHub Actions runners, le scheduler OS a un jitter qui
fait que ``time.sleep(0.01)`` peut prendre 30-90ms réels (pas 10).
Combiné avec le temps d'orchestration (queue, polling), un doc
"pair" peut dépasser 100ms et basculer en timeout.

Le fix correct n'est pas d'augmenter le timeout — c'est de **donner
une marge de robustesse** entre les deux régimes :

→ Timeout passé de 0.1s à **0.5s**.
→ Sleep pair passé de 0.01s à **0.05s** (10× sous le timeout).
→ Sleep impair passé de 0.5s à **2.0s** (4× au-dessus du timeout).

Ces marges absorbent le jitter scheduler de tous les OS sans
toucher à la sémantique du test. Plus une assertion enrichie qui
expose les compteurs réels en cas d'échec futur.

2. Windows 3.11/3.12 — README divergent par OS
----------------------------------------------
Log :

FAILED tests/docs/test_readme_dual_lang.py::test_readme_tables_consistent_with_code
Le README diverge du contenu généré par scripts/gen_readme_tables.py.
[gen_readme_tables] README divergent du code généré.

Cause racine identifiée : ``collect_test_count()`` du script lance
``pytest --collect-only`` et insère le compteur **exact** dans le
README via ``_replace_test_count``.

Mon fix précédent (``pytest.importorskip("resource")`` dans
``test_sprint_a14_s8_def_of_done.py``) skip 2 tests sur Windows.
Le compteur diverge donc selon l'OS qui régénère le README :

- Linux : 4519 tests collectés
- Windows : 4517 tests collectés (les 2 tests POSIX-only sautés)

Le test ``test_readme_tables_consistent_with_code`` exige égalité
**stricte** entre le README versionné (~4519) et la regénération
sur runner (~4517) → fail Windows.

Fix propre, dans l'esprit du projet : **arrondir le count à la
dizaine** (``round(count, -1)``). Tous les OS convergent vers
4520, indépendamment des skips OS-spécifiques. Le test
``test_readme_test_count_matches_baseline`` (consistency) tolère
déjà ±5 % donc 4520 vs 4517 réel passe largement.

Cette modif ne masque pas une vraie évolution — l'arrondi est
explicite, documenté, et le seuil de tolérance des tests
consistency reste à ±5 %.

Anti-bricolage assumé
---------------------
- Pas désactivé le test flaky S8 timeout — j'ai donné aux régimes
des marges de jitter saines.
- Pas désactivé le test README sur Windows — j'ai rendu le script
OS-déterministe.
- Pas dupliqué de logique — l'arrondi vit au seul endroit
pertinent (``_replace_test_count``).
- Pas augmenté arbitrairement la tolérance — j'ai éliminé la
source de variance.

Résultat local : 4504 passed, 11 skipped, 0 failed. Lint clean.

https://claude.ai/code/session_011XQZNitg1rCgia8ZD1a2hP

README.md CHANGED
@@ -396,7 +396,7 @@ ruff check picarones/ tests/
396
  python -m mypy picarones/core/
397
  ```
398
 
399
- **Test suite**: ~4519 tests, ~3 min on a modern laptop. Coverage
400
  floor at 85% (currently ~87%). The `network` marker excludes tests
401
  requiring live HTTP. A handful of tests depend on optional engines
402
  (`pero-ocr`, `pytesseract`) and are skipped/fail gracefully when
 
396
  python -m mypy picarones/core/
397
  ```
398
 
399
+ **Test suite**: ~4520 tests, ~3 min on a modern laptop. Coverage
400
  floor at 85% (currently ~87%). The `network` marker excludes tests
401
  requiring live HTTP. A handful of tests depend on optional engines
402
  (`pero-ocr`, `pytesseract`) and are skipped/fail gracefully when
scripts/gen_readme_tables.py CHANGED
@@ -214,7 +214,17 @@ def _replace_section(text: str, marker: str, content: str) -> str:
214
  def _replace_test_count(text: str, count: int) -> str:
215
  """Remplace les mentions ``N tests`` ou ``N passed`` qui citent un
216
  nombre dans la fenêtre [count*0.5, count*2]. Garde la formulation
217
- exacte (espace, ponctuation) intacte."""
 
 
 
 
 
 
 
 
 
 
218
  def _sub(match: re.Match) -> str:
219
  cited = int(match.group(1))
220
  # Ne touche pas si le nombre cité est complètement hors plage —
@@ -222,7 +232,7 @@ def _replace_test_count(text: str, count: int) -> str:
222
  # phrase qui parle d'autre chose).
223
  if cited < count * 0.5 or cited > count * 2:
224
  return match.group(0)
225
- return match.group(0).replace(str(cited), str(count))
226
 
227
  return re.sub(r"(\d{3,5})\s+(?:tests|passed)\b", _sub, text)
228
 
 
214
  def _replace_test_count(text: str, count: int) -> str:
215
  """Remplace les mentions ``N tests`` ou ``N passed`` qui citent un
216
  nombre dans la fenêtre [count*0.5, count*2]. Garde la formulation
217
+ exacte (espace, ponctuation) intacte.
218
+
219
+ Le count est **arrondi à la dizaine** pour rendre le résultat
220
+ OS-déterministe : sur Windows certains tests POSIX-only sont
221
+ skipés (cf. ``pytest.importorskip``) ce qui décale le compteur
222
+ de quelques unités. L'arrondi absorbe ces écarts mineurs sans
223
+ masquer une vraie évolution (le seuil de tolérance des tests
224
+ consistency reste à ±5 %).
225
+ """
226
+ rounded_count = round(count, -1) # -1 = arrondi à la dizaine
227
+
228
  def _sub(match: re.Match) -> str:
229
  cited = int(match.group(1))
230
  # Ne touche pas si le nombre cité est complètement hors plage —
 
232
  # phrase qui parle d'autre chose).
233
  if cited < count * 0.5 or cited > count * 2:
234
  return match.group(0)
235
+ return match.group(0).replace(str(cited), str(rounded_count))
236
 
237
  return re.sub(r"(\d{3,5})\s+(?:tests|passed)\b", _sub, text)
238
 
tests/pipeline/test_sprint_a14_s8_timeout.py CHANGED
@@ -124,7 +124,20 @@ def test_timeout_measured_from_real_start_not_submission() -> None:
124
  def test_some_docs_succeed_others_timeout() -> None:
125
  """Mix : la moitié des docs sont rapides, l'autre lente. Avec
126
  un timeout intermédiaire, les rapides réussissent et les lents
127
- timeout."""
 
 
 
 
 
 
 
 
 
 
 
 
 
128
 
129
  class _ConditionalSlow:
130
  name = "cond"
@@ -135,9 +148,9 @@ def test_some_docs_succeed_others_timeout() -> None:
135
  def execute(self, inputs, params, context):
136
  # Les docs avec id pair sont rapides.
137
  if int(context.document_id.removeprefix("d")) % 2 == 0:
138
- time.sleep(0.01)
139
  else:
140
- time.sleep(0.5)
141
  return {
142
  ArtifactType.RAW_TEXT: Artifact(
143
  id=f"{context.document_id}:raw_text",
@@ -147,10 +160,14 @@ def test_some_docs_succeed_others_timeout() -> None:
147
  }
148
 
149
  adapter = _ConditionalSlow()
150
- runner, spec = _build(adapter, timeout=0.1, max_in_flight=2)
151
  inputs, ctx = _factories()
152
  docs = [DocumentRef(id=f"d{i}") for i in range(6)]
153
 
154
  result = runner.run(spec, docs, inputs, ctx)
155
- assert result.n_succeeded == 3 # pairs : d0, d2, d4
156
- assert result.n_timed_out == 3 # impairs : d1, d3, d5
 
 
 
 
 
124
  def test_some_docs_succeed_others_timeout() -> None:
125
  """Mix : la moitié des docs sont rapides, l'autre lente. Avec
126
  un timeout intermédiaire, les rapides réussissent et les lents
127
+ timeout.
128
+
129
+ Marges de robustesse cross-OS
130
+ ------------------------------
131
+ - Timeout : **0.5s**.
132
+ - Docs pairs dorment **0.05s** (10× sous le timeout) — ne ratent
133
+ pas même sur runners macOS lents avec scheduler imprécis.
134
+ - Docs impairs dorment **2.0s** (4× au-dessus) — timeout
135
+ garanti.
136
+
137
+ L'ancienne version utilisait timeout=0.1s / sleep pair=0.01s
138
+ qui était à 10 ms du timeout — le jitter du scheduler macOS sur
139
+ runners GitHub Actions le faisait basculer aléatoirement.
140
+ """
141
 
142
  class _ConditionalSlow:
143
  name = "cond"
 
148
  def execute(self, inputs, params, context):
149
  # Les docs avec id pair sont rapides.
150
  if int(context.document_id.removeprefix("d")) % 2 == 0:
151
+ time.sleep(0.05) # 10× sous le timeout (0.5s)
152
  else:
153
+ time.sleep(2.0) # 4× au-dessus du timeout
154
  return {
155
  ArtifactType.RAW_TEXT: Artifact(
156
  id=f"{context.document_id}:raw_text",
 
160
  }
161
 
162
  adapter = _ConditionalSlow()
163
+ runner, spec = _build(adapter, timeout=0.5, max_in_flight=2)
164
  inputs, ctx = _factories()
165
  docs = [DocumentRef(id=f"d{i}") for i in range(6)]
166
 
167
  result = runner.run(spec, docs, inputs, ctx)
168
+ assert result.n_succeeded == 3, (
169
+ f"pairs (d0/d2/d4) auraient réussir, "
170
+ f"obtenu n_succeeded={result.n_succeeded}, "
171
+ f"n_timed_out={result.n_timed_out}"
172
+ )
173
+ assert result.n_timed_out == 3