docs(workshops): add test architecture workshop brief
4-track workshop targeting the class of bugs from Sprint 6-7: blocking I/O, serialization boundaries, state desync, off-by-one encoding. Participants: Tyre, Hoshe, Dudley, Stig. Covers integration test architecture, serialization boundary tests, client test infrastructure, and test gap prioritization. Includes catalogue of 6 real bugs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,115 @@
|
||||
# Test Architecture & Tooling Workshop Brief
|
||||
|
||||
**Goal:** Design the test strategy that catches the class of bugs we've been hitting — blocking I/O, serialization boundary errors, state desync, off-by-one encoding — before they reach manual playtesting. Produce concrete test specifications, not aspirational test plans.
|
||||
|
||||
**Participants:** Tyre, Hoshe, Dudley, Stig
|
||||
|
||||
**Context:** Sprint 6-7 development exposed a pattern of bugs that unit tests don't catch and manual `make game` testing finds too late. The bugs share a common profile: they live at integration boundaries (client-server, encode-decode, ECS schedule ordering, pause/unpause state transitions). D-030 defined the test architecture but implementation has lagged — this workshop closes the gap.
|
||||
|
||||
## The Bug Catalogue
|
||||
|
||||
These are real bugs from the current sprint. Every test specification produced by this workshop must prevent at least one from recurring.
|
||||
|
||||
| # | Bug | Root Cause | Category |
|
||||
|---|-----|-----------|----------|
|
||||
| 1 | Server never sends snapshots in live mode | `read_framed()` blocking TCP read stalls entire bevy Update schedule | IPC / schedule integration |
|
||||
| 2 | Camera doesn't center on player at startup | Client receives no snapshot until first keystroke (consequence of #1) | Client-server startup sequencing |
|
||||
| 3 | Player moves while game is paused | `process_player_input` processes movement regardless of TickRate | State transition / input filtering |
|
||||
| 4 | MessagePack encodes tick 128 as -128 | Signed int8 branch uses `<=` instead of `<` for upper bound | Serialization boundary |
|
||||
| 5 | Monologue lost when snapshots overwrite | Latest-wins snapshot buffer drops one-shot events | Event delivery reliability |
|
||||
| 6 | Snapshot overwrite warning spam | Server ticks faster than client consumes | Rate mismatch (design, not bug) |
|
||||
|
||||
## Workshop Tracks
|
||||
|
||||
### Track 1: Integration Test Architecture (Tyre, Dudley)
|
||||
|
||||
D-030 defined three IPC test layers but only Layer 1 (fixture roundtrips) and partial Layer 2 (bridge tests) exist. Layer 3 (real subprocess integration) is entirely missing. This track designs the concrete test infrastructure.
|
||||
|
||||
**Current state:**
|
||||
- Server: 352 unit tests, 9 integration tests (`tests/` dir), all pass
|
||||
- Client: 16 test files using gdUnit4, untested headless reliability
|
||||
- Cross-boundary: `bridge_tcp.rs` tests roundtrip serialization, but nothing tests the full `client sends input → server processes → server sends snapshot → client receives` loop
|
||||
- No test covers the bevy schedule execution order
|
||||
- No test covers non-blocking socket behavior under load
|
||||
|
||||
**Questions for Tyre:**
|
||||
1. What's the minimum viable Layer 3 test? A Rust test that spawns a real server, connects a mock client over TCP, sends input, and verifies a snapshot comes back — or something lighter?
|
||||
2. Should bevy schedule ordering be tested explicitly (e.g., assert that `compute_observer_snapshot` runs after `validate_movement`), or is the existing system ordering via `.after()/.before()` sufficient?
|
||||
3. The server game loop (`loop { app.update() }`) is untestable as-is — it's in `main()`. Should we extract a `GameLoop` struct with `tick()` that integration tests can drive?
|
||||
4. How do we test the non-blocking TCP behavior? The fix toggles between blocking/non-blocking per operation — what invariants should tests check?
|
||||
|
||||
**Questions for Dudley:**
|
||||
1. The `smoke.rs` test boots the world and ticks — but doesn't verify snapshots are produced. What's the minimal extension to catch bug #1?
|
||||
2. `process_player_input` has no test for pause-state filtering. Propose the test cases for the pause guard (movement discarded, unpause accepted, stance toggle during pause — allowed or not?).
|
||||
3. Content loading fails silently (`Failed to load content: manifest not found`). Should this be a test failure in integration tests, or is graceful degradation correct?
|
||||
4. The `EntityRegistry` ↔ `StableId` mapping is a correctness boundary — are there enough tests for entity lifecycle (spawn, register, lookup, despawn)?
|
||||
|
||||
### Track 2: Serialization Boundary Tests (Hoshe, Dudley)
|
||||
|
||||
The MessagePack off-by-one (bug #4) is a classic boundary value error. The project has TWO independent serialization paths: Godot `messagepack.gd` (client encode) and Rust `rmp_serde` (server decode). They must agree on every value.
|
||||
|
||||
**Current state:**
|
||||
- `test_protocol.gd` and `serialization.rs` test roundtrips within their respective languages
|
||||
- `bridge_tcp.rs` tests cross-language roundtrip for snapshots and inputs
|
||||
- No test specifically exercises boundary values (128, 256, 32768, 2^31, etc.)
|
||||
- The messagepack.gd encoder has unfixed boundary bugs at int16, int32, int64 boundaries (same pattern as the int8 bug)
|
||||
|
||||
**Questions for Hoshe:**
|
||||
1. Design a boundary value test matrix for the MessagePack encoder. Which values must be tested? (Hint: every power-of-two boundary where the format changes, both positive and negative.)
|
||||
2. Should boundary tests live in the client (GDScript), server (Rust), or both? Cross-language roundtrip tests catch the real bugs but are slower.
|
||||
3. The `gen_fixtures.rs` test generates MessagePack fixtures — should it generate boundary value fixtures that the client can verify?
|
||||
4. Propose a "golden file" approach: server generates canonical MessagePack bytes for known inputs, client verifies it decodes to the same values. How would this work in CI?
|
||||
|
||||
**Questions for Dudley:**
|
||||
1. The Rust `PlayerInput` uses `tick: u64` but the client sends Godot's signed `int` (63-bit range). What's the actual valid range, and should the server reject negative ticks explicitly?
|
||||
2. `rmp_serde` silently rejects `-128` for a `u64` field. Should the bridge layer pre-validate and log, or is the current error-and-skip behavior acceptable?
|
||||
|
||||
### Track 3: Client Test Infrastructure (Stig, Hoshe)
|
||||
|
||||
The client has 16 test files but reliability is uncertain. gdUnit4 headless mode needs validation. Scene-level tests (camera, renderer, input pipeline) are the gap that let bugs #2, #3, and #5 through.
|
||||
|
||||
**Current state:**
|
||||
- `make test-client` invokes gdUnit4 headless runner
|
||||
- Tests use `GdUnitTestSuite` base class
|
||||
- `test_camera_anchor.gd` was written this sprint (10 tests, all pass) but only tests test-mode (not live TCP mode)
|
||||
- No test for the `_process()` loop in `main.gd`
|
||||
- No test for the snapshot event carry-forward (bug #5 fix)
|
||||
- No test for pause toggle (InputMapper → SimBridge → wire encoding)
|
||||
|
||||
**Questions for Stig:**
|
||||
1. `main.gd._process()` drives the entire client game loop. What's testable here? Can we use `GdUnitSceneRunner` to drive the scene, inject mock snapshots, and verify camera + renderer state?
|
||||
2. The snapshot carry-forward logic (monologue/dialogue preserved across overwrites) is in `sim_bridge.gd`. Propose test cases.
|
||||
3. InputMapper's pause toggle reads `GameState.game_time.tick_rate` to decide Pause vs Unpause. How do we test this state-dependent branching?
|
||||
4. The cursor renderer does world-space hit detection via `get_canvas_transform()`. Is this testable in headless mode (no GPU)?
|
||||
|
||||
**Questions for Hoshe:**
|
||||
1. The client tests don't run in CI yet. What's the blocker — headless Godot availability, test runner stability, or just wiring?
|
||||
2. Propose a minimal CI pipeline: which tests are fast enough to run on every commit, which are PR-only, which are nightly?
|
||||
3. gdUnit4 `GdUnitSceneRunner` can simulate frames. Design a test that catches bug #2: scene loads → no snapshot → first snapshot arrives → camera anchors to player position (not 0,0).
|
||||
|
||||
### Track 4: Test Gaps & Priority (All)
|
||||
|
||||
D-030 defined test phases but we're past the timeline. Recalibrate.
|
||||
|
||||
**D-030 phases vs reality:**
|
||||
- Phase 1 (sprint 1-2): test infra + collision/pathfinding/time — **DONE** (server-side)
|
||||
- Phase 2 (sprint 3-4): monologue pipeline integration + info boundary negative tests — **NOT DONE**
|
||||
- Phase 3 (sprint 5+): CauseChain verification + divergent snapshots — **NOT STARTED**
|
||||
|
||||
**Questions for all:**
|
||||
1. Given the bug catalogue above, what test would you write FIRST if you could only write one?
|
||||
2. What's the highest-risk untested boundary in the codebase right now?
|
||||
3. The server has 352 unit tests but zero tests for the observer pipeline producing correct snapshots from a known world state. Is this the biggest gap?
|
||||
4. Should we invest in deterministic replay testing (D-030 #7) now, or is it still premature?
|
||||
|
||||
## Workshop Format
|
||||
|
||||
**Round 1 (analysis):** Each participant answers their track questions independently. Reference existing code, D-030, and the bug catalogue. Produce concrete test specifications (function names, assertions, setup), not general principles.
|
||||
|
||||
**Round 2 (synthesis):** Cross-review. Tyre validates Stig's client test proposals for architectural soundness. Hoshe validates Dudley's server test proposals for coverage completeness. All participants rank the proposed tests by bug-prevention value.
|
||||
|
||||
**Output:**
|
||||
- Prioritized test backlog (tickets) covering the bug catalogue
|
||||
- Concrete test specifications for the top 10 tests
|
||||
- CI pipeline design (what runs when)
|
||||
- Decision on Layer 3 integration test approach
|
||||
Reference in New Issue
Block a user