9.2 KiB
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 |
| Parent | NS-20 — Tech Debt |
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 underclient/linked from README) with install (if any beyond opening the project), local run commands, and how tests are laid out. - CI: Extend
.github/workflows/gdscript.ymlonly (no separate client test workflow): add a job (or extend the existing job) that downloads Godot 4.6-stable and runs the suite headless. Path filters on that workflow must include anything that should run tests—e.g.client/scripts/**/*.gd,client/test/**/*.gd,client/addons/**,client/project.godot, and the workflow file itself—so lint, format, and unit tests stay aligned whenever client code or tests change. - 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 fullmain.tscn.client/scripts/position_authority_client.gd— HTTP flow via a test double (subclass ofHTTPRequestor injected node) that invokesrequest_completedwith canned bodies/status codes: bootGET200 →authoritative_position_receivedwithapply_as_snap == true;POST400 →move_rejected; happyPOST200 →VERIFY_GET→GET200 → second argfalse. 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 +awaitchains 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).
Ongoing policy (effective once this story merges): Any change that adds or modifies production GDScript under client/scripts/ (and any future app .gd outside client/addons/ and client/test/) must add or update unit tests under client/test/ in the same change set, unless the PR or plan explicitly documents a justified exception (e.g. generated or no-logic glue). client/README.md should state this expectation for contributors; .cursor/rules/testing-expectations.md is updated to match.
Acceptance criteria checklist
- README or short doc under
client/explains install (if needed) and run commands for the test suite. .github/workflows/gdscript.ymlruns unit tests (in addition to gdlint / gdformat) on PR/push with path filters that cover production scripts,client/test/,client/addons/(harness),client/project.godot, and the workflow file.- New tests cover
player.gd,position_authority_client.gd, andground_pick.gdwhere feasible; any intentionally untested surface (e.g. full_inputray pick,main.gd) is briefly noted in the plan or README.
Technical approach
- Pick and pin GDUnit4 (or GUT): download/addon version verified against Godot 4.6; commit under
client/addons/…; enable the plugin inclient/project.godot. - Configure test discovery per the addon (e.g. GDUnit4 test directory / naming); keep no
class_nameon production scripts that the README already calls out for headless—tests attach scripts by path or scene instance. player.gd: Build a small SceneTree-hosted rig in setup:CharacterBody3D+NavigationAgent3Dnamed to matchplayer.gd’s$NavigationAgent3D; callsnap_to_server/ goals; advance physics frames (awaitor GDUnit helpers) and assert position,velocity, and_has_walk_goalbehavior where observable via public effects (arrival epsilon, clear goal).position_authority_client.gd: Replace or wrapHTTPRequestwith a double that recordsrequest()args and simulatesrequest_completed; drivesync_from_server/submit_move_targetand assert signals and_busy/ phase behavior.ground_pick.gd: Instantiate script onNode3D; buildStaticBody3D(or plainNode) in/out ofwalkablegroup; usecall("_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).- CI: In
.github/workflows/gdscript.yml, add a test step (new job or sequential step after lint—choose whichever keeps the YAML clear): download Linux 4.6-stable Godot perclient/README.md, run the addon’s headless test runner, fail on test failure. Extend the workflow’spaths/paths-ignoreso the workflow runs when tests, addons,project.godot, or production.gdunderclient/change. - Documentation: Add a “Automated tests (GDScript)” section to
client/README.mdwith 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, CI pointer, and policy: production GDScript changes require matching test adds/updates under client/test/ (see testing-expectations); note main.gd / full pick deferred. |
.github/workflows/gdscript.yml |
Add Godot headless test run; widen path filters so lint + format + tests run together whenever relevant client/ assets change. |
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 ARRIVE_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 underscorecall().