From 31bdec7b79cff2ed684495823a7d702899eae5d0 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Tue, 30 Jun 2026 15:29:38 -0400 Subject: [PATCH] cleaned up comments from tests --- .../counter_command_auth_test.cpp | 45 ++----------------- .../new_counter_id_test.cpp | 16 +++---- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/tests/command_zone_tests/counter_command_auth_test.cpp b/tests/command_zone_tests/counter_command_auth_test.cpp index 9dba0763b..bf61156b9 100644 --- a/tests/command_zone_tests/counter_command_auth_test.cpp +++ b/tests/command_zone_tests/counter_command_auth_test.cpp @@ -1,8 +1,7 @@ -// Unit tests for the pure authorization logic extracted from the counter command -// handlers. Server_Player::evaluateDelCounter(), evaluateSetCounterActive() and -// evaluateModifyCounter() are static and depend only on their arguments, so each -// guard branch can be exercised directly without a started game, network stack, or -// player fixture. +/** @file counter_command_auth_test.cpp + * @brief Tests for Server_Player counter-command authorization helpers. + * @ingroup Tests + */ #include "game/server_counter.h" #include "game/server_player.h" @@ -17,22 +16,16 @@ RNG_Abstract *rng = nullptr; // required by linked server code namespace { -// A user-range counter id that is never a reserved tax counter. constexpr int UserCounterId = CounterIds::FirstUserId; -// Builds a counter with the given id and current value; the other fields are -// irrelevant to the authorization decisions under test. Server_Counter makeCounter(int id, int count) { return Server_Counter(id, "c", color(), 20, count); } } // namespace -// --------------------------------------------------------------------------- // evaluateDelCounter -// --------------------------------------------------------------------------- -// The game must be started before any counter can be deleted. TEST(EvaluateDelCounter, RejectsWhenGameNotStarted) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -41,7 +34,6 @@ TEST(EvaluateDelCounter, RejectsWhenGameNotStarted) Response::RespGameNotStarted); } -// A conceded player may no longer mutate counters. TEST(EvaluateDelCounter, RejectsWhenPlayerConceded) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -49,7 +41,6 @@ TEST(EvaluateDelCounter, RejectsWhenPlayerConceded) Response::RespContextError); } -// Reserved tax counters are server-managed and may never be deleted by a client. TEST(EvaluateDelCounter, RejectsTaxCounters) { Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0); @@ -61,21 +52,17 @@ TEST(EvaluateDelCounter, RejectsTaxCounters) Response::RespFunctionNotAllowed); } -// A non-existent counter (null lookup) is reported as not found. TEST(EvaluateDelCounter, RejectsMissingCounter) { EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, nullptr), Response::RespNameNotFound); } -// A live user counter in a started, non-conceded game may be deleted. TEST(EvaluateDelCounter, AllowsDeletingUserCounter) { Server_Counter counter = makeCounter(UserCounterId, 7); EXPECT_EQ(Server_Player::evaluateDelCounter(true, false, UserCounterId, &counter), Response::RespOk); } -// The game-not-started guard is checked before the tax-counter guard: an attempt -// to delete a tax counter before the game starts reports RespGameNotStarted. TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard) { Server_Counter commander = makeCounter(CounterIds::CommanderTax, 0); @@ -83,11 +70,8 @@ TEST(EvaluateDelCounter, GameNotStartedTakesPrecedenceOverTaxGuard) Response::RespGameNotStarted); } -// --------------------------------------------------------------------------- // evaluateSetCounterActive -// --------------------------------------------------------------------------- -// The game must be started before a counter's active state can change. TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); @@ -97,7 +81,6 @@ TEST(EvaluateSetCounterActive, RejectsWhenGameNotStarted) Response::RespGameNotStarted); } -// A conceded player may no longer toggle counters. TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); @@ -106,8 +89,6 @@ TEST(EvaluateSetCounterActive, RejectsWhenPlayerConceded) Response::RespContextError); } -// Only the reserved tax counters support an active/inactive toggle; user counters -// are rejected as a disallowed operation. TEST(EvaluateSetCounterActive, RejectsNonTaxCounter) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -115,7 +96,6 @@ TEST(EvaluateSetCounterActive, RejectsNonTaxCounter) Response::RespFunctionNotAllowed); } -// Toggling a tax counter is only meaningful when the command zone is enabled. TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); @@ -124,15 +104,12 @@ TEST(EvaluateSetCounterActive, RejectsWhenCommandZoneDisabled) Response::RespContextError); } -// A missing counter is reported as not found. TEST(EvaluateSetCounterActive, RejectsMissingCounter) { EXPECT_EQ(Server_Player::evaluateSetCounterActive(true, false, true, CounterIds::CommanderTax, nullptr, true), Response::RespNameNotFound); } -// A tax counter may not be disabled while it still holds accumulated tax; the -// player must reset it to zero first. TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3); @@ -141,7 +118,6 @@ TEST(EvaluateSetCounterActive, RejectsDisablingWhenTaxAccumulated) Response::RespContextError); } -// Enabling a tax counter is allowed regardless of its current value. TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 3); @@ -150,7 +126,6 @@ TEST(EvaluateSetCounterActive, AllowsEnablingWithAccumulatedTax) Response::RespOk); } -// Disabling a tax counter that is already at zero is permitted. TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); @@ -159,8 +134,6 @@ TEST(EvaluateSetCounterActive, AllowsDisablingWhenCounterIsZero) Response::RespOk); } -// PartnerTax is the second reserved tax counter and must flow through the same auth -// path as CommanderTax: enabling is always permitted... TEST(EvaluateSetCounterActive, AllowsEnablingPartnerTax) { Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0); @@ -169,7 +142,6 @@ TEST(EvaluateSetCounterActive, AllowsEnablingPartnerTax) Response::RespOk); } -// ...and disabling is rejected while it still holds accumulated tax. TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated) { Server_Counter counter = makeCounter(CounterIds::PartnerTax, 2); @@ -178,11 +150,8 @@ TEST(EvaluateSetCounterActive, RejectsDisablingPartnerTaxWhenAccumulated) Response::RespContextError); } -// --------------------------------------------------------------------------- // evaluateModifyCounter (shared by cmdIncCounter / cmdSetCounter) -// --------------------------------------------------------------------------- -// No counter may be modified before the game starts. TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -191,7 +160,6 @@ TEST(EvaluateModifyCounter, RejectsWhenGameNotStarted) Response::RespGameNotStarted); } -// A conceded player may no longer modify counters. TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -199,7 +167,6 @@ TEST(EvaluateModifyCounter, RejectsWhenPlayerConceded) Response::RespContextError); } -// An ordinary user counter can be modified regardless of command-zone state. TEST(EvaluateModifyCounter, AllowsUserCounter) { Server_Counter counter = makeCounter(UserCounterId, 0); @@ -207,14 +174,12 @@ TEST(EvaluateModifyCounter, AllowsUserCounter) Response::RespOk); } -// A non-existent counter is reported as not found. TEST(EvaluateModifyCounter, RejectsMissingCounter) { EXPECT_EQ(Server_Player::evaluateModifyCounter(true, false, true, UserCounterId, nullptr), Response::RespNameNotFound); } -// A tax counter may not be modified outside a Commander game (command zone disabled). TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); @@ -223,7 +188,6 @@ TEST(EvaluateModifyCounter, RejectsTaxCounterWhenCommandZoneDisabled) Response::RespContextError); } -// An inactive (hidden) tax counter must not accumulate value behind the scenes. TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter) { Server_Counter counter = makeCounter(CounterIds::PartnerTax, 0); @@ -233,7 +197,6 @@ TEST(EvaluateModifyCounter, RejectsInactiveTaxCounter) Response::RespContextError); } -// An active tax counter in a Commander game may be modified. TEST(EvaluateModifyCounter, AllowsActiveTaxCounter) { Server_Counter counter = makeCounter(CounterIds::CommanderTax, 0); diff --git a/tests/command_zone_tests/new_counter_id_test.cpp b/tests/command_zone_tests/new_counter_id_test.cpp index 210cf5e39..ddf627983 100644 --- a/tests/command_zone_tests/new_counter_id_test.cpp +++ b/tests/command_zone_tests/new_counter_id_test.cpp @@ -1,3 +1,8 @@ +/** @file new_counter_id_test.cpp + * @brief Tests for Server_Player::newCounterId() id allocation. + * @ingroup Tests + */ + #include "../movecard_tests/server_test_helpers.h" #include "game/server_counter.h" #include "game/server_game.h" @@ -14,10 +19,6 @@ RNG_Abstract *rng = nullptr; // required by linked server code namespace { -// Wires up a Server_Player backed by a minimal fake game so that counters can be -// added directly via addCounter() to exercise newCounterId() in isolation. -// newCounterId() depends only on the player's counter map, not on game state, -// so the game never needs to be started. struct PlayerFixture { ServerInfo_User user; @@ -34,17 +35,12 @@ struct PlayerFixture }; } // namespace -// With no counters at all, the first allocated id must be FirstUserId, never 0. TEST(NewCounterId, ReturnsFirstUserIdWhenNoCounters) { PlayerFixture f; EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId); } -// Reserved ids must not drag a new user counter down into the reserved range. The -// ids here (3 and 5) are chosen so a naive "highest id + 1" implementation would -// return 6 (inside the reserved range) and fail this test; only the FirstUserId -// floor yields the correct 10, so this pins the floor against regression. TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist) { PlayerFixture f; @@ -53,7 +49,6 @@ TEST(NewCounterId, SkipsReservedRangeWhenOnlyReservedCountersExist) EXPECT_EQ(f.player.newCounterId(), CounterIds::FirstUserId); } -// Once user counters exist, the next id is one above the highest of them. TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter) { PlayerFixture f; @@ -62,7 +57,6 @@ TEST(NewCounterId, ReturnsNextIdAboveHighestUserCounter) EXPECT_EQ(f.player.newCounterId(), 16); } -// A reserved counter present alongside user counters must not affect the result. TEST(NewCounterId, IgnoresReservedCountersWhenUserCountersPresent) { PlayerFixture f;