diff --git a/ILSpy.Tests/Docking/RunInNewTabTests.cs b/ILSpy.Tests/Docking/RunInNewTabTests.cs index ba24db236..e821277fb 100644 --- a/ILSpy.Tests/Docking/RunInNewTabTests.cs +++ b/ILSpy.Tests/Docking/RunInNewTabTests.cs @@ -69,10 +69,13 @@ public class RunInNewTabTests dock.Documents!.VisibleDockables!.OfType().Count() .Should().BeGreaterThan(tabsBefore, "the long op opens its own tab"); - // Navigate while the op is running: select a type -> the preview tab decompiles. - var typeNode = vm.AssemblyTreeModel.FindNode( - "System.Linq", "System.Linq", "System.Linq.Enumerable"); - vm.AssemblyTreeModel.SelectNode(typeNode); + // Navigate while the op is running: select a node -> the preview tab decompiles. A C# + // namespace node decompiles to just its "// Some.Name.Space" comment line, which keeps + // this inside the headless decompile-wait budget on slow CI runners (a full type decompile + // would not). + var navNode = vm.AssemblyTreeModel.FindNode( + TreeNavigation.CoreLibName, "System.Runtime.Versioning"); + vm.AssemblyTreeModel.SelectNode(navNode); await dock.WaitForDecompiledTextAsync(); for (int i = 0; i < 6; i++) { diff --git a/ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs b/ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs index d3721ebef..8629596e6 100644 --- a/ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs +++ b/ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs @@ -23,6 +23,7 @@ using Avalonia.Headless.NUnit; using AwesomeAssertions; +using ILSpy.AssemblyTree; using ILSpy.Navigation; using ILSpy.TextView; using ILSpy.TreeNodes; @@ -41,6 +42,18 @@ namespace ICSharpCode.ILSpy.Tests.Navigation; [TestFixture] public class ViewStateRoundTripTests { + // These tests only exercise the navigation pipeline; what gets decompiled is irrelevant. + // Decompiling a CoreLib type in full is the slowest thing the suite does and overruns the + // headless 15s wait on slower CI runners. A namespace node in C# decompiles to just its + // "// Some.Name.Space" comment line (Language.DecompileNamespace) -- the cheapest possible + // target -- so navigate between two of those. Both namespaces are always present in CoreLib. + static (NamespaceTreeNode A, NamespaceTreeNode B) CheapNodes(AssemblyTreeModel atm) + { + var a = atm.FindNode(TreeNavigation.CoreLibName, "System.Runtime.Versioning"); + var b = atm.FindNode(TreeNavigation.CoreLibName, "System.Text"); + return (a, b); + } + [AvaloniaTest] public async Task Back_Restores_Caret_Position_The_User_Left_On_The_Previous_Node() { @@ -48,16 +61,13 @@ public class ViewStateRoundTripTests var dockWorkspace = vm.DockWorkspace; - // Pick two distinct decompiler targets so the navigation actually moves between - // them. CoreLib's System.Object and System.String are both always present. - var coreLibName = typeof(object).Assembly.GetName().Name!; - var objectNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); - var stringNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String"); + // Two distinct, cheap navigation targets so the navigation actually moves between them. + var (nodeA, nodeB) = CheapNodes(vm.AssemblyTreeModel); - vm.AssemblyTreeModel.SelectNode(objectNode); + vm.AssemblyTreeModel.SelectNode(nodeA); await dockWorkspace.WaitForDecompiledTextAsync(); var tab = dockWorkspace.ActiveDecompilerTab!; - TestCapture.Step("object-decompiled"); + TestCapture.Step("node-a-decompiled"); // State is pulled from the editor on demand (CaptureViewState) when DockWorkspace records // a navigation away -- not pushed per caret/scroll event. Headless has no laid-out editor, @@ -66,16 +76,16 @@ public class ViewStateRoundTripTests // Navigate to a different node — DockWorkspace pulls A's state into the current history // entry and records B as the new current. - vm.AssemblyTreeModel.SelectNode(stringNode); + vm.AssemblyTreeModel.SelectNode(nodeB); await dockWorkspace.WaitForDecompiledTextAsync(); - TestCapture.Step("string-decompiled"); + TestCapture.Step("node-b-decompiled"); // Verify the capture: the back stack's most recent entry should be A's, with the pulled // caret + scroll values. var backEntries = dockWorkspace.BackHistory.OfType().ToList(); backEntries.Should().NotBeEmpty("Select(B) must push A onto the back stack"); var captured = backEntries.Last(); - ReferenceEquals(captured.Node, objectNode).Should().BeTrue( + ReferenceEquals(captured.Node, nodeA).Should().BeTrue( "captured back-stack entry must reference node A"); captured.CaretOffset.Should().Be(500); captured.VerticalOffset.Should().Be(120.5); @@ -111,14 +121,12 @@ public class ViewStateRoundTripTests var (_, vm) = await TestHarness.BootAsync(); var dockWorkspace = vm.DockWorkspace; - var coreLibName = typeof(object).Assembly.GetName().Name!; - var objectNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); - var stringNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String"); + var (nodeA, nodeB) = CheapNodes(vm.AssemblyTreeModel); - vm.AssemblyTreeModel.SelectNode(objectNode); + vm.AssemblyTreeModel.SelectNode(nodeA); await dockWorkspace.WaitForDecompiledTextAsync(); var tab = dockWorkspace.ActiveDecompilerTab!; - TestCapture.Step("object-decompiled"); + TestCapture.Step("node-a-decompiled"); // Deterministic foldings snapshot — two expanded regions over a four-folding layout. // Compute via the helper to keep the checksum honest. (The snapshot itself is exercised @@ -133,13 +141,13 @@ public class ViewStateRoundTripTests // Navigate to a different node — DockWorkspace pulls the state and stamps the foldings // onto the OUTGOING entry on the back stack. - vm.AssemblyTreeModel.SelectNode(stringNode); + vm.AssemblyTreeModel.SelectNode(nodeB); await dockWorkspace.WaitForDecompiledTextAsync(); - TestCapture.Step("string-decompiled"); + TestCapture.Step("node-b-decompiled"); // Verify the capture: the back-stack entry for node A carries the snapshot. var captured = dockWorkspace.BackHistory.OfType().Last(); - ReferenceEquals(captured.Node, objectNode).Should().BeTrue( + ReferenceEquals(captured.Node, nodeA).Should().BeTrue( "captured back-stack entry must reference node A"); captured.Foldings.Should().NotBeNull("Select(B) must record A's foldings into the back stack"); captured.Foldings!.Value.Checksum.Should().Be(seeded.Checksum); @@ -168,14 +176,12 @@ public class ViewStateRoundTripTests ((object?)vm.DockWorkspace.NavigateBackCommand).Should().NotBeNull(); // Build up history: A → B, then go Back so Forward is enabled. - var coreLibName = typeof(object).Assembly.GetName().Name!; - var objectNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); - var stringNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String"); - vm.AssemblyTreeModel.SelectNode(objectNode); + var (nodeA, nodeB) = CheapNodes(vm.AssemblyTreeModel); + vm.AssemblyTreeModel.SelectNode(nodeA); await vm.DockWorkspace.WaitForDecompiledTextAsync(); - vm.AssemblyTreeModel.SelectNode(stringNode); + vm.AssemblyTreeModel.SelectNode(nodeB); await vm.DockWorkspace.WaitForDecompiledTextAsync(); - TestCapture.Step("string-decompiled"); + TestCapture.Step("node-b-decompiled"); vm.DockWorkspace.NavigateBackCommand.Execute(null); await vm.DockWorkspace.WaitForDecompiledTextAsync(); @@ -188,7 +194,7 @@ public class ViewStateRoundTripTests TestCapture.Step("navigated-forward"); // Back through Forward should land on B again. - ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, stringNode).Should().BeTrue( + ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, nodeB).Should().BeTrue( "NavigateForward must restore the tree selection to the entry that was just popped from forward"); } } diff --git a/ILSpy/NavigationHistory.cs b/ILSpy/NavigationHistory.cs index d7db288c6..2139da574 100644 --- a/ILSpy/NavigationHistory.cs +++ b/ILSpy/NavigationHistory.cs @@ -22,10 +22,11 @@ using System.Collections.Generic; namespace ILSpy.Navigation { /// - /// Two-stack browser-style history. Rapid successive calls (within - /// 0.5 s) replace the current entry instead of pushing, so a tree refresh that re-selects - /// the same node doesn't pollute the back stack with duplicates. Equality is delegated - /// to the entry's own implementation. + /// Two-stack browser-style history. A rapid successive of the SAME entry + /// (within 0.5 s) replaces the current entry instead of pushing, so the double-fire a single + /// click produces -- and tree refreshes that re-select the same node -- don't pollute the back + /// stack with duplicates. A rapid selection of a DIFFERENT entry still records normally. + /// Equality is delegated to the entry's own implementation. /// internal sealed class NavigationHistory where T : class, IEquatable { @@ -120,10 +121,15 @@ namespace ILSpy.Navigation var navigationTime = DateTime.Now; var period = navigationTime - lastNavigationTime; - if (period.TotalSeconds < NavigationSecondsBeforeNewEntry) + if (period.TotalSeconds < NavigationSecondsBeforeNewEntry && current != null && current.Equals(entry)) { - // Rapid successive selections collapse into a single entry — protects against - // tree refreshes that re-issue SelectedItem. + // A rapid RE-SELECTION of the SAME target collapses into the current entry. This + // swallows the double-fire a single click produces (SelectedItems.CollectionChanged + // and SelectedItem PropertyChanged both fan in) and tree refreshes that re-issue the + // same SelectedItem. A rapid selection of a DIFFERENT node is a genuine navigation + // and must fall through to push the old current onto the back stack -- gating only on + // elapsed time (the previous behaviour) silently dropped fast navigations whenever + // the decompile finished inside the window (cheap targets, fast machines). current = entry; } else