test: two tavern gamble tests re-implement the payout instead of calling gamble(), and encode the wrong multiplier

[dmccoystephenson]

Summary

During a triage pass, two tests in tests/location/test_tavern.py were found to perform the arithmetic they then assert on, without ever invoking the code under test.

tests/location/test_tavern.py:248-295 (test_gamble_win_shows_correct_amount) sets up a mock, abandons it, and hand-executes the win branch:

# We need to capture the state after the win but before the next iteration
# Let's test the win logic directly instead
input_value = 1
tavernInstance.diceThrow = 1

# Execute the win condition logic
winAmount = tavernInstance.currentBet
tavernInstance.player.money += tavernInstance.currentBet
tavernInstance.stats.totalMoneyMade += tavernInstance.currentBet
...
assert tavernInstance.player.money == 150  # Won 50

tests/location/test_tavern.py:298-321 (test_gamble_loss) has the same shape.

Replacing Tavern.gamble with a method that raises, then running the four gamble tests:

test_gamble_win_shows_correct_amount: PASSED with gamble() removed  <-- tests nothing
test_gamble_loss: PASSED with gamble() removed  <-- tests nothing
test_gamble_win_pays_multiple_of_bet: correctly failed -> gamble() was called
test_gamble_loss_via_real_loop: correctly failed -> gamble() was called

Why it matters

Beyond testing nothing, the hand-written arithmetic is wrong. test_gamble_win_shows_correct_amount pays even money (money += currentBet, asserting money == 150 on a $50 bet), while src/location/tavern.py:238 pays five times the stake:

winAmount = self.currentBet * DICE_WIN_MULTIPLIER

with DICE_WIN_MULTIPLIER = 5 (src/location/tavern.py:20). The real payout on that setup is $350, not $150.

So the suite contains a test named "shows correct amount" that asserts the payout the game stopped using when #126 was fixed, and it cannot fail — a future change to DICE_WIN_MULTIPLIER, or to the win branch entirely, leaves it green. It reads as coverage of the gambling payout while providing none, which is worse than an acknowledged gap.

Genuine coverage does exist alongside them (test_gamble_win_pays_multiple_of_bet:492 and test_gamble_loss_via_real_loop:512, both of which drive the real loop through showOptions and correctly fail when gamble() is removed), so the two hand-rolled tests are redundant as well as misleading.

Suggested fix

Delete both tests. test_gamble_win_pays_multiple_of_bet and test_gamble_loss_via_real_loop already cover the same branches properly and are written in the pattern the rest of the file uses (showOptions = MagicMock(side_effect=[...]) driving the real loop).

If the prompt-text assertions in test_gamble_win_shows_correct_amount are worth keeping — "You won $50!" rather than "You won $0!", which looks like a deliberate regression guard — move them into test_gamble_win_pays_multiple_of_bet and derive the expected figure from DICE_WIN_MULTIPLIER rather than hardcoding it.

Worth checking for the same shape elsewhere while in here: a test that assigns to the object and then asserts its own assignment will pass no matter what the source does.

Filed by Claude during an automated triage pass; claims above were verified against source.