From a02e54da2d940d2c816f69f5a77d8d93dfa052e1 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 5 Apr 2026 21:21:58 -0400 Subject: [PATCH] NS-21: fold test CI into gdscript.yml; document test-with-scripts policy --- .cursor/rules/testing-expectations.md | 7 ++++--- docs/plans/NS-21-implementation-plan.md | 14 ++++++++------ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index cfc5b5d..a6813b0 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -1,5 +1,5 @@ --- -description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot manual until harness. +description: Unit vs integration tests; integration when data persistence/mutation exists; C# xUnit; Godot client unit tests with script changes (NS-21 harness + gdscript.yml CI). alwaysApply: true --- @@ -23,8 +23,9 @@ alwaysApply: true ## Godot client (GDScript) -- Until the project adopts a harness (**GUT**, Godot unit tests, or CI export checks), rely on **manual verification** for gameplay and scene changes. -- For non-trivial client work, note **how you verified** (steps or scenario) in the PR or story plan so reviewers can repeat it. +- **Harness (NS-21):** The **target** setup is a **unit test stack** under `client/` with **`client/README.md`** instructions and **`.github/workflows/gdscript.yml`** running **gdlint**, **gdformat**, and **headless unit tests** on the same path triggers. Until NS-21 is merged, CI may not run that suite yet—use **manual verification** and PR notes as today. +- **Production script changes (after NS-21):** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently. +- **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress. ## Cross-cutting diff --git a/docs/plans/NS-21-implementation-plan.md b/docs/plans/NS-21-implementation-plan.md index 3dca1af..a3c5149 100644 --- a/docs/plans/NS-21-implementation-plan.md +++ b/docs/plans/NS-21-implementation-plan.md @@ -17,7 +17,7 @@ - **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`). +- **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.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. @@ -30,10 +30,12 @@ - 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`](../../.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. -- [ ] At least **one CI workflow** runs tests on **PR/push** with **path filters** appropriate to `client/` and the new test assets. +- [ ] **`.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 @@ -43,7 +45,7 @@ 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. +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 addon’s **headless test runner**, fail on test failure. **Extend the workflow’s `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 @@ -60,14 +62,14 @@ | 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). | +| `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 **ARIVE_EPS** / **VERT_ARRIVE_EPS** (using minimal tree + physics stepping). | +| `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. |