diff --git a/Server/tests/integration/test_edit_normalization_and_noop.py b/Server/tests/integration/test_edit_normalization_and_noop.py index 2eee7f3..90c5c08 100644 --- a/Server/tests/integration/test_edit_normalization_and_noop.py +++ b/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 test_normalizes_lsp_and_index_ranges(monkeypatch): @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 test_noop_evidence_shape(monkeypatch): @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 = {} diff --git a/Server/tests/integration/test_edit_strict_and_warnings.py b/Server/tests/integration/test_edit_strict_and_warnings.py index 21084de..55d0f4b 100644 --- a/Server/tests/integration/test_edit_strict_and_warnings.py +++ b/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 test_explicit_zero_based_normalized_warning(monkeypatch): @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): diff --git a/Server/tests/integration/test_find_in_file_minimal.py b/Server/tests/integration/test_find_in_file_minimal.py deleted file mode 100644 index 268ea15..0000000 --- a/Server/tests/integration/test_find_in_file_minimal.py +++ /dev/null @@ -1,8 +0,0 @@ -import asyncio -import pytest - -from .test_helpers import DummyContext - - -# Tests for resource_tools.py have been removed since the file was deleted -# These tests were confusing LLMs that read resources diff --git a/Server/tests/integration/test_get_sha.py b/Server/tests/integration/test_get_sha.py index d8632b0..4c8e89f 100644 --- a/Server/tests/integration/test_get_sha.py +++ b/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 = {} diff --git a/Server/tests/integration/test_helpers.py b/Server/tests/integration/test_helpers.py index 3c31533..cfa5593 100644 --- a/Server/tests/integration/test_helpers.py +++ b/Server/tests/integration/test_helpers.py @@ -53,3 +53,36 @@ class DummyContext: 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 diff --git a/Server/tests/integration/test_instance_routing_comprehensive.py b/Server/tests/integration/test_instance_routing_comprehensive.py index 676a430..e0b8be8 100644 --- a/Server/tests/integration/test_instance_routing_comprehensive.py +++ b/Server/tests/integration/test_instance_routing_comprehensive.py @@ -154,24 +154,6 @@ class TestInstanceRoutingToolCategories: 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: diff --git a/Server/tests/integration/test_logging_stdout.py b/Server/tests/integration/test_logging_stdout.py index cba13af..66d5d39 100644 --- a/Server/tests/integration/test_logging_stdout.py +++ b/Server/tests/integration/test_logging_stdout.py @@ -18,11 +18,6 @@ if SRC is None: ) -@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 = [] diff --git a/Server/tests/integration/test_resources_api.py b/Server/tests/integration/test_resources_api.py deleted file mode 100644 index 4c64e4b..0000000 --- a/Server/tests/integration/test_resources_api.py +++ /dev/null @@ -1,7 +0,0 @@ -import pytest - -from .test_helpers import DummyContext - - -# Tests for resource_tools.py have been removed since the file was deleted -# These tests were confusing LLMs that read resources diff --git a/Server/tests/integration/test_script_editing.py b/Server/tests/integration/test_script_editing.py deleted file mode 100644 index 88046d0..0000000 --- a/Server/tests/integration/test_script_editing.py +++ /dev/null @@ -1,36 +0,0 @@ -import pytest - - -@pytest.mark.xfail(strict=False, reason="pending: create new script, validate, apply edits, build and compile scene") -def test_script_edit_happy_path(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: multiple micro-edits debounce to single compilation") -def test_micro_edits_debounce(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: line ending variations handled correctly") -def test_line_endings_and_columns(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: regex_replace no-op with allow_noop honored") -def test_regex_replace_noop_allowed(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: large edit size boundaries and overflow protection") -def test_large_edit_size_and_overflow(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: symlink and junction protections on edits") -def test_symlink_and_junction_protection(): - pass - - -@pytest.mark.xfail(strict=False, reason="pending: atomic write guarantees") -def test_atomic_write_guarantees(): - pass diff --git a/Server/tests/integration/test_telemetry_queue_worker.py b/Server/tests/integration/test_telemetry_queue_worker.py index dbdbe12..012760f 100644 --- a/Server/tests/integration/test_telemetry_queue_worker.py +++ b/Server/tests/integration/test_telemetry_queue_worker.py @@ -34,8 +34,10 @@ def test_telemetry_queue_backpressure_and_single_worker(monkeypatch, caplog): 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) diff --git a/Server/tests/integration/test_transport_framing.py b/Server/tests/integration/test_transport_framing.py index f9999ea..78bc20a 100644 --- a/Server/tests/integration/test_transport_framing.py +++ b/Server/tests/integration/test_transport_framing.py @@ -201,21 +201,3 @@ def test_zero_length_payload_heartbeat(): 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 diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs index 1637a5b..dbb6585 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/CommandRegistryTests.cs @@ -43,11 +43,13 @@ namespace MCPForUnityTests.Editor.Tools 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"); } } } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs index 7521625..a5d158a 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MCPToolParameterTests.cs @@ -12,11 +12,8 @@ using System.Text.RegularExpressions; namespace MCPForUnityTests.Editor.Tools { /// - /// Tests specifically for MCP tool parameter handling issues. - /// These tests focus on the actual problems we encountered: - /// 1. JSON parameter parsing in manage_asset and manage_gameobject tools - /// 2. Material creation with properties through MCP tools - /// 3. Material assignment through MCP tools + /// Tests for MCP tool parameter handling - JSON parsing in manage_asset and manage_gameobject tools. + /// Consolidated from multiple redundant tests into focused, non-overlapping test cases. /// public class MCPToolParameterTests { @@ -35,7 +32,6 @@ namespace MCPForUnityTests.Editor.Tools private static void AssertShaderIsSupported(Shader s) { Assert.IsNotNull(s, "Shader should not be null"); - // Accept common defaults across pipelines var name = s.name; bool ok = name == "Universal Render Pipeline/Lit" || name == "HDRP/Lit" @@ -44,89 +40,44 @@ namespace MCPForUnityTests.Editor.Tools Assert.IsTrue(ok, $"Unexpected shader: {name}"); } + private static void EnsureTempFolders() + { + if (!AssetDatabase.IsValidFolder("Assets/Temp")) + AssetDatabase.CreateFolder("Assets", "Temp"); + if (!AssetDatabase.IsValidFolder(TempDir)) + AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); + if (!AssetDatabase.IsValidFolder(TempLiveDir)) + AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); + } + [TearDown] public void TearDown() { - // Clean up temp directories after each test if (AssetDatabase.IsValidFolder(TempDir)) - { AssetDatabase.DeleteAsset(TempDir); - } - if (AssetDatabase.IsValidFolder(TempLiveDir)) - { AssetDatabase.DeleteAsset(TempLiveDir); - } - // Clean up parent Temp folder if it's empty if (AssetDatabase.IsValidFolder("Assets/Temp")) { var remainingDirs = Directory.GetDirectories("Assets/Temp"); var remainingFiles = Directory.GetFiles("Assets/Temp"); if (remainingDirs.Length == 0 && remainingFiles.Length == 0) - { AssetDatabase.DeleteAsset("Assets/Temp"); - } - } - } - [Test] - public void Test_ManageAsset_ShouldAcceptJSONProperties() - { - // Arrange: create temp folder - if (!AssetDatabase.IsValidFolder("Assets/Temp")) - { - AssetDatabase.CreateFolder("Assets", "Temp"); - } - if (!AssetDatabase.IsValidFolder(TempDir)) - { - AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); - } - - var matPath = $"{TempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat"; - - // Build params with properties as a JSON string (to be coerced) - var p = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [0,0,1,1]}" - }; - - try - { - var raw = ManageAsset.HandleCommand(p); - var result = raw as JObject ?? JObject.FromObject(raw); - Assert.IsNotNull(result, "Handler should return a JObject result"); - Assert.IsTrue(result!.Value("success"), result.ToString()); - - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should be created at path"); - AssertShaderIsSupported(mat.shader); - if (mat.HasProperty("_Color")) - { - Assert.AreEqual(Color.blue, mat.GetColor("_Color")); - } - } - finally - { - if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { - AssetDatabase.DeleteAsset(matPath); - } - AssetDatabase.Refresh(); } } + /// + /// Tests GameObject componentProperties JSON string coercion path. + /// Verifies material assignment via JSON string works correctly. + /// [Test] - public void Test_ManageGameObject_ShouldAcceptJSONComponentProperties() + public void ManageGameObject_JSONComponentProperties_AssignsMaterial() { - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); - var matPath = $"{tempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat"; + EnsureTempFolders(); + var matPath = $"{TempDir}/Mat_{Guid.NewGuid():N}.mat"; - // Create a material first (object-typed properties) + // Create material with object-typed properties var createMat = new JObject { ["action"] = "create", @@ -160,7 +111,7 @@ namespace MCPForUnityTests.Editor.Tools var result = raw as JObject ?? JObject.FromObject(raw); Assert.IsTrue(result.Value("success"), result.ToString()); - // Verify material assignment and shader on the GameObject's MeshRenderer + // Verify material assignment var goVerify = GameObject.Find("MCPParamTestSphere"); Assert.IsNotNull(goVerify, "GameObject should exist"); var renderer = goVerify.GetComponent(); @@ -175,367 +126,54 @@ namespace MCPForUnityTests.Editor.Tools { var go = GameObject.Find("MCPParamTestSphere"); if (go != null) UnityEngine.Object.DestroyImmediate(go); - if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_JSONParsing_ShouldWorkInMCPTools() - { - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); - var matPath = $"{tempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat"; - - // manage_asset with JSON string properties (coercion path) - var createMat = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [0,0,1,1]}" - }; - var createResRaw = ManageAsset.HandleCommand(createMat); - var createRes = createResRaw as JObject ?? JObject.FromObject(createResRaw); - Assert.IsTrue(createRes.Value("success"), createRes.ToString()); - - // Create sphere and assign material (object-typed componentProperties) - var go = new JObject { ["action"] = "create", ["name"] = "MCPParamJSONSphere", ["primitiveType"] = "Sphere" }; - var goRes = ManageGameObject.HandleCommand(go); - var goObj = goRes as JObject ?? JObject.FromObject(goRes); - Assert.IsTrue(goObj.Value("success"), goObj.ToString()); - - try - { - var compJsonObj = new JObject { ["MeshRenderer"] = new JObject { ["sharedMaterial"] = matPath } }; - var compJson = compJsonObj.ToString(Newtonsoft.Json.Formatting.None); - var modify = new JObject - { - ["action"] = "modify", - ["target"] = "MCPParamJSONSphere", - ["searchMethod"] = "by_name", - ["componentProperties"] = compJson - }; - var modResRaw = ManageGameObject.HandleCommand(modify); - var modRes = modResRaw as JObject ?? JObject.FromObject(modResRaw); - Assert.IsTrue(modRes.Value("success"), modRes.ToString()); - - // Verify shader on created material - var createdMat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(createdMat); - AssertShaderIsSupported(createdMat.shader); - } - finally - { - var goObj2 = GameObject.Find("MCPParamJSONSphere"); - if (goObj2 != null) UnityEngine.Object.DestroyImmediate(goObj2); - if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_ManageAsset_JSONStringParsing_CreateMaterial() - { - // Test that JSON string properties are correctly parsed and applied - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - var matPath = $"{tempDir}/JsonStringTest_{Guid.NewGuid().ToString("N")}.mat"; - - var p = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [1,0,0,1]}" - }; - - try - { - var raw = ManageAsset.HandleCommand(p); - var result = raw as JObject ?? JObject.FromObject(raw); - Assert.IsNotNull(result, "Handler should return a JObject result"); - Assert.IsTrue(result!.Value("success"), $"Create failed: {result}"); - - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should be created at path"); - AssertShaderIsSupported(mat.shader); - if (mat.HasProperty("_Color")) - { - Assert.AreEqual(Color.red, mat.GetColor("_Color"), "Material should have red color"); - } - } - finally - { if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { AssetDatabase.DeleteAsset(matPath); - } AssetDatabase.Refresh(); } } + /// + /// Comprehensive end-to-end test covering all 10 property handling scenarios: + /// 1. Create material via JSON string + /// 2. Modify color and metallic (friendly names) + /// 3. Modify using structured float block + /// 4. Assign texture via direct prop alias + /// 5. Assign texture via structured block + /// 6. Create sphere and assign material via componentProperties JSON + /// 7. Use URP color alias key + /// 8. Invalid JSON handling (graceful degradation) + /// 9. Switch shader pipeline dynamically + /// 10. Mixed friendly and alias keys + /// [Test] - public void Test_ManageAsset_JSONStringParsing_ModifyMaterial() + public void EndToEnd_PropertyHandling_AllScenarios() { - // Test that JSON string properties work for modify operations - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - var matPath = $"{tempDir}/JsonStringModifyTest_{Guid.NewGuid().ToString("N")}.mat"; - - // First create a material - var createParams = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [0,1,0,1]}" - }; - - try - { - var createRaw = ManageAsset.HandleCommand(createParams); - var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); - Assert.IsTrue(createResult!.Value("success"), "Create should succeed"); - - // Now modify with JSON string - var modifyParams = new JObject - { - ["action"] = "modify", - ["path"] = matPath, - ["properties"] = "{\"color\": [0,0,1,1]}" - }; - - var modifyRaw = ManageAsset.HandleCommand(modifyParams); - var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); - Assert.IsNotNull(modifyResult, "Modify should return a result"); - Assert.IsTrue(modifyResult!.Value("success"), $"Modify failed: {modifyResult}"); - - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should exist"); - AssertShaderIsSupported(mat.shader); - if (mat.HasProperty("_Color")) - { - Assert.AreEqual(Color.blue, mat.GetColor("_Color"), "Material should have blue color after modify"); - } - } - finally - { - if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { - AssetDatabase.DeleteAsset(matPath); - } - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_ManageAsset_InvalidJSONString_HandledGracefully() - { - // Test that invalid JSON strings are handled gracefully - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - var matPath = $"{tempDir}/InvalidJsonTest_{Guid.NewGuid().ToString("N")}.mat"; - - var p = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"invalid\": json, \"missing\": quotes}" // Invalid JSON - }; - - try - { - LogAssert.Expect(LogType.Warning, new Regex("(failed to parse)|(Could not parse 'properties' JSON string)", RegexOptions.IgnoreCase)); - var raw = ManageAsset.HandleCommand(p); - var result = raw as JObject ?? JObject.FromObject(raw); - // Should either succeed with defaults or fail gracefully - Assert.IsNotNull(result, "Handler should return a result"); - // The result might be success (with defaults) or failure, both are acceptable - } - finally - { - if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { - AssetDatabase.DeleteAsset(matPath); - } - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_ManageAsset_JSONStringParsing_FloatProperty_Metallic_CreateAndModify() - { - // Validate float property handling via JSON string for create and modify - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); - var matPath = $"{tempDir}/JsonFloatTest_{Guid.NewGuid().ToString("N")}.mat"; - - var createParams = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"metallic\": 0.75}" - }; - - try - { - var createRaw = ManageAsset.HandleCommand(createParams); - var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); - Assert.IsTrue(createResult!.Value("success"), createResult.ToString()); - - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should be created at path"); - AssertShaderIsSupported(mat.shader); - if (mat.HasProperty("_Metallic")) - { - Assert.AreEqual(0.75f, mat.GetFloat("_Metallic"), 1e-3f, "Metallic should be ~0.75 after create"); - } - - var modifyParams = new JObject - { - ["action"] = "modify", - ["path"] = matPath, - ["properties"] = "{\"metallic\": 0.1}" - }; - - var modifyRaw = ManageAsset.HandleCommand(modifyParams); - var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); - Assert.IsTrue(modifyResult!.Value("success"), modifyResult.ToString()); - - var mat2 = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat2, "Material should still exist"); - if (mat2.HasProperty("_Metallic")) - { - Assert.AreEqual(0.1f, mat2.GetFloat("_Metallic"), 1e-3f, "Metallic should be ~0.1 after modify"); - } - } - finally - { - if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { - AssetDatabase.DeleteAsset(matPath); - } - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_ManageAsset_JSONStringParsing_TextureAssignment_CreateAndModify() - { - // Uses flexible direct property assignment to set _BaseMap/_MainTex by path - const string tempDir = "Assets/Temp/MCPToolParameterTests"; - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); - var matPath = $"{tempDir}/JsonTexTest_{Guid.NewGuid().ToString("N")}.mat"; - var texPath = "Assets/Temp/LiveTests/TempBaseTex.asset"; // created by GenTempTex - - // Ensure the texture exists BEFORE creating the material so assignment succeeds during create - var preTex = AssetDatabase.LoadAssetAtPath(texPath); - if (preTex == null) - { - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder("Assets/Temp/LiveTests")) AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); - var tex2D = new Texture2D(4, 4, TextureFormat.RGBA32, false); - var pixels = new Color[16]; - for (int i = 0; i < pixels.Length; i++) pixels[i] = Color.white; - tex2D.SetPixels(pixels); - tex2D.Apply(); - AssetDatabase.CreateAsset(tex2D, texPath); - AssetDatabase.SaveAssets(); - AssetDatabase.Refresh(); - } - - var createParams = new JObject - { - ["action"] = "create", - ["path"] = matPath, - ["assetType"] = "Material", - ["properties"] = new JObject - { - ["shader"] = "Universal Render Pipeline/Lit", - ["_BaseMap"] = texPath // resolves to _BaseMap or _MainTex internally - } - }; - - try - { - var createRaw = ManageAsset.HandleCommand(createParams); - var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); - Assert.IsTrue(createResult!.Value("success"), createResult.ToString()); - - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should be created at path"); - AssertShaderIsSupported(mat.shader); - var tex = AssetDatabase.LoadAssetAtPath(texPath); - if (tex == null) - { - // Create a tiny white texture if missing to make the test self-sufficient - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder("Assets/Temp/LiveTests")) AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); - var tex2D = new Texture2D(4, 4, TextureFormat.RGBA32, false); - var pixels = new Color[16]; - for (int i = 0; i < pixels.Length; i++) pixels[i] = Color.white; - tex2D.SetPixels(pixels); - tex2D.Apply(); - AssetDatabase.CreateAsset(tex2D, texPath); - AssetDatabase.SaveAssets(); - AssetDatabase.Refresh(); - tex = AssetDatabase.LoadAssetAtPath(texPath); - } - Assert.IsNotNull(tex, "Test texture should exist"); - // Verify either _BaseMap or _MainTex holds the texture - bool hasTexture = (mat.HasProperty("_BaseMap") && mat.GetTexture("_BaseMap") == tex) - || (mat.HasProperty("_MainTex") && mat.GetTexture("_MainTex") == tex); - Assert.IsTrue(hasTexture, "Material should reference the assigned texture"); - - // Modify by changing to same texture via alternate alias key - var modifyParams = new JObject - { - ["action"] = "modify", - ["path"] = matPath, - ["properties"] = new JObject { ["_MainTex"] = texPath } - }; - var modifyRaw = ManageAsset.HandleCommand(modifyParams); - var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); - Assert.IsTrue(modifyResult!.Value("success"), modifyResult.ToString()); - - var mat2 = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat2); - bool hasTexture2 = (mat2.HasProperty("_BaseMap") && mat2.GetTexture("_BaseMap") == tex) - || (mat2.HasProperty("_MainTex") && mat2.GetTexture("_MainTex") == tex); - Assert.IsTrue(hasTexture2, "Material should keep the assigned texture after modify"); - } - finally - { - if (AssetDatabase.LoadAssetAtPath(matPath) != null) - { - AssetDatabase.DeleteAsset(matPath); - } - AssetDatabase.Refresh(); - } - } - - [Test] - public void Test_EndToEnd_PropertyHandling_AllScenarios() - { - // Comprehensive end-to-end test of all 10 property handling scenarios - const string tempDir = "Assets/Temp/LiveTests"; - if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); - if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); + EnsureTempFolders(); string guidSuffix = Guid.NewGuid().ToString("N").Substring(0, 8); - string matPath = $"{tempDir}/Mat_{guidSuffix}.mat"; - string texPath = $"{tempDir}/TempBaseTex.asset"; + string matPath = $"{TempLiveDir}/Mat_{guidSuffix}.mat"; + string texPath = $"{TempLiveDir}/TempBaseTex_{guidSuffix}.asset"; string sphereName = $"LiveSphere_{guidSuffix}"; - string badJsonPath = $"{tempDir}/BadJson_{guidSuffix}.mat"; + string badJsonPath = $"{TempLiveDir}/BadJson_{guidSuffix}.mat"; - // Ensure clean state from previous runs + // Ensure clean state var preSphere = GameObject.Find(sphereName); if (preSphere != null) UnityEngine.Object.DestroyImmediate(preSphere); - if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); - if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) AssetDatabase.DeleteAsset(badJsonPath); + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + AssetDatabase.DeleteAsset(matPath); + if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) + AssetDatabase.DeleteAsset(badJsonPath); + if (AssetDatabase.LoadAssetAtPath(texPath) != null) + AssetDatabase.DeleteAsset(texPath); + + // Create test texture for texture-dependent scenarios (4, 5, 10) + var tex = new Texture2D(4, 4, TextureFormat.RGBA32, false); + var pixels = new Color[16]; + for (int i = 0; i < pixels.Length; i++) pixels[i] = Color.white; + tex.SetPixels(pixels); + tex.Apply(); + AssetDatabase.CreateAsset(tex, texPath); + AssetDatabase.SaveAssets(); AssetDatabase.Refresh(); try @@ -553,11 +191,10 @@ namespace MCPForUnityTests.Editor.Tools Assert.IsTrue(createResult.Value("success"), $"Test 1 failed: {createResult}"); var mat = AssetDatabase.LoadAssetAtPath(matPath); Assert.IsNotNull(mat, "Material should be created"); - var expectedRed = Color.red; if (mat.HasProperty("_BaseColor")) - Assert.AreEqual(expectedRed, mat.GetColor("_BaseColor"), "Test 1: _BaseColor should be red"); + Assert.AreEqual(Color.red, mat.GetColor("_BaseColor"), "Test 1: _BaseColor should be red"); else if (mat.HasProperty("_Color")) - Assert.AreEqual(expectedRed, mat.GetColor("_Color"), "Test 1: _Color should be red"); + Assert.AreEqual(Color.red, mat.GetColor("_Color"), "Test 1: _Color should be red"); else Assert.Inconclusive("Material has neither _BaseColor nor _Color"); @@ -577,8 +214,6 @@ namespace MCPForUnityTests.Editor.Tools Assert.AreEqual(expectedCyan, mat.GetColor("_BaseColor"), "Test 2: _BaseColor should be cyan"); else if (mat.HasProperty("_Color")) Assert.AreEqual(expectedCyan, mat.GetColor("_Color"), "Test 2: _Color should be cyan"); - else - Assert.Inconclusive("Material has neither _BaseColor nor _Color"); Assert.AreEqual(0.6f, mat.GetFloat("_Metallic"), 0.001f, "Test 2: Metallic should be 0.6"); // 3. Modify using structured float block @@ -597,46 +232,30 @@ namespace MCPForUnityTests.Editor.Tools mat = AssetDatabase.LoadAssetAtPath(matPath); Assert.AreEqual(0.1f, mat.GetFloat("_Metallic"), 0.001f, "Test 3: Metallic should be 0.1"); - // 4. Assign texture via direct prop alias (skip if texture doesn't exist) - if (AssetDatabase.LoadAssetAtPath(texPath) != null) + // 4. Assign texture via direct prop alias + var modify3 = new JObject { - var modify3 = new JObject - { - ["action"] = "modify", - ["path"] = matPath, - ["properties"] = "{\"_BaseMap\":\"" + texPath + "\"}" - }; - var modifyRaw3 = ManageAsset.HandleCommand(modify3); - var modifyResult3 = modifyRaw3 as JObject ?? JObject.FromObject(modifyRaw3); - Assert.IsTrue(modifyResult3.Value("success"), $"Test 4 failed: {modifyResult3}"); - Debug.Log("Test 4: Texture assignment successful"); - } - else - { - Debug.LogWarning("Test 4: Skipped - texture not found at " + texPath); - } + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"_BaseMap\":\"" + texPath + "\"}" + }; + var modifyRaw3 = ManageAsset.HandleCommand(modify3); + var modifyResult3 = modifyRaw3 as JObject ?? JObject.FromObject(modifyRaw3); + Assert.IsTrue(modifyResult3.Value("success"), $"Test 4 failed: {modifyResult3}"); - // 5. Assign texture via structured block (skip if texture doesn't exist) - if (AssetDatabase.LoadAssetAtPath(texPath) != null) + // 5. Assign texture via structured block + var modify4 = new JObject { - var modify4 = new JObject + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject { - ["action"] = "modify", - ["path"] = matPath, - ["properties"] = new JObject - { - ["texture"] = new JObject { ["name"] = "_MainTex", ["path"] = texPath } - } - }; - var modifyRaw4 = ManageAsset.HandleCommand(modify4); - var modifyResult4 = modifyRaw4 as JObject ?? JObject.FromObject(modifyRaw4); - Assert.IsTrue(modifyResult4.Value("success"), $"Test 5 failed: {modifyResult4}"); - Debug.Log("Test 5: Structured texture assignment successful"); - } - else - { - Debug.LogWarning("Test 5: Skipped - texture not found at " + texPath); - } + ["texture"] = new JObject { ["name"] = "_MainTex", ["path"] = texPath } + } + }; + var modifyRaw4 = ManageAsset.HandleCommand(modify4); + var modifyResult4 = modifyRaw4 as JObject ?? JObject.FromObject(modifyRaw4); + Assert.IsTrue(modifyResult4.Value("success"), $"Test 5 failed: {modifyResult4}"); // 6. Create sphere and assign material via componentProperties JSON string var createSphere = new JObject @@ -680,15 +299,9 @@ namespace MCPForUnityTests.Editor.Tools mat = AssetDatabase.LoadAssetAtPath(matPath); Color expectedColor = new Color(0.2f, 0.8f, 0.3f, 1f); if (mat.HasProperty("_BaseColor")) - { - Color actualColor = mat.GetColor("_BaseColor"); - AssertColorsEqual(expectedColor, actualColor, "Test 7: _BaseColor should be set"); - } + AssertColorsEqual(expectedColor, mat.GetColor("_BaseColor"), "Test 7: _BaseColor should be set"); else if (mat.HasProperty("_Color")) - { - Color actualColor = mat.GetColor("_Color"); - AssertColorsEqual(expectedColor, actualColor, "Test 7: Fallback _Color should be set"); - } + AssertColorsEqual(expectedColor, mat.GetColor("_Color"), "Test 7: Fallback _Color should be set"); // 8. Invalid JSON should warn (don't fail) var invalidJson = new JObject @@ -701,9 +314,7 @@ namespace MCPForUnityTests.Editor.Tools LogAssert.Expect(LogType.Warning, new Regex("(failed to parse)|(Could not parse 'properties' JSON string)", RegexOptions.IgnoreCase)); var invalidRaw = ManageAsset.HandleCommand(invalidJson); var invalidResult = invalidRaw as JObject ?? JObject.FromObject(invalidRaw); - // Should either succeed with defaults or fail gracefully Assert.IsNotNull(invalidResult, "Test 8: Should return a result"); - Debug.Log($"Test 8: Invalid JSON handled - {invalidResult!["success"]}"); // 9. Switch shader pipeline dynamically var modify6 = new JObject @@ -718,7 +329,9 @@ namespace MCPForUnityTests.Editor.Tools mat = AssetDatabase.LoadAssetAtPath(matPath); Assert.AreEqual("Standard", mat.shader.name, "Test 9: Shader should be Standard"); var c9 = mat.GetColor("_Color"); - Assert.IsTrue(Mathf.Abs(c9.r - 1f) < 0.02f && Mathf.Abs(c9.g - 1f) < 0.02f && Mathf.Abs(c9.b - 0f) < 0.02f, "Test 9: Color should be near yellow"); + // Looser tolerance (0.02) for shader-switched colors due to color space conversion differences + Assert.IsTrue(Mathf.Abs(c9.r - 1f) < 0.02f && Mathf.Abs(c9.g - 1f) < 0.02f && Mathf.Abs(c9.b - 0f) < 0.02f, + "Test 9: Color should be near yellow"); // 10. Mixed friendly and alias keys in one go var modify7 = new JObject @@ -729,7 +342,7 @@ namespace MCPForUnityTests.Editor.Tools { ["metallic"] = 0.8, ["smoothness"] = 0.3, - ["albedo"] = texPath // Texture path if exists + ["albedo"] = texPath } }; var modifyRaw7 = ManageAsset.HandleCommand(modify7); @@ -738,19 +351,19 @@ namespace MCPForUnityTests.Editor.Tools mat = AssetDatabase.LoadAssetAtPath(matPath); Assert.AreEqual(0.8f, mat.GetFloat("_Metallic"), 0.001f, "Test 10: Metallic should be 0.8"); Assert.AreEqual(0.3f, mat.GetFloat("_Glossiness"), 0.001f, "Test 10: Smoothness should be 0.3"); - - Debug.Log("All 10 end-to-end property handling tests completed successfully!"); } finally { - // Cleanup var sphere = GameObject.Find(sphereName); if (sphere != null) UnityEngine.Object.DestroyImmediate(sphere); - if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); - if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) AssetDatabase.DeleteAsset(badJsonPath); + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + AssetDatabase.DeleteAsset(matPath); + if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) + AssetDatabase.DeleteAsset(badJsonPath); + if (AssetDatabase.LoadAssetAtPath(texPath) != null) + AssetDatabase.DeleteAsset(texPath); AssetDatabase.Refresh(); } } - } -} \ No newline at end of file +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs index e42572e..ec89eda 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs @@ -37,7 +37,10 @@ namespace MCPForUnityTests.Editor.Tools var result = ManageGameObject.HandleCommand(null); Assert.IsNotNull(result, "Should return a result object"); - // Note: Actual error checking would need access to Response structure + // Verify the result indicates an error state + var errorResponse = result as MCPForUnity.Editor.Helpers.ErrorResponse; + Assert.IsNotNull(errorResponse, "Should return an ErrorResponse for null params"); + Assert.IsFalse(errorResponse.Success, "Success should be false for null params"); } [Test] @@ -47,6 +50,10 @@ namespace MCPForUnityTests.Editor.Tools var result = ManageGameObject.HandleCommand(emptyParams); Assert.IsNotNull(result, "Should return a result object for empty params"); + // Verify the result indicates an error state (missing required action) + var errorResponse = result as MCPForUnity.Editor.Helpers.ErrorResponse; + Assert.IsNotNull(errorResponse, "Should return an ErrorResponse for empty params"); + Assert.IsFalse(errorResponse.Success, "Success should be false for empty params"); } [Test] diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs index d3b129f..23c2142 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs @@ -163,7 +163,7 @@ namespace MCPForUnityTests.Editor.Tools #region Out of Bounds Test - Auto-Grow Arrays [Test] - public void OutOfBounds_SetElementBeyondArraySize_ShouldFailWithoutAutoGrow() + public void AutoGrow_SetElementBeyondArraySize_AutoResizesArray() { // Create an ArrayStressSO first var createResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject @@ -171,7 +171,7 @@ namespace MCPForUnityTests.Editor.Tools ["action"] = "create", ["typeName"] = "ArrayStressSO", ["folderPath"] = _runRoot, - ["assetName"] = "OutOfBounds", + ["assetName"] = "AutoGrow", ["overwrite"] = true })); Assert.IsTrue(createResult.Value("success"), createResult.ToString()); @@ -180,7 +180,7 @@ namespace MCPForUnityTests.Editor.Tools var guid = createResult["data"]?["guid"]?.ToString(); _createdAssets.Add(path); - // Try to set element at index 99 (array starts with 3 elements) + // Set element at index 99 (array starts with 3 elements) - should auto-grow var modifyResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject { ["action"] = "modify", @@ -196,16 +196,19 @@ namespace MCPForUnityTests.Editor.Tools } })); - // Document current behavior: this should fail without auto-grow - // After hardening, this should succeed var patchResults = modifyResult["data"]?["results"] as JArray; Assert.IsNotNull(patchResults); - + bool patchOk = patchResults[0]?.Value("ok") ?? false; - Debug.Log($"[OutOfBounds] Setting [99] on 3-element array: ok={patchOk}, message={patchResults[0]?["message"]}"); - - // Current expected behavior: fails with "Property not found" - // After Phase 1.2 (auto-resize): should succeed + Assert.IsTrue(patchOk, $"Auto-grow should succeed: {patchResults[0]?["message"]}"); + + // Verify the array was resized and value was set + var asset = AssetDatabase.LoadAssetAtPath(path); + Assert.IsNotNull(asset); + Assert.GreaterOrEqual(asset.floatArray.Length, 100, "Array should have been auto-grown to at least 100 elements"); + Assert.AreEqual(42.0f, asset.floatArray[99], 0.01f, "Value at index 99 should be set"); + + Debug.Log($"[AutoGrow] Array auto-resized to {asset.floatArray.Length} elements, value at [99] = {asset.floatArray[99]}"); } #endregion @@ -213,7 +216,7 @@ namespace MCPForUnityTests.Editor.Tools #region Friendly Path Syntax Test [Test] - public void FriendlySyntax_BracketNotation_ShouldBeNormalized() + public void FriendlySyntax_BracketNotation_IsNormalized() { // Create asset first var createResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject @@ -225,7 +228,6 @@ namespace MCPForUnityTests.Editor.Tools ["overwrite"] = true, ["patches"] = new JArray { - // Resize first using proper syntax new JObject { ["propertyPath"] = "floatArray.Array.size", ["op"] = "array_resize", ["value"] = 5 } } })); @@ -235,7 +237,7 @@ namespace MCPForUnityTests.Editor.Tools var guid = createResult["data"]?["guid"]?.ToString(); _createdAssets.Add(path); - // Try using friendly syntax: floatArray[2] instead of floatArray.Array.data[2] + // Use friendly syntax: floatArray[2] instead of floatArray.Array.data[2] var modifyResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject { ["action"] = "modify", @@ -244,7 +246,7 @@ namespace MCPForUnityTests.Editor.Tools { new JObject { - ["propertyPath"] = "floatArray[2]", // Friendly syntax! + ["propertyPath"] = "floatArray[2]", // Friendly syntax - gets normalized to floatArray.Array.data[2] ["op"] = "set", ["value"] = 123.456f } @@ -255,10 +257,14 @@ namespace MCPForUnityTests.Editor.Tools Assert.IsNotNull(patchResults); bool patchOk = patchResults[0]?.Value("ok") ?? false; - Debug.Log($"[FriendlySyntax] Using floatArray[2] syntax: ok={patchOk}, message={patchResults[0]?["message"]}"); + Assert.IsTrue(patchOk, $"Friendly bracket syntax should be normalized: {patchResults[0]?["message"]}"); - // Current expected behavior: fails with "Property not found: floatArray[2]" - // After Phase 1.1 (path normalization): should succeed + // Verify the value was actually set + var asset = AssetDatabase.LoadAssetAtPath(path); + Assert.IsNotNull(asset); + Assert.AreEqual(123.456f, asset.floatArray[2], 0.001f, "Value at index 2 should be set via friendly syntax"); + + Debug.Log($"[FriendlySyntax] floatArray[2] = {asset.floatArray[2]} (set via friendly bracket notation)"); } #endregion @@ -569,10 +575,10 @@ namespace MCPForUnityTests.Editor.Tools #endregion - #region Bulk Array Mapping Test (Phase 3 feature) + #region Bulk Array Mapping Test [Test] - public void BulkArrayMapping_PassArrayAsValue() + public void BulkArrayMapping_SetsEntireArrayFromJArray() { var createResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject { @@ -588,7 +594,7 @@ namespace MCPForUnityTests.Editor.Tools var guid = createResult["data"]?["guid"]?.ToString(); _createdAssets.Add(path); - // Try to set the entire intArray using a JArray value directly + // Set the entire intArray using a JArray value directly var modifyResult = ToJObject(ManageScriptableObject.HandleCommand(new JObject { ["action"] = "modify", @@ -599,7 +605,7 @@ namespace MCPForUnityTests.Editor.Tools { ["propertyPath"] = "intArray", ["op"] = "set", - ["value"] = new JArray(1, 2, 3, 4, 5) // Bulk array! + ["value"] = new JArray(1, 2, 3, 4, 5) // Bulk array mapping } } })); @@ -608,16 +614,20 @@ namespace MCPForUnityTests.Editor.Tools Assert.IsNotNull(patchResults); bool patchOk = patchResults[0]?.Value("ok") ?? false; - string message = patchResults[0]?["message"]?.ToString() ?? ""; - Debug.Log($"[BulkArrayMapping] Setting intArray to [1,2,3,4,5]: ok={patchOk}, message={message}"); + Assert.IsTrue(patchOk, $"Bulk array mapping should succeed: {patchResults[0]?["message"]}"); - // Current expected behavior: likely fails with unsupported type - // After Phase 3.1 (bulk array mapping): should succeed + // Verify the array was set correctly + var asset = AssetDatabase.LoadAssetAtPath(path); + Assert.IsNotNull(asset); + Assert.AreEqual(5, asset.intArray.Length, "Array should have 5 elements"); + CollectionAssert.AreEqual(new[] { 1, 2, 3, 4, 5 }, asset.intArray, "Array contents should match"); + + Debug.Log($"[BulkArrayMapping] intArray = [{string.Join(", ", asset.intArray)}]"); } #endregion - #region GUID Shorthand Test (Phase 4 feature) + #region GUID Shorthand Test [Test] public void GuidShorthand_PassPlainGuidString() @@ -666,7 +676,7 @@ namespace MCPForUnityTests.Editor.Tools #endregion - #region Dry Run Test (Phase 5 feature) + #region Dry Run Test [Test] public void DryRun_ValidatePatchesWithoutApplying()