1a9efa8f7e
Early returns in log_chaos, log_sac_bunt, and log_stealing left play locks permanently stuck because the lock was acquired but never released. The new locked_play async context manager wraps checks_log_interaction() and guarantees lock release on exception, early return, or normal exit. Migrated all 18 locking commands in gameplay.py and removed redundant double-locking in end_game_command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
480 lines
16 KiB
Python
480 lines
16 KiB
Python
"""
|
|
Test play locking idempotency guard.
|
|
|
|
Tests the play locking system including:
|
|
- Lock acquisition and rejection of duplicate submissions
|
|
- Automatic lock release on exception via global error handler
|
|
- Manual lock release via release_play_lock()
|
|
- Context manager safe_play_lock() behavior
|
|
"""
|
|
|
|
import pytest
|
|
from unittest.mock import MagicMock, AsyncMock, patch
|
|
from command_logic.logic_gameplay import checks_log_interaction, release_play_lock, safe_play_lock
|
|
from command_logic.play_context import locked_play
|
|
from exceptions import PlayLockedException
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_session():
|
|
"""Create a mock SQLAlchemy session."""
|
|
session = MagicMock()
|
|
return session
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_interaction():
|
|
"""Create a mock Discord interaction."""
|
|
interaction = MagicMock()
|
|
interaction.user.name = "TestUser"
|
|
interaction.user.id = 12345
|
|
interaction.channel.name = "test-game-channel"
|
|
interaction.channel_id = 98765
|
|
interaction.response = MagicMock()
|
|
interaction.response.defer = AsyncMock()
|
|
return interaction
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_game(mock_session):
|
|
"""Create a mock game with current play."""
|
|
game = MagicMock()
|
|
game.id = 100
|
|
game.current_play_or_none = MagicMock()
|
|
return game
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_team():
|
|
"""Create a mock team."""
|
|
team = MagicMock()
|
|
team.id = 50
|
|
team.abbrev = "TEST"
|
|
return team
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_play_unlocked():
|
|
"""Create an unlocked mock play."""
|
|
play = MagicMock()
|
|
play.id = 200
|
|
play.locked = False
|
|
return play
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_play_locked():
|
|
"""Create a locked mock play."""
|
|
play = MagicMock()
|
|
play.id = 200
|
|
play.locked = True
|
|
return play
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_unlocked_play_locks_successfully(
|
|
mock_session, mock_interaction, mock_game, mock_team, mock_play_unlocked
|
|
):
|
|
"""Verify unlocked play can be locked and processed."""
|
|
mock_game.current_play_or_none.return_value = mock_play_unlocked
|
|
mock_session.exec.return_value.one.return_value = mock_team
|
|
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_channel_game_or_none", return_value=mock_game
|
|
):
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_team_or_none", return_value=mock_team
|
|
):
|
|
result_game, result_team, result_play = await checks_log_interaction(
|
|
mock_session, mock_interaction, command_name="log xcheck"
|
|
)
|
|
|
|
assert result_play.locked is True
|
|
assert result_play.id == mock_play_unlocked.id
|
|
mock_session.commit.assert_called_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_rejects_duplicate_interaction(
|
|
mock_session, mock_interaction, mock_game, mock_team, mock_play_locked
|
|
):
|
|
"""Verify duplicate command on locked play raises PlayLockedException."""
|
|
mock_game.current_play_or_none.return_value = mock_play_locked
|
|
mock_session.exec.return_value.one.return_value = mock_team
|
|
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_channel_game_or_none", return_value=mock_game
|
|
):
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_team_or_none", return_value=mock_team
|
|
):
|
|
with pytest.raises(PlayLockedException) as exc_info:
|
|
await checks_log_interaction(
|
|
mock_session, mock_interaction, command_name="log xcheck"
|
|
)
|
|
|
|
assert "already being processed" in str(exc_info.value)
|
|
assert "wait" in str(exc_info.value).lower()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_logs_warning(
|
|
mock_session, mock_interaction, mock_game, mock_team, mock_play_locked, caplog
|
|
):
|
|
"""Verify locked play attempt is logged with warning."""
|
|
mock_game.current_play_or_none.return_value = mock_play_locked
|
|
mock_session.exec.return_value.one.return_value = mock_team
|
|
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_channel_game_or_none", return_value=mock_game
|
|
):
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_team_or_none", return_value=mock_team
|
|
):
|
|
try:
|
|
await checks_log_interaction(
|
|
mock_session, mock_interaction, command_name="log xcheck"
|
|
)
|
|
except PlayLockedException:
|
|
pass
|
|
|
|
assert any(
|
|
"attempted log xcheck on locked play" in record.message
|
|
for record in caplog.records
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_lock_released_after_successful_completion(mock_session):
|
|
"""
|
|
Verify play lock is released after successful command completion.
|
|
|
|
Tests the complete_play() function to ensure it:
|
|
- Sets play.locked = False
|
|
- Sets play.complete = True
|
|
- Commits changes to database
|
|
"""
|
|
from command_logic.logic_gameplay import complete_play
|
|
|
|
# Create mock play that's currently locked
|
|
mock_play = MagicMock()
|
|
mock_play.id = 300
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
mock_play.game = MagicMock()
|
|
mock_play.game.id = 100
|
|
mock_play.inning_num = 1
|
|
mock_play.inning_half = "top"
|
|
mock_play.starting_outs = 0
|
|
mock_play.away_score = 0
|
|
mock_play.home_score = 0
|
|
mock_play.on_base_code = 0
|
|
mock_play.batter = MagicMock()
|
|
mock_play.batter.team = MagicMock()
|
|
mock_play.pitcher = MagicMock()
|
|
mock_play.pitcher.team = MagicMock()
|
|
mock_play.managerai = MagicMock()
|
|
|
|
# Mock the session.exec queries
|
|
mock_session.exec.return_value.one.return_value = MagicMock()
|
|
|
|
# Execute complete_play
|
|
with patch("command_logic.logic_gameplay.get_one_lineup"):
|
|
with patch("command_logic.logic_gameplay.get_re24", return_value=0.0):
|
|
with patch("command_logic.logic_gameplay.get_wpa", return_value=0.0):
|
|
complete_play(mock_session, mock_play)
|
|
|
|
# Verify lock was released and play marked complete
|
|
assert mock_play.locked is False
|
|
assert mock_play.complete is True
|
|
|
|
# Verify changes were committed
|
|
mock_session.add.assert_called()
|
|
mock_session.commit.assert_called_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_different_commands_racing_on_locked_play(
|
|
mock_session, mock_interaction, mock_game, mock_team, mock_play_locked
|
|
):
|
|
"""
|
|
Verify different command types are blocked when play is locked.
|
|
|
|
Tests that the lock prevents ANY command from processing, not just
|
|
duplicates of the same command. This prevents race conditions where
|
|
different users try different commands simultaneously.
|
|
"""
|
|
mock_game.current_play_or_none.return_value = mock_play_locked
|
|
mock_session.exec.return_value.one.return_value = mock_team
|
|
|
|
# Test different command types all raise PlayLockedException
|
|
command_types = ["log walk", "log strikeout", "log single", "log xcheck"]
|
|
|
|
for command_name in command_types:
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_channel_game_or_none",
|
|
return_value=mock_game,
|
|
):
|
|
with patch(
|
|
"command_logic.logic_gameplay.get_team_or_none",
|
|
return_value=mock_team,
|
|
):
|
|
with pytest.raises(PlayLockedException) as exc_info:
|
|
await checks_log_interaction(
|
|
mock_session, mock_interaction, command_name=command_name
|
|
)
|
|
|
|
# Verify exception message is consistent
|
|
assert "already being processed" in str(exc_info.value)
|
|
assert "wait" in str(exc_info.value).lower()
|
|
|
|
|
|
def test_release_play_lock_unlocks_locked_play(mock_session):
|
|
"""
|
|
Verify release_play_lock() releases a locked play.
|
|
|
|
Tests that the release_play_lock helper function correctly:
|
|
- Sets play.locked = False
|
|
- Commits the change to the database
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 400
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
release_play_lock(mock_session, mock_play)
|
|
|
|
assert mock_play.locked is False
|
|
mock_session.add.assert_called_once_with(mock_play)
|
|
mock_session.commit.assert_called_once()
|
|
|
|
|
|
def test_release_play_lock_handles_unlocked_play(mock_session):
|
|
"""
|
|
Verify release_play_lock() safely handles already unlocked play.
|
|
|
|
Tests that calling release_play_lock on an already unlocked play
|
|
is a no-op and doesn't cause errors.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 400
|
|
mock_play.locked = False
|
|
mock_play.complete = True
|
|
|
|
# Should not raise exception even if play is already unlocked
|
|
release_play_lock(mock_session, mock_play)
|
|
|
|
# Should still be unlocked
|
|
assert mock_play.locked is False
|
|
|
|
|
|
def test_safe_play_lock_releases_on_exception(mock_session):
|
|
"""
|
|
Verify safe_play_lock context manager releases lock on exception.
|
|
|
|
Tests that when an exception occurs within the context manager,
|
|
the play lock is automatically released before re-raising.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 500
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with pytest.raises(ValueError):
|
|
with safe_play_lock(mock_session, mock_play):
|
|
# Simulate exception during command processing
|
|
raise ValueError("Test exception")
|
|
|
|
# Lock should be released
|
|
assert mock_play.locked is False
|
|
mock_session.add.assert_called()
|
|
mock_session.commit.assert_called()
|
|
|
|
|
|
def test_safe_play_lock_allows_normal_completion(mock_session):
|
|
"""
|
|
Verify safe_play_lock doesn't interfere with normal completion.
|
|
|
|
Tests that when no exception occurs, the context manager doesn't
|
|
prematurely release the lock (complete_play should handle that).
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 600
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with safe_play_lock(mock_session, mock_play):
|
|
# Normal processing
|
|
pass
|
|
|
|
# Lock should remain (will be released by complete_play)
|
|
# Note: The finally block will force unlock, but this simulates
|
|
# the case where complete_play() is called within the context
|
|
mock_play.complete = True
|
|
|
|
with safe_play_lock(mock_session, mock_play):
|
|
pass
|
|
|
|
# Completed play should not be unlocked by context manager
|
|
|
|
|
|
# --- locked_play context manager tests ---
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_releases_on_exception(mock_session, mock_interaction, mock_game, mock_team):
|
|
"""
|
|
Verify locked_play releases the lock when an exception is raised inside the block.
|
|
|
|
This ensures that if command processing throws (e.g. a dice roll function errors),
|
|
the play lock is released so the game isn't permanently stuck.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 700
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
def fake_release(session, play):
|
|
play.locked = False
|
|
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
return_value=(mock_game, mock_team, mock_play),
|
|
):
|
|
with patch("command_logic.play_context.release_play_lock", side_effect=fake_release) as mock_release:
|
|
with pytest.raises(ValueError):
|
|
async with locked_play(mock_session, mock_interaction, "log test") as (g, t, p):
|
|
raise ValueError("processing error")
|
|
|
|
# Called once in except block; finally sees locked=False and skips
|
|
mock_release.assert_called_once_with(mock_session, mock_play)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_releases_on_early_return(mock_session, mock_interaction, mock_game, mock_team):
|
|
"""
|
|
Verify locked_play releases the lock when the block exits normally without
|
|
calling complete_play (i.e. an early return from validation).
|
|
|
|
This is the primary bug fix — commands like log_chaos, log_sac_bunt, and
|
|
log_stealing had early returns after lock acquisition that left the lock stuck.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 701
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
return_value=(mock_game, mock_team, mock_play),
|
|
):
|
|
with patch("command_logic.play_context.release_play_lock") as mock_release:
|
|
async with locked_play(mock_session, mock_interaction, "log bunt") as (g, t, p):
|
|
# Simulate early return: block exits without calling complete_play
|
|
pass
|
|
|
|
# Lock should be released because play.locked=True and play.complete=False
|
|
mock_release.assert_called_once_with(mock_session, mock_play)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_skips_release_when_complete(mock_session, mock_interaction, mock_game, mock_team):
|
|
"""
|
|
Verify locked_play does NOT attempt to release the lock when complete_play()
|
|
has already run (play.complete=True, play.locked=False).
|
|
|
|
This is the normal success path — complete_play handles the lock release,
|
|
so the context manager should be a no-op.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 702
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
return_value=(mock_game, mock_team, mock_play),
|
|
):
|
|
with patch("command_logic.play_context.release_play_lock") as mock_release:
|
|
async with locked_play(mock_session, mock_interaction, "log single") as (g, t, p):
|
|
# Simulate complete_play() having been called
|
|
mock_play.locked = False
|
|
mock_play.complete = True
|
|
|
|
# Should NOT attempt release since play is already complete
|
|
mock_release.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_propagates_checks_exception(mock_session, mock_interaction):
|
|
"""
|
|
Verify that when checks_log_interaction raises PlayLockedException (because
|
|
the play is already locked by another command), the exception propagates
|
|
without any cleanup attempt — the lock was never ours.
|
|
"""
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
side_effect=PlayLockedException("Play is already being processed. Please wait."),
|
|
):
|
|
with patch("command_logic.play_context.release_play_lock") as mock_release:
|
|
with pytest.raises(PlayLockedException):
|
|
async with locked_play(mock_session, mock_interaction, "log walk") as (g, t, p):
|
|
pass # Should never reach here
|
|
|
|
# No release attempted — lock was never acquired
|
|
mock_release.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_reraises_original_exception(mock_session, mock_interaction, mock_game, mock_team):
|
|
"""
|
|
Verify that the original exception is preserved and re-raised after
|
|
the lock is released. The caller (global error handler) should see
|
|
the real error, not a lock-release error.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 704
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
return_value=(mock_game, mock_team, mock_play),
|
|
):
|
|
with patch("command_logic.play_context.release_play_lock"):
|
|
with pytest.raises(RuntimeError, match="dice roll failed"):
|
|
async with locked_play(mock_session, mock_interaction, "log xcheck") as (g, t, p):
|
|
raise RuntimeError("dice roll failed")
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_locked_play_handles_release_failure(mock_session, mock_interaction, mock_game, mock_team):
|
|
"""
|
|
Verify that if release_play_lock itself fails (e.g. DB connection lost),
|
|
the original exception is still propagated rather than being masked by
|
|
the release failure.
|
|
"""
|
|
mock_play = MagicMock()
|
|
mock_play.id = 705
|
|
mock_play.locked = True
|
|
mock_play.complete = False
|
|
|
|
with patch(
|
|
"command_logic.play_context.checks_log_interaction",
|
|
new_callable=AsyncMock,
|
|
return_value=(mock_game, mock_team, mock_play),
|
|
):
|
|
with patch(
|
|
"command_logic.play_context.release_play_lock",
|
|
side_effect=Exception("DB connection lost"),
|
|
):
|
|
# The original ValueError should propagate, not the release failure
|
|
with pytest.raises(ValueError, match="original error"):
|
|
async with locked_play(mock_session, mock_interaction, "log chaos") as (g, t, p):
|
|
raise ValueError("original error")
|