Removed erroneous Disosal and fixed poorly coded unit test: A toplevel manually creatged must be disposed by creator

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 ();
     }

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"
                                                 );
         }
     }

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<Toplevel> when already initialized with a Driver will throw (because Toplevel is not dervied from TopLevel)
+        // Run<Toplevel> when already initialized with a Driver will throw (because Toplevel is not derived from TopLevel)
         Assert.Throws<ArgumentException> (() => Application.Run<Toplevel> ());
 
         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<Toplevel> when already initialized with a Driver will throw (because Toplevel is not dervied from TopLevel)
+        // Run<Toplevel> when already initialized with a Driver will throw (because Toplevel is not derivied from TopLevel)
         Assert.Throws<ArgumentException> (() => Application.Run<Toplevel> (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<TestToplevel> ();
+
+#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<TestTopLevel>. Bad
+        // Init has been called, selecting FakeDriver; we're passing no driver to Run<TestTopLevel>. Should be fine.
         Application.Run<TestToplevel> ();
 
         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 ();

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 ();