wrongmove: stabilise setup_periodic_tasks tests after market-aggregator add
CI test-unit failed on pipeline #49 because the three TestSetupPeriodicTasks cases asserted exact call counts on `sender.add_periodic_task` and the new unconditional `daily-market-aggregator` registration bumped each by one. Fix: - `tasks/listing_tasks.py`: lifted the market-aggregator registration out of the SchedulesConfig try-block — it's now independent of the user's SCRAPE_SCHEDULES (a malformed scrape config no longer takes the aggregator down with it). - `tests/unit/test_listing_tasks.py`: updated the four cases to account for the +1 unconditional aggregator call. `test_handles_config_error_ gracefully` now asserts the aggregator still registers when SchedulesConfig.from_env raises (regression coverage for the independence guarantee). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
49e3514780
commit
9fdf7fd356
2 changed files with 40 additions and 22 deletions
|
|
@ -635,27 +635,33 @@ async def _dump_listings_full_inner(
|
||||||
|
|
||||||
@app.on_after_finalize.connect
|
@app.on_after_finalize.connect
|
||||||
def setup_periodic_tasks(sender, **kwargs):
|
def setup_periodic_tasks(sender, **kwargs):
|
||||||
"""Register periodic tasks from environment configuration."""
|
"""Register periodic tasks from environment configuration.
|
||||||
|
|
||||||
|
The daily market aggregator is registered unconditionally so it stays
|
||||||
|
healthy even when the user's `SCRAPE_SCHEDULES` env var is malformed —
|
||||||
|
the two systems are orthogonal.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
config = SchedulesConfig.from_env()
|
config: SchedulesConfig | None = SchedulesConfig.from_env()
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
celery_logger.error(f"Failed to load schedule configuration: {e}")
|
celery_logger.error(f"Failed to load schedule configuration: {e}")
|
||||||
return
|
config = None
|
||||||
|
|
||||||
for schedule in config.get_enabled_schedules():
|
if config is not None:
|
||||||
celery_logger.info(
|
for schedule in config.get_enabled_schedules():
|
||||||
f"Registering periodic task: {schedule.name} at {schedule.hour}:{schedule.minute}"
|
celery_logger.info(
|
||||||
)
|
f"Registering periodic task: {schedule.name} at {schedule.hour}:{schedule.minute}"
|
||||||
|
)
|
||||||
|
|
||||||
sender.add_periodic_task(
|
sender.add_periodic_task(
|
||||||
crontab(
|
crontab(
|
||||||
minute=schedule.minute,
|
minute=schedule.minute,
|
||||||
hour=schedule.hour,
|
hour=schedule.hour,
|
||||||
day_of_week=schedule.day_of_week,
|
day_of_week=schedule.day_of_week,
|
||||||
),
|
),
|
||||||
dump_listings_task.s(schedule.to_query_parameters().model_dump_json()),
|
dump_listings_task.s(schedule.to_query_parameters().model_dump_json()),
|
||||||
name=schedule.name,
|
name=schedule.name,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Daily market aggregator — fires after the 03:00 RENT scrape so the
|
# Daily market aggregator — fires after the 03:00 RENT scrape so the
|
||||||
# snapshot reflects today's freshly-pulled data. Imported lazily to
|
# snapshot reflects today's freshly-pulled data. Imported lazily to
|
||||||
|
|
|
||||||
|
|
@ -162,6 +162,10 @@ class TestDumpListingsTask:
|
||||||
class TestSetupPeriodicTasks:
|
class TestSetupPeriodicTasks:
|
||||||
"""Tests for setup_periodic_tasks."""
|
"""Tests for setup_periodic_tasks."""
|
||||||
|
|
||||||
|
# NOTE: every call to setup_periodic_tasks also registers the unconditional
|
||||||
|
# `daily-market-aggregator` task (one extra call per invocation), so
|
||||||
|
# call_count assertions below account for that +1.
|
||||||
|
|
||||||
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
||||||
def test_registers_enabled_schedules(self, mock_from_env):
|
def test_registers_enabled_schedules(self, mock_from_env):
|
||||||
from config.schedule_config import ScheduleConfig
|
from config.schedule_config import ScheduleConfig
|
||||||
|
|
@ -180,18 +184,23 @@ class TestSetupPeriodicTasks:
|
||||||
sender = MagicMock()
|
sender = MagicMock()
|
||||||
module.setup_periodic_tasks(sender)
|
module.setup_periodic_tasks(sender)
|
||||||
|
|
||||||
sender.add_periodic_task.assert_called_once()
|
# 1 schedule + 1 market aggregator.
|
||||||
call_args = sender.add_periodic_task.call_args
|
assert sender.add_periodic_task.call_count == 2
|
||||||
assert call_args[1]["name"] == "Test Schedule"
|
names = [c.kwargs["name"] for c in sender.add_periodic_task.call_args_list]
|
||||||
|
assert "Test Schedule" in names
|
||||||
|
assert "daily-market-aggregator" in names
|
||||||
|
|
||||||
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
||||||
def test_handles_config_error_gracefully(self, mock_from_env):
|
def test_handles_config_error_gracefully(self, mock_from_env):
|
||||||
|
"""A malformed SCRAPE_SCHEDULES must not block the market aggregator."""
|
||||||
mock_from_env.side_effect = ValueError("bad config")
|
mock_from_env.side_effect = ValueError("bad config")
|
||||||
|
|
||||||
sender = MagicMock()
|
sender = MagicMock()
|
||||||
module.setup_periodic_tasks(sender)
|
module.setup_periodic_tasks(sender)
|
||||||
|
|
||||||
sender.add_periodic_task.assert_not_called()
|
# Aggregator still registers (the two systems are independent).
|
||||||
|
assert sender.add_periodic_task.call_count == 1
|
||||||
|
assert sender.add_periodic_task.call_args.kwargs["name"] == "daily-market-aggregator"
|
||||||
|
|
||||||
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
||||||
def test_registers_nothing_when_no_schedules(self, mock_from_env):
|
def test_registers_nothing_when_no_schedules(self, mock_from_env):
|
||||||
|
|
@ -202,7 +211,9 @@ class TestSetupPeriodicTasks:
|
||||||
sender = MagicMock()
|
sender = MagicMock()
|
||||||
module.setup_periodic_tasks(sender)
|
module.setup_periodic_tasks(sender)
|
||||||
|
|
||||||
sender.add_periodic_task.assert_not_called()
|
# Only the market aggregator registered — no user schedules.
|
||||||
|
assert sender.add_periodic_task.call_count == 1
|
||||||
|
assert sender.add_periodic_task.call_args.kwargs["name"] == "daily-market-aggregator"
|
||||||
|
|
||||||
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
@patch("tasks.listing_tasks.SchedulesConfig.from_env")
|
||||||
def test_registers_multiple_schedules(self, mock_from_env):
|
def test_registers_multiple_schedules(self, mock_from_env):
|
||||||
|
|
@ -220,7 +231,8 @@ class TestSetupPeriodicTasks:
|
||||||
sender = MagicMock()
|
sender = MagicMock()
|
||||||
module.setup_periodic_tasks(sender)
|
module.setup_periodic_tasks(sender)
|
||||||
|
|
||||||
assert sender.add_periodic_task.call_count == 2
|
# 2 schedules + 1 market aggregator.
|
||||||
|
assert sender.add_periodic_task.call_count == 3
|
||||||
|
|
||||||
|
|
||||||
class TestPipelineState:
|
class TestPipelineState:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue