Skip to content

Commit f4a8b05

Browse files
dsarnoclaude
andauthored
Clean up Unity and Python tests (CoplayDev#548)
* Test suite audit: remove placeholders, consolidate duplicates, fix stale tests Python tests: - Delete 3 vestigial placeholder files (test_script_editing.py, test_find_in_file_minimal.py, test_resources_api.py) - Remove empty skip-marked tests from test_transport_framing.py and test_logging_stdout.py - Consolidate DummyMCP/setup_tools() into test_helpers.py - Fix flaky timing in test_telemetry_queue_worker.py (200ms → 500ms) - Remove placeholder from test_instance_routing_comprehensive.py C# tests: - Consolidate MCPToolParameterTests.cs (755 → 361 lines, -52%) - Fix 3 tests with stale "future feature" comments - features ARE implemented: - AutoGrow arrays (Phase 1.2) - Path normalization (Phase 1.1) - Bulk array mapping (Phase 3.1) - Strengthen assertions in ManageGameObjectTests.cs and CommandRegistryTests.cs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address code review feedback: remove dead code, fix silent test skips - Remove unused tools_struct setup in test_edit_normalization_and_noop.py - Create test texture in EndToEnd_PropertyHandling_AllScenarios so texture-dependent scenarios (4, 5, 10) actually run instead of silently skipping - Add texture cleanup in finally block Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 53c0ed2 commit f4a8b05

15 files changed

Lines changed: 188 additions & 693 deletions

Server/tests/integration/test_edit_normalization_and_noop.py

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,11 @@
11
import pytest
22

3-
from .test_helpers import DummyContext
4-
5-
6-
class DummyMCP:
7-
def __init__(self): self.tools = {}
8-
9-
def tool(self, *args, **kwargs):
10-
def deco(fn): self.tools[fn.__name__] = fn; return fn
11-
return deco
12-
13-
14-
def setup_tools():
15-
mcp = DummyMCP()
16-
# Import the tools module to trigger decorator registration
17-
import services.tools.manage_script
18-
# Get the registered tools from the registry
19-
from services.registry import get_registered_tools
20-
tools = get_registered_tools()
21-
# Add all script-related tools to our dummy MCP
22-
for tool_info in tools:
23-
tool_name = tool_info['name']
24-
if any(keyword in tool_name for keyword in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
25-
mcp.tools[tool_name] = tool_info['func']
26-
return mcp.tools
3+
from .test_helpers import DummyContext, DummyMCP, setup_script_tools
274

285

296
@pytest.mark.asyncio
307
async def test_normalizes_lsp_and_index_ranges(monkeypatch):
31-
tools = setup_tools()
8+
tools = setup_script_tools()
329
apply = tools["apply_text_edits"]
3310
calls = []
3411

@@ -88,7 +65,7 @@ async def fake_read(cmd, params, **kwargs):
8865

8966
@pytest.mark.asyncio
9067
async def test_noop_evidence_shape(monkeypatch):
91-
tools = setup_tools()
68+
tools = setup_script_tools()
9269
apply = tools["apply_text_edits"]
9370
# Route response from Unity indicating no-op
9471

@@ -120,19 +97,8 @@ async def fake_send(cmd, params, **kwargs):
12097

12198
@pytest.mark.asyncio
12299
async def test_atomic_multi_span_and_relaxed(monkeypatch):
123-
tools_text = setup_tools()
100+
tools_text = setup_script_tools()
124101
apply_text = tools_text["apply_text_edits"]
125-
tools_struct = DummyMCP()
126-
# Import the tools module to trigger decorator registration
127-
import services.tools.script_apply_edits
128-
# Get the registered tools from the registry
129-
from services.registry import get_registered_tools
130-
tools = get_registered_tools()
131-
# Add all script-related tools to our dummy MCP
132-
for tool_info in tools:
133-
tool_name = tool_info['name']
134-
if any(keyword in tool_name for keyword in ['script_apply', 'apply_edits']):
135-
tools_struct.tools[tool_name] = tool_info['func']
136102
# Fake send for read and write; verify atomic applyMode and validate=relaxed passes through
137103
sent = {}
138104

Server/tests/integration/test_edit_strict_and_warnings.py

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,11 @@
11
import pytest
22

3-
from .test_helpers import DummyContext
4-
5-
6-
class DummyMCP:
7-
def __init__(self): self.tools = {}
8-
9-
def tool(self, *args, **kwargs):
10-
def deco(fn): self.tools[fn.__name__] = fn; return fn
11-
return deco
12-
13-
14-
def setup_tools():
15-
mcp = DummyMCP()
16-
# Import tools to trigger decorator-based registration
17-
import services.tools.manage_script
18-
from services.registry import get_registered_tools
19-
for tool_info in get_registered_tools():
20-
name = tool_info['name']
21-
if any(k in name for k in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
22-
mcp.tools[name] = tool_info['func']
23-
return mcp.tools
3+
from .test_helpers import DummyContext, setup_script_tools
244

255

266
@pytest.mark.asyncio
277
async def test_explicit_zero_based_normalized_warning(monkeypatch):
28-
tools = setup_tools()
8+
tools = setup_script_tools()
299
apply_edits = tools["apply_text_edits"]
3010

3111
async def fake_send(cmd, params, **kwargs):
@@ -60,7 +40,7 @@ async def fake_send(cmd, params, **kwargs):
6040

6141
@pytest.mark.asyncio
6242
async def test_strict_zero_based_error(monkeypatch):
63-
tools = setup_tools()
43+
tools = setup_script_tools()
6444
apply_edits = tools["apply_text_edits"]
6545

6646
async def fake_send(cmd, params, **kwargs):

Server/tests/integration/test_find_in_file_minimal.py

Lines changed: 0 additions & 8 deletions
This file was deleted.

Server/tests/integration/test_get_sha.py

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,11 @@
11
import pytest
22

3-
from .test_helpers import DummyContext
4-
5-
6-
class DummyMCP:
7-
def __init__(self):
8-
self.tools = {}
9-
10-
def tool(self, *args, **kwargs):
11-
def deco(fn):
12-
self.tools[fn.__name__] = fn
13-
return fn
14-
return deco
15-
16-
17-
def setup_tools():
18-
mcp = DummyMCP()
19-
# Import the tools module to trigger decorator registration
20-
import services.tools.manage_script
21-
# Get the registered tools from the registry
22-
from services.registry import get_registered_tools
23-
tools = get_registered_tools()
24-
# Add all script-related tools to our dummy MCP
25-
for tool_info in tools:
26-
tool_name = tool_info['name']
27-
if any(keyword in tool_name for keyword in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
28-
mcp.tools[tool_name] = tool_info['func']
29-
return mcp.tools
3+
from .test_helpers import DummyContext, setup_script_tools
304

315

326
@pytest.mark.asyncio
337
async def test_get_sha_param_shape_and_routing(monkeypatch):
34-
tools = setup_tools()
8+
tools = setup_script_tools()
359
get_sha = tools["get_sha"]
3610

3711
captured = {}

Server/tests/integration/test_helpers.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,36 @@ def set_state(self, key, value):
5353
def get_state(self, key, default=None):
5454
"""Get state value (mimics FastMCP context.get_state)"""
5555
return self._state.get(key, default)
56+
57+
58+
class DummyMCP:
59+
"""Mock MCP server for testing tool registration patterns."""
60+
61+
def __init__(self):
62+
self.tools = {}
63+
64+
def tool(self, *args, **kwargs):
65+
def deco(fn):
66+
self.tools[fn.__name__] = fn
67+
return fn
68+
return deco
69+
70+
71+
def setup_script_tools():
72+
"""
73+
Setup script-related tools for testing.
74+
75+
Returns a dict mapping tool names to their async handler functions.
76+
Useful for testing parameter serialization and SDK behavior.
77+
"""
78+
mcp = DummyMCP()
79+
# Import tools to trigger decorator-based registration
80+
import services.tools.manage_script
81+
from services.registry import get_registered_tools
82+
83+
for tool_info in get_registered_tools():
84+
name = tool_info['name']
85+
if any(k in name for k in ['script', 'apply_text', 'create_script',
86+
'delete_script', 'validate_script', 'get_sha']):
87+
mcp.tools[name] = tool_info['func']
88+
return mcp.tools

Server/tests/integration/test_instance_routing_comprehensive.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -154,24 +154,6 @@ def _create_mock_context_with_instance(self, instance_id: str):
154154

155155
return ctx
156156

157-
@pytest.mark.parametrize("tool_category,tool_names", [
158-
("GameObject", ["manage_gameobject"]),
159-
("Asset", ["manage_asset"]),
160-
("Scene", ["manage_scene"]),
161-
("Editor", ["manage_editor"]),
162-
("Console", ["read_console"]),
163-
("Menu", ["execute_menu_item"]),
164-
("Shader", ["manage_shader"]),
165-
("Prefab", ["manage_prefabs"]),
166-
("Tests", ["run_tests"]),
167-
("Script", ["create_script", "delete_script",
168-
"apply_text_edits", "script_apply_edits"]),
169-
("Resources", ["unity_instances", "menu_items", "tests"]),
170-
])
171-
def test_tool_category_respects_active_instance(self, tool_category, tool_names):
172-
"""All tool categories must respect set_active_instance."""
173-
# This is a specification test - individual tools need separate implementation tests
174-
pass # Placeholder for category-level test
175157

176158

177159
class TestInstanceRoutingHTTP:

Server/tests/integration/test_logging_stdout.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@
1818
)
1919

2020

21-
@pytest.mark.skip(reason="TODO: ensure server logs only to stderr and rotating file")
22-
def test_no_stdout_output_from_tools():
23-
pass
24-
25-
2621
def test_no_print_statements_in_codebase():
2722
"""Ensure no stray print/sys.stdout writes remain in server source."""
2823
offenders = []

Server/tests/integration/test_resources_api.py

Lines changed: 0 additions & 7 deletions
This file was deleted.

Server/tests/integration/test_script_editing.py

Lines changed: 0 additions & 36 deletions
This file was deleted.

Server/tests/integration/test_telemetry_queue_worker.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ def slow_send(self, rec):
3434
elapsed_ms = (time.perf_counter() - start) * 1000.0
3535

3636
# Should be fast despite backpressure (non-blocking enqueue or drop)
37-
# Timeout relaxed to 200ms to handle thread scheduling variance in CI/local environments
38-
assert elapsed_ms < 200.0, f"Took {elapsed_ms:.1f}ms (expected <200ms)"
37+
# Threshold set high (500ms) to accommodate CI environments with variable load.
38+
# The key assertion is that 50 record() calls don't block on a full queue;
39+
# even under heavy CI load, non-blocking calls should complete well under 500ms.
40+
assert elapsed_ms < 500.0, f"Took {elapsed_ms:.1f}ms (expected <500ms for non-blocking calls)"
3941

4042
# Allow worker to process some
4143
time.sleep(0.3)

0 commit comments

Comments
 (0)