Add comprehensive tests for Player model functionality by No-bodyq · Pull Request #195 · Prometheus-A/The-Velvet-Ace

No-bodyq · 2025-07-27T21:17:23Z

Description

This PR introduces a comprehensive suite of unit tests for the playerImplementation

Related Issues

closes #98

Changes Made

Added helper function: mock_poker_game for simulating an active game state
Added helper function: mock_allowable_game for simulating a joinable game state
Implemented test: test_exit_with_out_true_succeeds
Implemented test: test_exit_with_out_false_succeeds
Implemented test: test_exit_without_lock_fails
Implemented test: test_exit_with_splitted_showdown_returns_locked_chips
Implemented test: test_exit_with_zero_player_count_fails
Implemented test: test_exit_with_invalid_player_fails
Implemented test: test_enter_succeeds_new_player

How to Test

Run the tests using dojo:

sozo test

Checklist

My code follows the project's coding style.
I have tested these changes locally.
Documentation has been updated where necessary.

No-bodyq · 2025-07-28T17:23:46Z

Ready for review @Birdmannn

Birdmannn

Hey champ @No-bodyq, please check this out 👀

Birdmannn · 2025-07-29T13:47:14Z

poker-texas-hold-em/contract/src/models/player.cairo

+    fn PLAYER_1() -> ContractAddress {
+        starknet::contract_address_const::<'PLAYER_1'>()
+    }
+
+    fn PLAYER_2() -> ContractAddress {
+        starknet::contract_address_const::<'PLAYER_2'>()
+    }
+
+    fn PLAYER_3() -> ContractAddress {
+        starknet::contract_address_const::<'PLAYER_3'>()
+    }
+
+    fn PLAYER_4() -> ContractAddress {
+        starknet::contract_address_const::<'PLAYER_4'>()
+    }
+
+    fn mock_poker_game(ref world: WorldStorage) {
+        let game = Game {
+            id: 1,
+            in_progress: true,
+            has_ended: false,
+            current_round: 1,
+            round_in_progress: true,
+            current_player_count: 2,
+            players: array![PLAYER_1(), PLAYER_2(), PLAYER_3()],


This code is also in test_actions.cairo. make all functions public there, then import from there please

Birdmannn · 2025-07-29T13:47:36Z

poker-texas-hold-em/contract/src/models/player.cairo

+            deck: array![],
+            next_player: Option::Some(PLAYER_1()),
+            community_cards: array![],
+            pots: array![0],
+            current_bet: 0,
+            params: get_default_game_params(),
+            reshuffled: 0,
+            should_end: false,
+            deck_root: 0,
+            dealt_cards_root: 0,
+            nonce: 0,
+            community_dealing: false,
+            showdown: false,
+            round_count: 0,
+            highest_staker: Option::None,
+            previous_offset: 0,
+        };
+
+        let player_1 = Player {
+            id: PLAYER_1(),
+            alias: 'dub_zn',
+            chips: 2000,
+            current_bet: 0,
+            total_rounds: 1,
+            locked: (true, 1),
+            is_dealer: false,
+            in_round: true,
+            out: (0, 0),
+            pub_key: 0x1,
+            locked_chips: 0,
+            is_blacklisted: false,
+            eligible_pots: 1,


This too.. and it goes down

Birdmannn · 2025-07-29T13:48:12Z

poker-texas-hold-em/contract/src/models/player.cairo

+        };
+
+        let player_2 = Player {
+            id: PLAYER_2(),
+            alias: 'Birdmannn',
+            chips: 5000,
+            current_bet: 0,
+            total_rounds: 1,
+            locked: (true, 2),
+            is_dealer: false,
+            in_round: true,
+            out: (0, 0),
+            pub_key: 0x2,
+            locked_chips: 0,
+            is_blacklisted: false,
+            eligible_pots: 1,
+        };
+
+        let player_3 = Player {
+            id: PLAYER_3(),
+            alias: 'chiscookeke11',
+            chips: 5000,
+            current_bet: 0,
+            total_rounds: 1,
+            locked: (false, 1),
+            is_dealer: false,
+            in_round: true,
+            out: (0, 0),


this too.. it still goes down

Birdmannn · 2025-07-29T13:49:15Z

poker-texas-hold-em/contract/src/models/player.cairo

+            pub_key: 0x3,
+            locked_chips: 0,
+            is_blacklisted: false,
+            eligible_pots: 1,
+        };
+
+        world.write_model(@game);
+        world.write_models(array![@player_1, @player_2, @player_3].span());
+    }


Yes, I think for the first part of your refactor, it'll end here 💯

Birdmannn · 2025-07-29T13:55:29Z

poker-texas-hold-em/contract/src/models/player.cairo

+        world.write_models(array![@player_1, @player_2, @player_3].span());
+    }
+
+    fn mock_allowable_game(ref world: WorldStorage) {


from here Line167 to Line322, all new mocks would be to refactor the original mock function to take in parameters that target specific fields, and reuse this function.

SO basically all functions mocking the player state must be merged into one player function that takes in the specific parameters you wish to mock.
For all functions mocking the game state, then you must refactor the original function in test_actions.cairo to take in the specific mock parameters and adjust the tests there to pass.

Please, I hope you understand.

Birdmannn · 2025-07-29T14:06:50Z

poker-texas-hold-em/contract/src/models/player.cairo

+        };
+
+        world.write_model(@game);
+    }


…ed readability and maintainability

No-bodyq · 2025-07-30T10:40:27Z

all done ser @Birdmannn

Birdmannn · 2025-07-31T21:23:49Z

all done ser @Birdmannn

Just to ask.... you normally use copilot to assist you, right? @No-bodyq

No-bodyq · 2025-08-01T07:58:59Z

all done ser @Birdmannn

Just to ask.... you normally use copilot to assist you, right? @No-bodyq

Yes ser, why? @Birdmannn

Birdmannn · 2025-08-01T09:13:47Z

Yes ser, why? @Birdmannn

@No-bodyq Your code is bloated. Too much lines of code for little matters. And it causes an overhead during review when I want to optimize those lines but you and copilot are not helping matters.

I'll review this code one more time, and I'll show you something...
Maybe if you had worked with Oleh, MSGhais and GianM, you might've understood.

No-bodyq · 2025-08-01T09:33:02Z

Okay ser, let me know what I should change and I'll do it manually
I apologise

Birdmannn

Very good @No-bodyq. One last set of adjustments, if you do it well, we merge in 1. Arigato. 🙏🏻

Birdmannn · 2025-08-03T07:11:27Z

poker-texas-hold-em/contract/src/models/player.cairo

+            false,
+            true,
+            (0, 0),
+            0x3,
+            0,
+            false,
+            1,


For example, something like this is repeated. including Line87 and Line88. Don't bother about the public key, you never tested it. This is what I wanted to show you. In the mock_player function you created, what should enter the params are the variables that are actually tested and that they vary for each player

So your function would resemble something like this

fn mock_player(id: ContractAddress, alias: felt252, chips: u256, out: (bool, u16)) -> Player { let mut player: Player = Default::default(); player.id = id; player.chips = chips; player.alias = alias; player.out = out; player }

Implement this type of stuff too when mocking the game.
You see how neat and easy to read that is? That's how it should probably look... I didn't say that is the exact function... I'm not sure if I omitted some fields or parameters

Birdmannn · 2025-08-03T07:19:03Z

poker-texas-hold-em/contract/src/models/player.cairo

+        mock_poker_game_flex(
+            ref world,
+            2,
+            true,
+            false,
+            1,
+            false,
+            2,
+            array![PLAYER_1(), PLAYER_2(), PLAYER_3()],
+            array![],
+            Option::Some(PLAYER_1()),
+            array![],
+            array![0],
+            0,
+            get_default_game_params(),
+            0,
+            false,
+            0,
+            0,
+            0,
+            false,
+            false,
+            0,
+            Option::None,
+            0,
+            array![player_1, player_2, player_3],
+        );
+    }


do the same with this function too. It takes in too many default fields... and yes, the pots might not be passed in as a parameter, you can just set the game.pots = array![0] since that is always the case for the pots, and the game.pots must be set like this inside the function because Default::default() makes it an empty array which is not a valid pot.

Birdmannn · 2025-08-03T07:25:39Z

poker-texas-hold-em/contract/src/models/player.cairo

+
+
+    #[test]
+    fn test_exit_with_out_true_succeeds() {


Birdmannn · 2025-08-03T07:25:54Z

poker-texas-hold-em/contract/src/models/player.cairo

+    }
+
+    #[test]
+    fn test_exit_with_out_false_succeeds() {


Birdmannn · 2025-08-03T07:38:34Z

poker-texas-hold-em/contract/src/models/player.cairo

+        let (mut world, _) = deploy_contracts(array![CoreContract::Actions]);
+        mock_poker_game_flex(
+            ref world,
+            3,


by the way, if the id is not useful in mocking, you can default it to 1 in the mock function, and remove it as a parameter in the function's signature

No-bodyq · 2025-08-03T19:13:26Z

I think we finally have it ser @Birdmannn

Birdmannn · 2025-08-06T04:14:52Z

Trés bien, Mister Asher @No-bodyq. Thanks for keeping up with this kind of review, and yes, thank you for your contribution.🫡. Ohayo 🧘🏻‍♂️

No-bodyq · 2025-08-06T06:20:31Z

Thank you ser @Birdmannn

No-bodyq added 2 commits July 27, 2025 22:13

Add comprehensive tests for Player model functionality

dafbb1b

Add tests for player model and game mechanics

cec47d7

No-bodyq marked this pull request as ready for review July 28, 2025 17:17

Birdmannn self-requested a review July 29, 2025 13:30

Birdmannn reviewed Jul 29, 2025

View reviewed changes

No-bodyq added 2 commits July 30, 2025 11:36

Refactor player model tests to use flexible mock functions for improv…

1860bcc

…ed readability and maintainability

fmt

06a7b6b

No-bodyq requested a review from Birdmannn July 30, 2025 10:40

No-bodyq added 2 commits August 2, 2025 10:40

Refactor player model tests

1838e91

Refactor player model tests

2095295

Birdmannn reviewed Aug 3, 2025

View reviewed changes

No-bodyq added 2 commits August 3, 2025 18:40

Merge branch 'Prometheus-A:v1' into v1

786d787

Refactor player model tests

a55faac

No-bodyq requested a review from Birdmannn August 3, 2025 19:12

Birdmannn merged commit 4d5af0c into Prometheus-A:v1 Aug 6, 2025
1 check passed

+                          false,
+                          true,
+                          (0, 0),
+x3,
+,
+                          false,
+,

Conversation

No-bodyq commented Jul 27, 2025

Description

Related Issues

Changes Made

How to Test

Checklist

Uh oh!

No-bodyq commented Jul 28, 2025

Uh oh!

Birdmannn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

No-bodyq commented Jul 30, 2025

Uh oh!

Birdmannn commented Jul 31, 2025

Uh oh!

No-bodyq commented Aug 1, 2025

Uh oh!

Birdmannn commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

No-bodyq commented Aug 1, 2025

Uh oh!

Birdmannn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

No-bodyq commented Aug 3, 2025

Uh oh!

Birdmannn commented Aug 6, 2025

Uh oh!

Uh oh!

No-bodyq commented Aug 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Birdmannn commented Aug 1, 2025 •

edited

Loading