fix: make cooldown_action decorator properly await async finish() (#75)

* fix: make cooldown_action decorator properly await async finish()

The decorator was incorrectly wrapping the async finish() method with a sync
wrapper, causing cooldown to be recorded BEFORE the action actually executed.

This fix:
- Changes wrapper to async def finish()
- Awaits original_finish() before recording cooldown
- Ensures cooldown is only recorded on successful execution

Fixes #74

* test: add test to verify cooldown not recorded on failure

This test actually reveals the async/await bug:
- Creates a FailingAction that raises in finish()
- Verifies cooldown is NOT recorded when action fails
- Fails with buggy sync wrapper, passes with async fix
This commit is contained in:
Zihao Xu
2026-01-19 23:32:14 -08:00
committed by GitHub
parent 5f236361dc
commit a1241a9156
2 changed files with 142 additions and 2 deletions

View File

@@ -34,8 +34,10 @@ def cooldown_action(cls: type) -> type:
if hasattr(cls, "finish"):
original_finish = cls.finish
def finish(self, **params): # type: ignore[no-redef]
events = original_finish(self, **params)
async def finish(self, **params): # type: ignore[no-redef]
# Must await original_finish first, then record cooldown.
# This ensures cooldown is only recorded after finish() succeeds.
events = await original_finish(self, **params)
last_map = getattr(self.avatar, "_action_cd_last_months", None)
if last_map is not None:
last_map[self.__class__.__name__] = self.world.month_stamp

138
tests/test_cooldown.py Normal file
View File

@@ -0,0 +1,138 @@
"""
Tests for src/classes/action/cooldown.py
Tests the cooldown_action decorator behavior.
"""
import pytest
from unittest.mock import MagicMock, patch
from src.classes.action.cooldown import cooldown_action
from src.classes.mutual_action.impart import Impart
class TestCooldownAction:
"""Tests for cooldown_action decorator."""
@pytest.mark.asyncio
async def test_cooldown_recorded_after_finish_executes(self):
"""
Test that cooldown is recorded AFTER finish() executes, not before.
This was a bug where the sync wrapper recorded cooldown before
awaiting the async original_finish().
"""
world = MagicMock()
world.month_stamp = 100
master = MagicMock()
master.name = "Master"
master._action_cd_last_months = {}
action = Impart(master, world)
action._impart_success = True
action._impart_exp_gain = 2000
target = MagicMock()
target.name = "Disciple"
target.id = "disciple_id"
# Before calling finish, no cooldown recorded.
assert "Impart" not in master._action_cd_last_months
# Call finish and await it.
events = await action.finish(target_avatar=target)
# After await, cooldown should be recorded.
assert "Impart" in master._action_cd_last_months
assert master._action_cd_last_months["Impart"] == 100
def test_cooldown_check_in_can_start(self):
"""Test that can_start checks cooldown correctly."""
world = MagicMock()
world.month_stamp = 100
master = MagicMock()
master.name = "Master"
master._action_cd_last_months = {"Impart": 98} # Used 2 months ago
action = Impart(master, world)
# Impart has 6 month cooldown, only 2 months passed.
can_start, reason = action.can_start(target_avatar=MagicMock())
assert can_start is False
assert "冷却中" in reason
assert "4" in reason # 6 - 2 = 4 months remaining
def test_cooldown_expired_allows_start(self):
"""Test that can_start allows action after cooldown expires."""
world = MagicMock()
world.month_stamp = 110
master = MagicMock()
master.name = "Master"
master.get_relation = MagicMock(return_value=None)
master._action_cd_last_months = {"Impart": 100} # Used 10 months ago
master.cultivation_progress = MagicMock()
master.cultivation_progress.level = 50
action = Impart(master, world)
target = MagicMock()
target.is_dead = False
# Impart has 6 month cooldown, 10 months passed - should be allowed.
# Note: will fail on other checks (relation), but not on cooldown.
with patch("src.classes.observe.is_within_observation", return_value=True):
can_start, reason = action.can_start(target_avatar=target)
# Should not fail due to cooldown.
assert "冷却" not in reason
@pytest.mark.asyncio
async def test_cooldown_not_recorded_on_finish_failure(self):
"""
Test that cooldown is NOT recorded if finish() raises an exception.
This is the key test that reveals the async/await bug:
- Buggy code: records cooldown BEFORE awaiting, so cooldown is recorded even on failure
- Fixed code: awaits first, so exception prevents cooldown from being recorded
"""
class ActionError(Exception):
pass
# Create a simple action class with finish that raises.
@cooldown_action
class FailingAction:
ACTION_CD_MONTHS = 6
def __init__(self, avatar, world):
self.avatar = avatar
self.world = world
def can_start(self, **params):
return True, ""
async def finish(self, **params):
raise ActionError("Action failed!")
world = MagicMock()
world.month_stamp = 100
avatar = MagicMock()
avatar.name = "Test"
avatar._action_cd_last_months = {}
action = FailingAction(avatar, world)
# Before calling finish, no cooldown recorded.
assert "FailingAction" not in avatar._action_cd_last_months
# Call finish - should raise.
with pytest.raises(ActionError):
await action.finish()
# Cooldown should NOT be recorded because action failed.
assert "FailingAction" not in avatar._action_cd_last_months