NEO-86: Address code review test gaps and doc alignment.
Promote E5.M1 to Ready in alignment register, fix NEO-85 README link, add schema-mismatch and deny-path GdUnit cases; strike review suggestions.pull/122/head
parent
d5fc648dd6
commit
8d4c048bbe
|
|
@ -164,7 +164,7 @@ Epic 5 Slice 1 capstone — tab-target lock, cast, defeat, and gig XP visibility
|
|||
| Deny | **1** on defeated target | **`ability_cast_denied: target_defeated`** |
|
||||
| Clear | **Esc** | HP label **`— (no lock)`** |
|
||||
|
||||
**Cross-links:** [NEO-23](../docs/plans/NEO-23-implementation-plan.md) targeting · [NEO-28](../docs/manual-qa/NEO-28.md) cast deny HUD · [NEO-31](../docs/manual-qa/NEO-31.md) cast POST · [NEO-85](NEO-85.md) combat feedback + HP · [NEO-44](../docs/manual-qa/NEO-44.md) server gig grant.
|
||||
**Cross-links:** [NEO-23](../docs/plans/NEO-23-implementation-plan.md) targeting · [NEO-28](../docs/manual-qa/NEO-28.md) cast deny HUD · [NEO-31](../docs/manual-qa/NEO-31.md) cast POST · [NEO-85](../docs/manual-qa/NEO-85.md) combat feedback + HP · [NEO-44](../docs/manual-qa/NEO-44.md) server gig grant.
|
||||
|
||||
**Scripts:** `ability_cast_client.gd`, `combat_targets_client.gd`, `gig_progression_client.gd` — wired from `main.gd`.
|
||||
|
||||
|
|
|
|||
|
|
@ -101,3 +101,24 @@ func test_non_defeat_cast_skips_gig_progression_sync() -> void:
|
|||
# Assert
|
||||
await assert_signal(harness._cast).is_emitted("cast_result_received", true, "", any())
|
||||
assert_that(harness.gig_sync_calls).is_equal(0)
|
||||
|
||||
|
||||
func test_denied_cast_skips_gig_progression_sync() -> void:
|
||||
# Arrange
|
||||
var transport := MockCastTransport.new()
|
||||
transport.body_json = (
|
||||
'{"schemaVersion":1,"accepted":false,"reasonCode":"target_defeated"}'
|
||||
)
|
||||
var harness := GigRefreshHarness.new()
|
||||
auto_free(transport)
|
||||
auto_free(harness)
|
||||
add_child(harness)
|
||||
harness.setup(self, transport)
|
||||
monitor_signals(harness._cast)
|
||||
# Act
|
||||
harness.request_cast()
|
||||
# Assert
|
||||
await assert_signal(harness._cast).is_emitted(
|
||||
"cast_result_received", false, "target_defeated", any()
|
||||
)
|
||||
assert_that(harness.gig_sync_calls).is_equal(0)
|
||||
|
|
|
|||
|
|
@ -92,3 +92,17 @@ func test_http_404_emits_progression_sync_failed() -> void:
|
|||
c.call("request_sync_from_server")
|
||||
# Assert
|
||||
await assert_signal(c).is_emitted("progression_sync_failed", "HTTP 404 (player unknown)")
|
||||
|
||||
|
||||
func test_invalid_schema_emits_progression_sync_failed() -> void:
|
||||
# Arrange
|
||||
var transport := MockHttpTransport.new()
|
||||
transport.body_json = '{"schemaVersion":2,"mainGigId":"breach","gigs":[]}'
|
||||
var c := _make_client(transport)
|
||||
monitor_signals(c)
|
||||
# Act
|
||||
c.call("request_sync_from_server")
|
||||
# Assert
|
||||
await assert_signal(c).is_emitted(
|
||||
"progression_sync_failed", "non-JSON body or schemaVersion mismatch"
|
||||
)
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -151,7 +151,7 @@ Run capstone QA on **`main`** wiring before gig client work; if any step fails,
|
|||
| Path | Change |
|
||||
|------|--------|
|
||||
| `client/test/gig_progression_client_test.gd` | **Add:** GET parse v1 with **`breach`** row; invalid schema → failure signal; 404 path. |
|
||||
| `client/test/gig_feedback_refresh_test.gd` | **Add:** cast accept with **`targetDefeated: true`** triggers **`GigProgressionClient.request_sync_from_server`**; deny / non-defeat accept does not. |
|
||||
| `client/test/gig_feedback_refresh_test.gd` | **Add:** cast accept with **`targetDefeated: true`** triggers sync; non-defeat accept and deny do not. |
|
||||
|
||||
**Manual verification:** [`docs/manual-qa/NEO-86.md`](manual-qa/NEO-86.md) — server + Godot; full defeat spine + gig XP visibility without curl.
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
**Date:** 2026-05-25
|
||||
**Scope:** Branch `NEO-86-playable-combat-capstone` · commits `8190614`–`b97cace` vs `origin/main`
|
||||
**Follow-up:** Code review suggestions 1–4 addressed (alignment **Ready**, README link, two GdUnit cases).
|
||||
**Base:** `origin/main`
|
||||
|
||||
## Verdict
|
||||
|
|
@ -20,9 +21,9 @@ NEO-86 closes **E5M1-12** and Epic 5 Slice 1 client acceptance: a capstone manua
|
|||
| [`docs/plans/E5M1-prototype-backlog.md`](../plans/E5M1-prototype-backlog.md) · **E5M1-12** | **Matches** — AC checked; landed note cites gig client, manual QA, README. |
|
||||
| [`docs/decomposition/modules/E5_M1_CombatRulesEngine.md`](../decomposition/modules/E5_M1_CombatRulesEngine.md) | **Matches** — Status **Ready**; **E5M1-12 (NEO-86)** slice added; Slice 1 client capstone complete. |
|
||||
| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E5.M1 note cites **NEO-86 landed** + capstone manual QA link. |
|
||||
| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) · **E5.M1** | **Partially matches** — row updated with NEO-86 landed + Slice 1 client capstone note, but **Status** column still **In Progress** while `E5_M1_CombatRulesEngine.md` header is **Ready** (see Suggestion 1). |
|
||||
| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) · **E5.M1** | **Matches** — row updated with NEO-86 landed + Slice 1 client capstone note; **Status** **Ready** (aligned with `E5_M1_CombatRulesEngine.md`). |
|
||||
| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — gig XP from server GET only; defeat refresh triggered by wire **`targetDefeated`**, not client-side XP math. |
|
||||
| [`client/README.md`](../../client/README.md) | **Partially matches** — end-to-end combat loop section and test scope list are good; **NEO-85** cross-link path is wrong (see Suggestion 2). |
|
||||
| [`client/README.md`](../../client/README.md) | **Matches** — end-to-end combat loop section and test scope list; NEO-85 cross-link uses `../docs/manual-qa/NEO-85.md`. |
|
||||
| [`docs/manual-qa/NEO-86.md`](../manual-qa/NEO-86.md) | **Matches** — single-session Godot script, combat math table, economy HUD toggle step, no Bruno/curl; acceptance boxes intentionally unchecked pre-human QA. |
|
||||
| Full-stack epic decomposition | **Matches** — client capstone with Godot manual QA; not Bruno-only prototype proof. |
|
||||
|
||||
|
|
@ -32,13 +33,13 @@ None.
|
|||
|
||||
## Suggestions
|
||||
|
||||
1. **E5.M1 alignment status vs module doc** — `E5_M1_CombatRulesEngine.md` header is **Ready** with E5M1-01–12 landed; `documentation_and_implementation_alignment.md` E5.M1 row still **In Progress**. Promote to **Ready** (or add a one-line note why server slice remains in progress) so register, module page, and alignment table stay consistent.
|
||||
1. ~~**E5.M1 alignment status vs module doc** — `E5_M1_CombatRulesEngine.md` header is **Ready** with E5M1-01–12 landed; `documentation_and_implementation_alignment.md` E5.M1 row still **In Progress**. Promote to **Ready** (or add a one-line note why server slice remains in progress) so register, module page, and alignment table stay consistent.~~ **Done.** E5.M1 **Status** column promoted to **Ready**.
|
||||
|
||||
2. **Broken NEO-85 README link** — In `client/README.md` § End-to-end combat loop, `[NEO-85](NEO-85.md)` resolves relative to `client/` and 404s. Use `[NEO-85](../docs/manual-qa/NEO-85.md)` like the NEO-28/NEO-31 links in the same line.
|
||||
2. ~~**Broken NEO-85 README link** — In `client/README.md` § End-to-end combat loop, `[NEO-85](NEO-85.md)` resolves relative to `client/` and 404s. Use `[NEO-85](../docs/manual-qa/NEO-85.md)` like the NEO-28/NEO-31 links in the same line.~~ **Done.**
|
||||
|
||||
3. **Schema-mismatch HTTP test gap** — Plan §Tests lists invalid schema → **`progression_sync_failed`** for `gig_progression_client_test.gd`; implementation covers parse unit test + 404 only. Add a mock 200 with `schemaVersion: 2` (or missing `mainGigId`) asserting **`progression_sync_failed`** with the schema mismatch reason — parity with `combat_targets_client_test.gd` **`test_invalid_schema_emits_sync_failed`**.
|
||||
3. ~~**Schema-mismatch HTTP test gap** — Plan §Tests lists invalid schema → **`progression_sync_failed`** for `gig_progression_client_test.gd`; implementation covers parse unit test + 404 only. Add a mock 200 with `schemaVersion: 2` (or missing `mainGigId`) asserting **`progression_sync_failed`** with the schema mismatch reason — parity with `combat_targets_client_test.gd` **`test_invalid_schema_emits_sync_failed`**.~~ **Done.** `test_invalid_schema_emits_progression_sync_failed`.
|
||||
|
||||
4. **Deny-path refresh test gap** — Plan §Tests lists deny cast must not trigger gig sync; `gig_feedback_refresh_test.gd` covers defeat accept and non-defeat accept only. Add a third case with `accepted: false` and assert **`gig_sync_calls == 0`**.
|
||||
4. ~~**Deny-path refresh test gap** — Plan §Tests lists deny cast must not trigger gig sync; `gig_feedback_refresh_test.gd` covers defeat accept and non-defeat accept only. Add a third case with `accepted: false` and assert **`gig_sync_calls == 0`**.~~ **Done.** `test_denied_cast_skips_gig_progression_sync`.
|
||||
|
||||
## Nits
|
||||
|
||||
|
|
@ -57,6 +58,6 @@ godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreH
|
|||
-a res://test/gig_feedback_refresh_test.gd
|
||||
```
|
||||
|
||||
**GdUnit (local run):** 5/5 passed (3 + 2 suites).
|
||||
**GdUnit (local run):** 7/7 passed (4 + 3 suites).
|
||||
|
||||
**Manual:** [`docs/manual-qa/NEO-86.md`](../manual-qa/NEO-86.md) — fresh server restart, Godot **F5**, Tab lock **`prototype_target_alpha`**, four **`prototype_pulse`** casts, verify **`GigXpLabel`** **`breach: L1 · 25 xp`**, deny on defeated target, economy HUD collapse step 9.
|
||||
|
|
|
|||
Loading…
Reference in New Issue