diff --git a/Makefile b/Makefile index c1491d875..b2ed08a7d 100644 --- a/Makefile +++ b/Makefile @@ -158,13 +158,13 @@ pre-pr-validate: validate-content check-fact-ids @echo "--- Content validation: PASS ---" pre-pr-fixtures: - @echo "Checking fixture staleness..." + @echo "Checking Rust->GDScript fixture staleness..." @cd server && cargo test --test gen_fixtures -- --ignored @if git diff --quiet client/tests/fixtures/; then \ - echo "--- Fixtures: UP TO DATE ---"; \ + echo "--- Rust fixtures: UP TO DATE ---"; \ else \ echo ""; \ - echo "--- FIXTURES STALE ---"; \ + echo "--- RUST FIXTURES STALE ---"; \ echo " Protocol changed but fixtures not regenerated."; \ echo " Stale fixtures make all client tests FALSE POSITIVES."; \ echo ""; \ @@ -174,6 +174,22 @@ pre-pr-fixtures: echo " Fix: commit the updated fixtures with your protocol change."; \ exit 1; \ fi + @echo "Checking GDScript->Rust fixture staleness..." + @$(MAKE) fixtures-client + @if git diff --quiet server/tests/fixtures/gdscript/; then \ + echo "--- GDScript fixtures: UP TO DATE ---"; \ + else \ + echo ""; \ + echo "--- GDSCRIPT FIXTURES STALE ---"; \ + echo " Protocol changed but GDScript fixtures not regenerated."; \ + echo " Stale fixtures make cross-encoder tests FALSE POSITIVES."; \ + echo ""; \ + echo " Changed files:"; \ + git diff --stat server/tests/fixtures/gdscript/; \ + echo ""; \ + echo " Fix: commit the updated fixtures with your protocol change."; \ + exit 1; \ + fi # Branch-specific variants (faster, scope-appropriate) diff --git a/client/tests/gen_client_fixtures.gd b/client/tests/gen_client_fixtures.gd index f3bbf6df1..20383136a 100644 --- a/client/tests/gen_client_fixtures.gd +++ b/client/tests/gen_client_fixtures.gd @@ -12,6 +12,7 @@ extends SceneTree var _Msgpack: GDScript var _count := 0 +var _errors := 0 var _output_dir: String @@ -22,27 +23,48 @@ func _init(): func _run(): _Msgpack = load("res://addons/messagepack/messagepack.gd") + # Resolve repo root from Godot project root (client/). + # Assumes client/ is one level below repo root — validated below. var project_root := ProjectSettings.globalize_path("res://") var repo_root := project_root.rstrip("/").get_base_dir() _output_dir = repo_root.path_join("server/tests/fixtures/gdscript") + if not DirAccess.dir_exists_absolute(repo_root.path_join("server")): + push_error("Repo root detection failed: %s/server/ does not exist" % repo_root) + quit(1) + return + DirAccess.make_dir_recursive_absolute(_output_dir) _generate_inputs() _generate_boundary_inputs() _generate_batch() + if _errors > 0: + push_error("FAILED: %d encode errors encountered" % _errors) + quit(1) + return + if _count == 0: + push_error("FAILED: no fixtures generated") + quit(1) + return print("Generated %d GDScript fixtures at %s" % [_count, _output_dir]) quit() func _write_fixture(name: String, bytes: PackedByteArray) -> void: + if bytes.is_empty(): + push_error("Encode produced empty bytes for fixture: %s" % name) + _errors += 1 + return var path := _output_dir.path_join(name + ".msgpack") var file := FileAccess.open(path, FileAccess.WRITE) if file == null: push_error("Failed to write fixture: %s (error: %d)" % [path, FileAccess.get_open_error()]) + _errors += 1 return file.store_buffer(bytes) + file.flush() file.close() print(" Wrote %s (%d bytes)" % [name, bytes.size()]) _count += 1 @@ -90,10 +112,11 @@ func _encode_inputs(inputs: Array) -> PackedByteArray: func _generate_inputs() -> void: - # Unit variants: movement directions (tick=100) - _write_fixture("input_move_north", - _encode_input(100, "MoveNorth")) - for dir_name in ["MoveNortheast", "MoveSoutheast", "MoveSouthwest", "MoveNorthwest"]: + # Unit variants: all 8 movement directions (tick=100) + for dir_name in [ + "MoveNorth", "MoveSouth", "MoveEast", "MoveWest", + "MoveNortheast", "MoveSoutheast", "MoveSouthwest", "MoveNorthwest", + ]: _write_fixture("input_%s" % dir_name.to_snake_case(), _encode_input(100, dir_name)) @@ -108,8 +131,14 @@ func _generate_inputs() -> void: # Other unit variants _write_fixture("input_pause", _encode_input(100, "Pause")) + _write_fixture("input_unpause", + _encode_input(100, "Unpause")) _write_fixture("input_toggle_stance_up", _encode_input(100, "ToggleStanceUp")) + _write_fixture("input_toggle_stance_down", + _encode_input(100, "ToggleStanceDown")) + _write_fixture("input_walk_away", + _encode_input(100, "WalkAway")) func _generate_boundary_inputs() -> void: diff --git a/docs/DEVOPS.md b/docs/DEVOPS.md index 56c47d6c7..162c07568 100644 --- a/docs/DEVOPS.md +++ b/docs/DEVOPS.md @@ -82,6 +82,12 @@ The bidirectional protocol is validated by two sets of committed fixtures: Regenerate both after any protocol change. Commit the updated fixtures alongside the code change. +**Troubleshooting fixture failures:** + +- **`make fixtures-client` fails with encode errors:** Check that `client/addons/messagepack/messagepack.gd` is up to date. The script exits non-zero on any encode failure. +- **`gdscript_generated_fixtures_deserialize` fails:** Fixtures in `server/tests/fixtures/gdscript/` are stale or corrupted. Re-run `make fixtures-client` and commit the updated files. +- **Fixture staleness in `make pre-pr`:** Protocol changed but fixtures were not regenerated. Run `make fixtures && make fixtures-client`, then commit both `client/tests/fixtures/` and `server/tests/fixtures/gdscript/`. + ### Lint ```bash diff --git a/server/tests/serialization.rs b/server/tests/serialization.rs index 3366279fe..2a094c5d4 100644 --- a/server/tests/serialization.rs +++ b/server/tests/serialization.rs @@ -1013,15 +1013,14 @@ fn malformed_input_in_batch_rejects_entire_batch() { /// (runs client/tests/gen_client_fixtures.gd via Godot headless) #[test] fn gdscript_generated_fixtures_deserialize() { + // CWD is server/ when cargo test runs (Cargo sets it to the package root) let fixture_dir = std::path::Path::new("tests/fixtures/gdscript"); - if !fixture_dir.exists() { - eprintln!( - "SKIP: GDScript fixtures not found at {}. Run `make fixtures-client` to generate.", - fixture_dir.display() - ); - return; - } + assert!( + fixture_dir.exists(), + "GDScript fixture directory not found at {}. Run `make fixtures-client` to generate.", + fixture_dir.display() + ); let mut count = 0; for entry in fs::read_dir(&fixture_dir).expect("read gdscript fixture dir") { @@ -1088,13 +1087,11 @@ fn gdscript_generated_fixtures_deserialize() { count += 1; } - if count == 0 { - eprintln!( - "SKIP: no .msgpack files found in {}. Run `make fixtures-client` to generate.", - fixture_dir.display() - ); - return; - } + assert!( + count > 0, + "No .msgpack files found in {}. Run `make fixtures-client` to generate.", + fixture_dir.display() + ); eprintln!("Verified {} GDScript-generated fixtures", count); }