neon-sprawl/docs/plans/NEO-12-implementation-plan.md

9.2 KiB
Raw Permalink Blame History

NEO-12 — Implementation plan

Story reference

Field Value
Key NEO-12
Title Set up GDScript unit testing and cover existing client scripts
Linear NEO-12
Parent 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 under client/ 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.yml only (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 full main.tscn.
    • client/scripts/position_authority_client.gdHTTP 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_GETGET 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 Linear).
  • Replacing server tests or adding E2E browser/gameplay automation.
  • Changing production gameplay contracts (NEO-7/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.yml runs 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, 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.gds $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: 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 per client/README.md, run the addons headless test runner, fail on test failure. Extend the workflows paths / paths-ignore so the workflow runs when tests, addons, project.godot, or production .gd under client/ change.
  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, 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 200authoritative_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().