Clean up Unity and Python tests

Server/tests/integration/test_edit_normalization_and_noop.py

@@ -1,34 +1,11 @@
 import pytest
 
-from .test_helpers import DummyContext
-
-
-class DummyMCP:
-    def __init__(self): self.tools = {}
-
-    def tool(self, *args, **kwargs):
-        def deco(fn): self.tools[fn.__name__] = fn; return fn
-        return deco
-
-
-def setup_tools():
-    mcp = DummyMCP()
-    # Import the tools module to trigger decorator registration
-    import services.tools.manage_script
-    # Get the registered tools from the registry
-    from services.registry import get_registered_tools
-    tools = get_registered_tools()
-    # Add all script-related tools to our dummy MCP
-    for tool_info in tools:
-        tool_name = tool_info['name']
-        if any(keyword in tool_name for keyword in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
-            mcp.tools[tool_name] = tool_info['func']
-    return mcp.tools
+from .test_helpers import DummyContext, DummyMCP, setup_script_tools
 
 
 @pytest.mark.asyncio
 async def test_normalizes_lsp_and_index_ranges(monkeypatch):
-    tools = setup_tools()
+    tools = setup_script_tools()
     apply = tools["apply_text_edits"]
     calls = []
 
@@ -88,7 +65,7 @@ async def fake_read(cmd, params, **kwargs):
 
 @pytest.mark.asyncio
 async def test_noop_evidence_shape(monkeypatch):
-    tools = setup_tools()
+    tools = setup_script_tools()
     apply = tools["apply_text_edits"]
     # Route response from Unity indicating no-op
 
@@ -120,19 +97,8 @@ async def fake_send(cmd, params, **kwargs):
 
 @pytest.mark.asyncio
 async def test_atomic_multi_span_and_relaxed(monkeypatch):
-    tools_text = setup_tools()
+    tools_text = setup_script_tools()
     apply_text = tools_text["apply_text_edits"]
-    tools_struct = DummyMCP()
-    # Import the tools module to trigger decorator registration
-    import services.tools.script_apply_edits
-    # Get the registered tools from the registry
-    from services.registry import get_registered_tools
-    tools = get_registered_tools()
-    # Add all script-related tools to our dummy MCP
-    for tool_info in tools:
-        tool_name = tool_info['name']
-        if any(keyword in tool_name for keyword in ['script_apply', 'apply_edits']):
-            tools_struct.tools[tool_name] = tool_info['func']
     # Fake send for read and write; verify atomic applyMode and validate=relaxed passes through
     sent = {}
 

Server/tests/integration/test_edit_strict_and_warnings.py

@@ -1,31 +1,11 @@
 import pytest
 
-from .test_helpers import DummyContext
-
-
-class DummyMCP:
-    def __init__(self): self.tools = {}
-
-    def tool(self, *args, **kwargs):
-        def deco(fn): self.tools[fn.__name__] = fn; return fn
-        return deco
-
-
-def setup_tools():
-    mcp = DummyMCP()
-    # Import tools to trigger decorator-based registration
-    import services.tools.manage_script
-    from services.registry import get_registered_tools
-    for tool_info in get_registered_tools():
-        name = tool_info['name']
-        if any(k in name for k in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
-            mcp.tools[name] = tool_info['func']
-    return mcp.tools
+from .test_helpers import DummyContext, setup_script_tools
 
 
 @pytest.mark.asyncio
 async def test_explicit_zero_based_normalized_warning(monkeypatch):
-    tools = setup_tools()
+    tools = setup_script_tools()
     apply_edits = tools["apply_text_edits"]
 
     async def fake_send(cmd, params, **kwargs):
@@ -60,7 +40,7 @@ async def fake_send(cmd, params, **kwargs):
 
 @pytest.mark.asyncio
 async def test_strict_zero_based_error(monkeypatch):
-    tools = setup_tools()
+    tools = setup_script_tools()
     apply_edits = tools["apply_text_edits"]
 
     async def fake_send(cmd, params, **kwargs):

Server/tests/integration/test_get_sha.py

@@ -1,37 +1,11 @@
 import pytest
 
-from .test_helpers import DummyContext
-
-
-class DummyMCP:
-    def __init__(self):
-        self.tools = {}
-
-    def tool(self, *args, **kwargs):
-        def deco(fn):
-            self.tools[fn.__name__] = fn
-            return fn
-        return deco
-
-
-def setup_tools():
-    mcp = DummyMCP()
-    # Import the tools module to trigger decorator registration
-    import services.tools.manage_script
-    # Get the registered tools from the registry
-    from services.registry import get_registered_tools
-    tools = get_registered_tools()
-    # Add all script-related tools to our dummy MCP
-    for tool_info in tools:
-        tool_name = tool_info['name']
-        if any(keyword in tool_name for keyword in ['script', 'apply_text', 'create_script', 'delete_script', 'validate_script', 'get_sha']):
-            mcp.tools[tool_name] = tool_info['func']
-    return mcp.tools
+from .test_helpers import DummyContext, setup_script_tools
 
 
 @pytest.mark.asyncio
 async def test_get_sha_param_shape_and_routing(monkeypatch):
-    tools = setup_tools()
+    tools = setup_script_tools()
     get_sha = tools["get_sha"]
 
     captured = {}

Server/tests/integration/test_helpers.py

@@ -53,3 +53,36 @@ def set_state(self, key, value):
     def get_state(self, key, default=None):
         """Get state value (mimics FastMCP context.get_state)"""
         return self._state.get(key, default)
+
+
+class DummyMCP:
+    """Mock MCP server for testing tool registration patterns."""
+
+    def __init__(self):
+        self.tools = {}
+
+    def tool(self, *args, **kwargs):
+        def deco(fn):
+            self.tools[fn.__name__] = fn
+            return fn
+        return deco
+
+
+def setup_script_tools():
+    """
+    Setup script-related tools for testing.
+
+    Returns a dict mapping tool names to their async handler functions.
+    Useful for testing parameter serialization and SDK behavior.
+    """
+    mcp = DummyMCP()
+    # Import tools to trigger decorator-based registration
+    import services.tools.manage_script
+    from services.registry import get_registered_tools
+
+    for tool_info in get_registered_tools():
+        name = tool_info['name']
+        if any(k in name for k in ['script', 'apply_text', 'create_script',
+                                    'delete_script', 'validate_script', 'get_sha']):
+            mcp.tools[name] = tool_info['func']
+    return mcp.tools

Server/tests/integration/test_instance_routing_comprehensive.py

@@ -154,24 +154,6 @@ def _create_mock_context_with_instance(self, instance_id: str):
 
         return ctx
 
-    @pytest.mark.parametrize("tool_category,tool_names", [
-        ("GameObject", ["manage_gameobject"]),
-        ("Asset", ["manage_asset"]),
-        ("Scene", ["manage_scene"]),
-        ("Editor", ["manage_editor"]),
-        ("Console", ["read_console"]),
-        ("Menu", ["execute_menu_item"]),
-        ("Shader", ["manage_shader"]),
-        ("Prefab", ["manage_prefabs"]),
-        ("Tests", ["run_tests"]),
-        ("Script", ["create_script", "delete_script",
-         "apply_text_edits", "script_apply_edits"]),
-        ("Resources", ["unity_instances", "menu_items", "tests"]),
-    ])
-    def test_tool_category_respects_active_instance(self, tool_category, tool_names):
-        """All tool categories must respect set_active_instance."""
-        # This is a specification test - individual tools need separate implementation tests
-        pass  # Placeholder for category-level test
 
 
 class TestInstanceRoutingHTTP:

Server/tests/integration/test_logging_stdout.py

@@ -18,11 +18,6 @@
     )
 
 
-@pytest.mark.skip(reason="TODO: ensure server logs only to stderr and rotating file")
-def test_no_stdout_output_from_tools():
-    pass
-
-
 def test_no_print_statements_in_codebase():
     """Ensure no stray print/sys.stdout writes remain in server source."""
     offenders = []

Server/tests/integration/test_telemetry_queue_worker.py

@@ -34,8 +34,10 @@ def slow_send(self, rec):
     elapsed_ms = (time.perf_counter() - start) * 1000.0
 
     # Should be fast despite backpressure (non-blocking enqueue or drop)
-    # Timeout relaxed to 200ms to handle thread scheduling variance in CI/local environments
-    assert elapsed_ms < 200.0, f"Took {elapsed_ms:.1f}ms (expected <200ms)"
+    # Threshold set high (500ms) to accommodate CI environments with variable load.
+    # The key assertion is that 50 record() calls don't block on a full queue;
+    # even under heavy CI load, non-blocking calls should complete well under 500ms.
+    assert elapsed_ms < 500.0, f"Took {elapsed_ms:.1f}ms (expected <500ms for non-blocking calls)"
 
     # Allow worker to process some
     time.sleep(0.3)

Server/tests/integration/test_transport_framing.py

@@ -201,21 +201,3 @@ def _run():
         conn.disconnect()
 
 
-@pytest.mark.skip(reason="TODO: oversized payload should disconnect")
-def test_oversized_payload_rejected():
-    pass
-
-
-@pytest.mark.skip(reason="TODO: partial header/payload triggers timeout and disconnect")
-def test_partial_frame_timeout():
-    pass
-
-
-@pytest.mark.skip(reason="TODO: concurrency test with parallel tool invocations")
-def test_parallel_invocations_no_interleaving():
-    pass
-
-
-@pytest.mark.skip(reason="TODO: reconnection after drop mid-command")
-def test_reconnect_mid_command():
-    pass

TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs

@@ -43,11 +43,13 @@ public void AutoDiscovery_RegistersAllBuiltInTools()
 
             foreach (var toolName in expectedTools)
             {
-                Assert.DoesNotThrow(() =>
-                {
-                    var handler = CommandRegistry.GetHandler(toolName);
-                    Assert.IsNotNull(handler, $"Handler for '{toolName}' should not be null");
-                }, $"Expected tool '{toolName}' to be auto-registered");
+                var handler = CommandRegistry.GetHandler(toolName);
+                Assert.IsNotNull(handler, $"Handler for '{toolName}' should not be null");
+
+                // Verify the handler is actually callable (returns a result, not throws)
+                var emptyParams = new Newtonsoft.Json.Linq.JObject();
+                var result = handler(emptyParams);
+                Assert.IsNotNull(result, $"Handler for '{toolName}' should return a result even for empty params");
             }
         }
     }