don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

NEO-32: defer review suggestion #1 (integration test AAA)

pull/59/head
VinPropane 2026-04-30 22:04:11 -04:00
parent 27daef70a7
commit cdf7c61ce4
1 changed files with 1 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/reviews/2026-04-30-NEO-32.md
View File

@ -33,7 +33,7 @@ The branch delivers the planned NEO-32 slice: authoritative in-memory per-player
## Suggestions ## Suggestions
1. **C# AAA layout on new integration tests** — `CooldownSnapshotApiTests.GetCooldownSnapshot_ShouldExposeSlotEnd_WhenCastAccepted_ThenClear_WhenAdvancedPastDuration` and the two new cooldown tests in `AbilityCastApiTests` bundle HTTP calls and `FakeClock.Advance` inside a single `// Act` block. Per [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), split so **Act** is only the invocations under assertion (or use clearly separated Act steps per phase) and keep clock advances in Arrange or interleaved with labeled sections. Low priority but keeps CI lint/review consistency for future edits. 1. ~~**C# AAA layout on new integration tests** — `CooldownSnapshotApiTests.GetCooldownSnapshot_ShouldExposeSlotEnd_WhenCastAccepted_ThenClear_WhenAdvancedPastDuration` and the two new cooldown tests in `AbilityCastApiTests` bundle HTTP calls and `FakeClock.Advance` inside a single `// Act` block. Per [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), split so **Act** is only the invocations under assertion (or use clearly separated Act steps per phase) and keep clock advances in Arrange or interleaved with labeled sections. Low priority but keeps CI lint/review consistency for future edits.~~ **Deferred (won’t do).** Scenario-style integration tests read better with one narrated timeline; strict per-call Act splits are optional here.
2. ~~**Stale cooldown entries in the in-memory store** — `TryGetCooldownEndUtc` does not prune expired ends; `BuildSnapshot` omits them from JSON, so wire behavior is correct. Expired rows linger until `IsOnCooldown` runs (e.g. next cast to that slot). For a long-lived prototype process this is negligible; if you ever extend GET-heavy paths, consider pruning in `TryGetCooldownEndUtc` or snapshot build for symmetry with `IsOnCooldown`.~~ **Done.** `TryGetCooldownEndUtc` now takes `DateTimeOffset now` and shares pruning with `IsOnCooldown`; `InMemoryPlayerAbilityCooldownStoreTests` covers prune-on-read. 2. ~~**Stale cooldown entries in the in-memory store** — `TryGetCooldownEndUtc` does not prune expired ends; `BuildSnapshot` omits them from JSON, so wire behavior is correct. Expired rows linger until `IsOnCooldown` runs (e.g. next cast to that slot). For a long-lived prototype process this is negligible; if you ever extend GET-heavy paths, consider pruning in `TryGetCooldownEndUtc` or snapshot build for symmetry with `IsOnCooldown`.~~ **Done.** `TryGetCooldownEndUtc` now takes `DateTimeOffset now` and shares pruning with `IsOnCooldown`; `InMemoryPlayerAbilityCooldownStoreTests` covers prune-on-read.