feat: Hot reload responses now report time outside the phases, name sibling rows in the Skipped count, and note added test methods

Assets/Tests/Editor/HotReload/HotReloadGroupProcessorTests.cs

@@ -734,6 +734,10 @@ private static HotReloadGroupFile CreatePhaseFile()
         private static async Task<HotReloadTimingBreakdown> RunGroupWithPhaseStubsAsync(PhaseStubs stubs)
         {
             HotReloadRunTiming timing = new HotReloadRunTiming();
+            // Why time the call instead of passing a fixed total: the stubs really wait, so the
+            // phases hold measured times, and only the time around the call is a total that
+            // covers them, as in the production run.
+            Stopwatch total = new Stopwatch();
             using (HotReloadServicesTestScope.BeginWithDependencies(collaborators =>
                 HotReloadGroupProcessorDependencies.Create(
                     files => true,
@@ -745,11 +749,13 @@ private static async Task<HotReloadTimingBreakdown> RunGroupWithPhaseStubsAsync(
             {
                 try
                 {
+                    total.Start();
                     await HotReloadCompositionRoot.Services.GroupProcessor.ProcessGroupAsync(
                         new[] { stubs.GroupFile },
                         "phase-timing-test",
                         timing,
                         CancellationToken.None);
+                    total.Stop();
                 }
                 catch (OperationCanceledException exception)
                 {
@@ -759,7 +765,7 @@ await HotReloadCompositionRoot.Services.GroupProcessor.ProcessGroupAsync(
                 }
             }
 
-            return timing.Complete(0);
+            return timing.Complete(total.ElapsedMilliseconds);
         }
 
         private static TransformWorkerEntryDto CreateAtomicEntry(string projectRelativePath)

Assets/Tests/Editor/HotReload/HotReloadRunTimingTests.cs

@@ -70,5 +70,48 @@ public void Complete_NegativeTotal_Throws()
 
             Assert.Throws<ArgumentOutOfRangeException>(() => timing.Complete(-1));
         }
+
+        /// <summary>
+        /// What: the time outside the three phases is reported as the total minus the phases.
+        /// </summary>
+        [Test]
+        public void Complete_ReportsOtherAsTotalMinusThePhases()
+        {
+            HotReloadRunTiming timing = new HotReloadRunTiming();
+            timing.AddAnalysis(100);
+            timing.AddShimCompile(200);
+            timing.AddPatch(30);
+
+            Assert.That(timing.Complete(1000).OtherMs, Is.EqualTo(670));
+        }
+
+        /// <summary>
+        /// What: a total that the phases fill exactly leaves nothing outside them.
+        /// </summary>
+        [Test]
+        public void Complete_TotalEqualToThePhases_ReportsZeroOther()
+        {
+            HotReloadRunTiming timing = new HotReloadRunTiming();
+            timing.AddAnalysis(100);
+            timing.AddShimCompile(200);
+            timing.AddPatch(30);
+
+            Assert.That(timing.Complete(330).OtherMs, Is.EqualTo(0));
+        }
+
+        /// <summary>
+        /// What: a total shorter than the phases inside it is refused instead of reporting a
+        /// negative time outside the phases.
+        /// </summary>
+        [Test]
+        public void Complete_TotalBelowThePhases_Throws()
+        {
+            HotReloadRunTiming timing = new HotReloadRunTiming();
+            timing.AddAnalysis(100);
+            timing.AddShimCompile(200);
+            timing.AddPatch(30);
+
+            Assert.Throws<ArgumentOutOfRangeException>(() => timing.Complete(329));
+        }
     }
 }

Assets/Tests/Editor/HotReload/HotReloadToolTests.cs

@@ -851,6 +851,7 @@ public void BuildApplyResponse_WithTiming_CopiesEveryPhase()
             Assert.That(response.Timing.AnalysisMs, Is.EqualTo(5), "AnalysisMs");
             Assert.That(response.Timing.ShimCompileMs, Is.EqualTo(7), "ShimCompileMs");
             Assert.That(response.Timing.PatchMs, Is.EqualTo(1), "PatchMs");
+            Assert.That(response.Timing.OtherMs, Is.EqualTo(37), "OtherMs");
             Assert.That(response.Timing.TotalMs, Is.EqualTo(50), "TotalMs");
         }
 
@@ -2368,7 +2369,8 @@ public void BuildApplyResponse_AddedAndSkippedOutcomes_CountsSkippedAfterAdded()
         /// <summary>
         /// What: an applied run that re-applied a sibling's earlier changes says how many of the
         /// Patched and Added rows came from those siblings, right after the counts they are part of,
-        /// so a reader who edited one method is not left wondering where the rest came from.
+        /// so a reader who edited one method is not left wondering where the rest came from. Its
+        /// Skipped count says the same of the Skipped rows.
         /// </summary>
         [Test]
         public void BuildApplyResponse_SiblingRowsReapplied_SaysHowManyOfTheCountsCameFromSiblings()
@@ -2392,7 +2394,8 @@ public void BuildApplyResponse_SiblingRowsReapplied_SaysHowManyOfTheCountsCameFr
                 response.Message,
                 Does.StartWith(
                     "Hot reload applied. PatchedTotal=2, ActivePatchTotal=2. Added: 1. "
-                    + "2 of the patched and added rows re-applied changes from earlier reloads in sibling files. Skipped: 1."));
+                    + "2 of the patched and added rows re-applied changes from earlier reloads in sibling files. "
+                    + "Skipped: 1 (1 in sibling files the run re-applied on its own; Outcome does not count those)."));
         }
 
         /// <summary>
@@ -2433,7 +2436,7 @@ public void BuildApplyResponse_SiblingRows_AreMarkedReappliedFromSiblingWhatever
 
         /// <summary>
         /// What: a pulled-in sibling whose rows were all Skipped re-applied nothing, so the message
-        /// adds no re-applied count for it.
+        /// adds no re-applied count for it, while its Skipped count names the row as a sibling's.
         /// </summary>
         [Test]
         public void BuildApplyResponse_SiblingRowsAllSkipped_AddsNoReappliedCount()
@@ -2452,6 +2455,34 @@ public void BuildApplyResponse_SiblingRowsAllSkipped_AddsNoReappliedCount()
             HotReloadResponse response = HotReloadTool.BuildApplyResponse(result);
 
             Assert.That(response.Message, Does.Not.Contain("re-applied changes from earlier reloads"));
+            Assert.That(response.Message, Does.Contain(" Skipped: 1 (1 in sibling files"));
+        }
+
+        /// <summary>
+        /// What: when the requested file and a sibling both have Skipped rows, the Skipped count
+        /// still covers every row and says how many of them are the sibling's. The sibling also has
+        /// a Patched row, so counting every sibling row instead of its Skipped ones fails.
+        /// </summary>
+        [Test]
+        public void BuildApplyMessage_RequestedAndSiblingRowsSkipped_SaysHowManySkippedRowsAreSiblings()
+        {
+            HotReloadOrchestratorResult result = new HotReloadOrchestratorResult(
+                new List<HotReloadMethodOutcome>
+                {
+                    HotReloadMethodOutcome.Patched("Type.Edited", "Assets/Requested.cs"),
+                    HotReloadMethodOutcome.Skipped("Type.Skip", "reason", "Assets/Requested.cs"),
+                    HotReloadMethodOutcome.Patched("Sibling.Earlier", "Assets/Sibling.cs"),
+                    HotReloadMethodOutcome.Skipped("Sibling.FirstSkip", "reason", "Assets/Sibling.cs"),
+                    HotReloadMethodOutcome.Skipped("Sibling.SecondSkip", "reason", "Assets/Sibling.cs")
+                },
+                new List<string>(),
+                patchedTotal: 2,
+                activePatchTotal: 2,
+                reappliedSiblingPaths: new[] { "Assets/Sibling.cs" });
+
+            HotReloadResponse response = HotReloadTool.BuildApplyResponse(result);
+
+            Assert.That(response.Message, Does.Contain(" Skipped: 3 (2 in sibling files"));
         }
 
         /// <summary>
@@ -2835,6 +2866,38 @@ public void BuildApplyResponse_IntroducedTypeBesideSkippedMethods_CountsTheSkipp
                     + " Skipped: 2."));
         }
 
+        /// <summary>
+        /// What: a run that only introduced a type also says when its Skipped row belongs to a
+        /// sibling it re-applied on its own, as the ordinary applied message does.
+        /// </summary>
+        [Test]
+        public void BuildApplyResponse_IntroducedTypeOnlyWithSiblingSkipped_SaysTheSkippedRowIsASibling()
+        {
+            HotReloadResponse response = HotReloadTool.BuildApplyResponse(
+                new HotReloadOrchestratorResult(
+                    new List<HotReloadMethodOutcome>
+                    {
+                        HotReloadMethodOutcome.Skipped("Sibling.Skip", "reason", "Assets/Sibling.cs")
+                    },
+                    new List<string>(),
+                    patchedTotal: 0,
+                    activePatchTotal: 0,
+                    reappliedSiblingPaths: new[] { "Assets/Sibling.cs" },
+                    introducedTypes: new[]
+                    {
+                        HotReloadIntroducedTypeOutcome.Introduced(
+                            "Example.Introduced",
+                            "IntroducedAssembly",
+                            "Assets/Requested.cs")
+                    }));
+
+            Assert.That(
+                response.Message,
+                Does.StartWith(
+                    string.Format(HotReloadConstants.IntroducedTypesOnlyApplyMessageFormat, 1)
+                    + " Skipped: 1 (1 in sibling files"));
+        }
+
         /// <summary>
         /// What: a run that skipped an edit outside Play Mode asks the CLI for a compile and leaves
         /// the run's own next action alone.

Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs

@@ -719,6 +719,91 @@ public async Task Warn_AddedTestMethod_KeepsEntryAndEmitsWarning()
                 Has.Some.Contain("AddedProbe").And.Contain("Unity Test Runner"));
         }
 
+        /// <summary>
+        /// What: an added method with a test attribute says on its own entry that the Unity Test
+        /// Runner will not discover it until a compile, so a reader of its row alone learns it.
+        /// </summary>
+        [TestCase("[Test]\n        public void AddedProbe()\n        {\n        }", "AddedProbe")]
+        [TestCase(
+            "[UnityTest]\n        public System.Collections.IEnumerator AddedUnityProbe()\n        {\n"
+            + "            yield break;\n        }",
+            "AddedUnityProbe")]
+        public async Task Emit_AddedTestMethod_CarriesTestRunnerLifecycleNote(string addedMember, string methodName)
+        {
+            // Why written out: the worker's constant lives in its own program, which the tests
+            // cannot reference.
+            const string expectedNote =
+                "Test method: not discovered by the Unity Test Runner until 'uloop compile'; "
+                + "'uloop run-tests --skip-compile' will not find or run it.";
+            string onDisk = File.ReadAllText(ResolveHostPath());
+            string edited = WithHostMembers(onDisk, addedMember);
+            string sourcePath = WriteEdited("AddedTestMethodNote" + methodName + ".cs", edited);
+
+            TransformWorkerClientResult result = await RunWorkerOnSourceAsync(
+                sourcePath,
+                HostProjectRelativePath,
+                snapshotSource: onDisk);
+            Assert.That(result.Success, Is.True, result.ErrorMessage);
+
+            TransformWorkerEntryDto added = FindEntry(result, methodName);
+            Assert.That(added, Is.Not.Null, methodName + " must be an entry.");
+            Assert.That(added.patchKind, Is.EqualTo(HotReloadConstants.PatchKindAddedMethod));
+            Assert.That(added.lifecycleNote, Is.EqualTo(expectedNote));
+        }
+
+        /// <summary>
+        /// What: a compiled method whose body was edited and that gained a test attribute is not
+        /// an added method, so its entry carries no Test Runner note, as it gets no warning.
+        /// </summary>
+        [Test]
+        public async Task Emit_EditedExistingTestMethod_HasNoLifecycleNote()
+        {
+            string onDisk = File.ReadAllText(ResolveHostPath());
+            const string originalExistingValue =
+                "        public int ExistingValue()\n        {\n            return 1;\n        }";
+            Assert.That(onDisk, Does.Contain(originalExistingValue));
+            // Why the body changes too: an unchanged body is recorded as unchanged and has no
+            // entry, so the note could not be checked.
+            string edited = onDisk.Replace(
+                originalExistingValue,
+                "        [Test]\n        public int ExistingValue()\n        {\n            return 99;\n        }",
+                StringComparison.Ordinal);
+            string sourcePath = WriteEdited("EditedExistingTestMethodNote.cs", edited);
+
+            TransformWorkerClientResult result = await RunWorkerOnSourceAsync(
+                sourcePath,
+                HostProjectRelativePath,
+                snapshotSource: onDisk);
+            Assert.That(result.Success, Is.True, result.ErrorMessage);
+
+            TransformWorkerEntryDto entry = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingValue));
+            Assert.That(entry, Is.Not.Null, "The edited ExistingValue must be an entry.");
+            Assert.That(entry.patchKind, Is.Not.EqualTo(HotReloadConstants.PatchKindAddedMethod));
+            Assert.That(entry.lifecycleNote, Is.Null.Or.Empty);
+        }
+
+        /// <summary>
+        /// What: an added method without a test attribute carries no Test Runner note.
+        /// </summary>
+        [Test]
+        public async Task Emit_AddedPlainMethod_HasNoLifecycleNote()
+        {
+            string onDisk = File.ReadAllText(ResolveHostPath());
+            string edited = WithHostMembers(onDisk, "public void AddedPlainProbe()\n        {\n        }");
+            string sourcePath = WriteEdited("AddedPlainMethodNote.cs", edited);
+
+            TransformWorkerClientResult result = await RunWorkerOnSourceAsync(
+                sourcePath,
+                HostProjectRelativePath,
+                snapshotSource: onDisk);
+            Assert.That(result.Success, Is.True, result.ErrorMessage);
+
+            TransformWorkerEntryDto added = FindEntry(result, "AddedPlainProbe");
+            Assert.That(added, Is.Not.Null, "AddedPlainProbe must be an entry.");
+            Assert.That(added.patchKind, Is.EqualTo(HotReloadConstants.PatchKindAddedMethod));
+            Assert.That(added.lifecycleNote, Is.Null.Or.Empty);
+        }
+
         /// <summary>
         /// What: added methods with a qualified [TestCase], [global::NUnit.Framework.Test],
         /// [UnityTest], or [SetUp] each produce a Unity Test Runner warning that names that method.