fix: admin/api apontava para admin/code (inexistente) — crash no app
Ao dividir _shared.py em admin/api/*.py ontem, o cálculo `Path(__file__).resolve().parent.parent / "code"` foi copiado sem ajustar para o nível de diretório novo. No arquivo original (admin/models_api.py, direto em admin/) dois `.parent` chegavam na raiz do repo. Em admin/api/shared.py, um nível mais fundo, dois `.parent` param em admin/ — e admin/code nunca existiu. sys.path nunca recebia code/, então toda ação que passa por `server` (analisar voz, aplicar decisões) crashava o app com ModuleNotFoundError: server_tools. O bug sobreviveu a duas rodadas de validação da sessão anterior — lint zero, 1454 testes verdes, comando testado manualmente pela ponte — porque todos rodam num venv com install editável (__editable__.fcp_mcp_server.pth) que já deixa fcpxml/server_tools importáveis por conta própria, mascarando qualquer erro no cálculo manual de sys.path. Só o app real, no fallback sem uv, expõe o bug. Correção: o cálculo de sys.path sai de cada módulo de comando (estava duplicado em nove arquivos) e passa a existir uma única vez em admin/api/__init__.py, que roda antes de qualquer submódulo — nenhum precisa mais da própria cópia. O teste de regressão precisou de duas tentativas pelo mesmo motivo do bug: a primeira versão também passava com o bug presente, por rodar no mesmo venv "de sorte". Só ficou confiável isolando um subprocess que remove site-packages do sys.path antes de importar — confirmado nos dois sentidos, falha com o bug reintroduzido e passa com a correção (TestCodeDirResolution). Detalhe completo, incluindo por que o comando manual não pegou: Engine/docs/05_EXPERIENCIAS.md #25. Lint zerado, 1457 testes passando (3 novos), app compilado. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
cbd9297751
commit
711c397dfe
@@ -10,6 +10,7 @@ patch keeps capturing the output of all of them.
|
||||
"""
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
@@ -23,6 +24,87 @@ if str(_REPO_ROOT) not in sys.path:
|
||||
|
||||
from admin.api import models, shared, subtitles, transcription # noqa: E402
|
||||
|
||||
_MODELS_API = _REPO_ROOT / "admin" / "models_api.py"
|
||||
|
||||
|
||||
class TestCodeDirResolution:
|
||||
"""Pins the sys.path computation itself, independent of the environment.
|
||||
|
||||
`code/` has an editable install (`__editable__.fcp_mcp_server*.pth`) that
|
||||
makes `fcpxml`/`server_tools` importable regardless of this calculation —
|
||||
which is exactly why TestSubprocessEntryPoint below can't be trusted alone
|
||||
to catch a regression here: it passes in an environment with the editable
|
||||
install even when the path math is wrong. The app's real subprocess call
|
||||
can fall back to a plain `python3` with no such install (see
|
||||
PythonBridge.swift), where this calculation is the only thing that puts
|
||||
`code/` on sys.path — so pin the math directly, not just its usual
|
||||
workaround.
|
||||
"""
|
||||
|
||||
def test_importing_the_package_puts_a_real_code_dir_on_sys_path(self):
|
||||
"""Checks the observable effect (what `admin.api`'s own `__init__.py`
|
||||
puts on sys.path), with the editable install that normally masks this
|
||||
stripped out of `sys.path` first — otherwise `server_tools` imports
|
||||
fine regardless of whether the path math is right, and the assertion
|
||||
proves nothing. A fresh subprocess is required too: importing
|
||||
`admin.api` in-process reuses whatever `sys.modules` already cached
|
||||
from an earlier test in this file, silently skipping `__init__.py`
|
||||
the second time around.
|
||||
"""
|
||||
script = (
|
||||
"import sys\n"
|
||||
"sys.path = [p for p in sys.path if 'site-packages' not in p]\n"
|
||||
"import admin.api\n"
|
||||
"import server_tools\n" # only resolvable if __init__.py did its job
|
||||
"print('OK')\n"
|
||||
)
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
capture_output=True, text=True, timeout=15, cwd=str(_REPO_ROOT),
|
||||
)
|
||||
assert result.returncode == 0 and "OK" in result.stdout, result.stderr
|
||||
|
||||
|
||||
class TestSubprocessEntryPoint:
|
||||
"""Runs models_api.py as the real subprocess the app launches.
|
||||
|
||||
Every other test in this file imports admin.api directly, which resolves
|
||||
`code/` onto sys.path a completely different way than a fresh Python
|
||||
process does. That gap let a real bug through: `admin/api/shared.py` sits
|
||||
one directory deeper than the old single-file `admin/models_api.py`, so
|
||||
its `Path(__file__).resolve().parent.parent / "code"` pointed at
|
||||
`admin/code` (nonexistent) instead of `code/` — `server_tools` (which
|
||||
lives in `code/`) was then unimportable the moment a handler tried
|
||||
`from server import ...`. Every in-process test still passed, because
|
||||
none of them launch a real subprocess with a real `__file__`.
|
||||
"""
|
||||
|
||||
def _run(self, command: str, args: dict | None = None) -> dict:
|
||||
argv = [sys.executable, str(_MODELS_API), command]
|
||||
if args is not None:
|
||||
argv.append(json.dumps(args))
|
||||
result = subprocess.run(
|
||||
argv, capture_output=True, text=True, timeout=30, cwd=str(_REPO_ROOT / "code")
|
||||
)
|
||||
assert result.returncode in (0, 1), (
|
||||
f"crashed (exit {result.returncode}):\n{result.stderr}"
|
||||
)
|
||||
return json.loads(result.stdout.strip().splitlines()[-1])
|
||||
|
||||
def test_simple_command_runs_as_a_real_subprocess(self):
|
||||
assert self._run("acoustics_capability")["ok"] is True
|
||||
|
||||
def test_command_that_imports_server_tools_runs_as_a_real_subprocess(self):
|
||||
"""The exact failure mode: a handler reaching into `server` (and
|
||||
transitively `server_tools`) from a process where `code/` was never
|
||||
actually on sys.path."""
|
||||
result = self._run("analyze_voice", {"path": "/nonexistent/project.fcpxml"})
|
||||
# The path is bogus on purpose — the point is that it FAILS on a
|
||||
# missing project, not on `ModuleNotFoundError: server_tools`.
|
||||
assert result["ok"] is False
|
||||
assert "server_tools" not in result.get("error", "")
|
||||
assert "ModuleNotFoundError" not in result.get("error", "")
|
||||
|
||||
|
||||
def _capture(monkeypatch):
|
||||
captured: list[dict] = []
|
||||
|
||||
Reference in New Issue
Block a user