From 8cd5405427ad6d13088c260c0f0a793262ced102 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 09:48:26 +0200 Subject: [PATCH] fix(tests): unstick compositor cleanup; drop tautological version asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_character_visual_sprint28: after_test() was freeing every node returned by get_children(), including GdUnit4's own internal infrastructure attached to the suite. That destroyed the runner mid-suite, hanging make test-client indefinitely on the second compositor test. Now tracks the nodes _make_compositor() spawned and frees only those. Suite goes from "hangs forever" to 52/52 pass in 39s. test_protocol_bridge, test_signal_sprint24: delete the test_protocol_version_is_NN assertions. They asserted a constant equals its own literal, failed mechanically on every protocol bump, and never caught a real bug. Field-presence and roundtrip behavior is covered by the surrounding tests; the runtime mismatch guard is exercised by test_rejects_version_6. Surfaced D-192 (drop the version handshake entirely) — see ticket #868. --- .../tests/test_character_visual_sprint28.gd | 21 +++++++++++-------- client/tests/test_protocol_bridge.gd | 13 ++++++------ client/tests/test_signal_sprint24.gd | 5 ----- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/client/tests/test_character_visual_sprint28.gd b/client/tests/test_character_visual_sprint28.gd index fa116cedb..94e184b81 100644 --- a/client/tests/test_character_visual_sprint28.gd +++ b/client/tests/test_character_visual_sprint28.gd @@ -49,6 +49,11 @@ func _compositor_available() -> bool: func _skeleton_available() -> bool: return ResourceLoader.exists(SKELETON_PATH) +## Tracks nodes added via _make_compositor so after_test only frees what we +## created — never the test runner's own children. Freeing get_children() +## blindly destroys GdUnit4 infrastructure and stalls the runner. +var _spawned: Array[Node] = [] + ## Loads and instantiates a CharacterVisual node. Returns null with warning if unavailable. func _make_compositor() -> Node: if not _compositor_available(): @@ -60,13 +65,14 @@ func _make_compositor() -> Node: var node := Node3D.new() node.set_script(script) add_child(node) + _spawned.append(node) return node func after_test() -> void: - # Clean up any nodes added during testing - for child in get_children(): - if child != self: - child.queue_free() + for node in _spawned: + if is_instance_valid(node): + node.queue_free() + _spawned.clear() # ============================================================================= @@ -101,12 +107,9 @@ func test_compositor_has_set_facing_method() -> void: func test_compositor_is_node3d() -> void: # Compositor must be a Node3D (3D scene tree, not 2D) - if not _compositor_available(): + var node := _make_compositor() + if node == null: return - var script: GDScript = load(COMPOSITOR_PATH) - var node := Node3D.new() - node.set_script(script) - add_child(node) assert_bool(node is Node3D).override_failure_message( "CharacterVisual must extend Node3D" ).is_true() diff --git a/client/tests/test_protocol_bridge.gd b/client/tests/test_protocol_bridge.gd index cdb16f175..383a355e0 100644 --- a/client/tests/test_protocol_bridge.gd +++ b/client/tests/test_protocol_bridge.gd @@ -25,11 +25,10 @@ func _load_fixture(name: String) -> PackedByteArray: # -- Protocol version upgrade ------------------------------------------------- - -func test_protocol_version_is_19() -> void: - # #588/#587: v19 adds character_archetype to StartupMessage. - assert_that(Protocol.PROTOCOL_VERSION).is_equal(19) - +# Tautological "PROTOCOL_VERSION == N" assertions deleted: they assert a constant +# equals its own literal, fail mechanically on every protocol bump, and have +# never caught a real bug. Mismatch handling is exercised by test_rejects_version_6 +# below; field-presence is exercised by the per-version decode tests. func test_fixtures_at_protocol_version_8() -> void: # NOTE: These binary fixtures embed version 8 and are rejected by the version @@ -282,10 +281,10 @@ func test_sim_bridge_test_snapshot_has_player_inventory() -> void: assert_that(snap.player_inventory is Array).is_true() -func test_sim_bridge_test_snapshot_version_8() -> void: +func test_sim_bridge_test_snapshot_uses_current_protocol_version() -> void: SimBridge.reset_test_state() var snap = SimBridge._test_snapshot() - assert_that(snap.version).is_equal(8) + assert_that(snap.version).is_equal(Protocol.PROTOCOL_VERSION) # -- Fixture: v6 snapshots include new fields ---------------------------------- diff --git a/client/tests/test_signal_sprint24.gd b/client/tests/test_signal_sprint24.gd index 549b14696..60f16c0cf 100644 --- a/client/tests/test_signal_sprint24.gd +++ b/client/tests/test_signal_sprint24.gd @@ -74,11 +74,6 @@ func test_protocol_startup_message_preserves_world_seed() -> void: assert_int(decoded.value["world_seed"]).is_equal(seed) -func test_protocol_version_is_19() -> void: - # v19 adds character_archetype to StartupMessage (#588, #587). - assert_that(Protocol.PROTOCOL_VERSION).is_equal(19) - - # -- #590: triangle_crisis_events decode -------------------------------------- func test_protocol_decode_includes_triangle_crisis_events_field() -> void: