diff --git a/docs/plans/NS-21-implementation-plan.md b/docs/plans/NS-21-implementation-plan.md new file mode 100644 index 0000000..3dca1af --- /dev/null +++ b/docs/plans/NS-21-implementation-plan.md @@ -0,0 +1,80 @@ +# NS-21 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NS-21 | +| **Title** | Set up GDScript unit testing and cover existing client scripts | +| **Jira** | [NS-21](https://neon-sprawl.atlassian.net/browse/NS-21) | +| **Parent** | [NS-20 — Tech Debt](https://neon-sprawl.atlassian.net/browse/NS-20) | + +## Goal, scope, and out-of-scope + +**Goal:** Introduce **automated GDScript tests** for the Neon Sprawl Godot client (**Godot 4.6** per `client/project.godot`) and add **meaningful coverage** for scripts that exist today, alongside **contributor docs** and **CI** that runs the suite when client code or tests change. + +**In scope** + +- **Harness:** Add a maintained test stack (**GDUnit4** preferred for Godot 4.x headless + CI; **GUT** acceptable if GDUnit4 blocks on 4.6—decide during implementation and record in README). Vendor the addon under `client/addons/` (pinned release) so CI and clones match. +- **Documentation:** `client/README.md` (or a short dedicated doc under `client/` linked from README) with **install** (if any beyond opening the project), **local run commands**, and how tests are laid out. +- **CI:** At least one GitHub Actions job that runs the suite on **push/PR** to `main`, with **path filters** on `client/` (including `client/addons/` test plugin paths, `client/**/*.gd` tests, workflow file). Align **Godot binary version** with **4.6-stable** (same pattern as existing headless smoke notes in `client/README.md`). +- **Tests:** Cover current behavior where feasible for: + - `client/scripts/player.gd` — public API: `snap_to_server`, `set_authoritative_nav_goal`, `clear_nav_goal`, and **physics-process outcomes** that can be asserted with a **minimal scene tree** (CharacterBody3D + child NavigationAgent3D matching `$NavigationAgent3D`) without the full `main.tscn`. + - `client/scripts/position_authority_client.gd` — **HTTP flow** via a **test double** (subclass of `HTTPRequest` or injected node) that invokes `request_completed` with canned bodies/status codes: boot `GET` 200 → `authoritative_position_received` with `apply_as_snap == true`; `POST` 400 → `move_rejected`; happy `POST` 200 → `VERIFY_GET` → `GET` 200 → second arg `false`. Cover `_base_root` / URL trimming indirectly through successful request URLs if practical. + - `client/scripts/ground_pick.gd` — **`_collider_is_walkable`** (walkable group ancestry) using a **minimal node hierarchy** under test; **steep-segment / wall cutoff** behavior only if we can do it without brittle full viewport ray setup—otherwise **document** deferral to manual / future scene test. +- **Lint alignment:** Keep **gdlint** / **gdformat** green; extend lint paths if new roots are added (e.g. `client/test/`). + +**Out of scope** + +- **Full integration** tests of `main.gd` / `main.tscn` (navigation bake timing, label timers, full pick pipeline)—**intentionally deferred** here: thin composition + `await` chains are brittle without a dedicated scene test harness; README should state that explicitly (per Jira). +- Replacing **server** tests or adding **E2E** browser/gameplay automation. +- Changing **production** gameplay contracts (NS-16/19/23) except where a test exposes a real bug (unlikely in this story). + +## Acceptance criteria checklist + +- [ ] README or short doc under `client/` explains **install** (if needed) and **run** commands for the test suite. +- [ ] At least **one CI workflow** runs tests on **PR/push** with **path filters** appropriate to `client/` and the new test assets. +- [ ] New tests cover **`player.gd`**, **`position_authority_client.gd`**, and **`ground_pick.gd`** where feasible; any **intentionally untested** surface (e.g. full `_input` ray pick, `main.gd`) is **briefly noted** in the plan or README. + +## Technical approach + +1. **Pick and pin GDUnit4** (or GUT): download/addon version verified against **Godot 4.6**; commit under `client/addons/…`; enable the plugin in `client/project.godot`. +2. **Configure test discovery** per the addon (e.g. GDUnit4 test directory / naming); keep **no `class_name`** on production scripts that the README already calls out for headless—tests attach scripts by path or scene instance. +3. **`player.gd`:** Build a small **SceneTree**-hosted rig in setup: `CharacterBody3D` + `NavigationAgent3D` named to match `player.gd`’s `$NavigationAgent3D`; call `snap_to_server` / goals; advance **physics frames** (`await` or GDUnit helpers) and assert position, `velocity`, and `_has_walk_goal` behavior where observable via public effects (arrival epsilon, clear goal). +4. **`position_authority_client.gd`:** Replace or wrap **`HTTPRequest`** with a **double** that records `request()` args and simulates **`request_completed`**; drive `sync_from_server` / `submit_move_target` and assert signals and `_busy` / phase behavior. +5. **`ground_pick.gd`:** Instantiate script on `Node3D`; build `StaticBody3D` (or plain `Node`) in/out of **`walkable`** group; use `call("_collider_is_walkable", collider)` **only if** needed for white-box tests, or test via a thin extracted helper if the team prefers no underscore calls (optional refactor—keep minimal). +6. **CI:** Reuse the **official Linux 4.6-stable** Godot binary pattern from `client/README.md`; run the addon’s **headless test runner** CLI; fail the job on test failure. Optionally **merge** into `.github/workflows/gdscript.yml` as a second job or add `client-tests.yml`—whichever keeps caches and Godot download maintainable. +7. **Documentation:** Add a **“Automated tests (GDScript)”** section to `client/README.md` with exact commands. + +## Files to add + +| Path | Purpose | +|------|---------| +| `client/addons/gdUnit4/` (or chosen plugin root) | Vendored **GDUnit4** (or **GUT**) addon, **pinned** to a release compatible with **Godot 4.6**. | +| `client/test/player_test.gd` | GDUnit tests for **`player.gd`** (snap, nav goal, clear, arrival / idle paths where assertable). | +| `client/test/position_authority_client_test.gd` | Tests for **`position_authority_client.gd`** using **HTTPRequest double** + signal assertions. | +| `client/test/ground_pick_test.gd` | Tests for **`ground_pick.gd`** walkable collider logic (and optional ray/segment cases if implemented). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `client/project.godot` | Enable **test plugin**; set **GDUnit4** (or GUT) project options / test root if required. | +| `client/README.md` | Document **how to run tests locally** and CI pointer; note **`main.gd` / full pick** deferred. | +| `.github/workflows/gdscript.yml` and/or **new** `client`-focused workflow under `.github/workflows/` | Add job(s) to **download Godot 4.6** and **run the suite** with **path filters** covering `client/` (including addons + tests). | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `client/test/player_test.gd` | `snap_to_server` resets transform/velocity and nav target; `set_authoritative_nav_goal` / `clear_nav_goal`; movement toward goal / arrival within **ARIVE_EPS** / **VERT_ARRIVE_EPS** (using minimal tree + physics stepping). | +| `client/test/position_authority_client_test.gd` | Boot `GET` success → `authoritative_position_received(..., true)`; `POST` `400` + JSON → `move_rejected`; `POST` `200` + verify `GET` `200` → `authoritative_position_received(..., false)`; malformed JSON does not emit position. | +| `client/test/ground_pick_test.gd` | `_collider_is_walkable` true when ancestor in **`walkable`** group, false otherwise; document in-file if ray stepping remains manual-only. | + +**Manual verification (supplemental):** Run **F5** main scene after plugin install to ensure editor still loads; run documented **headless** Godot command from README; confirm CI green on a PR touching `client/scripts/player.gd`. + +## Open questions / risks + +- **Addon choice lock-in:** If **GDUnit4** lags a **4.6** patch, switch to **GUT** for this story and document—low risk if decided early. +- **CI time:** Downloading **Godot** each run vs **cache** action—balance simplicity (download URL) vs speed; may iterate after first green pipeline. +- **White-box calls** to `_*` methods in tests vs **small extractions**—prefer smallest change; refactor only if reviews object to underscore `call()`.