test: Add automated tests for gambling/wheel of snacks functionality - #292
test: Add automated tests for gambling/wheel of snacks functionality#2920nly-Fir3 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds automated tests for the gambling/wheel of snacks functionality in the GuiApp. The tests focus on navigation to the gambling screen under various conditions such as different credit levels, snack availability, and user scenarios.
Key Changes:
- Adds a new test file
test_gambling.pywith 8 test cases covering gambling screen navigation - Tests include validation for minimum snack requirements, back navigation, screen refresh behavior, and multi-user scenarios
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Verify screen loaded properly | ||
| wheel_screen = app.screenManager.current_screen | ||
| assert wheel_screen is not None |
There was a problem hiding this comment.
The assertion on line 178 "assert wheel_screen is not None" is redundant because if wheel_screen were None, the code would have already failed when accessing current_screen. Consider either removing this assertion or adding more meaningful checks, such as verifying specific properties of the wheel screen or the loaded snacks.
| assert wheel_screen is not None |
| async def test_gambling_with_insufficient_credits(app_with_users_and_snacks): | ||
| """Test that gambling works even with low credits""" |
There was a problem hiding this comment.
The test docstring says "gambling works even with low credits" but the test name suggests it's testing "insufficient credits". This test only verifies navigation to the wheel screen, not the actual spinning behavior with low credits. Consider either renaming the test to "test_gambling_navigation_with_low_credits" or expanding it to test what happens when attempting to spin with insufficient funds.
| async def test_gambling_screen_loads_snacks(app_with_users_and_snacks): | ||
| """Test that gambling screen loads available snacks""" | ||
| app = app_with_users_and_snacks | ||
|
|
||
| # Get user | ||
| user = app.screenManager.database.getPatronByEmployeeId(987654321) | ||
| app.screenManager.database.addCredits(user.patronId, 100) | ||
| app.screenManager.setCurrentPatron(user) | ||
|
|
||
| # Navigate to gambling screen | ||
| app.screenManager.current = "mainUserPage" | ||
| await asyncio.sleep(0.1) | ||
| app.screenManager.current_screen.ids.gambleOption.dispatch("on_release") | ||
| await asyncio.sleep(0.2) | ||
|
|
||
| # Verify screen loaded | ||
| assert app.screenManager.current == "wheelOfSnacksScreen" | ||
|
|
||
| # Verify screen loaded properly | ||
| wheel_screen = app.screenManager.current_screen | ||
| assert wheel_screen is not None |
There was a problem hiding this comment.
The test name and docstring say it tests "loads available snacks" but there are no assertions that actually verify snacks were loaded. The test only checks if the screen exists. Consider adding assertions that verify the wheel widget has the expected snacks loaded or that the win table is populated correctly with snack data.
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_gambling_with_insufficient_credits(app_with_users_and_snacks): |
There was a problem hiding this comment.
The comment in this test is misleading. The test name suggests it's checking that gambling is "blocked" with insufficient credits, but the test actually verifies that gambling works even with low credits (as stated in the docstring). Either the test name should be changed to match the behavior (e.g., "test_gambling_with_low_credits") or the test should verify that gambling is blocked.
| async def test_gambling_with_insufficient_credits(app_with_users_and_snacks): | |
| async def test_gambling_with_low_credits(app_with_users_and_snacks): |
| # Click gamble option button | ||
| main_screen = app.screenManager.current_screen | ||
| main_screen.ids.gambleOption.dispatch("on_release") | ||
| await asyncio.sleep(0.2) |
There was a problem hiding this comment.
The asyncio.sleep durations are inconsistent across tests without clear reasoning. This test uses 0.2 seconds (lines 46, 69, 95, 171) while most other tests use 0.1 seconds. Consider standardizing sleep durations unless there's a specific reason for the variation, as this can make tests unpredictable and harder to maintain.
| # Get user and set as current patron | ||
| user = app.screenManager.database.getPatronByEmployeeId(987654321) | ||
| app.screenManager.database.addCredits(user.patronId, 100) | ||
| app.screenManager.setCurrentPatron(user) |
There was a problem hiding this comment.
There's significant code duplication across multiple tests for the setup phase (getting user, adding credits, setting current patron, navigating to main screen). Consider extracting this into a helper function or additional fixture to improve maintainability and reduce repetition.
Summary
Fixes #274
Adds automated tests for gambling/wheel of snacks functionality.