mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-01 19:06:01 +00:00
test(schedule): close the domain's coverage and documentation gaps
A review pass over domain/schedule found three genuinely untested branches, all of which now have cases: - ensure_launchable with a naive `now`. next_after already had this covered; the delay floor did not, so a caller handing over a naive clock reading could have had it shifted by the local offset unnoticed. - update_task changing the prompt. Only the title path was exercised. - _save finding the row gone. get_task saw it and save no longer does, which is a concurrent delete; the caller must get the same not-found it would have got a moment earlier rather than a None leaking out. That takes service.py and every model module to 100%. The one remaining uncovered line is croniter's naive-return guard, carried over verbatim from schedules.py and unreachable with an aware input -- it now says so instead of looking like an untested branch. Also fills in the documentation the migration skipped: TaskStatus, ContextMode and RunStatus arrived from the original draft without docstrings while their newer siblings had them, and ScheduleService plus four of its use cases were undocumented. Each now records the reasoning a reader would otherwise have to reconstruct -- why RUNNING is not "the agent is executing", why SKIPPED never passes through QUEUED, why INTERRUPTED is not FAILED. CRON_FIELD_COUNT stops being exported: it has no consumer outside the module that defines it.
This commit is contained in:
parent
d8527dd0f7
commit
2ded78fdad
@ -9,7 +9,6 @@ tests, not everyday call-site symbols.
|
||||
|
||||
from deerflow.domain.schedule.model import (
|
||||
ACTIVE_RUN_STATUSES,
|
||||
CRON_FIELD_COUNT,
|
||||
TERMINAL_RUN_STATUSES,
|
||||
TERMINAL_TASK_STATUSES,
|
||||
ActiveRunConflictError,
|
||||
@ -35,7 +34,6 @@ from deerflow.domain.schedule.model import (
|
||||
|
||||
__all__ = [
|
||||
"ACTIVE_RUN_STATUSES",
|
||||
"CRON_FIELD_COUNT",
|
||||
"TERMINAL_RUN_STATUSES",
|
||||
"TERMINAL_TASK_STATUSES",
|
||||
"ActiveRunConflictError",
|
||||
|
||||
@ -39,12 +39,11 @@ from deerflow.domain.schedule.model.errors import (
|
||||
ThreadNotFoundError,
|
||||
)
|
||||
from deerflow.domain.schedule.model.run import ACTIVE_RUN_STATUSES, TERMINAL_RUN_STATUSES, ScheduledRun
|
||||
from deerflow.domain.schedule.model.spec import CRON_FIELD_COUNT, SchedulePolicy, ScheduleSpec
|
||||
from deerflow.domain.schedule.model.spec import SchedulePolicy, ScheduleSpec
|
||||
from deerflow.domain.schedule.model.task import TERMINAL_TASK_STATUSES, ScheduledTask
|
||||
|
||||
__all__ = [
|
||||
"ACTIVE_RUN_STATUSES",
|
||||
"CRON_FIELD_COUNT",
|
||||
"TERMINAL_RUN_STATUSES",
|
||||
"TERMINAL_TASK_STATUSES",
|
||||
"ActiveRunConflictError",
|
||||
|
||||
@ -4,6 +4,16 @@ from enum import StrEnum
|
||||
|
||||
|
||||
class TaskStatus(StrEnum):
|
||||
"""Lifecycle of a standing instruction.
|
||||
|
||||
RUNNING does not mean "the agent is executing" -- it means this dispatch
|
||||
round's scheduling ownership is held. That is why `ensure_mutable` refuses
|
||||
edits in that state, and why a crash can strand a task there.
|
||||
|
||||
The last three are terminal for the *current* schedule, not forever: moving
|
||||
the schedule into the future re-arms them (see TERMINAL_TASK_STATUSES).
|
||||
"""
|
||||
|
||||
ENABLED = "enabled"
|
||||
PAUSED = "paused"
|
||||
RUNNING = "running"
|
||||
@ -13,16 +23,41 @@ class TaskStatus(StrEnum):
|
||||
|
||||
|
||||
class ContextMode(StrEnum):
|
||||
"""Which thread a dispatch executes in.
|
||||
|
||||
REUSE_THREAD lets consecutive executions see each other's history and
|
||||
requires a thread up front; the default gives each one a fresh thread and
|
||||
therefore no shared context.
|
||||
"""
|
||||
|
||||
FRESH_THREAD_PER_RUN = "fresh_thread_per_run"
|
||||
REUSE_THREAD = "reuse_thread"
|
||||
|
||||
|
||||
class ScheduleType(StrEnum):
|
||||
"""How the next fire time is derived.
|
||||
|
||||
The two branch almost every rule in this context: CRON always has a next
|
||||
occurrence, ONCE is consumed by its first one.
|
||||
"""
|
||||
|
||||
ONCE = "once"
|
||||
CRON = "cron"
|
||||
|
||||
|
||||
class RunStatus(StrEnum):
|
||||
"""Lifecycle of one execution record.
|
||||
|
||||
QUEUED and RUNNING are the two that occupy a task's single active slot;
|
||||
everything else is terminal.
|
||||
|
||||
SKIPPED is created directly terminal and never passes through QUEUED -- a
|
||||
queued tombstone would collide with the very run it is recording the
|
||||
overlap with (see ScheduledRun.skipped_tombstone). INTERRUPTED is kept
|
||||
distinct from FAILED because a cancel or takeover is not an execution
|
||||
failure, and the two lead a `once` task to different terminal states.
|
||||
"""
|
||||
|
||||
QUEUED = "queued"
|
||||
RUNNING = "running"
|
||||
SUCCESS = "success"
|
||||
|
||||
@ -118,6 +118,9 @@ class ScheduleSpec:
|
||||
zone = ZoneInfo(self.timezone)
|
||||
next_local = croniter(self.cron, now.astimezone(zone)).get_next(datetime)
|
||||
if next_local.tzinfo is None:
|
||||
# Unreachable with an aware input on today's croniter, and kept
|
||||
# verbatim from schedules.py:51-52 rather than dropped: it costs
|
||||
# one branch and guards a library detail we do not control.
|
||||
next_local = next_local.replace(tzinfo=zone)
|
||||
return next_local.astimezone(UTC)
|
||||
|
||||
|
||||
@ -80,6 +80,14 @@ class DispatchResult:
|
||||
|
||||
|
||||
class ScheduleService:
|
||||
"""Every scheduled-task use case, orchestrated over the four output ports.
|
||||
|
||||
The input port of this context: primary adapters (an HTTP router, the
|
||||
poller, the run-completion hook) call these methods and translate what
|
||||
comes back. Domain errors are allowed to propagate -- mapping them onto a
|
||||
protocol is the adapter's job, not this class's.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
@ -98,9 +106,15 @@ class ScheduleService:
|
||||
# ------------------------------------------------------------------ reads
|
||||
|
||||
async def list_tasks(self, user_id: str) -> list[ScheduledTask]:
|
||||
"""Every task the user owns, newest first."""
|
||||
return await self._tasks.list_by_user(user_id)
|
||||
|
||||
async def list_tasks_by_thread(self, user_id: str, thread_id: str) -> list[ScheduledTask]:
|
||||
"""The user's tasks bound to one thread.
|
||||
|
||||
Only reuse_thread tasks can appear: a fresh-thread task carries no
|
||||
binding to show.
|
||||
"""
|
||||
return await self._tasks.list_by_user_and_thread(user_id, thread_id)
|
||||
|
||||
async def get_task(self, task_id: str, *, user_id: str) -> ScheduledTask:
|
||||
@ -190,10 +204,15 @@ class ScheduleService:
|
||||
return await self._save(task)
|
||||
|
||||
async def pause_task(self, task_id: str, *, user_id: str) -> ScheduledTask:
|
||||
"""Stop claiming this task until it is resumed.
|
||||
|
||||
Refused while the task is being dispatched -- see `ensure_mutable`.
|
||||
"""
|
||||
task = await self.get_task(task_id, user_id=user_id)
|
||||
return await self._save(task.paused())
|
||||
|
||||
async def resume_task(self, task_id: str, *, user_id: str) -> ScheduledTask:
|
||||
"""Re-admit this task to claiming. Same gate as `pause_task`."""
|
||||
task = await self.get_task(task_id, user_id=user_id)
|
||||
return await self._save(task.resumed())
|
||||
|
||||
|
||||
@ -174,6 +174,12 @@ class TestEnsureLaunchable:
|
||||
spec = cron_spec("* * * * *", "UTC")
|
||||
assert spec.ensure_launchable(NOW, MIN_60) is not None
|
||||
|
||||
def test_naive_now_is_read_as_utc(self):
|
||||
"""Same tolerance as next_after: a caller handing over a naive clock
|
||||
reading must not silently shift the delay floor by the local offset."""
|
||||
spec = once_spec(after_seconds=3600)
|
||||
assert spec.ensure_launchable(NOW.replace(tzinfo=None), MIN_60) == spec.ensure_launchable(NOW, MIN_60)
|
||||
|
||||
def test_matches_next_after_for_a_valid_once_schedule(self):
|
||||
spec = once_spec(after_seconds=3600)
|
||||
assert spec.ensure_launchable(NOW, MIN_60) == spec.next_after(NOW)
|
||||
|
||||
@ -555,6 +555,33 @@ class TestTaskManagement:
|
||||
assert updated.prompt == task.prompt
|
||||
assert updated.schedule == task.schedule
|
||||
|
||||
async def test_update_can_change_the_prompt(self):
|
||||
service = make_service()
|
||||
task = await create_cron_task(service)
|
||||
|
||||
updated = await service.update_task(task.task_id, user_id="user-1", now=NOW, prompt="new instructions")
|
||||
|
||||
assert updated.prompt == "new instructions"
|
||||
assert updated.title == task.title
|
||||
|
||||
async def test_a_task_deleted_mid_update_reports_as_missing(self):
|
||||
"""get_task saw it, save no longer does -- a concurrent delete landed
|
||||
in between. The caller gets the same not-found it would have got a
|
||||
moment earlier, rather than a None leaking out."""
|
||||
|
||||
class _VanishingRepo(InMemoryScheduledTaskRepository):
|
||||
async def save(self, _task):
|
||||
return None
|
||||
|
||||
tasks = _VanishingRepo()
|
||||
service = make_service(tasks=tasks)
|
||||
task = await create_cron_task(service)
|
||||
|
||||
with pytest.raises(TaskNotFoundError):
|
||||
await service.update_task(task.task_id, user_id="user-1", now=NOW, title="x")
|
||||
with pytest.raises(TaskNotFoundError):
|
||||
await service.pause_task(task.task_id, user_id="user-1")
|
||||
|
||||
async def test_context_is_changed_through_the_packaged_value(self):
|
||||
"""context_mode and thread_id move together, so they are supplied
|
||||
together -- which is what lets every other update field use plain
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user