56007aaeec
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m54s
Restructure roll_for_cards into three phases: dice rolling (CPU-only), batched player fetches (one per rarity tier via asyncio.gather), and gathered writes (cards + pack patches concurrent). Reduces 20-30 sequential API calls to ~6 gathered calls for 5 packs. Also fixes leaked `x` variable bug in dupe branch, removes dead `all_players` accumulation, and bumps open-packs limit from 5 to 20. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
531 lines
20 KiB
Python
531 lines
20 KiB
Python
"""Tests for roll_for_cards parallelized implementation.
|
|
|
|
Validates dice rolling, batched player fetches, card creation,
|
|
pack marking, MVP backfill, cardset-23 dupe fallback, and notifications.
|
|
"""
|
|
|
|
import os
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
|
|
def _make_player(
|
|
player_id, rarity_value=0, rarity_name="Replacement", p_name="Test Player"
|
|
):
|
|
"""Factory for player dicts matching API shape."""
|
|
return {
|
|
"player_id": player_id,
|
|
"rarity": {"value": rarity_value, "name": rarity_name},
|
|
"p_name": p_name,
|
|
"description": f"2024 {p_name}",
|
|
}
|
|
|
|
|
|
_UNSET = object()
|
|
|
|
|
|
def _make_pack(
|
|
pack_id, team_id=1, pack_type="Standard", pack_team=None, pack_cardset=_UNSET
|
|
):
|
|
"""Factory for pack dicts matching API shape."""
|
|
return {
|
|
"id": pack_id,
|
|
"team": {"id": team_id, "abbrev": "TST"},
|
|
"pack_type": {"name": pack_type},
|
|
"pack_team": pack_team,
|
|
"pack_cardset": {"id": 10} if pack_cardset is _UNSET else pack_cardset,
|
|
}
|
|
|
|
|
|
def _random_response(players):
|
|
"""Wrap a list of player dicts in the API response shape."""
|
|
return {"count": len(players), "players": players}
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_db():
|
|
"""Patch db_get, db_post, db_patch in helpers.main."""
|
|
with (
|
|
patch("helpers.main.db_get", new_callable=AsyncMock) as mock_get,
|
|
patch("helpers.main.db_post", new_callable=AsyncMock) as mock_post,
|
|
patch("helpers.main.db_patch", new_callable=AsyncMock) as mock_patch,
|
|
):
|
|
mock_post.return_value = True
|
|
yield mock_get, mock_post, mock_patch
|
|
|
|
|
|
class TestSinglePack:
|
|
"""Single pack opening — verifies basic flow."""
|
|
|
|
async def test_single_standard_pack_creates_cards_and_marks_opened(self, mock_db):
|
|
"""A single standard pack should fetch players, create cards, and patch open_time.
|
|
|
|
Why: Validates the core happy path — dice roll → fetch → create → mark opened.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
# Return enough players for any rarity tier requested
|
|
mock_get.return_value = _random_response([_make_player(i) for i in range(10)])
|
|
|
|
pack = _make_pack(100)
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [100]
|
|
# At least one db_get for player fetches
|
|
assert mock_get.call_count >= 1
|
|
# Exactly one db_post for cards (may have notif posts too)
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 1
|
|
# Exactly one db_patch for marking pack opened
|
|
assert mock_patch.call_count == 1
|
|
assert mock_patch.call_args.kwargs["object_id"] == 100
|
|
|
|
async def test_checkin_pack_uses_extra_val(self, mock_db):
|
|
"""Check-In Player packs should apply extra_val modifier to dice range.
|
|
|
|
Why: extra_val shifts the d1000 ceiling, affecting rarity odds for check-in rewards.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(1)])
|
|
|
|
pack = _make_pack(200, pack_type="Check-In Player")
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards([pack], extra_val=500)
|
|
|
|
assert result == [200]
|
|
assert mock_get.call_count >= 1
|
|
|
|
async def test_unknown_pack_type_raises(self, mock_db):
|
|
"""Unrecognized pack types must raise TypeError.
|
|
|
|
Why: Guards against silent failures if a new pack type is added without dice logic.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
pack = _make_pack(300, pack_type="Unknown")
|
|
from helpers.main import roll_for_cards
|
|
|
|
with pytest.raises(TypeError, match="Pack type not recognized"):
|
|
await roll_for_cards([pack])
|
|
|
|
|
|
class TestMultiplePacks:
|
|
"""Multiple packs — verifies batching and distribution."""
|
|
|
|
async def test_multiple_packs_return_all_ids(self, mock_db):
|
|
"""Opening multiple packs should return all pack IDs.
|
|
|
|
Why: Callers use the returned IDs to know which packs were successfully opened.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(i) for i in range(50)])
|
|
|
|
packs = [_make_pack(i) for i in range(5)]
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards(packs)
|
|
|
|
assert result == [0, 1, 2, 3, 4]
|
|
|
|
async def test_multiple_packs_batch_fetches(self, mock_db):
|
|
"""Multiple packs should batch fetches — one db_get per rarity tier, not per pack.
|
|
|
|
Why: This is the core performance optimization. 5 packs should NOT make 20-30 calls.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(i) for i in range(50)])
|
|
|
|
packs = [_make_pack(i) for i in range(5)]
|
|
from helpers.main import roll_for_cards
|
|
|
|
await roll_for_cards(packs)
|
|
|
|
# Standard packs have up to 6 rarity tiers, but typically fewer are non-zero.
|
|
# The key assertion: far fewer fetches than 5 packs * ~4 tiers = 20.
|
|
player_fetches = [
|
|
c for c in mock_get.call_args_list if c.args[0] == "players/random"
|
|
]
|
|
# At most 6 tier fetches + possible 1 MVP backfill = 7
|
|
assert len(player_fetches) <= 7
|
|
|
|
async def test_multiple_packs_create_cards_per_pack(self, mock_db):
|
|
"""Each pack should get its own db_post('cards') call with correct pack_id.
|
|
|
|
Why: Cards must be associated with the correct pack for display and tracking.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(i) for i in range(50)])
|
|
|
|
packs = [_make_pack(i) for i in range(3)]
|
|
from helpers.main import roll_for_cards
|
|
|
|
await roll_for_cards(packs)
|
|
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 3
|
|
# Each card post should reference the correct pack_id
|
|
for i, post_call in enumerate(card_posts):
|
|
payload = post_call.kwargs["payload"]
|
|
pack_ids_in_cards = {card["pack_id"] for card in payload["cards"]}
|
|
assert pack_ids_in_cards == {i}
|
|
|
|
|
|
class TestMVPBackfill:
|
|
"""MVP fallback when a rarity tier returns fewer players than requested."""
|
|
|
|
async def test_shortfall_triggers_mvp_backfill(self, mock_db):
|
|
"""When a tier returns fewer players than needed, MVP backfill should fire.
|
|
|
|
Why: Packs must always contain the expected number of cards. Shortfalls are
|
|
filled with MVP-tier players as a fallback.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
call_count = 0
|
|
|
|
async def side_effect(endpoint, params=None):
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if params and any(
|
|
p[0] == "min_rarity"
|
|
and p[1] == 5
|
|
and any(q[0] == "max_rarity" for q in params) is False
|
|
for p in params
|
|
):
|
|
# MVP backfill call (no max_rarity)
|
|
return _random_response([_make_player(900, 5, "MVP")])
|
|
|
|
# For tier-specific calls, check if this is the MVP backfill
|
|
if params:
|
|
param_dict = dict(params)
|
|
if "max_rarity" not in param_dict:
|
|
return _random_response([_make_player(900, 5, "MVP")])
|
|
|
|
# Return fewer than requested to trigger shortfall
|
|
requested = 5
|
|
if params:
|
|
for key, val in params:
|
|
if key == "limit":
|
|
requested = val
|
|
break
|
|
return _random_response(
|
|
[_make_player(i) for i in range(max(0, requested - 1))]
|
|
)
|
|
|
|
mock_get.side_effect = side_effect
|
|
|
|
pack = _make_pack(100)
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [100]
|
|
# Should have at least the tier fetch + backfill call
|
|
assert mock_get.call_count >= 2
|
|
|
|
|
|
class TestCardsetExclusion:
|
|
"""Cardset 23 should duplicate existing players instead of MVP backfill."""
|
|
|
|
async def test_cardset_23_duplicates_instead_of_mvp(self, mock_db):
|
|
"""For cardset 23, shortfalls should duplicate existing players, not fetch MVPs.
|
|
|
|
Why: Cardset 23 (special/limited cardset) shouldn't pull from the MVP pool —
|
|
it should fill gaps by duplicating from what's already available.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
async def side_effect(endpoint, params=None):
|
|
if params:
|
|
param_dict = dict(params)
|
|
# If this is a backfill call (no max_rarity), it shouldn't happen
|
|
if "max_rarity" not in param_dict:
|
|
pytest.fail("Should not make MVP backfill call for cardset 23")
|
|
# Return fewer than requested
|
|
return _random_response([_make_player(1)])
|
|
|
|
mock_get.side_effect = side_effect
|
|
|
|
pack = _make_pack(100, pack_cardset={"id": 23})
|
|
from helpers.main import roll_for_cards
|
|
|
|
# Force specific dice rolls to ensure a shortfall
|
|
with patch("helpers.main.random.randint", return_value=1):
|
|
# d1000=1 for Standard: Rep, Rep, Rep, Rep, Rep → 5 Reps needed
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [100]
|
|
|
|
|
|
class TestNotifications:
|
|
"""Rare pull notifications should be gathered and sent."""
|
|
|
|
async def test_rare_pulls_generate_notifications(self, mock_db):
|
|
"""Players with rarity >= 3 should trigger notification posts.
|
|
|
|
Why: Rare pulls are announced to the community — all notifs should be sent.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
rare_player = _make_player(
|
|
42, rarity_value=3, rarity_name="All-Star", p_name="Mike Trout"
|
|
)
|
|
mock_get.return_value = _random_response([rare_player])
|
|
|
|
pack = _make_pack(100)
|
|
# Force all dice to land on All-Star tier (d1000=951 for card 3)
|
|
from helpers.main import roll_for_cards
|
|
|
|
with patch("helpers.main.random.randint", return_value=960):
|
|
await roll_for_cards([pack])
|
|
|
|
notif_posts = [c for c in mock_post.call_args_list if c.args[0] == "notifs"]
|
|
assert len(notif_posts) >= 1
|
|
payload = notif_posts[0].kwargs["payload"]
|
|
assert payload["title"] == "Rare Pull"
|
|
assert "Mike Trout" in payload["field_name"]
|
|
|
|
async def test_no_notifications_for_common_pulls(self, mock_db):
|
|
"""Players with rarity < 3 should NOT trigger notifications.
|
|
|
|
Why: Only rare pulls are noteworthy — common cards would spam the notif feed.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
|
|
common_player = _make_player(1, rarity_value=0, rarity_name="Replacement")
|
|
mock_get.return_value = _random_response([common_player])
|
|
|
|
pack = _make_pack(100)
|
|
from helpers.main import roll_for_cards
|
|
|
|
# Force low dice rolls (all Replacement)
|
|
with patch("helpers.main.random.randint", return_value=1):
|
|
await roll_for_cards([pack])
|
|
|
|
notif_posts = [c for c in mock_post.call_args_list if c.args[0] == "notifs"]
|
|
assert len(notif_posts) == 0
|
|
|
|
|
|
class TestErrorHandling:
|
|
"""Error propagation from gathered writes."""
|
|
|
|
async def test_card_creation_failure_raises(self, mock_db):
|
|
"""If db_post('cards') returns falsy, ConnectionError must propagate.
|
|
|
|
Why: Card creation failure means the pack wasn't properly opened — caller
|
|
needs to know so it can report the error to the user.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(1)])
|
|
mock_post.return_value = False # Simulate failure
|
|
|
|
pack = _make_pack(100)
|
|
from helpers.main import roll_for_cards
|
|
|
|
with pytest.raises(ConnectionError, match="Failed to create"):
|
|
await roll_for_cards([pack])
|
|
|
|
|
|
class TestPackTeamFiltering:
|
|
"""Verify correct filter params are passed to player fetch."""
|
|
|
|
async def test_pack_team_adds_franchise_filter(self, mock_db):
|
|
"""When pack has a pack_team, franchise filter should be applied.
|
|
|
|
Why: Team-specific packs should only contain players from that franchise.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(1)])
|
|
|
|
pack = _make_pack(
|
|
100,
|
|
pack_team={"sname": "NYY"},
|
|
pack_cardset=None,
|
|
)
|
|
from helpers.main import roll_for_cards
|
|
|
|
with patch("helpers.main.random.randint", return_value=1):
|
|
await roll_for_cards([pack])
|
|
|
|
# Check that tier-fetch calls (those with max_rarity) include franchise filter
|
|
tier_calls = [
|
|
c
|
|
for c in mock_get.call_args_list
|
|
if any(p[0] == "max_rarity" for p in (c.kwargs.get("params") or []))
|
|
]
|
|
assert len(tier_calls) >= 1
|
|
for c in tier_calls:
|
|
param_dict = dict(c.kwargs.get("params") or [])
|
|
assert param_dict.get("franchise") == "NYY"
|
|
assert param_dict.get("in_packs") is True
|
|
|
|
async def test_no_team_no_cardset_adds_in_packs(self, mock_db):
|
|
"""When pack has no team or cardset, in_packs filter should be applied.
|
|
|
|
Why: Generic packs still need the in_packs filter to exclude non-packable players.
|
|
"""
|
|
mock_get, mock_post, mock_patch = mock_db
|
|
mock_get.return_value = _random_response([_make_player(1)])
|
|
|
|
pack = _make_pack(100, pack_team=None, pack_cardset=None)
|
|
from helpers.main import roll_for_cards
|
|
|
|
with patch("helpers.main.random.randint", return_value=1):
|
|
await roll_for_cards([pack])
|
|
|
|
# Check that tier-fetch calls (those with max_rarity) include in_packs filter
|
|
tier_calls = [
|
|
c
|
|
for c in mock_get.call_args_list
|
|
if any(p[0] == "max_rarity" for p in (c.kwargs.get("params") or []))
|
|
]
|
|
assert len(tier_calls) >= 1
|
|
for c in tier_calls:
|
|
param_dict = dict(c.kwargs.get("params") or [])
|
|
assert param_dict.get("in_packs") is True
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Integration tests — hit real dev API for reads, mock all writes
|
|
# ---------------------------------------------------------------------------
|
|
requires_api = pytest.mark.skipif(
|
|
not os.environ.get("API_TOKEN"),
|
|
reason="API_TOKEN not set — skipping integration tests",
|
|
)
|
|
|
|
|
|
@requires_api
|
|
class TestIntegrationRealFetches:
|
|
"""Integration tests that hit the real dev API for player fetches.
|
|
|
|
Only db_get is real — db_post and db_patch are mocked to prevent writes.
|
|
Run with: API_TOKEN=<token> python -m pytest tests/test_roll_for_cards.py -k integration -v
|
|
"""
|
|
|
|
@pytest.fixture
|
|
def mock_writes(self):
|
|
"""Mock only write operations, let reads hit the real API."""
|
|
with (
|
|
patch("helpers.main.db_post", new_callable=AsyncMock) as mock_post,
|
|
patch("helpers.main.db_patch", new_callable=AsyncMock) as mock_patch,
|
|
):
|
|
mock_post.return_value = True
|
|
yield mock_post, mock_patch
|
|
|
|
async def test_integration_single_pack_fetches_real_players(self, mock_writes):
|
|
"""A single standard pack should fetch real players from the dev API.
|
|
|
|
Why: Validates that the batched fetch params (min_rarity, max_rarity, limit,
|
|
in_packs) produce valid responses from the real API and that the returned
|
|
players have the expected structure.
|
|
"""
|
|
mock_post, mock_patch = mock_writes
|
|
|
|
pack = _make_pack(9999)
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [9999]
|
|
# Cards were "created" (mocked)
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 1
|
|
payload = card_posts[0].kwargs["payload"]
|
|
# Standard pack produces 5 cards
|
|
assert len(payload["cards"]) == 5
|
|
# Each card has the expected structure
|
|
for card in payload["cards"]:
|
|
assert "player_id" in card
|
|
assert card["team_id"] == 1
|
|
assert card["pack_id"] == 9999
|
|
|
|
async def test_integration_multiple_packs_batch_correctly(self, mock_writes):
|
|
"""Multiple packs should batch fetches and distribute players correctly.
|
|
|
|
Why: Validates the core optimization — summing counts across packs, making
|
|
fewer API calls, and slicing players back into per-pack groups with real data.
|
|
"""
|
|
mock_post, mock_patch = mock_writes
|
|
|
|
packs = [_make_pack(i + 9000) for i in range(3)]
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards(packs)
|
|
|
|
assert result == [9000, 9001, 9002]
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 3
|
|
# Each pack should have exactly 5 cards (Standard packs)
|
|
total_cards = 0
|
|
for post_call in card_posts:
|
|
cards = post_call.kwargs["payload"]["cards"]
|
|
assert len(cards) == 5
|
|
total_cards += len(cards)
|
|
assert total_cards == 15
|
|
|
|
async def test_integration_players_have_valid_rarity(self, mock_writes):
|
|
"""Fetched players should have rarity values matching their requested tier.
|
|
|
|
Why: Confirms the API respects min_rarity/max_rarity filters and that
|
|
the player distribution logic assigns correct-tier players to each pack.
|
|
"""
|
|
mock_post, mock_patch = mock_writes
|
|
|
|
pack = _make_pack(9999)
|
|
from helpers.main import roll_for_cards
|
|
|
|
# Use fixed dice to get known rarity distribution
|
|
# d1000=500 for Standard: Rep, Res, Sta, Res, Sta (mix of low tiers)
|
|
with patch("helpers.main.random.randint", return_value=500):
|
|
await roll_for_cards([pack])
|
|
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 1
|
|
cards = card_posts[0].kwargs["payload"]["cards"]
|
|
# All cards should have valid player_ids (positive integers from real API)
|
|
for card in cards:
|
|
assert isinstance(card["player_id"], int)
|
|
assert card["player_id"] > 0
|
|
|
|
async def test_integration_cardset_filter(self, mock_writes):
|
|
"""Packs with a specific cardset should only fetch players from that cardset.
|
|
|
|
Why: Validates that the cardset_id parameter is correctly passed through
|
|
the batched fetch and the API filters accordingly.
|
|
"""
|
|
mock_post, mock_patch = mock_writes
|
|
|
|
pack = _make_pack(9999, pack_cardset={"id": 24})
|
|
from helpers.main import roll_for_cards
|
|
|
|
with patch("helpers.main.random.randint", return_value=500):
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [9999]
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 1
|
|
assert len(card_posts[0].kwargs["payload"]["cards"]) == 5
|
|
|
|
async def test_integration_checkin_pack(self, mock_writes):
|
|
"""Check-In Player pack should fetch exactly 1 player from the real API.
|
|
|
|
Why: Check-in packs produce a single card — validates the simplest
|
|
path through the batched fetch logic with real data.
|
|
"""
|
|
mock_post, mock_patch = mock_writes
|
|
|
|
pack = _make_pack(9999, pack_type="Check-In Player")
|
|
from helpers.main import roll_for_cards
|
|
|
|
result = await roll_for_cards([pack])
|
|
|
|
assert result == [9999]
|
|
card_posts = [c for c in mock_post.call_args_list if c.args[0] == "cards"]
|
|
assert len(card_posts) == 1
|
|
# Check-in packs produce exactly 1 card
|
|
assert len(card_posts[0].kwargs["payload"]["cards"]) == 1
|