Skip to content

Commit bc36034

Browse files
dsarnoclaude
andauthored
Tighten 7 tests that passed without verifying behavior (#458)
* Tighten 7 tests that passed without verifying behavior Audit of the GDScript + Python test suites surfaced seven tests whose assertions didn't actually pin down the behavior they claimed to test — they would have stayed green under regressions in the code paths they exercise. GDScript (test_project/tests/): - test_theme.gd::test_theme_set_color_accepts_dict only checked the response envelope. Now reloads the theme and asserts the stored value is a Color with the right channels, so a missing dict->Color coercion can't pass. - test_theme.gd::test_create_theme_overwrite_allowed and test_material.gd::test_create_overwrite_allowed now assert overwritten=true on the response and that the target file still exists, instead of just confirming the call returned data. - test_material.gd::test_set_param_bool now reads emission_enabled back off the loaded material (matching the read-back pattern its float and color siblings already use). - test_animation.gd::test_add_property_track_transition_raw_float now reads the stored keyframes and asserts both that the dict values coerced to Vector3 and that the raw-float transition landed on each key — the prior version would pass even if either silently no-oped. Python (tests/unit/): - test_runtime_handlers.py::test_audio_player_create_blocks_when_not_writable was catching bare Exception, which would mask unrelated bugs. Narrowed to GodotCommandError to match the surrounding readiness-gate tests. - test_self_update_smoke_harness.py::test_v240_preflight_passes_when_both_files_present had zero assertions — it called the validator and returned. Added an assertion that the call returns None so the runner's zero-assertion guard has something to verify and the intent is documented. https://claude.ai/code/session_01H8CLhwuNUA4NoEZ4neJk78 * Retrigger CI for Windows Godot tests Linux + macOS Godot tests passed on the previous run; Windows failed in ~89s (vs ~134s on green main-branch runs), suggesting either a flake or an early-pipeline failure. Pushing this empty commit to disambiguate. https://claude.ai/code/session_01H8CLhwuNUA4NoEZ4neJk78 * Address review follow-ups on PR #458 - test_animation.gd: add value assert for v0 so both keyframes carry the same depth of check (was: only v1 had a value assert; v0 was type-only). - test_theme.gd: assert the alpha channel in test_theme_set_color_accepts_dict — the test dict explicitly passes a=1.0, so leaving it unchecked left a gap in the dict→Color coercion coverage. https://claude.ai/code/session_01H8CLhwuNUA4NoEZ4neJk78 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3e6ac49 commit bc36034

5 files changed

Lines changed: 42 additions & 2 deletions

File tree

test_project/tests/test_animation.gd

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,21 @@ func test_add_property_track_transition_raw_float() -> void:
569569
],
570570
})
571571
assert_has_key(result, "data")
572+
# Read back the stored keyframes so a broken dict→Vector3 coercion or a
573+
# silently-dropped transition can't pass this test.
574+
var scene_root := EditorInterface.get_edited_scene_root()
575+
var player := McpScenePath.resolve(player_path, scene_root) as AnimationPlayer
576+
var anim: Animation = player.get_animation("anim")
577+
var v0 = anim.track_get_key_value(0, 0)
578+
var v1 = anim.track_get_key_value(0, 1)
579+
assert_true(v0 is Vector3, "from value must coerce to Vector3, not stay as Dict")
580+
assert_true(v1 is Vector3, "to value must coerce to Vector3, not stay as Dict")
581+
assert_eq(v0.x, 0.0)
582+
assert_eq(v1.x, 100.0)
583+
assert_true(abs(anim.track_get_key_transition(0, 0) - 3.0) < 0.0001,
584+
"raw-float transition should be stored on key 0")
585+
assert_true(abs(anim.track_get_key_transition(0, 1) - 3.0) < 0.0001,
586+
"raw-float transition should be stored on key 1")
572587
_remove_node(player_path)
573588

574589

test_project/tests/test_material.gd

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ func test_create_overwrite_allowed() -> void:
119119
_make_material()
120120
var result := _handler.create_material({"path": TEST_MATERIAL_PATH, "overwrite": true})
121121
assert_has_key(result, "data")
122+
assert_eq(result.data.overwritten, true,
123+
"overwritten flag must reflect the pre-existing file")
124+
assert_true(FileAccess.file_exists(TEST_MATERIAL_PATH),
125+
"material file should still exist after overwrite")
122126

123127

124128
func test_create_shader_requires_shader_path() -> void:
@@ -179,6 +183,9 @@ func test_set_param_bool() -> void:
179183
"value": true,
180184
})
181185
assert_has_key(result, "data")
186+
var mat: Material = ResourceLoader.load(TEST_MATERIAL_PATH)
187+
assert_eq(mat.get("emission_enabled"), true,
188+
"emission_enabled should be stored as the bool we passed")
182189

183190

184191
func test_set_param_transparency_enum_by_name() -> void:

test_project/tests/test_theme.gd

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ func test_create_theme_overwrite_allowed() -> void:
6565
_make_theme()
6666
var result := _handler.create_theme({"path": TEST_THEME_PATH, "overwrite": true})
6767
assert_has_key(result, "data")
68+
assert_eq(result.data.overwritten, true,
69+
"overwritten flag must reflect the pre-existing file")
70+
assert_true(FileAccess.file_exists(TEST_THEME_PATH),
71+
"theme file should still exist after overwrite")
6872

6973

7074
# ----- theme_set_color -----
@@ -96,6 +100,16 @@ func test_theme_set_color_accepts_dict() -> void:
96100
"value": {"r": 0.5, "g": 0.3, "b": 0.1, "a": 1.0},
97101
})
98102
assert_has_key(result, "data")
103+
# Read back from disk so a missing dict→Color coercion can't pass by
104+
# returning a successful envelope while storing a raw Dict.
105+
var theme: Theme = ResourceLoader.load(TEST_THEME_PATH)
106+
assert_true(theme.has_color("font_color", "Label"))
107+
var c := theme.get_color("font_color", "Label")
108+
assert_true(c is Color, "Stored value must be a Color, not a raw Dict")
109+
assert_true(abs(c.r - 0.5) < 0.01)
110+
assert_true(abs(c.g - 0.3) < 0.01)
111+
assert_true(abs(c.b - 0.1) < 0.01)
112+
assert_true(abs(c.a - 1.0) < 0.01)
99113

100114

101115
func test_theme_set_color_rejects_garbage_string() -> void:

tests/unit/test_runtime_handlers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4869,6 +4869,8 @@ async def test_audio_list_handler_is_read_only():
48694869

48704870
async def test_audio_player_create_blocks_when_not_writable():
48714871
"""audio_player_create requires a writable session (uses require_writable)."""
4872+
from godot_ai.godot_client.client import GodotCommandError
4873+
48724874
client = StubClient()
48734875
client.live_readiness = "playing"
48744876
session = Session(
@@ -4881,7 +4883,7 @@ async def test_audio_player_create_blocks_when_not_writable():
48814883
registry = SessionRegistry()
48824884
registry.register(session)
48834885
runtime = DirectRuntime(registry=registry, client=client)
4884-
with pytest.raises(Exception) as exc_info:
4886+
with pytest.raises(GodotCommandError) as exc_info:
48854887
await audio_handlers.audio_player_create(runtime, parent_path="/Main")
48864888
assert "play mode" in str(exc_info.value).lower()
48874889
## The gate fires one `get_editor_state` probe to confirm the cache

tests/unit/test_self_update_smoke_harness.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,9 @@ def test_v240_preflight_passes_when_both_files_present(tmp_path: Path) -> None:
229229
tmp_path,
230230
present=("utils/server_lifecycle.gd", "utils/update_manager.gd"),
231231
)
232-
smoke._require_v240_plus_addon_shape(addon, "2.4.0")
232+
# Validator returns None on success and raises HarnessError otherwise; the
233+
# assertion both documents intent and trips the runner's zero-assertion guard.
234+
assert smoke._require_v240_plus_addon_shape(addon, "2.4.0") is None
233235

234236

235237
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)