From ab07ec665b282e4c7d7e965c3091a8cd8ebf4137 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 17 Mar 2024 07:41:37 -0800 Subject: [PATCH] Removed erroneous Disosal and fixed poorly coded unit test: A toplevel manually creatged must be disposed by creator --- Terminal.Gui/Application.cs | 16 ++++-- Terminal.Gui/RunState.cs | 5 +- UnitTests/Application/ApplicationTests.cs | 67 +++++++++++++++++++++-- UnitTests/Views/ToplevelTests.cs | 2 + 4 files changed, 77 insertions(+), 13 deletions(-) diff --git a/Terminal.Gui/Application.cs b/Terminal.Gui/Application.cs index 7f891da2c..6513fd23b 100644 --- a/Terminal.Gui/Application.cs +++ b/Terminal.Gui/Application.cs @@ -543,9 +543,17 @@ public static partial class Application { if (_initialized) { + // Init created Application.Top. If it hasn't been disposed... + if (Top is { }) + { + Top.Dispose (); + Top = null; + } + if (Driver is { }) { // Init() has been called and we have a driver, so just run the app. + // This Toplevel will get disposed in `Shutdown` var top = new T (); Type type = top.GetType ().BaseType; @@ -1032,12 +1040,8 @@ public static partial class Application Refresh (); } - //if (Top == runState.Toplevel) - //{ - // Top = null; - //} - - runState.Toplevel?.Dispose (); + // Do NOT dispose .Toplevel here. It was not created by + // Run, but Init or someone else. runState.Toplevel = null; runState.Dispose (); } diff --git a/Terminal.Gui/RunState.cs b/Terminal.Gui/RunState.cs index ebfb32b30..0c1387ff8 100644 --- a/Terminal.Gui/RunState.cs +++ b/Terminal.Gui/RunState.cs @@ -32,8 +32,11 @@ public class RunState : IDisposable { if (Toplevel is { } && disposing) { + // Previously we were requiring Toplevel be disposed here. + // But that is not correct becaue `Begin` didn't create the TopLevel, `Init` did; thus + // disposing should be done by `Shutdown`, not `End`. throw new InvalidOperationException ( - "You must clean up (Dispose) the Toplevel before calling Application.RunState.Dispose" + "Toplevel must be null before calling Application.RunState.Dispose" ); } } diff --git a/UnitTests/Application/ApplicationTests.cs b/UnitTests/Application/ApplicationTests.cs index 7d07a6ae2..9105fe7bb 100644 --- a/UnitTests/Application/ApplicationTests.cs +++ b/UnitTests/Application/ApplicationTests.cs @@ -47,7 +47,7 @@ public class ApplicationTests } [Fact] - public void End_Disposes_Runstate_Toplevel () + public void End_Should_Not_Dispose_ApplicationTop_Shutdown_Should () { Init (); @@ -56,13 +56,19 @@ public class ApplicationTests #if DEBUG_IDISPOSABLE Assert.True (rs.WasDisposed); - Assert.True (Application.Top.WasDisposed); + Assert.False (Application.Top.WasDisposed); #endif Assert.Null (rs.Toplevel); + var top = Application.Top; + Shutdown (); +#if DEBUG_IDISPOSABLE + Assert.True (top.WasDisposed); +#endif + Assert.Null (Application.Top); } @@ -490,12 +496,13 @@ public class ApplicationTests #region RunTests [Fact] + public void Run_T_After_InitWithDriver_with_TopLevel_Throws () { // Setup Mock driver Init (); - // Run when already initialized with a Driver will throw (because Toplevel is not dervied from TopLevel) + // Run when already initialized with a Driver will throw (because Toplevel is not derived from TopLevel) Assert.Throws (() => Application.Run ()); Shutdown (); @@ -506,12 +513,13 @@ public class ApplicationTests } [Fact] + public void Run_T_After_InitWithDriver_with_TopLevel_and_Driver_Throws () { // Setup Mock driver Init (); - // Run when already initialized with a Driver will throw (because Toplevel is not dervied from TopLevel) + // Run when already initialized with a Driver will throw (because Toplevel is not derivied from TopLevel) Assert.Throws (() => Application.Run (null, new FakeDriver ())); Shutdown (); @@ -522,6 +530,40 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + + public void Run_T_After_Init_Disposes_Application_Top () + { + Init (); + + // Init created a Toplevel and assigned it to Application.Top + var initTop = Application.Top; + + Application.Iteration += (s, a) => + { + Assert.NotEqual(initTop, Application.Top); +#if DEBUG_IDISPOSABLE + Assert.True (initTop.WasDisposed); +#endif + Application.RequestStop (); + }; + + Application.Run (); + +#if DEBUG_IDISPOSABLE + Assert.True (initTop.WasDisposed); +#endif + + Shutdown (); + + Assert.Null (Application.Top); + Assert.Null (Application.MainLoop); + Assert.Null (Application.Driver); + } + + [Fact] + [TestRespondersDisposed] + public void Run_T_After_InitWithDriver_with_TestTopLevel_DoesNotThrow () { // Setup Mock driver @@ -540,7 +582,9 @@ public class ApplicationTests } [Fact] - public void Run_T_After_InitNullDriver_with_TestTopLevel_Throws () + [TestRespondersDisposed] + + public void Run_T_After_InitNullDriver_with_TestTopLevel_DoesNotThrow () { Application.ForceDriver = "FakeDriver"; @@ -549,7 +593,7 @@ public class ApplicationTests Application.Iteration += (s, a) => { Application.RequestStop (); }; - // Init has been called without selecting a driver and we're passing no driver to Run. Bad + // Init has been called, selecting FakeDriver; we're passing no driver to Run. Should be fine. Application.Run (); Shutdown (); @@ -560,6 +604,8 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + public void Run_T_Init_Driver_Cleared_with_TestTopLevel_Throws () { Init (); @@ -579,6 +625,7 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] public void Run_T_NoInit_DoesNotThrow () { Application.ForceDriver = "FakeDriver"; @@ -596,6 +643,8 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + public void Run_T_NoInit_WithDriver_DoesNotThrow () { Application.Iteration += (s, a) => { Application.RequestStop (); }; @@ -611,6 +660,8 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + public void Run_RequestStop_Stops () { // Setup Mock driver @@ -633,6 +684,8 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + public void Run_RunningFalse_Stops () { // Setup Mock driver @@ -655,6 +708,8 @@ public class ApplicationTests } [Fact] + [TestRespondersDisposed] + public void Run_Loaded_Ready_Unlodaded_Events () { Init (); diff --git a/UnitTests/Views/ToplevelTests.cs b/UnitTests/Views/ToplevelTests.cs index 9eaee637f..8bc487c70 100644 --- a/UnitTests/Views/ToplevelTests.cs +++ b/UnitTests/Views/ToplevelTests.cs @@ -2030,6 +2030,7 @@ public class ToplevelTests ); Application.Run (firstWindow); + firstWindow.Dispose (); } void SecondWindow (object sender, EventArgs args) @@ -2060,6 +2061,7 @@ public class ToplevelTests ); Application.Run (testWindow); + testWindow.Dispose (); } Application.Run ();