Browse Source

Fixes #4300 - Scrolling intermittently causes All_Scenarios_Quit_And_Init_Shutdown_Properly to fail (#4301)

* Refactor Scrolling.cs for timer management and nullability

Enabled nullable reference types with `#nullable enable` for improved safety. Replaced the `pulsing` flag with a new `_progressTimer` object to better manage the progress bar's timer lifecycle. Updated `AppOnInitialized` and `AppUnloaded` methods to handle timer initialization and cleanup properly, preventing potential memory leaks.

Simplified code by removing unnecessary comments and aligning method signatures with nullable reference type annotations.

* Enable nullable types and add debugging utilities

* Runu tests 5 times

* Adjust abortTime in ScenarioTests to 1800

* Revert "Runu tests 5 times"

This reverts commit b9b5282315fcbc9667ac7423e494a7a4cabfb74d.

* Improve CI diagnostics and adjust scenario abort time

* Fix typos, update metadata,

Corrected typos in `ScenarioCategory` and comments in `Mazing.cs`.
Added the `System.Text` namespace to support additional functionality.
Introduced a new "Games" category in `ScenarioCategory`.
Updated `.DotSettings` to include "Mazing" in the user dictionary.
Improved overall code comments and metadata for clarity.

* Remove unnecessary null assertion for _progressTimer

* Update Examples/UICatalog/Scenarios/Scrolling.cs

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
Tig 1 month ago
parent
commit
b502471ee6

+ 0 - 2
.github/workflows/integration-tests.yml

@@ -47,8 +47,6 @@ jobs:
       run: |
        dotnet test Tests/IntegrationTests --no-build --verbosity normal --diag:logs/${{ runner.os }}/logs.txt --blame --blame-crash --blame-hang --blame-hang-timeout 60s --blame-crash-collect-always -- xunit.stopOnFail=true
      
-       # mv -v Tests/IntegrationTests/TestResults/*/*.* TestResults/IntegrationTests/
-
     - name: Upload Test Logs
       if: always()
       uses: actions/upload-artifact@v4

+ 2 - 2
Examples/UICatalog/Scenarios/Mazing.cs

@@ -5,7 +5,7 @@ namespace UICatalog.Scenarios;
 
 [ScenarioMetadata ("A Mazing", "Illustrates how to make a basic maze game.")]
 [ScenarioCategory ("Drawing")]
-[ScenarioCategory ("Mouse and KeyBoard")]
+[ScenarioCategory ("Mouse and Keyboard")]
 [ScenarioCategory ("Games")]
 public class Mazing : Scenario
 {
@@ -33,7 +33,7 @@ public class Mazing : Scenario
         _top.KeyBindings.Add (Key.CursorDown, Command.Down);
 
         // Changing the key-bindings of a View is not allowed, however,
-        // by default, Toplevel does't bind any of our movement keys, so
+        // by default, Toplevel doesn't bind any of our movement keys, so
         // we can take advantage of the CommandNotBound event to handle them
         // 
         // An alternative implementation would be to create a TopLevel subclass that

+ 19 - 15
Examples/UICatalog/Scenarios/Scrolling.cs

@@ -1,4 +1,8 @@
-namespace UICatalog.Scenarios;
+#nullable enable
+
+using System.Diagnostics;
+
+namespace UICatalog.Scenarios;
 
 [ScenarioMetadata ("Scrolling", "Content scrolling, IScrollBars, etc...")]
 [ScenarioCategory ("Controls")]
@@ -6,6 +10,8 @@
 [ScenarioCategory ("Tests")]
 public class Scrolling : Scenario
 {
+    private object? _progressTimer = null;
+
     public override void Main ()
     {
         Application.Init ();
@@ -38,10 +44,6 @@ public class Scrolling : Scenario
 
         app.Add (demoView);
 
-        //// NOTE: This call to EnableScrollBar is technically not needed because the reference
-        //// NOTE: to demoView.HorizontalScrollBar below will cause it to be lazy created.
-        //// NOTE: The call included in this sample to for illustration purposes.
-        //demoView.EnableScrollBar (Orientation.Horizontal);
         var hCheckBox = new CheckBox
         {
             X = Pos.X (demoView),
@@ -52,10 +54,6 @@ public class Scrolling : Scenario
         app.Add (hCheckBox);
         hCheckBox.CheckedStateChanged += (sender, args) => { demoView.HorizontalScrollBar.Visible = args.Value == CheckState.Checked; };
 
-        //// NOTE: This call to EnableScrollBar is technically not needed because the reference
-        //// NOTE: to demoView.HorizontalScrollBar below will cause it to be lazy created.
-        //// NOTE: The call included in this sample to for illustration purposes.
-        //demoView.EnableScrollBar (Orientation.Vertical);
         var vCheckBox = new CheckBox
         {
             X = Pos.Right (hCheckBox) + 3,
@@ -96,8 +94,6 @@ public class Scrolling : Scenario
 
         app.Add (progress);
 
-        var pulsing = true;
-
         app.Initialized += AppOnInitialized;
         app.Unloaded += AppUnloaded;
 
@@ -108,17 +104,25 @@ public class Scrolling : Scenario
 
         return;
 
-        void AppOnInitialized (object sender, EventArgs e)
+        void AppOnInitialized (object? sender, EventArgs e)
         {
             bool TimerFn ()
             {
                 progress.Pulse ();
 
-                return pulsing;
+                return _progressTimer is { };
             }
 
-            Application.AddTimeout (TimeSpan.FromMilliseconds (200), TimerFn);
+            _progressTimer = Application.AddTimeout (TimeSpan.FromMilliseconds (200), TimerFn);
+        }
+
+        void AppUnloaded (object? sender, EventArgs args)
+        {
+            if (_progressTimer is { })
+            {
+                Application.RemoveTimeout (_progressTimer);
+                _progressTimer = null;
+            }
         }
-        void AppUnloaded (object sender, EventArgs args) { pulsing = false; }
     }
 }

+ 1 - 0
Terminal.sln.DotSettings

@@ -418,6 +418,7 @@
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Guppie/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Justifier/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=langword/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Mazing/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=ogonek/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Roslynator/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Toplevel/@EntryIndexedValue">True</s:Boolean>

+ 1 - 1
Tests/IntegrationTests/UICatalog/ScenarioTests.cs

@@ -43,7 +43,7 @@ public class ScenarioTests : TestsAllViews
         var scenario = Activator.CreateInstance (scenarioType) as Scenario;
         var scenarioName = scenario!.GetName ();
 
-        uint abortTime = 2200;
+        uint abortTime = 5000;  // Scrolling scenario can take up to 3 seconds to init on slow CI machines
         object? timeout = null;
         var initialized = false;
         var shutdownGracefully = false;