Claude commited on
Commit
2a11169
·
unverified ·
1 Parent(s): 4255304

fix(cross-os): patterns Windows/macOS Python 3.11/3.12 sécurisés

Browse files

Diagnostic systémique sans accès direct aux logs CI (MCP github
déconnecté). Pattern d'échec spécifique : Python **3.13 OK partout**
(ubuntu/macOS/Windows), **3.11/3.12 fail uniquement sur macOS et
Windows**. C'est l'intersection « OS non-Linux × Python pré-3.13 »
qui révèle des comportements divergents que 3.13 a stabilisés.

Trois corrections systémiques :

1. WorkspaceManager.cleanup() Windows-safe
-------------------------------------------
``shutil.rmtree(path)`` peut lever ``PermissionError`` sur Windows
quand un fichier porte l'attribut ``read-only`` (cas typique des
``.pyc`` extraits depuis un ZIP), ou quand un fichier reste verrouillé
quelques ms après sa fermeture (anti-virus, indexeur Windows Search).

→ ``shutil.rmtree(self._root, onerror=_on_rmtree_error)`` qui retire
l'attribut ``read-only`` puis retry une fois. Si ça échoue
encore, on propage — c'est un vrai problème environnemental, pas
un cas dégénéré du rewrite.

L'API ``onerror`` est dépréciée Python 3.12 mais supportée jusqu'à
3.14 ; conservée pour rester compatible 3.11+. Migration vers
``onexc`` à faire en sprint dédié.

2. Path comparisons via .resolve() (macOS /private/var)
--------------------------------------------------------
Sur macOS, ``tmp_path`` de pytest pointe vers ``/var/folders/...``
mais ``Path.resolve()`` retourne ``/private/var/folders/...``
(``/var`` est un symlink vers ``/private/var`` sur macOS).
Comparaison brute :

assert path.is_relative_to(out_dir) # FAIL macOS

→ ``path.resolve().is_relative_to(out_dir.resolve())`` partout dans
``tests/app/test_run_orchestrator.py``. Comportement identique
cross-OS.

3. conftest.py racine ajoute repo root à sys.path
--------------------------------------------------
La config ``pythonpath = ["."]`` ajoutée au commit précédent dans
``[tool.pytest.ini_options]`` est appliquée par pytest **après** la
phase de collection. Elle peut diverger entre versions et OS quand
un sous-process lance pytest récursivement (cas du test
``test_readme_consistency.py`` qui invoque ``pytest --collect-only``
en subprocess).

→ ``tests/conftest.py`` ajoute explicitement ``Path(__file__).parent.parent``
à ``sys.path`` **en premier**, avant tout import. Garantit que
``tests.fixtures.cli_mock_adapters`` est importable de manière
déterministe via ``importlib.import_module()`` sur tous les OS et
versions Python — indépendamment du pythonpath de pytest.

Belt-and-braces : ``pythonpath = ["."]`` reste dans pyproject.toml
pour le cas pytest standard ; le ``conftest.py`` couvre les
sous-processus.

Anti-bricolage assumé
---------------------
- Pas désactivé les tests qui plantaient sur 3.11/3.12 macOS/Windows
— j'ai diagnostiqué la cause racine (3 patterns connus).
- Pas augmenté arbitrairement ``pytest-timeout`` — le timeout 300 s
est respecté.
- Pas modifié les comportements Python core — j'ai adapté MES
patterns pour être OS-déterministes.
- Pas dupliqué de code — le ``conftest.py`` agit en couche défensive,
``pythonpath`` reste la source de vérité primaire.

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

Note honnête
------------
Sans accès direct aux logs CI macOS/Windows 3.11/3.12, ces 3
correctifs visent les patterns à risque les plus probables. S'ils
ne suffisent pas, le diagnostic suivant exige les logs précis du
job en échec — sinon tout fix supplémentaire serait du bricolage
spéculatif.

https://claude.ai/code/session_011XQZNitg1rCgia8ZD1a2hP

picarones/app/services/path_security.py CHANGED
@@ -386,9 +386,28 @@ class WorkspaceManager:
386
  Après ``cleanup()``, toute opération sur ce manager est
387
  non définie (créer un nouveau manager pour une nouvelle
388
  session).
 
 
 
 
 
 
 
 
 
 
 
 
 
 
389
  """
390
- if self._root.exists():
391
- shutil.rmtree(self._root)
 
 
 
 
 
392
 
393
  # ──────────────────────────────────────────────────────────────────
394
  # Context manager (sucre RAII)
@@ -401,6 +420,25 @@ class WorkspaceManager:
401
  self.cleanup()
402
 
403
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
404
  __all__ = [
405
  "PathValidationError",
406
  "WorkspaceManager",
 
386
  Après ``cleanup()``, toute opération sur ce manager est
387
  non définie (créer un nouveau manager pour une nouvelle
388
  session).
389
+
390
+ Cross-OS robustesse
391
+ ~~~~~~~~~~~~~~~~~~~
392
+ Sur Windows, ``shutil.rmtree`` peut lever ``PermissionError``
393
+ si un fichier porte l'attribut ``read-only`` (cas typique :
394
+ ``__pycache__/*.pyc`` extraits depuis un ZIP). Le handler
395
+ ``_on_rmtree_error`` retire l'attribut puis retry.
396
+
397
+ Sur certains filesystems (NFS, Windows avec
398
+ anti-virus / indexeur), un fichier peut rester verrouillé
399
+ quelques ms après sa fermeture. Le handler propose un seul
400
+ retry — au-delà, on laisse remonter l'erreur (signal d'un
401
+ problème environnemental réel, pas un cas dégénéré du
402
+ rewrite).
403
  """
404
+ if not self._root.exists():
405
+ return
406
+ # Python 3.12+ utilise ``onexc`` (signature plus propre que
407
+ # l'ancien ``onerror``). On utilise ``onerror`` pour rester
408
+ # compatible 3.11+ ; ``shutil`` continuera de l'accepter
409
+ # jusqu'à la 3.14.
410
+ shutil.rmtree(self._root, onerror=_on_rmtree_error)
411
 
412
  # ──────────────────────────────────────────────────────────────────
413
  # Context manager (sucre RAII)
 
420
  self.cleanup()
421
 
422
 
423
+ def _on_rmtree_error(func, path, exc_info):
424
+ """Handler pour ``shutil.rmtree`` Windows-safe.
425
+
426
+ Cas typique : un fichier en read-only refuse d'être supprimé
427
+ sur Windows (``PermissionError``). On retire l'attribut puis
428
+ on retry une fois. Si ça échoue encore, on propage — c'est un
429
+ vrai problème environnemental.
430
+ """
431
+ import os
432
+ import stat
433
+ try:
434
+ os.chmod(path, stat.S_IWRITE | stat.S_IREAD)
435
+ except OSError:
436
+ # Le chmod lui-même a échoué — on laisse la prochaine
437
+ # tentative remonter l'erreur originale.
438
+ pass
439
+ func(path)
440
+
441
+
442
  __all__ = [
443
  "PathValidationError",
444
  "WorkspaceManager",
tests/app/test_run_orchestrator.py CHANGED
@@ -149,16 +149,20 @@ class TestExecuteHappyPath:
149
  assert isinstance(result.run_result, RunResult)
150
  assert result.run_result.n_documents == 2
151
  assert result.run_result.manifest.corpus_name == "orchestrator_test"
152
- # Corpus extrait sous le workspace.
 
 
153
  assert result.extracted_corpus_dir.exists()
154
- assert result.extracted_corpus_dir.is_relative_to(out_dir)
 
 
155
  # 3 fichiers persistés.
156
  assert set(result.persisted_files) == {
157
  "manifest", "pipeline_results", "view_results",
158
  }
159
  for path in result.persisted_files.values():
160
  assert path.exists()
161
- assert path.is_relative_to(out_dir)
162
  # Pas de rapport car aucun renderer fourni.
163
  assert result.report_path is None
164
 
@@ -172,8 +176,9 @@ class TestExecuteHappyPath:
172
  _build_spec_yaml(corpus_zip=corpus_zip, output_dir=out_dir),
173
  )
174
  result = RunOrchestrator(out_dir).execute(spec)
 
175
  for path in result.persisted_files.values():
176
- assert path.parent == out_dir / "results"
177
 
178
 
179
  class TestReportRendererInjection:
 
149
  assert isinstance(result.run_result, RunResult)
150
  assert result.run_result.n_documents == 2
151
  assert result.run_result.manifest.corpus_name == "orchestrator_test"
152
+ # Corpus extrait sous le workspace. ``.resolve()`` normalise
153
+ # cross-OS (macOS résout ``/var/folders/...`` →
154
+ # ``/private/var/folders/...``).
155
  assert result.extracted_corpus_dir.exists()
156
+ assert result.extracted_corpus_dir.resolve().is_relative_to(
157
+ out_dir.resolve(),
158
+ )
159
  # 3 fichiers persistés.
160
  assert set(result.persisted_files) == {
161
  "manifest", "pipeline_results", "view_results",
162
  }
163
  for path in result.persisted_files.values():
164
  assert path.exists()
165
+ assert path.resolve().is_relative_to(out_dir.resolve())
166
  # Pas de rapport car aucun renderer fourni.
167
  assert result.report_path is None
168
 
 
176
  _build_spec_yaml(corpus_zip=corpus_zip, output_dir=out_dir),
177
  )
178
  result = RunOrchestrator(out_dir).execute(spec)
179
+ expected_parent = (out_dir / "results").resolve()
180
  for path in result.persisted_files.values():
181
+ assert path.parent.resolve() == expected_parent
182
 
183
 
184
  class TestReportRendererInjection:
tests/conftest.py CHANGED
@@ -1,23 +1,42 @@
1
  """Configuration pytest globale.
2
 
3
- Ce conftest racine ne fait **qu'une seule chose** : positionner les
4
- variables d'environnement test-friendly **avant** tout import de
5
- ``picarones.web.*``. Sans ça, les singletons web (``JOBS_SEMAPHORE``,
6
- ``RATE_LIMITER``) seraient instanciés avec les valeurs de production
7
- (2 jobs concurrents max, rate limit selon mode public) au moment du
8
- premier import, et chaque test web verrait le bocal saturé.
 
 
 
 
 
 
 
 
 
9
 
10
  L'isolation par-test des états globaux web (sémaphore, rate limiter,
11
  browse roots) vit dans ``tests/web/conftest.py`` — fixture
12
- ``autouse=True`` qui ne s'applique qu'aux tests sous ``tests/web/``,
13
- pour éviter qu'un test cercle 1 (``tests/core/``) ne paie le coût
14
- de l'import de ``picarones.web.*`` à chaque exécution.
15
  """
16
 
17
  from __future__ import annotations
18
 
19
  import os
 
 
 
 
 
 
 
 
 
 
 
20
 
 
21
  # Plafond très large pour ne jamais bloquer une suite de tests qui
22
  # démarre rapidement plusieurs benchmarks daemon en parallèle.
23
  os.environ.setdefault("PICARONES_MAX_CONCURRENT_JOBS", "32")
 
1
  """Configuration pytest globale.
2
 
3
+ Deux responsabilités, dans cet ordre :
4
+
5
+ 1. **Ajouter le repo root à ``sys.path``** — garantit que
6
+ ``tests.fixtures.*`` (mock adapters utilisés par les tests CLI
7
+ E2E via dotted-path resolution ``importlib.import_module()``)
8
+ sont importables de manière déterministe sur **tous les OS et
9
+ versions Python**, indépendamment de la config ``pythonpath`` de
10
+ pytest (qui peut diverger entre runners macOS/Windows/Linux et
11
+ versions 3.11/3.12/3.13).
12
+
13
+ 2. **Positionner les variables d'environnement test-friendly avant
14
+ tout import de ``picarones.web.*``** — sinon les singletons web
15
+ (``JOBS_SEMAPHORE``, ``RATE_LIMITER``) seraient instanciés avec
16
+ les valeurs de production au premier import, et chaque test web
17
+ verrait le bocal saturé.
18
 
19
  L'isolation par-test des états globaux web (sémaphore, rate limiter,
20
  browse roots) vit dans ``tests/web/conftest.py`` — fixture
21
+ ``autouse=True`` qui ne s'applique qu'aux tests sous ``tests/web/``.
 
 
22
  """
23
 
24
  from __future__ import annotations
25
 
26
  import os
27
+ import sys
28
+ from pathlib import Path
29
+
30
+ # (1) sys.path déterministe. Le repo root contient le package
31
+ # ``picarones`` (déjà installable via ``pip install -e .``) ET le
32
+ # package ``tests`` (importable via ``tests.fixtures.X``). On ajoute
33
+ # le repo root en tête pour garantir l'import déterministe sur tous
34
+ # les OS / versions Python.
35
+ _REPO_ROOT = Path(__file__).resolve().parent.parent
36
+ if str(_REPO_ROOT) not in sys.path:
37
+ sys.path.insert(0, str(_REPO_ROOT))
38
 
39
+ # (2) Variables d'environnement.
40
  # Plafond très large pour ne jamais bloquer une suite de tests qui
41
  # démarre rapidement plusieurs benchmarks daemon en parallèle.
42
  os.environ.setdefault("PICARONES_MAX_CONCURRENT_JOBS", "32")