refactor(tests): optimize unit test structure

2026-03-24 13:37:06 +08:00
parent d25985be0b
commit 72acba5d27
52 changed files with 104 additions and 13 deletions
--- a/tests/cron/test_cron_service.py
+++ b/tests/cron/test_cron_service.py
@@ -0,0 +1,143 @@
+import asyncio
+import json
+
+import pytest
+
+from nanobot.cron.service import CronService
+from nanobot.cron.types import CronSchedule
+
+
+def test_add_job_rejects_unknown_timezone(tmp_path) -> None:
+    service = CronService(tmp_path / "cron" / "jobs.json")
+
+    with pytest.raises(ValueError, match="unknown timezone 'America/Vancovuer'"):
+        service.add_job(
+            name="tz typo",
+            schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="America/Vancovuer"),
+            message="hello",
+        )
+
+    assert service.list_jobs(include_disabled=True) == []
+
+
+def test_add_job_accepts_valid_timezone(tmp_path) -> None:
+    service = CronService(tmp_path / "cron" / "jobs.json")
+
+    job = service.add_job(
+        name="tz ok",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="America/Vancouver"),
+        message="hello",
+    )
+
+    assert job.schedule.tz == "America/Vancouver"
+    assert job.state.next_run_at_ms is not None
+
+
+@pytest.mark.asyncio
+async def test_execute_job_records_run_history(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    service = CronService(store_path, on_job=lambda _: asyncio.sleep(0))
+    job = service.add_job(
+        name="hist",
+        schedule=CronSchedule(kind="every", every_ms=60_000),
+        message="hello",
+    )
+    await service.run_job(job.id)
+
+    loaded = service.get_job(job.id)
+    assert loaded is not None
+    assert len(loaded.state.run_history) == 1
+    rec = loaded.state.run_history[0]
+    assert rec.status == "ok"
+    assert rec.duration_ms >= 0
+    assert rec.error is None
+
+
+@pytest.mark.asyncio
+async def test_run_history_records_errors(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+
+    async def fail(_):
+        raise RuntimeError("boom")
+
+    service = CronService(store_path, on_job=fail)
+    job = service.add_job(
+        name="fail",
+        schedule=CronSchedule(kind="every", every_ms=60_000),
+        message="hello",
+    )
+    await service.run_job(job.id)
+
+    loaded = service.get_job(job.id)
+    assert len(loaded.state.run_history) == 1
+    assert loaded.state.run_history[0].status == "error"
+    assert loaded.state.run_history[0].error == "boom"
+
+
+@pytest.mark.asyncio
+async def test_run_history_trimmed_to_max(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    service = CronService(store_path, on_job=lambda _: asyncio.sleep(0))
+    job = service.add_job(
+        name="trim",
+        schedule=CronSchedule(kind="every", every_ms=60_000),
+        message="hello",
+    )
+    for _ in range(25):
+        await service.run_job(job.id)
+
+    loaded = service.get_job(job.id)
+    assert len(loaded.state.run_history) == CronService._MAX_RUN_HISTORY
+
+
+@pytest.mark.asyncio
+async def test_run_history_persisted_to_disk(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    service = CronService(store_path, on_job=lambda _: asyncio.sleep(0))
+    job = service.add_job(
+        name="persist",
+        schedule=CronSchedule(kind="every", every_ms=60_000),
+        message="hello",
+    )
+    await service.run_job(job.id)
+
+    raw = json.loads(store_path.read_text())
+    history = raw["jobs"][0]["state"]["runHistory"]
+    assert len(history) == 1
+    assert history[0]["status"] == "ok"
+    assert "runAtMs" in history[0]
+    assert "durationMs" in history[0]
+
+    fresh = CronService(store_path)
+    loaded = fresh.get_job(job.id)
+    assert len(loaded.state.run_history) == 1
+    assert loaded.state.run_history[0].status == "ok"
+
+
+@pytest.mark.asyncio
+async def test_running_service_honors_external_disable(tmp_path) -> None:
+    store_path = tmp_path / "cron" / "jobs.json"
+    called: list[str] = []
+
+    async def on_job(job) -> None:
+        called.append(job.id)
+
+    service = CronService(store_path, on_job=on_job)
+    job = service.add_job(
+        name="external-disable",
+        schedule=CronSchedule(kind="every", every_ms=200),
+        message="hello",
+    )
+    await service.start()
+    try:
+        # Wait slightly to ensure file mtime is definitively different
+        await asyncio.sleep(0.05)
+        external = CronService(store_path)
+        updated = external.enable_job(job.id, enabled=False)
+        assert updated is not None
+        assert updated.enabled is False
+
+        await asyncio.sleep(0.35)
+        assert called == []
+    finally:
+        service.stop()
--- a/tests/cron/test_cron_tool_list.py
+++ b/tests/cron/test_cron_tool_list.py
@@ -0,0 +1,250 @@
+"""Tests for CronTool._list_jobs() output formatting."""
+
+from nanobot.agent.tools.cron import CronTool
+from nanobot.cron.service import CronService
+from nanobot.cron.types import CronJobState, CronSchedule
+
+
+def _make_tool(tmp_path) -> CronTool:
+    service = CronService(tmp_path / "cron" / "jobs.json")
+    return CronTool(service)
+
+
+# -- _format_timing tests --
+
+
+def test_format_timing_cron_with_tz() -> None:
+    s = CronSchedule(kind="cron", expr="0 9 * * 1-5", tz="America/Denver")
+    assert CronTool._format_timing(s) == "cron: 0 9 * * 1-5 (America/Denver)"
+
+
+def test_format_timing_cron_without_tz() -> None:
+    s = CronSchedule(kind="cron", expr="*/5 * * * *")
+    assert CronTool._format_timing(s) == "cron: */5 * * * *"
+
+
+def test_format_timing_every_hours() -> None:
+    s = CronSchedule(kind="every", every_ms=7_200_000)
+    assert CronTool._format_timing(s) == "every 2h"
+
+
+def test_format_timing_every_minutes() -> None:
+    s = CronSchedule(kind="every", every_ms=1_800_000)
+    assert CronTool._format_timing(s) == "every 30m"
+
+
+def test_format_timing_every_seconds() -> None:
+    s = CronSchedule(kind="every", every_ms=30_000)
+    assert CronTool._format_timing(s) == "every 30s"
+
+
+def test_format_timing_every_non_minute_seconds() -> None:
+    s = CronSchedule(kind="every", every_ms=90_000)
+    assert CronTool._format_timing(s) == "every 90s"
+
+
+def test_format_timing_every_milliseconds() -> None:
+    s = CronSchedule(kind="every", every_ms=200)
+    assert CronTool._format_timing(s) == "every 200ms"
+
+
+def test_format_timing_at() -> None:
+    s = CronSchedule(kind="at", at_ms=1773684000000)
+    result = CronTool._format_timing(s)
+    assert result.startswith("at 2026-")
+
+
+def test_format_timing_fallback() -> None:
+    s = CronSchedule(kind="every")  # no every_ms
+    assert CronTool._format_timing(s) == "every"
+
+
+# -- _format_state tests --
+
+
+def test_format_state_empty() -> None:
+    state = CronJobState()
+    assert CronTool._format_state(state) == []
+
+
+def test_format_state_last_run_ok() -> None:
+    state = CronJobState(last_run_at_ms=1773673200000, last_status="ok")
+    lines = CronTool._format_state(state)
+    assert len(lines) == 1
+    assert "Last run:" in lines[0]
+    assert "ok" in lines[0]
+
+
+def test_format_state_last_run_with_error() -> None:
+    state = CronJobState(last_run_at_ms=1773673200000, last_status="error", last_error="timeout")
+    lines = CronTool._format_state(state)
+    assert len(lines) == 1
+    assert "error" in lines[0]
+    assert "timeout" in lines[0]
+
+
+def test_format_state_next_run_only() -> None:
+    state = CronJobState(next_run_at_ms=1773684000000)
+    lines = CronTool._format_state(state)
+    assert len(lines) == 1
+    assert "Next run:" in lines[0]
+
+
+def test_format_state_both() -> None:
+    state = CronJobState(
+        last_run_at_ms=1773673200000, last_status="ok", next_run_at_ms=1773684000000
+    )
+    lines = CronTool._format_state(state)
+    assert len(lines) == 2
+    assert "Last run:" in lines[0]
+    assert "Next run:" in lines[1]
+
+
+def test_format_state_unknown_status() -> None:
+    state = CronJobState(last_run_at_ms=1773673200000, last_status=None)
+    lines = CronTool._format_state(state)
+    assert "unknown" in lines[0]
+
+
+# -- _list_jobs integration tests --
+
+
+def test_list_empty(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    assert tool._list_jobs() == "No scheduled jobs."
+
+
+def test_list_cron_job_shows_expression_and_timezone(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Morning scan",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * 1-5", tz="America/Denver"),
+        message="scan",
+    )
+    result = tool._list_jobs()
+    assert "cron: 0 9 * * 1-5 (America/Denver)" in result
+
+
+def test_list_every_job_shows_human_interval(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Frequent check",
+        schedule=CronSchedule(kind="every", every_ms=1_800_000),
+        message="check",
+    )
+    result = tool._list_jobs()
+    assert "every 30m" in result
+
+
+def test_list_every_job_hours(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Hourly check",
+        schedule=CronSchedule(kind="every", every_ms=7_200_000),
+        message="check",
+    )
+    result = tool._list_jobs()
+    assert "every 2h" in result
+
+
+def test_list_every_job_seconds(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Fast check",
+        schedule=CronSchedule(kind="every", every_ms=30_000),
+        message="check",
+    )
+    result = tool._list_jobs()
+    assert "every 30s" in result
+
+
+def test_list_every_job_non_minute_seconds(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Ninety-second check",
+        schedule=CronSchedule(kind="every", every_ms=90_000),
+        message="check",
+    )
+    result = tool._list_jobs()
+    assert "every 90s" in result
+
+
+def test_list_every_job_milliseconds(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Sub-second check",
+        schedule=CronSchedule(kind="every", every_ms=200),
+        message="check",
+    )
+    result = tool._list_jobs()
+    assert "every 200ms" in result
+
+
+def test_list_at_job_shows_iso_timestamp(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="One-shot",
+        schedule=CronSchedule(kind="at", at_ms=1773684000000),
+        message="fire",
+    )
+    result = tool._list_jobs()
+    assert "at 2026-" in result
+
+
+def test_list_shows_last_run_state(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    job = tool._cron.add_job(
+        name="Stateful job",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="UTC"),
+        message="test",
+    )
+    # Simulate a completed run by updating state in the store
+    job.state.last_run_at_ms = 1773673200000
+    job.state.last_status = "ok"
+    tool._cron._save_store()
+
+    result = tool._list_jobs()
+    assert "Last run:" in result
+    assert "ok" in result
+
+
+def test_list_shows_error_message(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    job = tool._cron.add_job(
+        name="Failed job",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="UTC"),
+        message="test",
+    )
+    job.state.last_run_at_ms = 1773673200000
+    job.state.last_status = "error"
+    job.state.last_error = "timeout"
+    tool._cron._save_store()
+
+    result = tool._list_jobs()
+    assert "error" in result
+    assert "timeout" in result
+
+
+def test_list_shows_next_run(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool._cron.add_job(
+        name="Upcoming job",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="UTC"),
+        message="test",
+    )
+    result = tool._list_jobs()
+    assert "Next run:" in result
+
+
+def test_list_excludes_disabled_jobs(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    job = tool._cron.add_job(
+        name="Paused job",
+        schedule=CronSchedule(kind="cron", expr="0 9 * * *", tz="UTC"),
+        message="test",
+    )
+    tool._cron.enable_job(job.id, enabled=False)
+
+    result = tool._list_jobs()
+    assert "Paused job" not in result
+    assert result == "No scheduled jobs."