Files
golfgame/server/test_game_lifecycle_logging.py
adlee-was-taken c02b0054c2 fix(server): winner_id on completed games + stats idempotency latch
Two issues in the GAME_OVER broadcast path:

1. log_game_end called update_game_completed with winner_id=None default,
   so games_v2.winner_id was NULL on all 17 completed staging rows. The
   denormalized column existed but carried no information. Compute winner
   (lowest total; None on tie) in broadcast_game_state and thread through.

2. _process_stats_safe had no idempotency guard. log_game_end was already
   self-guarding via game_log_id=None after first fire, but nothing
   stopped repeated GAME_OVER broadcasts from re-firing stats and
   double-counting games_played/games_won. Add Room.stats_processed latch;
   reset it in handle_start_game so a re-used room still records.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-18 00:47:53 -04:00

328 lines
12 KiB
Python

# SPDX-License-Identifier: GPL-3.0-or-later
"""
Tests for game lifecycle logging — ensuring games_v2 rows never leak as
stranded 'active' when a room is removed from memory without the game
transitioning to GAME_OVER.
Context: on staging we observed 42 games stuck in status='active' because
handle_player_leave and handle_end_game removed the room from memory
without updating games_v2. The periodic cleanup only scans in-memory rooms,
so those rows were orphaned forever.
These tests pin down the fix:
1. GameLogger.log_game_abandoned_async calls event_store.update_game_abandoned
2. handle_end_game marks the game abandoned when the host ends the game
"""
import asyncio
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
from game import GameOptions
from room import Room, RoomManager
from services.game_logger import GameLogger
from handlers import ConnectionContext, handle_end_game, handle_start_game
from test_handlers import MockWebSocket, make_ctx
# ---------------------------------------------------------------------------
# GameLogger.log_game_abandoned — unit
# ---------------------------------------------------------------------------
class TestLogGameAbandoned:
@pytest.mark.asyncio
async def test_calls_update_game_abandoned(self):
"""log_game_abandoned_async delegates to event_store.update_game_abandoned."""
event_store = MagicMock()
event_store.update_game_abandoned = AsyncMock()
logger = GameLogger(event_store)
await logger.log_game_abandoned_async("game-uuid-123")
event_store.update_game_abandoned.assert_awaited_once_with("game-uuid-123")
@pytest.mark.asyncio
async def test_sync_wrapper_fires_task(self):
"""Sync log_game_abandoned fires an async task in async context."""
event_store = MagicMock()
event_store.update_game_abandoned = AsyncMock()
logger = GameLogger(event_store)
logger.log_game_abandoned("game-uuid-456")
# Let the fire-and-forget task run
await asyncio.sleep(0)
await asyncio.sleep(0)
event_store.update_game_abandoned.assert_awaited_once_with("game-uuid-456")
def test_sync_wrapper_noop_on_empty_id(self):
"""Empty game_id is a no-op (nothing to abandon)."""
event_store = MagicMock()
event_store.update_game_abandoned = AsyncMock()
logger = GameLogger(event_store)
logger.log_game_abandoned("")
logger.log_game_abandoned(None)
event_store.update_game_abandoned.assert_not_called()
@pytest.mark.asyncio
async def test_swallows_db_exceptions(self):
"""DB errors are logged, not re-raised (fire-and-forget guarantee)."""
event_store = MagicMock()
event_store.update_game_abandoned = AsyncMock(side_effect=Exception("db down"))
logger = GameLogger(event_store)
# Must not raise
await logger.log_game_abandoned_async("game-uuid-789")
# ---------------------------------------------------------------------------
# handle_end_game integration
# ---------------------------------------------------------------------------
class TestHandleEndGameMarksAbandoned:
@pytest.mark.asyncio
async def test_marks_game_abandoned_before_room_removal(self):
"""When host ends the game, games_v2 must be marked abandoned."""
rm = RoomManager()
room = rm.create_room()
host_ws = MockWebSocket()
room.add_player("host", "Host", host_ws)
room.get_player("host").is_host = True
room.game_log_id = "game-uuid-end"
mock_logger = MagicMock()
mock_logger.log_game_abandoned = MagicMock()
ctx = make_ctx(websocket=host_ws, player_id="host", room=room)
with patch("handlers.get_logger", return_value=mock_logger):
await handle_end_game(
{},
ctx,
room_manager=rm,
cleanup_room_profiles=lambda _code: None,
)
mock_logger.log_game_abandoned.assert_called_once_with("game-uuid-end")
assert room.code not in rm.rooms # room still removed
@pytest.mark.asyncio
async def test_no_log_when_game_log_id_missing(self):
"""If the game never logged a start, there's nothing to mark abandoned."""
rm = RoomManager()
room = rm.create_room()
host_ws = MockWebSocket()
room.add_player("host", "Host", host_ws)
room.get_player("host").is_host = True
# room.game_log_id stays None
mock_logger = MagicMock()
mock_logger.log_game_abandoned = MagicMock()
ctx = make_ctx(websocket=host_ws, player_id="host", room=room)
with patch("handlers.get_logger", return_value=mock_logger):
await handle_end_game(
{},
ctx,
room_manager=rm,
cleanup_room_profiles=lambda _code: None,
)
mock_logger.log_game_abandoned.assert_not_called()
@pytest.mark.asyncio
async def test_non_host_cannot_trigger_abandonment(self):
"""Only the host ends games — non-host requests are rejected unchanged."""
rm = RoomManager()
room = rm.create_room()
room.add_player("host", "Host", MockWebSocket())
room.get_player("host").is_host = True
joiner_ws = MockWebSocket()
room.add_player("joiner", "Joiner", joiner_ws)
room.game_log_id = "game-uuid-untouchable"
mock_logger = MagicMock()
mock_logger.log_game_abandoned = MagicMock()
ctx = make_ctx(websocket=joiner_ws, player_id="joiner", room=room)
with patch("handlers.get_logger", return_value=mock_logger):
await handle_end_game(
{},
ctx,
room_manager=rm,
cleanup_room_profiles=lambda _code: None,
)
mock_logger.log_game_abandoned.assert_not_called()
assert room.code in rm.rooms
class TestLogGameStartPopulatesMetadata:
"""
create_game only writes id/room_code/host_id/options. update_game_started
(which fills in started_at, num_players, num_rounds, player_ids) existed
but had zero callers — 100% of staging's 289 games had those fields NULL.
log_game_start must call both so the row is complete after start_game.
"""
@pytest.mark.asyncio
async def test_start_calls_create_and_update(self):
event_store = MagicMock()
event_store.create_game = AsyncMock()
event_store.update_game_started = AsyncMock()
logger = GameLogger(event_store)
options = GameOptions(initial_flips=0)
await logger.log_game_start_async(
room_code="ABCD",
num_players=3,
num_rounds=9,
player_ids=["p1", "p2", "p3"],
options=options,
)
event_store.create_game.assert_awaited_once()
event_store.update_game_started.assert_awaited_once()
call = event_store.update_game_started.await_args
assert call.kwargs.get("num_players", call.args[1] if len(call.args) > 1 else None) == 3
assert call.kwargs.get("num_rounds", call.args[2] if len(call.args) > 2 else None) == 9
assert call.kwargs.get("player_ids", call.args[3] if len(call.args) > 3 else None) == ["p1", "p2", "p3"]
@pytest.mark.asyncio
async def test_update_uses_same_game_id_as_create(self):
event_store = MagicMock()
event_store.create_game = AsyncMock()
event_store.update_game_started = AsyncMock()
logger = GameLogger(event_store)
await logger.log_game_start_async(
room_code="XYZW",
num_players=2,
num_rounds=1,
player_ids=["a", "b"],
options=GameOptions(initial_flips=0),
)
create_args = event_store.create_game.await_args
update_args = event_store.update_game_started.await_args
created_id = create_args.kwargs.get("game_id", create_args.args[0] if create_args.args else None)
updated_id = update_args.args[0] if update_args.args else update_args.kwargs.get("game_id")
assert created_id == updated_id
assert created_id # non-empty
@pytest.mark.asyncio
async def test_create_failure_skips_update(self):
"""If the row never landed, don't try to update a non-existent id."""
event_store = MagicMock()
event_store.create_game = AsyncMock(side_effect=Exception("db down"))
event_store.update_game_started = AsyncMock()
logger = GameLogger(event_store)
await logger.log_game_start_async(
room_code="ABCD",
num_players=2,
num_rounds=1,
player_ids=["a", "b"],
options=GameOptions(initial_flips=0),
)
event_store.update_game_started.assert_not_awaited()
class TestLogGameEndWinnerId:
"""
update_game_completed accepts winner_id but the existing sync wrapper
called it with the default None → every completed games_v2 row had
winner_id NULL. Thread the winner through so the denormalized column
is actually useful.
"""
@pytest.mark.asyncio
async def test_winner_id_passed_through(self):
event_store = MagicMock()
event_store.update_game_completed = AsyncMock()
logger = GameLogger(event_store)
await logger.log_game_end_async("game-uuid", winner_id="player-7")
event_store.update_game_completed.assert_awaited_once_with("game-uuid", "player-7")
@pytest.mark.asyncio
async def test_winner_id_optional(self):
"""A tie or abandonment-style end without a clear winner still works."""
event_store = MagicMock()
event_store.update_game_completed = AsyncMock()
logger = GameLogger(event_store)
await logger.log_game_end_async("game-uuid")
event_store.update_game_completed.assert_awaited_once_with("game-uuid", None)
@pytest.mark.asyncio
async def test_sync_wrapper_forwards_winner(self):
event_store = MagicMock()
event_store.update_game_completed = AsyncMock()
logger = GameLogger(event_store)
logger.log_game_end("game-uuid", winner_id="player-9")
await asyncio.sleep(0)
await asyncio.sleep(0)
event_store.update_game_completed.assert_awaited_once_with("game-uuid", "player-9")
class TestStatsIdempotency:
"""
broadcast_game_state can fire multiple times with phase=GAME_OVER
(double-click next-round, reconnect flush, etc.). log_game_end is
already idempotent because it nulls game_log_id immediately after.
_process_stats_safe had no such guard → every extra broadcast would
double-count games_played/games_won on the same game.
Solution: Room.stats_processed flag. Set True before firing the task.
"""
def test_room_has_stats_processed_flag_defaulting_false(self):
room = Room(code="TEST")
assert room.stats_processed is False
def test_stats_processed_survives_touch(self):
"""touch() updates last_activity but must not clobber stats_processed."""
room = Room(code="TEST")
room.stats_processed = True
room.touch()
assert room.stats_processed is True
@pytest.mark.asyncio
async def test_start_game_resets_stats_processed(self):
"""When a room is reused for a second game, the latch must reset —
otherwise the new game's stats would be silently dropped."""
from handlers import handle_start_game
rm = RoomManager()
room = rm.create_room()
host_ws = MockWebSocket()
room.add_player("host", "Host", host_ws)
room.get_player("host").is_host = True
room.add_player("p2", "P2", MockWebSocket())
# Previous game already had stats processed
room.stats_processed = True
ctx = make_ctx(websocket=host_ws, player_id="host", room=room)
with patch("handlers.get_logger", return_value=None):
await handle_start_game(
{"decks": 1, "rounds": 1},
ctx,
broadcast_game_state=AsyncMock(),
check_and_run_cpu_turn=lambda _r: None,
)
assert room.stats_processed is False