From b34dcc390c8d89ec7c2ff1f2923ad0c5a6eb9d4d Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Tue, 24 Mar 2026 08:59:18 -0500 Subject: [PATCH] test: add comprehensive refractor test cases (T1-5 through T3-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements all gap tests identified by PO agents across three existing test files. No new files created — tests added to existing modules. T1-5 (test_refractor_notifs): Expose WP-14 integration bug — minimal stub dict {player_id, old_tier, new_tier} causes KeyError in build_tier_up_embed because player_name/track_name use bare dict access. Documents the bug contract so WP-14 implementers know what to fix. T1-6 (test_refractor_commands): Divergence tripwire — imports TIER_NAMES from both cogs.refractor and helpers.refractor_notifs and asserts deep equality. Will fail the moment the two copies fall out of sync. T1-7 (test_card_embed_refractor): TIER_BADGES format contract — asserts that wrapping helpers.main badge values in brackets produces cogs.refractor badge values (e.g. "BC" -> "[BC]") for all tiers. T2-7 (test_refractor_notifs): notify_tier_completion with None channel must not raise — the try/except absorbs AttributeError from None.send(). T2-8 (test_refractor_commands): All-T4 apply_close_filter returns empty list. Documents intended behaviour for tier=4 + progress="close" combo. T3-2 (test_refractor_commands): Malformed API response handling — format_refractor_entry must use fallbacks ("Unknown", 0) for missing keys. T3-3 (test_refractor_commands): Progress bar boundary precision — 1/100, 99/100, 0/100, and negative current values. T3-4 (test_refractor_commands): RP formula label — card_type="rp" shows "IP+K" (previously only "sp" was tested). T3-5 (test_refractor_commands): Unknown card_type falls back to raw string as the formula label without crashing. 112 tests pass (23 new, 89 pre-existing). Co-Authored-By: Claude Sonnet 4.6 --- tests/test_card_embed_refractor.py | 79 +++++++ tests/test_refractor_commands.py | 339 ++++++++++++++++++++++++++++- tests/test_refractor_notifs.py | 89 ++++++++ 3 files changed, 505 insertions(+), 2 deletions(-) diff --git a/tests/test_card_embed_refractor.py b/tests/test_card_embed_refractor.py index 764a96c..f4bfc06 100644 --- a/tests/test_card_embed_refractor.py +++ b/tests/test_card_embed_refractor.py @@ -246,6 +246,85 @@ class TestEmbedColorUnchanged: assert embeds[0].color == discord.Color(int(rarity_color, 16)) +# --------------------------------------------------------------------------- +# T1-7: TIER_BADGES format consistency check across modules +# --------------------------------------------------------------------------- + + +class TestTierBadgesFormatConsistency: + """ + T1-7: Assert that TIER_BADGES in cogs.refractor (format: "[BC]") and + helpers.main (format: "BC") are consistent — wrapping the helpers.main + value in brackets must produce the cogs.refractor value. + + Why: The two modules intentionally use different formats for different + rendering contexts: + - helpers.main uses bare strings ("BC") because get_card_embeds + wraps them in brackets when building the embed title. + - cogs.refractor uses bracket strings ("[BC]") because + format_refractor_entry inlines them directly into the display string. + + If either definition is updated without updating the other, embed titles + and /refractor status output will display inconsistent badges. This test + acts as an explicit contract check so any future change to either dict + is immediately surfaced here. + """ + + def test_cogs_badge_equals_bracketed_helpers_badge_for_all_tiers(self): + """ + For every tier in cogs.refractor TIER_BADGES, wrapping the + helpers.main TIER_BADGES value in square brackets must produce + the cogs.refractor value. + + i.e., f"[{helpers_badge}]" == cog_badge for all tiers. + """ + from cogs.refractor import TIER_BADGES as cog_badges + from helpers.main import TIER_BADGES as helpers_badges + + assert set(cog_badges.keys()) == set(helpers_badges.keys()), ( + "TIER_BADGES key sets differ between cogs.refractor and helpers.main. " + f"cogs keys: {set(cog_badges.keys())}, helpers keys: {set(helpers_badges.keys())}" + ) + + for tier, cog_badge in cog_badges.items(): + helpers_badge = helpers_badges[tier] + expected = f"[{helpers_badge}]" + assert cog_badge == expected, ( + f"Tier {tier} badge mismatch: " + f"cogs.refractor={cog_badge!r}, " + f"helpers.main={helpers_badge!r} " + f"(expected cog badge to equal '[{helpers_badge}]')" + ) + + def test_t1_badge_relationship(self): + """T1: helpers.main 'BC' wrapped in brackets equals cogs.refractor '[BC]'.""" + from cogs.refractor import TIER_BADGES as cog_badges + from helpers.main import TIER_BADGES as helpers_badges + + assert f"[{helpers_badges[1]}]" == cog_badges[1] + + def test_t2_badge_relationship(self): + """T2: helpers.main 'R' wrapped in brackets equals cogs.refractor '[R]'.""" + from cogs.refractor import TIER_BADGES as cog_badges + from helpers.main import TIER_BADGES as helpers_badges + + assert f"[{helpers_badges[2]}]" == cog_badges[2] + + def test_t3_badge_relationship(self): + """T3: helpers.main 'GR' wrapped in brackets equals cogs.refractor '[GR]'.""" + from cogs.refractor import TIER_BADGES as cog_badges + from helpers.main import TIER_BADGES as helpers_badges + + assert f"[{helpers_badges[3]}]" == cog_badges[3] + + def test_t4_badge_relationship(self): + """T4: helpers.main 'SF' wrapped in brackets equals cogs.refractor '[SF]'.""" + from cogs.refractor import TIER_BADGES as cog_badges + from helpers.main import TIER_BADGES as helpers_badges + + assert f"[{helpers_badges[4]}]" == cog_badges[4] + + # --------------------------------------------------------------------------- # Helper: call get_card_embeds and return embed list # --------------------------------------------------------------------------- diff --git a/tests/test_refractor_commands.py b/tests/test_refractor_commands.py index 12cb035..87a9fcf 100644 --- a/tests/test_refractor_commands.py +++ b/tests/test_refractor_commands.py @@ -186,8 +186,6 @@ class TestFormatRefractorEntry: assert len(lines) == 2 - - # --------------------------------------------------------------------------- # TIER_BADGES # --------------------------------------------------------------------------- @@ -257,6 +255,7 @@ class TestTierBadges: name_pos = first_line.find("Mike Trout") assert badge_pos < name_pos + # --------------------------------------------------------------------------- # apply_close_filter # --------------------------------------------------------------------------- @@ -421,6 +420,342 @@ def mock_interaction(): return interaction +# --------------------------------------------------------------------------- +# T1-6: TIER_NAMES duplication divergence check +# --------------------------------------------------------------------------- + + +class TestTierNamesDivergenceCheck: + """ + T1-6: Assert that TIER_NAMES in cogs.refractor and helpers.refractor_notifs + are identical (same keys, same values). + + Why: TIER_NAMES is duplicated in two modules. If one is updated and the + other is not (e.g. a tier is renamed or a new tier is added), tier labels + in the /refractor status embed and the tier-up notification embed will + diverge silently. This test acts as a divergence tripwire — it will fail + the moment the two copies fall out of sync, forcing an explicit fix. + """ + + def test_tier_names_are_identical_across_modules(self): + """ + Import TIER_NAMES from both modules and assert deep equality. + + The test imports the name at call-time rather than at module level to + ensure it always reads the current definition and is not affected by + module-level caching or monkeypatching in other tests. + """ + from cogs.refractor import TIER_NAMES as cog_tier_names + from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names + + assert cog_tier_names == notifs_tier_names, ( + "TIER_NAMES differs between cogs.refractor and helpers.refractor_notifs. " + "Both copies must be kept in sync. " + f"cogs.refractor: {cog_tier_names!r} " + f"helpers.refractor_notifs: {notifs_tier_names!r}" + ) + + def test_tier_names_have_same_keys(self): + """Keys (tier numbers) must be identical in both modules.""" + from cogs.refractor import TIER_NAMES as cog_tier_names + from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names + + assert set(cog_tier_names.keys()) == set(notifs_tier_names.keys()), ( + "TIER_NAMES key sets differ between modules." + ) + + def test_tier_names_have_same_values(self): + """Display strings (values) must be identical for every shared key.""" + from cogs.refractor import TIER_NAMES as cog_tier_names + from helpers.refractor_notifs import TIER_NAMES as notifs_tier_names + + for tier, name in cog_tier_names.items(): + assert notifs_tier_names.get(tier) == name, ( + f"Tier {tier} name mismatch: " + f"cogs.refractor={name!r}, " + f"helpers.refractor_notifs={notifs_tier_names.get(tier)!r}" + ) + + +# --------------------------------------------------------------------------- +# T2-8: Filter combination — tier=4 + progress="close" yields empty result +# --------------------------------------------------------------------------- + + +class TestApplyCloseFilterWithAllT4Cards: + """ + T2-8: When all cards in the list are T4 (fully evolved), apply_close_filter + must return an empty list. + + Why: T4 cards have no next tier to advance to, so they have no threshold. + The close filter explicitly excludes fully evolved cards (tier >= 4 or + next_threshold is None). If a user passes both tier=4 and progress="close" + to /refractor status, the combined result should be empty — the command + already handles this by showing "No cards are currently close to a tier + advancement." This test documents and protects that behaviour. + """ + + def test_all_t4_cards_returns_empty(self): + """ + A list of T4-only card states should produce an empty result from + apply_close_filter, because T4 cards are fully evolved and have no + next threshold to be "close" to. + + This is the intended behaviour when tier=4 and progress="close" are + combined: there are no qualifying cards, and the command should show + the "no cards close to advancement" message rather than an empty embed. + """ + t4_cards = [ + {"current_tier": 4, "formula_value": 300, "next_threshold": None}, + {"current_tier": 4, "formula_value": 500, "next_threshold": None}, + {"current_tier": 4, "formula_value": 275, "next_threshold": None}, + ] + result = apply_close_filter(t4_cards) + assert result == [], ( + "apply_close_filter must return [] for fully evolved T4 cards — " + "they have no next threshold and cannot be 'close' to advancement." + ) + + def test_t4_cards_excluded_even_with_high_formula_value(self): + """ + T4 cards are excluded regardless of their formula_value, since the + filter is based on tier (>= 4) and threshold (None), not raw values. + """ + t4_high_value = { + "current_tier": 4, + "formula_value": 9999, + "next_threshold": None, + } + assert apply_close_filter([t4_high_value]) == [] + + +# --------------------------------------------------------------------------- +# T3-2: Malformed API response handling in format_refractor_entry +# --------------------------------------------------------------------------- + + +class TestFormatRefractorEntryMalformedInput: + """ + T3-2: format_refractor_entry should not crash when given a card state dict + that is missing expected keys. + + Why: API responses can be incomplete due to migration states, partially + populated records, or future schema changes. format_refractor_entry uses + .get() with fallbacks for all keys, so missing fields should gracefully + degrade to sensible defaults ("Unknown" for name, 0 for values) rather than + raising a KeyError or TypeError. + """ + + def test_missing_player_name_uses_unknown(self): + """ + When player_name is absent, the output should contain "Unknown" rather + than crashing with a KeyError. + """ + state = { + "card_type": "batter", + "current_tier": 1, + "formula_value": 100, + "next_threshold": 150, + } + result = format_refractor_entry(state) + assert "Unknown" in result + + def test_missing_formula_value_uses_zero(self): + """ + When formula_value is absent, the progress calculation should use 0 + without raising a TypeError. + """ + state = { + "player_name": "Test Player", + "card_type": "batter", + "current_tier": 1, + "next_threshold": 150, + } + result = format_refractor_entry(state) + assert "0/150" in result + + def test_completely_empty_dict_does_not_crash(self): + """ + An entirely empty dict should produce a valid (if sparse) string using + all fallback values, not raise any exception. + """ + result = format_refractor_entry({}) + # Should not raise; output should be a string with two lines + assert isinstance(result, str) + lines = result.split("\n") + assert len(lines) == 2 + + def test_missing_card_type_uses_raw_fallback(self): + """ + When card_type is absent, the code defaults to 'batter' internally + (via .get("card_type", "batter")), so "PA+TB×2" should appear as the + formula label. + """ + state = { + "player_name": "Test Player", + "current_tier": 1, + "formula_value": 50, + "next_threshold": 100, + } + result = format_refractor_entry(state) + assert "PA+TB×2" in result + + +# --------------------------------------------------------------------------- +# T3-3: Progress bar boundary precision +# --------------------------------------------------------------------------- + + +class TestRenderProgressBarBoundaryPrecision: + """ + T3-3: Verify the progress bar behaves correctly at edge values — near zero, + near full, exactly at extremes, and for negative input. + + Why: Off-by-one errors in rounding or integer truncation can make a nearly- + full bar look full (or vice versa), confusing users about how close their + card is to a tier advancement. Defensive handling of negative values ensures + no bar is rendered longer than its declared width. + """ + + def test_one_of_hundred_shows_mostly_empty(self): + """ + 1/100 = 1% — should produce a bar with 0 or 1 filled segment and the + rest empty. The bar must not appear more than minimally filled. + """ + bar = render_progress_bar(1, 100) + # Interior is 10 chars: count '=' vs '-' + interior = bar[1:-1] # strip '[' and ']' + filled_count = interior.count("=") + assert filled_count <= 1, ( + f"1/100 should show 0 or 1 filled segment, got {filled_count}: {bar!r}" + ) + + def test_ninety_nine_of_hundred_is_nearly_full(self): + """ + 99/100 = 99% — should produce a bar with 9 or 10 filled segments. + The bar must NOT be completely empty or show fewer than 9 filled. + """ + bar = render_progress_bar(99, 100) + interior = bar[1:-1] + filled_count = interior.count("=") + assert filled_count >= 9, ( + f"99/100 should show 9 or 10 filled segments, got {filled_count}: {bar!r}" + ) + # But it must not overflow the bar width + assert len(interior) == 10 + + def test_zero_of_hundred_is_completely_empty(self): + """0/100 = all dashes — re-verify the all-empty baseline.""" + assert render_progress_bar(0, 100) == "[----------]" + + def test_negative_current_does_not_overflow_bar(self): + """ + A negative formula_value (data anomaly) must not produce a bar with + more filled segments than the width. The min(..., 1.0) clamp in + render_progress_bar should handle this, but this test guards against + a future refactor removing the clamp. + """ + bar = render_progress_bar(-5, 100) + interior = bar[1:-1] + # No filled segments should exist for a negative value + filled_count = interior.count("=") + assert filled_count == 0, ( + f"Negative current should produce 0 filled segments, got {filled_count}: {bar!r}" + ) + # Bar width must be exactly 10 + assert len(interior) == 10 + + +# --------------------------------------------------------------------------- +# T3-4: RP formula label +# --------------------------------------------------------------------------- + + +class TestRPFormulaLabel: + """ + T3-4: Verify that relief pitchers (card_type="rp") show the "IP+K" formula + label in format_refractor_entry output. + + Why: FORMULA_LABELS maps both "sp" and "rp" to "IP+K". The existing test + suite only verifies "sp" (via the sp_state fixture). Adding "rp" explicitly + prevents a future refactor from accidentally giving RPs a different label + or falling through to the raw card_type fallback. + """ + + def test_rp_formula_label_is_ip_plus_k(self): + """ + A card with card_type="rp" must show "IP+K" as the formula label + in its progress line. + """ + rp_state = { + "player_name": "Edwin Diaz", + "card_type": "rp", + "current_tier": 1, + "formula_value": 45, + "next_threshold": 60, + } + result = format_refractor_entry(rp_state) + assert "IP+K" in result, ( + f"Relief pitcher card should show 'IP+K' formula label, got: {result!r}" + ) + + +# --------------------------------------------------------------------------- +# T3-5: Unknown card_type fallback +# --------------------------------------------------------------------------- + + +class TestUnknownCardTypeFallback: + """ + T3-5: format_refractor_entry should use the raw card_type string as the + formula label when the type is not in FORMULA_LABELS, rather than crashing. + + Why: FORMULA_LABELS only covers "batter", "sp", and "rp". If the API + introduces a new card type (e.g. "util" for utility players) before the + bot is updated, FORMULA_LABELS.get(card_type, card_type) will fall back to + the raw string. This test ensures that fallback path produces readable + output rather than an error, and explicitly documents what to expect. + """ + + def test_unknown_card_type_uses_raw_string_as_label(self): + """ + card_type="util" is not in FORMULA_LABELS. The output should include + "util" as the formula label (the raw fallback) and must not raise. + """ + util_state = { + "player_name": "Ben Zobrist", + "card_type": "util", + "current_tier": 2, + "formula_value": 80, + "next_threshold": 120, + } + result = format_refractor_entry(util_state) + assert "util" in result, ( + f"Unknown card_type should appear verbatim as the formula label, got: {result!r}" + ) + + def test_unknown_card_type_does_not_crash(self): + """ + Any unknown card_type must produce a valid two-line string without + raising an exception. + """ + state = { + "player_name": "Test Player", + "card_type": "dh", + "current_tier": 1, + "formula_value": 30, + "next_threshold": 50, + } + result = format_refractor_entry(state) + assert isinstance(result, str) + assert len(result.split("\n")) == 2 + + +# --------------------------------------------------------------------------- +# Slash command: empty roster / no-team scenarios +# --------------------------------------------------------------------------- + + @pytest.mark.asyncio async def test_refractor_status_no_team(mock_bot, mock_interaction): """ diff --git a/tests/test_refractor_notifs.py b/tests/test_refractor_notifs.py index 737ad4a..688ff13 100644 --- a/tests/test_refractor_notifs.py +++ b/tests/test_refractor_notifs.py @@ -257,3 +257,92 @@ class TestNotifyTierCompletion: for event in tier_up_events: await notify_tier_completion(channel, event) channel.send.assert_not_called() + + +# --------------------------------------------------------------------------- +# T1-5: tier_up dict shape mismatch — WP-14 integration blocker +# --------------------------------------------------------------------------- + + +class TestTierUpDictShapeMismatch: + """ + T1-5: Expose the latent integration bug where the post-game hook passes a + minimal tier_up dict (only player_id, old_tier, new_tier) but + build_tier_up_embed expects player_name, old_tier, new_tier, track_name, + and current_value. + + Why this matters: the hook test (test_complete_game_hook.py) confirms the + plumbing forwards tier_up dicts from the API response to notify_tier_completion. + However, the real API response may omit player_name/track_name. If + build_tier_up_embed does a bare dict access (tier_up["player_name"]) without + a fallback, it will raise KeyError in production. This test documents the + current behaviour (crash vs. graceful degradation) so WP-14 implementers + know to either harden the embed builder or ensure the API always returns + the full shape. + """ + + def test_minimal_stub_shape_raises_key_error(self): + """ + Calling build_tier_up_embed with only {player_id, old_tier, new_tier} + (the minimal shape used by the post-game hook stub) raises KeyError + because player_name and track_name are accessed via bare dict lookup. + + This is the latent bug: the hook passes stub-shaped dicts but the embed + builder expects the full notification shape. WP-14 must ensure either + (a) the API returns the full shape or (b) build_tier_up_embed degrades + gracefully with .get() fallbacks. + """ + minimal_stub = { + "player_id": 101, + "old_tier": 1, + "new_tier": 2, + } + # Document that this raises — it's the bug we're exposing, not a passing test. + with pytest.raises(KeyError): + build_tier_up_embed(minimal_stub) + + def test_full_shape_does_not_raise(self): + """ + Confirm that supplying the full expected shape (player_name, old_tier, + new_tier, track_name, current_value) does NOT raise, establishing the + correct contract for callers. + """ + full_shape = make_tier_up( + player_name="Mike Trout", + old_tier=1, + new_tier=2, + track_name="Batter", + current_value=150, + ) + # Must not raise + embed = build_tier_up_embed(full_shape) + assert embed is not None + + +# --------------------------------------------------------------------------- +# T2-7: notify_tier_completion with None channel +# --------------------------------------------------------------------------- + + +class TestNotifyTierCompletionNoneChannel: + """ + T2-7: notify_tier_completion must not propagate exceptions when the channel + is None. + + Why: the post-game hook may call notify_tier_completion before a valid + channel is resolved (e.g. in tests, or if the scoreboard channel lookup + fails). The try/except in notify_tier_completion should catch the + AttributeError from None.send() so game flow is never interrupted. + """ + + @pytest.mark.asyncio + async def test_none_channel_does_not_raise(self): + """ + Passing None as the channel argument must not raise. + + None.send() raises AttributeError; the try/except in + notify_tier_completion is expected to absorb it silently. + """ + tier_up = make_tier_up(new_tier=2) + # Should not raise regardless of channel being None + await notify_tier_completion(None, tier_up)