busya
4726fd110d
Create AsyncJobRunner — centralized bridge between APScheduler (BackgroundScheduler, sync thread pool) and async coroutines. Fixes: - P0: execute_run() called without await from APScheduler thread, causing coroutine to be silently discarded (root cause: no translation history) - P0: get_async_job_runner() deadlock when called from APScheduler thread pool without running event loop - P1: ID mismatch in disable_schedule/delete_schedule routes (job_id passed instead of schedule_id) - P1: asyncio.run() in APScheduler callbacks incompatible with running event loop - Delete unused llm_analysis/scheduler.py (not used in production) Changes: core/async_job_runner.py — new: AsyncJobRunner class core/scheduler.py — use runner.run()/run_later() translate/scheduler.py — use runner.run() for execute_run mapping_service.py — remove unused BackgroundScheduler dependencies.py — add get_async_job_runner() DI app.py — init runner in lifespan api/routes/migration.py — use runner.run() _schedule_routes.py — fix ID mismatch plugins/migration.py — use runner.run() llm_analysis/scheduler.py — delete (unused) tests: 151 new/updated tests, all passing
321 lines
14 KiB
Python
321 lines
14 KiB
Python
# #region TestTranslateSchedulerAudit [C:3] [TYPE Module] [SEMANTICS test,translate,scheduler,audit,async-sync-mismatch,id-mismatch]
|
|
# @BRIEF Orthogonal audit of scheduled translation execution — async/sync mismatch and ID mismatch bugs.
|
|
# @RELATION BINDS_TO -> [execute_scheduled_translation]
|
|
# @RELATION BINDS_TO -> [TranslationScheduler]
|
|
# @RELATION BINDS_TO -> [SchedulerService]
|
|
# @RELATION BINDS_TO -> [AsyncJobRunner]
|
|
# @TEST_EDGE: async_coroutine_not_awaited — execute_run returns coroutine but never awaited
|
|
# @TEST_EDGE: disable_schedule_wrong_id — disable_schedule passes job_id instead of schedule_id
|
|
# @TEST_EDGE: delete_schedule_wrong_id — delete_schedule passes job_id instead of schedule_id
|
|
# @TEST_INVARIANT: execute_run_must_be_awaited -> VERIFIED_BY: [test_scheduler_uses_runner_for_execute_run]
|
|
# @TEST_INVARIANT: remove_job_uses_schedule_id -> VERIFIED_BY: [test_disable_schedule_uses_schedule_id, test_delete_schedule_uses_schedule_id]
|
|
import asyncio
|
|
import inspect
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch, AsyncMock
|
|
|
|
import pytest
|
|
|
|
sys_path = str(Path(__file__).parent.parent / "src")
|
|
import sys
|
|
if sys_path not in sys.path:
|
|
sys.path.insert(0, sys_path)
|
|
|
|
|
|
# =============================================================================
|
|
# BUG #1 FIX: async/sync mismatch — now uses AsyncJobRunner
|
|
# =============================================================================
|
|
|
|
class TestAsyncSyncMismatch:
|
|
"""Verify that execute_run is async and uses AsyncJobRunner."""
|
|
|
|
# #region test_execute_run_is_async_coroutine [C:2] [TYPE Function]
|
|
# @BRIEF Confirm execute_run is an async method that returns a coroutine.
|
|
def test_execute_run_is_async_coroutine(self):
|
|
"""execute_run is defined as async def — calling it without await returns a coroutine object."""
|
|
from src.plugins.translate.orchestrator import TranslationOrchestrator
|
|
|
|
# Verify the method is actually async
|
|
assert inspect.iscoroutinefunction(TranslationOrchestrator.execute_run), (
|
|
"TranslationOrchestrator.execute_run must be an async method"
|
|
)
|
|
# #endregion test_execute_run_is_async_coroutine
|
|
|
|
# #region test_scheduler_uses_runner_for_execute_run [C:2] [TYPE Function]
|
|
# @BRIEF The scheduler uses AsyncJobRunner.run() to execute translations (FIXED).
|
|
def test_scheduler_uses_runner_for_execute_run(self):
|
|
"""In execute_scheduled_translation, orch.execute_run(run) is called via runner.run()."""
|
|
import ast
|
|
import inspect
|
|
|
|
# Read the source code of execute_scheduled_translation
|
|
from src.plugins.translate.scheduler import execute_scheduled_translation
|
|
source = inspect.getsource(execute_scheduled_translation)
|
|
|
|
# Parse the AST to find the call to execute_run
|
|
tree = ast.parse(source)
|
|
|
|
# Find all Call nodes that call execute_run
|
|
execute_run_calls = []
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Call):
|
|
if isinstance(node.func, ast.Attribute) and node.func.attr == 'execute_run':
|
|
execute_run_calls.append(node)
|
|
|
|
assert len(execute_run_calls) > 0, "No calls to execute_run found in execute_scheduled_translation"
|
|
|
|
# Check if any call is wrapped in runner.run()
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Call):
|
|
if isinstance(node.func, ast.Attribute) and node.func.attr == 'run':
|
|
# Check if the argument is an execute_run call
|
|
for arg in node.args:
|
|
if isinstance(arg, ast.Call):
|
|
if isinstance(arg.func, ast.Attribute) and arg.func.attr == 'execute_run':
|
|
# Found runner.run(orch.execute_run(run)) — this is correct
|
|
return
|
|
|
|
pytest.fail(
|
|
"FIX NOT APPLIED: execute_run should be called via runner.run() in execute_scheduled_translation. "
|
|
"Expected: runner.run(orch.execute_run(run))"
|
|
)
|
|
# #endregion test_scheduler_uses_runner_for_execute_run
|
|
|
|
# #region test_manual_trigger_uses_asyncio_create_task [C:2] [TYPE Function]
|
|
# @BRIEF Manual trigger correctly uses asyncio.create_task for async execution.
|
|
def test_manual_trigger_uses_asyncio_create_task(self):
|
|
"""The manual trigger route uses asyncio.create_task() which correctly handles async execution."""
|
|
import ast
|
|
import inspect
|
|
from src.api.routes.translate._run_routes import run_translation
|
|
|
|
source = inspect.getsource(run_translation)
|
|
tree = ast.parse(source)
|
|
|
|
# Look for asyncio.create_task calls
|
|
has_create_task = False
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Call):
|
|
if isinstance(node.func, ast.Attribute):
|
|
if node.func.attr == 'create_task':
|
|
has_create_task = True
|
|
elif isinstance(node.func, ast.Name):
|
|
if node.func.id == 'create_task':
|
|
has_create_task = True
|
|
|
|
assert has_create_task, (
|
|
"Manual trigger should use asyncio.create_task() for async execution"
|
|
)
|
|
# #endregion test_manual_trigger_uses_asyncio_create_task
|
|
|
|
|
|
# =============================================================================
|
|
# BUG #2: ID mismatch — disable_schedule and delete_schedule pass wrong ID
|
|
# =============================================================================
|
|
|
|
class TestScheduleIdMismatch:
|
|
"""Verify that disable_schedule and delete_schedule use correct IDs."""
|
|
|
|
# #region test_add_translation_job_uses_schedule_id [C:2] [TYPE Function]
|
|
# @BRIEF add_translation_job constructs APScheduler job ID from schedule_id.
|
|
def test_add_translation_job_uses_schedule_id(self):
|
|
"""add_translation_job creates job ID as 'translate_{schedule_id}'."""
|
|
from src.core.scheduler import SchedulerService
|
|
|
|
mock_task_manager = MagicMock()
|
|
mock_config = MagicMock()
|
|
|
|
svc = SchedulerService(mock_task_manager, mock_config)
|
|
svc.scheduler = MagicMock()
|
|
|
|
schedule_id = "sched-uuid-123"
|
|
job_id = "job-uuid-456"
|
|
|
|
svc.add_translation_job(
|
|
schedule_id=schedule_id,
|
|
job_id=job_id,
|
|
cron_expression="*/30 * * * *",
|
|
timezone="UTC",
|
|
execution_mode="full"
|
|
)
|
|
|
|
# Verify the APScheduler job ID is based on schedule_id
|
|
call_args = svc.scheduler.add_job.call_args
|
|
aps_job_id = call_args.kwargs.get('id') or call_args.args[1] if len(call_args.args) > 1 else None
|
|
|
|
assert aps_job_id == f"translate_{schedule_id}", (
|
|
f"APScheduler job ID should be 'translate_{schedule_id}', got '{aps_job_id}'"
|
|
)
|
|
# #endregion test_add_translation_job_uses_schedule_id
|
|
|
|
# #region test_remove_translation_job_uses_schedule_id [C:2] [TYPE Function]
|
|
# @BRIEF remove_translation_job constructs APScheduler job ID from schedule_id.
|
|
def test_remove_translation_job_uses_schedule_id(self):
|
|
"""remove_translation_job removes job with ID 'translate_{schedule_id}'."""
|
|
from src.core.scheduler import SchedulerService
|
|
|
|
mock_task_manager = MagicMock()
|
|
mock_config = MagicMock()
|
|
|
|
svc = SchedulerService(mock_task_manager, mock_config)
|
|
svc.scheduler = MagicMock()
|
|
|
|
schedule_id = "sched-uuid-123"
|
|
svc.remove_translation_job(schedule_id=schedule_id)
|
|
|
|
# Verify it tries to remove the correct job ID
|
|
svc.scheduler.remove_job.assert_called_once_with(f"translate_{schedule_id}")
|
|
# #endregion test_remove_translation_job_uses_schedule_id
|
|
|
|
# #region test_disable_schedule_uses_schedule_id [C:2] [TYPE Function]
|
|
# @BRIEF disable_schedule uses schedule.id (not job_id) for remove_translation_job (FIXED).
|
|
def test_disable_schedule_uses_schedule_id(self):
|
|
"""FIXED: disable_schedule route uses schedule.id for remove_translation_job."""
|
|
import ast
|
|
import inspect
|
|
from src.api.routes.translate._schedule_routes import disable_schedule
|
|
|
|
source = inspect.getsource(disable_schedule)
|
|
tree = ast.parse(source)
|
|
|
|
# Find calls to remove_translation_job
|
|
remove_calls = []
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Call):
|
|
if isinstance(node.func, ast.Attribute) and node.func.attr == 'remove_translation_job':
|
|
remove_calls.append(node)
|
|
|
|
assert len(remove_calls) > 0, "No calls to remove_translation_job found"
|
|
|
|
# Check what argument is passed — should NOT be job_id directly
|
|
for call in remove_calls:
|
|
for kw in call.keywords:
|
|
if kw.arg == 'schedule_id':
|
|
# Should NOT be job_id (the parameter name)
|
|
if isinstance(kw.value, ast.Name) and kw.value.id == 'job_id':
|
|
pytest.fail(
|
|
"BUG STILL EXISTS: disable_schedule passes job_id to remove_translation_job. "
|
|
"Expected: schedule.id from set_schedule_active() return value."
|
|
)
|
|
# #endregion test_disable_schedule_uses_schedule_id
|
|
|
|
# #region test_delete_schedule_uses_schedule_id [C:2] [TYPE Function]
|
|
# @BRIEF delete_schedule uses schedule.id (not job_id) for remove_translation_job (FIXED).
|
|
def test_delete_schedule_uses_schedule_id(self):
|
|
"""FIXED: delete_schedule route uses schedule.id for remove_translation_job."""
|
|
import ast
|
|
import inspect
|
|
from src.api.routes.translate._schedule_routes import delete_schedule
|
|
|
|
source = inspect.getsource(delete_schedule)
|
|
tree = ast.parse(source)
|
|
|
|
# Find calls to remove_translation_job
|
|
remove_calls = []
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Call):
|
|
if isinstance(node.func, ast.Attribute) and node.func.attr == 'remove_translation_job':
|
|
remove_calls.append(node)
|
|
|
|
assert len(remove_calls) > 0, "No calls to remove_translation_job found"
|
|
|
|
# Check what argument is passed — should NOT be job_id directly
|
|
for call in remove_calls:
|
|
for kw in call.keywords:
|
|
if kw.arg == 'schedule_id':
|
|
# Should NOT be job_id (the parameter name)
|
|
if isinstance(kw.value, ast.Name) and kw.value.id == 'job_id':
|
|
pytest.fail(
|
|
"BUG STILL EXISTS: delete_schedule passes job_id to remove_translation_job. "
|
|
"Expected: schedule_id from get_schedule() return value."
|
|
)
|
|
# #endregion test_delete_schedule_uses_schedule_id
|
|
|
|
# #region test_schedule_id_differs_from_job_id [C:2] [TYPE Function]
|
|
# @BRIEF TranslationSchedule.id and TranslationSchedule.job_id are different UUIDs.
|
|
def test_schedule_id_differs_from_job_id(self):
|
|
"""TranslationSchedule has separate id and job_id fields — they are different UUIDs."""
|
|
from src.models.translate import TranslationSchedule
|
|
|
|
# Verify the model has both fields
|
|
assert hasattr(TranslationSchedule, 'id'), "TranslationSchedule should have 'id' field"
|
|
assert hasattr(TranslationSchedule, 'job_id'), "TranslationSchedule should have 'job_id' field"
|
|
|
|
# The id is the primary key, job_id is a foreign key to TranslationJob
|
|
# They are different UUIDs
|
|
import uuid
|
|
schedule_id = str(uuid.uuid4())
|
|
job_id = str(uuid.uuid4())
|
|
|
|
assert schedule_id != job_id, (
|
|
"schedule_id and job_id are different UUIDs — "
|
|
"using job_id where schedule_id is expected will cause ID mismatch"
|
|
)
|
|
# #endregion test_schedule_id_differs_from_job_id
|
|
|
|
|
|
# =============================================================================
|
|
# Integration verification: end-to-end scheduled execution flow
|
|
# =============================================================================
|
|
|
|
class TestScheduledExecutionFlow:
|
|
"""Verify the complete scheduled execution flow has the async/sync bug."""
|
|
|
|
# #region test_scheduled_execution_uses_runner [C:2] [TYPE Function]
|
|
# @BRIEF Full flow: scheduler triggers execute_run via AsyncJobRunner (FIXED).
|
|
def test_scheduled_execution_uses_runner(self):
|
|
"""
|
|
Simulate what happens when APScheduler triggers execute_scheduled_translation:
|
|
1. TranslationOrchestrator is created
|
|
2. start_run creates a TranslationRun
|
|
3. execute_run is called via runner.run()
|
|
4. The translation actually executes
|
|
"""
|
|
from src.plugins.translate.scheduler import execute_scheduled_translation
|
|
from src.plugins.translate.orchestrator import TranslationOrchestrator
|
|
|
|
mock_db = MagicMock()
|
|
mock_session_maker = MagicMock(return_value=mock_db)
|
|
mock_config = MagicMock()
|
|
|
|
# Setup schedule
|
|
schedule = MagicMock()
|
|
schedule.id = "sched-1"
|
|
schedule.job_id = "job-1"
|
|
schedule.is_active = True
|
|
schedule.execution_mode = "full"
|
|
|
|
q1 = MagicMock()
|
|
q1.filter.return_value.first.return_value = schedule
|
|
|
|
q2 = MagicMock()
|
|
q2.filter.return_value.order_by.return_value.first.return_value = None # no concurrent
|
|
|
|
q3 = MagicMock()
|
|
q3.filter.return_value.order_by.return_value.first.return_value = None # no recent run
|
|
|
|
mock_db.query.side_effect = [q1, q2, q3]
|
|
|
|
# Setup orchestrator mock
|
|
mock_run = MagicMock()
|
|
mock_run.id = "run-1"
|
|
mock_run.status = "PENDING"
|
|
|
|
# Mock the runner to track calls
|
|
mock_runner = MagicMock()
|
|
|
|
with patch.object(TranslationOrchestrator, 'execute_run', return_value=mock_run):
|
|
with patch.object(TranslationOrchestrator, 'start_run', return_value=mock_run):
|
|
with patch('src.dependencies.get_async_job_runner', return_value=mock_runner):
|
|
execute_scheduled_translation(
|
|
schedule_id="sched-1",
|
|
job_id="job-1",
|
|
db_session_maker=mock_session_maker,
|
|
config_manager=mock_config,
|
|
execution_mode="full",
|
|
)
|
|
|
|
# Verify that runner.run was called (the fix is applied)
|
|
mock_runner.run.assert_called_once()
|
|
# #endregion test_scheduled_execution_uses_runner
|