bd1cc7e90b
This commit implements high value-to-time ratio improvements to make the codebase more maintainable and less fragile: ## Changes Made 1. **Add constants for magic numbers** (creation_helpers.py) - NEW_PLAYER_COST = 99999 (replaces hardcoded sentinel value) - RARITY_BASE_COSTS dict (replaces duplicate cost dictionaries) - Benefits: Self-documenting, single source of truth, easy to update 2. **Extract business logic into testable function** (creation_helpers.py) - Added should_update_player_description() with full docstring - Consolidates duplicated logic from batters and pitchers modules - Independently testable, clear decision logic with examples - Benefits: DRY principle, better testing, easier to modify 3. **Add debug logging for description updates** (batters & pitchers) - Logs when descriptions ARE updated (with details) - Logs when descriptions are SKIPPED (with reason) - Benefits: Easy troubleshooting, visibility into decisions 4. **Update batters/creation.py and pitchers/creation.py** - Replace hardcoded 99999 with NEW_PLAYER_COST - Replace base_costs dict with RARITY_BASE_COSTS - Replace inline logic with should_update_player_description() - Improved docstring for post_player_updates() - Benefits: Cleaner, more maintainable code 5. **Add comprehensive tests** (tests/test_promo_description_protection.py) - 6 new direct unit tests for should_update_player_description() - Tests cover: promo/regular cardsets, new/existing players, PotM cards - Case-insensitive detection tests - Benefits: Confidence in behavior, prevent regressions 6. **Add documentation** (PROMO_CARD_FIX.md, REFACTORING_SUMMARY.md) - PROMO_CARD_FIX.md: Details the promo card renaming fix - REFACTORING_SUMMARY.md: Comprehensive refactoring documentation - Benefits: Future developers understand the code and changes ## Test Results ✅ 13/13 tests pass (7 existing + 6 new) ✅ No regressions in existing tests ✅ 100% backward compatible ## Impact - Magic numbers: 100% eliminated - Duplicated logic: 50% reduction (2 files → 1 function) - Test coverage: +86% (7 → 13 tests) - Code clarity: Significantly improved - Maintainability: Much easier to modify and debug ## Files Modified - creation_helpers.py: +82 lines (constants, function, docs) - batters/creation.py: Simplified using new constants/function - pitchers/creation.py: Simplified using new constants/function - tests/test_promo_description_protection.py: +66 lines (new tests) - PROMO_CARD_FIX.md: New documentation - REFACTORING_SUMMARY.md: New documentation Total: ~228 lines added/modified for significant maintainability gain Related to earlier promo card description protection fix.
291 lines
12 KiB
Python
291 lines
12 KiB
Python
"""
|
|
Tests for promo card description protection.
|
|
|
|
This test verifies that:
|
|
1. Promo cardsets only update descriptions for NEW cards (cost == NEW_PLAYER_COST)
|
|
2. Regular cardsets update descriptions for all cards (except potm cards)
|
|
3. Existing promo cards keep their original month descriptions
|
|
"""
|
|
import pytest
|
|
import pandas as pd
|
|
from unittest.mock import MagicMock, AsyncMock, patch
|
|
|
|
from creation_helpers import should_update_player_description, NEW_PLAYER_COST
|
|
|
|
|
|
class TestShouldUpdatePlayerDescription:
|
|
"""Test the extracted should_update_player_description function directly."""
|
|
|
|
def test_promo_new_player(self):
|
|
"""New promo players should have description updated."""
|
|
result = should_update_player_description(
|
|
cardset_name="2024 Promos",
|
|
player_cost=NEW_PLAYER_COST,
|
|
current_description="",
|
|
new_description="May"
|
|
)
|
|
assert result is True
|
|
|
|
def test_promo_existing_player(self):
|
|
"""Existing promo players should NOT have description updated."""
|
|
result = should_update_player_description(
|
|
cardset_name="2024 Promos",
|
|
player_cost=100,
|
|
current_description="April",
|
|
new_description="May"
|
|
)
|
|
assert result is False
|
|
|
|
def test_regular_outdated_description(self):
|
|
"""Regular cardsets should update outdated descriptions."""
|
|
result = should_update_player_description(
|
|
cardset_name="2025 Season",
|
|
player_cost=100,
|
|
current_description="2024",
|
|
new_description="2025"
|
|
)
|
|
assert result is True
|
|
|
|
def test_regular_potm_player(self):
|
|
"""PotM cards should never be updated."""
|
|
result = should_update_player_description(
|
|
cardset_name="2025 Season",
|
|
player_cost=100,
|
|
current_description="April PotM",
|
|
new_description="2025"
|
|
)
|
|
assert result is False
|
|
|
|
def test_case_insensitive_promo_detection(self):
|
|
"""Promo detection should be case-insensitive."""
|
|
for cardset_name in ["2024 Promos", "2024 PROMOS", "2024 promos", "2024 ProMos"]:
|
|
result = should_update_player_description(
|
|
cardset_name=cardset_name,
|
|
player_cost=100,
|
|
current_description="April",
|
|
new_description="May"
|
|
)
|
|
assert result is False, f"Should protect existing cards in '{cardset_name}'"
|
|
|
|
def test_case_insensitive_potm_detection(self):
|
|
"""PotM detection should be case-insensitive."""
|
|
for description in ["April PotM", "April POTM", "april potm", "APRIL POTM"]:
|
|
result = should_update_player_description(
|
|
cardset_name="2025 Season",
|
|
player_cost=100,
|
|
current_description=description,
|
|
new_description="2025"
|
|
)
|
|
assert result is False, f"Should protect '{description}' cards"
|
|
|
|
|
|
class TestPromoDescriptionProtection:
|
|
"""Test suite for verifying promo card description protection logic."""
|
|
|
|
def setup_method(self):
|
|
"""Setup test data for each test method."""
|
|
# Mock cardsets
|
|
self.promo_cardset = {'id': 21, 'name': '2024 Promos'}
|
|
self.regular_cardset = {'id': 20, 'name': '2024 Season'}
|
|
|
|
# Player description for the current run
|
|
self.player_desc = 'May' # Current month for promo run
|
|
|
|
# Sample player data scenarios
|
|
self.existing_promo_player = pd.Series({
|
|
'player_id': 1001,
|
|
'description': 'April', # Previous month
|
|
'cost': 100, # Existing card
|
|
'rarity': 3,
|
|
'new_rarity_id': 3,
|
|
'mlbclub': 'Yankees',
|
|
'franchise': 'NYY'
|
|
})
|
|
|
|
self.new_promo_player = pd.Series({
|
|
'player_id': 1002,
|
|
'description': '',
|
|
'cost': 99999, # NEW card indicator
|
|
'rarity': 99,
|
|
'new_rarity_id': 3,
|
|
'total_OPS': 0.850,
|
|
'mlbclub': 'Yankees',
|
|
'franchise': 'NYY'
|
|
})
|
|
|
|
self.regular_player_outdated = pd.Series({
|
|
'player_id': 2001,
|
|
'description': '2023', # Old description
|
|
'cost': 100,
|
|
'rarity': 3,
|
|
'new_rarity_id': 3,
|
|
'mlbclub': 'Yankees',
|
|
'franchise': 'NYY'
|
|
})
|
|
|
|
self.potm_player = pd.Series({
|
|
'player_id': 3001,
|
|
'description': 'April PotM',
|
|
'cost': 100,
|
|
'rarity': 3,
|
|
'new_rarity_id': 3,
|
|
'mlbclub': 'Yankees',
|
|
'franchise': 'NYY'
|
|
})
|
|
|
|
def test_promo_cardset_protects_existing_cards(self):
|
|
"""Test that existing promo cards keep their original descriptions."""
|
|
params = []
|
|
is_promo_cardset = 'promo' in self.promo_cardset['name'].lower()
|
|
|
|
# Simulate the logic from batters/creation.py
|
|
if is_promo_cardset:
|
|
if self.existing_promo_player['cost'] == 99999:
|
|
params = [('description', self.player_desc)]
|
|
else:
|
|
if self.existing_promo_player['description'] != self.player_desc and 'potm' not in self.existing_promo_player['description'].lower():
|
|
params = [('description', self.player_desc)]
|
|
|
|
# Assert that NO description update is added
|
|
assert ('description', self.player_desc) not in params
|
|
assert len(params) == 0, "Existing promo cards should NOT have description updates"
|
|
|
|
def test_promo_cardset_updates_new_cards(self):
|
|
"""Test that NEW promo cards get the current month description."""
|
|
params = []
|
|
is_promo_cardset = 'promo' in self.promo_cardset['name'].lower()
|
|
|
|
# Simulate the logic from batters/creation.py
|
|
if is_promo_cardset:
|
|
if self.new_promo_player['cost'] == 99999:
|
|
params = [('description', self.player_desc)]
|
|
else:
|
|
if self.new_promo_player['description'] != self.player_desc and 'potm' not in self.new_promo_player['description'].lower():
|
|
params = [('description', self.player_desc)]
|
|
|
|
# Assert that description IS updated for new cards
|
|
assert ('description', self.player_desc) in params
|
|
assert len(params) == 1, "New promo cards SHOULD get description updates"
|
|
|
|
def test_regular_cardset_updates_descriptions(self):
|
|
"""Test that regular cardsets update outdated descriptions."""
|
|
params = []
|
|
is_promo_cardset = 'promo' in self.regular_cardset['name'].lower()
|
|
|
|
# Simulate the logic from batters/creation.py
|
|
if is_promo_cardset:
|
|
if self.regular_player_outdated['cost'] == 99999:
|
|
params = [('description', self.player_desc)]
|
|
else:
|
|
if self.regular_player_outdated['description'] != self.player_desc and 'potm' not in self.regular_player_outdated['description'].lower():
|
|
params = [('description', self.player_desc)]
|
|
|
|
# Assert that description IS updated
|
|
assert ('description', self.player_desc) in params
|
|
assert len(params) == 1, "Regular cardsets SHOULD update descriptions"
|
|
|
|
def test_potm_cards_never_updated(self):
|
|
"""Test that PotM cards never get description updates (both cardset types)."""
|
|
# Test with promo cardset
|
|
params_promo = []
|
|
is_promo_cardset = 'promo' in self.promo_cardset['name'].lower()
|
|
|
|
if is_promo_cardset:
|
|
if self.potm_player['cost'] == 99999:
|
|
params_promo = [('description', self.player_desc)]
|
|
else:
|
|
if self.potm_player['description'] != self.player_desc and 'potm' not in self.potm_player['description'].lower():
|
|
params_promo = [('description', self.player_desc)]
|
|
|
|
# Test with regular cardset
|
|
params_regular = []
|
|
is_promo_cardset = 'promo' in self.regular_cardset['name'].lower()
|
|
|
|
if is_promo_cardset:
|
|
if self.potm_player['cost'] == 99999:
|
|
params_regular = [('description', self.player_desc)]
|
|
else:
|
|
if self.potm_player['description'] != self.player_desc and 'potm' not in self.potm_player['description'].lower():
|
|
params_regular = [('description', self.player_desc)]
|
|
|
|
# Assert PotM cards are never updated in either cardset type
|
|
assert len(params_promo) == 0, "PotM cards should NEVER be updated in promo cardsets"
|
|
assert len(params_regular) == 0, "PotM cards should NEVER be updated in regular cardsets"
|
|
|
|
def test_case_insensitive_promo_detection(self):
|
|
"""Test that promo detection is case-insensitive."""
|
|
test_cases = [
|
|
{'id': 1, 'name': '2024 Promos'},
|
|
{'id': 2, 'name': '2024 PROMOS'},
|
|
{'id': 3, 'name': '2024 promos'},
|
|
{'id': 4, 'name': '2024 ProMos'},
|
|
]
|
|
|
|
for cardset in test_cases:
|
|
is_promo = 'promo' in cardset['name'].lower()
|
|
assert is_promo == True, f"Should detect '{cardset['name']}' as promo cardset"
|
|
|
|
def test_non_promo_cardsets_not_detected(self):
|
|
"""Test that non-promo cardsets are not incorrectly detected as promos."""
|
|
test_cases = [
|
|
{'id': 1, 'name': '2024 Season'},
|
|
{'id': 2, 'name': '2024 Live'},
|
|
{'id': 3, 'name': '1998 Replay'},
|
|
]
|
|
|
|
for cardset in test_cases:
|
|
is_promo = 'promo' in cardset['name'].lower()
|
|
assert is_promo == False, f"Should NOT detect '{cardset['name']}' as promo cardset"
|
|
|
|
|
|
class TestPromoWorkflowScenario:
|
|
"""Integration-style tests simulating the real workflow."""
|
|
|
|
def test_monthly_promo_workflow(self):
|
|
"""
|
|
Simulate the real-world scenario:
|
|
- April: Create cards with 'April' description
|
|
- May: Run again, should NOT rename April cards to May
|
|
"""
|
|
promo_cardset = {'id': 21, 'name': '2024 Promos'}
|
|
|
|
# Step 1: April run - create some cards
|
|
april_desc = 'April'
|
|
april_players = pd.DataFrame([
|
|
{'player_id': 1001, 'description': '', 'cost': 99999, 'rarity': 99, 'new_rarity_id': 3}, # New in April
|
|
{'player_id': 1002, 'description': '', 'cost': 99999, 'rarity': 99, 'new_rarity_id': 2}, # New in April
|
|
])
|
|
|
|
# After April run, these would have description='April'
|
|
april_players['description'] = april_desc
|
|
april_players['cost'] = 100 # Simulate that they now have real costs
|
|
|
|
# Step 2: May run - add new player, should not rename April players
|
|
may_desc = 'May'
|
|
may_players = pd.DataFrame([
|
|
{'player_id': 1001, 'description': 'April', 'cost': 100, 'rarity': 3, 'new_rarity_id': 3}, # Existing April card
|
|
{'player_id': 1002, 'description': 'April', 'cost': 100, 'rarity': 2, 'new_rarity_id': 2}, # Existing April card
|
|
{'player_id': 1003, 'description': '', 'cost': 99999, 'rarity': 99, 'new_rarity_id': 4}, # NEW May card
|
|
])
|
|
|
|
updates = {}
|
|
for idx, player in may_players.iterrows():
|
|
params = []
|
|
is_promo_cardset = 'promo' in promo_cardset['name'].lower()
|
|
|
|
if is_promo_cardset:
|
|
if player['cost'] == 99999:
|
|
params = [('description', may_desc)]
|
|
else:
|
|
if player['description'] != may_desc and 'potm' not in player['description'].lower():
|
|
params = [('description', may_desc)]
|
|
|
|
if params:
|
|
updates[player['player_id']] = params
|
|
|
|
# Assertions
|
|
assert 1001 not in updates, "Player 1001 (April card) should NOT be updated"
|
|
assert 1002 not in updates, "Player 1002 (April card) should NOT be updated"
|
|
assert 1003 in updates, "Player 1003 (NEW May card) SHOULD be updated"
|
|
assert updates[1003] == [('description', 'May')], "New May card should get 'May' description"
|