test: cover new ReflectorNet atomic API in Unity EditMode by IvanMurzak · Pull Request #687 · IvanMurzak/Unity-MCP

IvanMurzak · 2026-04-28T23:33:08Z

Summary

Adds Unity EditMode tests under Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/ that exercise the new ReflectorNet atomic API surface introduced by IvanMurzak/ReflectorNet#71: TryModifyAt, TryPatch, TryReadAt, View, Grep. The goal is to validate that these APIs continue to work end-to-end inside a real Unity Editor process — managed assembly load context, Unity-specific edge cases, Editor-time object graphs.

5 test files + 1 fixtures file (AtomicApiTestFixtures.cs) added under a new AtomicApi/ subfolder. No existing files were modified.
Tests use small POCOs defined in the test asmdef (CelestialBody, StarSystemPoco, Animal/Dog, CyclicNode). They do NOT consume ReflectorNet's own SolarSystem fixture because that lives in ReflectorNet's test assembly and is not shipped to Unity.
Coverage mirrors what ReflectorNet's own AtomicModifyTests / ViewTests do, scoped to what's reachable from Reflector (no internal helpers).

Coverage

TryModifyAt: root field, array element, dictionary string/int key, partial-element SerializedMember patch, out-of-range index error, missing-member detailed error.
TryPatch: multi-field top-level patch, nested array+dictionary patch, JsonElement overload, "$type" polymorphic replacement (Animal → Dog), null-value erasure (RFC 7396), invalid-JSON failure path.
TryReadAt: read↔write symmetry against the same paths exercised by TryModifyAt, plus invalid-path detailed error.
View / Grep: NameRegex + TypeFilter on a graph with circular references, MaxDepth pruning, flat ViewMatch list, null-input + invalid-regex paths.

Test plan

Local Unity EditMode test gate: 815/815 passed in 3m 35.8s.
Validated against ReflectorNet feature/atomic-api at SHA 2252f623495ff9355f022ca68990807b5280b429 (locally built and dropped into Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.0.0/ReflectorNet.dll for the gate — DLL is not committed; the committed tests target whatever ReflectorNet version is the active dependency at PR-merge time).
CI: awaits Add atomic modification API and JSON patch support with enhancements ReflectorNet#71 to merge AND a ReflectorNet release/NuGet bump that lands the new APIs in com.IvanMurzak.ReflectorNet's shipped DLL. CI will fail until that dependency lands; this is by design.

Notes / follow-ups

The "$type"-based polymorphic replacement is exercised via TryPatch (which has documented $type semantics in the API). The same scenario via TryModifyAt+SerializedMember was investigated and intentionally narrowed to a partial-field patch instead, since member-level type replacement through SerializedMember is an edge case not covered by ReflectorNet's own test suite.

Add Unity EditMode tests under Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi covering the atomic-modify / atomic-read / view / grep API surface introduced by IvanMurzak/ReflectorNet PR #71: - TryModifyAt: root field, array element, dictionary string/int key, partial SerializedMember patch, out-of-range index error, missing-member error. - TryPatch: multi-field top-level patch, nested array+dictionary patch, JsonElement overload, "$type" polymorphic replacement (Animal -> Dog), null-value erasure (RFC 7396), invalid-JSON failure path. - TryReadAt: read-write symmetry against the same paths exercised by TryModifyAt, plus invalid-path detailed error. - View / Grep: NameRegex + TypeFilter on a graph with circular references, MaxDepth pruning, flat ViewMatch list, null-input + invalid-regex paths. Tests use small POCO fixtures defined in the test asmdef (CelestialBody, StarSystemPoco, Animal/Dog, CyclicNode) — they intentionally do NOT consume ReflectorNet.Tests.Model.SolarSystem because that fixture lives in ReflectorNet's own test assembly which is not shipped to Unity. Validated against ReflectorNet feature/atomic-api branch at SHA 2252f623495ff9355f022ca68990807b5280b429 (locally built and dropped into Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.0.0/ReflectorNet.dll for the test gate; the DLL itself is NOT committed — the committed tests target whatever ReflectorNet version is the active dependency at PR-merge time). Local Unity EditMode test gate: 815/815 passed in 3m35.8s.

Copilot

Pull request overview

Adds Unity EditMode tests to validate ReflectorNet’s new “atomic API” behavior end-to-end inside the Unity Editor test environment.

Changes:

Added new AtomicApi/ Editor test suite for TryModifyAt, TryPatch, TryReadAt, View, and Grep.
Added local POCO fixtures to avoid relying on ReflectorNet’s own test assembly.
Added Unity .meta assets for the new folder and test files.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi.meta	Adds new AtomicApi test folder asset.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/AtomicApiTestFixtures.cs	Adds POCO fixtures used by atomic API tests.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/AtomicApiTestFixtures.cs.meta	Unity meta for fixtures file.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryModifyAtTests.cs	Adds EditMode coverage for `Reflector.TryModifyAt`.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryModifyAtTests.cs.meta	Unity meta for TryModifyAt tests.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryPatchTests.cs	Adds EditMode coverage for `Reflector.TryPatch` (merge patch + `$type`).
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryPatchTests.cs.meta	Unity meta for TryPatch tests.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryReadAtTests.cs	Adds EditMode coverage for `Reflector.TryReadAt` symmetry with writes.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/TryReadAtTests.cs.meta	Unity meta for TryReadAt tests.
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/ViewAndGrepTests.cs	Adds EditMode coverage for `Reflector.View` and `Reflector.Grep` (filters, cycles, errors).
Unity-MCP-Plugin/Packages/com.ivanmurzak.unity.mcp/Tests/Editor/AtomicApi/ViewAndGrepTests.cs.meta	Unity meta for View/Grep tests.

Copilot · 2026-04-28T23:38:06Z

+    /// TypeFilter compose as documented.
+    /// </summary>
+    public class ViewAndGrepTests : BaseTest


BaseTest is declared under com.IvanMurzak.Unity.MCP.Editor.Tests, but this file is in com.IvanMurzak.Unity.MCP.Editor.Tests.AtomicApi and doesn't import the parent namespace. This will prevent compilation (BaseTest won't resolve). Add a using for the parent namespace, fully qualify BaseTest, or move the class into the existing test namespace.

Copilot · 2026-04-28T23:38:06Z

+        {
+            var reflector = UnityMcpPluginEditor.Instance.Reflector
+                ?? throw new Exception("Reflector is not available.");
+            var system = new StarSystemPoco { globalOrbitSpeedMultiplier = 1f, globalSizeMultiplier = 2f };
+            object? obj = system;
+
+            var success = reflector.TryModifyAt<float>(ref obj, "globalOrbitSpeedMultiplier", 5f);
+


These tests call the new ReflectorNet atomic APIs (e.g., TryModifyAt, TryPatch, TryReadAt, View, Grep), but the repo currently vendors Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.0.0/ReflectorNet.dll. Unless that DLL already includes this API surface, the Editor test assembly will fail to compile and CI will be red. Consider bumping the ReflectorNet dependency (and committed DLL) in the same PR, or gating these tests behind a compile-time define/version-define until the dependency lands.

Copilot · 2026-04-28T23:38:07Z

+    /// the Unity-MCP plugin.
+    /// </summary>
+    public class TryModifyAtTests : BaseTest


BaseTest is declared in namespace com.IvanMurzak.Unity.MCP.Editor.Tests, but this file is in com.IvanMurzak.Unity.MCP.Editor.Tests.AtomicApi and does not import the parent namespace. As written, : BaseTest will not resolve and the test assembly won't compile. Add using com.IvanMurzak.Unity.MCP.Editor.Tests; or fully-qualify the base class / adjust the namespace to match existing tests.

Copilot · 2026-04-28T23:38:07Z

+    /// single call. Includes the polymorphic "$type" hint scenario.
+    /// </summary>
+    public class TryPatchTests : BaseTest


BaseTest lives in com.IvanMurzak.Unity.MCP.Editor.Tests, but this file is in com.IvanMurzak.Unity.MCP.Editor.Tests.AtomicApi and doesn't import the parent namespace. : BaseTest will fail to compile unless you add the appropriate using or fully-qualify the type (or move this test class into the existing ...Editor.Tests namespace).

Copilot · 2026-04-28T23:38:07Z

+    /// value written by TryModifyAt is observed back through TryReadAt.
+    /// </summary>
+    public class TryReadAtTests : BaseTest


BaseTest is in namespace com.IvanMurzak.Unity.MCP.Editor.Tests, but this file declares com.IvanMurzak.Unity.MCP.Editor.Tests.AtomicApi and does not import the parent namespace. : BaseTest will not compile without a using com.IvanMurzak.Unity.MCP.Editor.Tests; / fully-qualified base class / namespace alignment.

github-actions · 2026-04-29T00:00:37Z

Test Results

12 files 492 suites 39m 21s ⏱️
824 tests 824 ✅ 0 💤 0 ❌
4 944 runs 4 944 ✅ 0 💤 0 ❌

Results for commit 43bae14.

♻️ This comment has been updated with latest results.

The committed tests in this PR exercise the new atomic-modify and view APIs (TryModifyAt, TryPatch, TryReadAt, View, Grep) introduced by IvanMurzak/ReflectorNet#71 (HEAD 2252f623, branch feature/atomic-api). Those APIs are not in the published com.IvanMurzak.ReflectorNet 5.0.0 NuGet, so building this PR against the released DLL fails compilation on every Unity matrix job. Replace the 5.0.0 DLL with a Release build of ReflectorNet at feature/atomic-api SHA 2252f623 so CI compiles. The .csproj HintPath locations are unchanged; only the binary content is updated. This is a temporary cross-repo dependency stub. Once ReflectorNet#71 merges and a NuGet release ships with the atomic API surface, the proper path is: 1. Bump the Unity-MCP-Plugin NuGet folder name + .csproj HintPaths to the released ReflectorNet version (e.g. 5.1.0). 2. Replace this dev DLL with the release artefact. DLL provenance: - Source: ReflectorNet/ReflectorNet/bin/Release/netstandard2.1/ReflectorNet.dll - Built from: feature/atomic-api @ 2252f623495ff9355f022ca68990807b5280b429 - md5: cdf5e92be0f2e5084d5b8ea16b57b4de - Size: 227328 bytes (was 206336 bytes for released 5.0.0)

Each Unity-Tests/<version>/ has its own committed copy of Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.0.0/ReflectorNet.dll. The previous commit only updated the Unity-MCP-Plugin/ copy, so CI EditMode runs (which use Unity-Tests/<version>/ as the project path) compiled against the old shipped 5.0.0 NuGet DLL and failed with "Reflector does not contain a definition for TryModifyAt / View / Grep". Built ReflectorNet from source via the infra cli (`uv run --directory .scripts python cli.py build-plugins`) and propagated the resulting netstandard2.1 DLL to all six Unity NuGet drop folders. The build is deterministic for this source: the produced DLL hashes byte-identical to the one already committed at Unity-MCP-Plugin/.../ReflectorNet.dll in commit ed4977d, so only the 5 Unity-Tests/<version>/ copies needed to change. Verified locally: every ReflectorNet.dll across the six Unity project trees now hashes to 564de27 (227 328 bytes).

Copilot

Pull request overview

Copilot reviewed 11 out of 17 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-29T08:28:05Z

+            Assert.IsNotNull(result);
+            // Top-level field celestialBodies is present
+            var bodies = result!.fields?.FirstOrDefault(f => f.name == "celestialBodies");
+            Assert.IsNotNull(bodies, "MaxDepth=1 must keep top-level field celestialBodies.");


The test name/description claims nested members are stripped at MaxDepth = 1, but the assertions only check that the top-level celestialBodies field exists. This can pass even if nested children are still being serialized. Add an assertion that bodies.fields (or its equivalent child collection) is null/empty to actually validate the pruning behavior.

Suggested change

Assert.IsNotNull(bodies, "MaxDepth=1 must keep top-level field celestialBodies.");

Assert.IsNotNull(bodies, "MaxDepth=1 must keep top-level field celestialBodies.");

Assert.IsTrue(bodies!.fields == null || bodies.fields.Count == 0,

"MaxDepth=1 should strip nested members from celestialBodies.");

Copilot · 2026-04-29T08:28:06Z

+            // ".*Speed" matches orbitSpeed and rotationSpeed (both float) but not "name" (string).
+            var matches = reflector.Grep(obj, ".*Speed$");
+
+            Debug.Log($"[Grep_Speed] found {matches.Count} matches:");
+            foreach (var m in matches)
+                Debug.Log($"  {m.Path}");
+


reflector.Grep(...) is used without any null check, but the test immediately dereferences matches (matches.Count, matches.All). If Grep returns null, this will fail with a NullReferenceException rather than a clear assertion. Add Assert.IsNotNull(matches) (before any logging/iteration) and then proceed with the rest of the assertions.

Copilot · 2026-04-29T08:28:06Z

+
+            Assert.IsTrue(matches.Count >= 2, "Expected at least orbitSpeed and rotationSpeed.");
+            Assert.IsTrue(matches.All(m => m.Path.EndsWith("orbitSpeed") || m.Path.EndsWith("rotationSpeed")),
+                "All Grep matches must end in a 'Speed' segment by regex contract.");


The comment says this test narrows to float-typed members and verifies the result type, but the assertions only check the path suffixes. To actually validate the “float fields” aspect, assert that each match’s Value can be read as float (or that Value.typeName/equivalent indicates Single) so the test will catch type-filtering regressions rather than just name matching.

Suggested change

"All Grep matches must end in a 'Speed' segment by regex contract.");

"All Grep matches must end in a 'Speed' segment by regex contract.");

Assert.IsTrue(matches.All(m => m.Value is float),

"All Grep matches must expose float values to verify float-field narrowing.");

Copilot · 2026-04-29T08:28:07Z

+
+            Debug.Log($"[Grep_NameRegex] found {matches.Count} matches:");
+            foreach (var m in matches)
+                Debug.Log($"  {m.Path} = {m.Value.GetValue<float>(reflector)}");
+
+            Assert.IsNotNull(matches);
+            Assert.AreEqual(3, matches.Count, "Expected exactly one orbitRadius match per array element.");
+            // Each entry must be a ViewMatch (flat list, not a tree)
+            foreach (var m in matches)
+            {
+                Assert.IsInstanceOf<ViewMatch>(m);
+                Assert.IsNotNull(m.Path);
+                Assert.IsNotNull(m.Value);
+            }
+            // Paths must include the array-index bracket notation
+            Assert.IsTrue(matches.Any(m => m.Path.Contains("[0]") && m.Value.GetValue<float>(reflector) == 10f));
+            Assert.IsTrue(matches.Any(m => m.Path.Contains("[1]") && m.Value.GetValue<float>(reflector) == 20f));
+            Assert.IsTrue(matches.Any(m => m.Path.Contains("[2]") && m.Value.GetValue<float>(reflector) == 30f));


In this test you dereference matches (via matches.Count / foreach) before asserting it is non-null. If Grep ever returns null (e.g., regression), the test will throw a NullReferenceException instead of producing a clear assertion failure. Assert matches is not null before logging/iterating, or guard the logging accordingly.

Suggested change

Debug.Log($"[Grep_NameRegex] found {matches.Count} matches:");

foreach (var m in matches)

Debug.Log($" {m.Path} = {m.Value.GetValue<float>(reflector)}");

Assert.IsNotNull(matches);

Assert.AreEqual(3, matches.Count, "Expected exactly one orbitRadius match per array element.");

// Each entry must be a ViewMatch (flat list, not a tree)

foreach (var m in matches)

{

Assert.IsInstanceOf<ViewMatch>(m);

Assert.IsNotNull(m.Path);

Assert.IsNotNull(m.Value);

}

// Paths must include the array-index bracket notation

Assert.IsTrue(matches.Any(m => m.Path.Contains("[0]") && m.Value.GetValue<float>(reflector) == 10f));

Assert.IsTrue(matches.Any(m => m.Path.Contains("[1]") && m.Value.GetValue<float>(reflector) == 20f));

Assert.IsTrue(matches.Any(m => m.Path.Contains("[2]") && m.Value.GetValue<float>(reflector) == 30f));

Assert.IsNotNull(matches);

var nonNullMatches = matches!;

Debug.Log($"[Grep_NameRegex] found {nonNullMatches.Count} matches:");

foreach (var m in nonNullMatches)

Debug.Log($" {m.Path} = {m.Value.GetValue<float>(reflector)}");

Assert.AreEqual(3, nonNullMatches.Count, "Expected exactly one orbitRadius match per array element.");

// Each entry must be a ViewMatch (flat list, not a tree)

foreach (var m in nonNullMatches)

{

Assert.IsInstanceOf<ViewMatch>(m);

Assert.IsNotNull(m.Path);

Assert.IsNotNull(m.Value);

}

// Paths must include the array-index bracket notation

Assert.IsTrue(nonNullMatches.Any(m => m.Path.Contains("[0]") && m.Value.GetValue<float>(reflector) == 10f));

Assert.IsTrue(nonNullMatches.Any(m => m.Path.Contains("[1]") && m.Value.GetValue<float>(reflector) == 20f));

Assert.IsTrue(nonNullMatches.Any(m => m.Path.Contains("[2]") && m.Value.GetValue<float>(reflector) == 30f));

ReflectorNet 5.1.0 was just released to NuGet (https://www.nuget.org/packages/com.IvanMurzak.ReflectorNet/5.1.0), introducing the atomic API surface (TryModifyAt, TryPatch, TryReadAt, View, Grep) that the EditMode tests under Tests/Editor/AtomicApi/ depend on. Update path: Unity-MCP-Plugin/ + every Unity-Tests/<unity-version>/: Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.0.0/ -> Assets/Plugins/NuGet/com.IvanMurzak.ReflectorNet.5.1.0/ (directory + sibling .meta + inner .dll.meta renamed via git mv; the inner ReflectorNet.dll content swapped to the released NuGet 5.1.0 bytes — blob dfd4a43) NuGetConfig.cs: Added com.IvanMurzak.ReflectorNet@5.1.0 as a top-level pinned package right after McpPlugin. Without this, the resolver would fall back to McpPlugin 6.1.0's transitive 5.0.0 nuspec on dev machines (CI is unaffected — IsCi() short-circuits the resolver). Verified locally: every ReflectorNet.dll across the six Unity project trees now hashes byte-identical to the NuGet 5.1.0 lib/netstandard2.1 payload.

Copilot AI review requested due to automatic review settings April 28, 2026 23:33

Copilot started reviewing on behalf of IvanMurzak April 28, 2026 23:33 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

IvanMurzak mentioned this pull request Apr 29, 2026

fix: config priority — flags > env vars > on-disk config > defaults #689

Merged

15 tasks

IvanMurzak self-assigned this Apr 29, 2026

IvanMurzak added the enhancement New feature or request label Apr 29, 2026

IvanMurzak added 2 commits April 28, 2026 22:22

Copilot AI review requested due to automatic review settings April 29, 2026 08:22

Copilot started reviewing on behalf of IvanMurzak April 29, 2026 08:23 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

IvanMurzak merged commit 34f88e2 into main Apr 29, 2026
17 checks passed

IvanMurzak deleted the worktree-reflectornet-atomic-api-coverage branch April 29, 2026 10:11

Uh oh!

Conversation

IvanMurzak commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Coverage

Test plan

Notes / follow-ups

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IvanMurzak commented Apr 28, 2026 •

edited

Loading

github-actions Bot commented Apr 29, 2026 •

edited

Loading