Browse Source

Refactor to make IDriver dependency explicit

Updated `AnsiEscapeSequenceRequest.Send` to accept an `IDriver?` parameter, replacing reliance on `Application.Driver`. Refactored `AnsiRequestScheduler` methods (`SendOrSchedule`, `RunSchedule`, and private `Send`) to propagate the `IDriver?` parameter, ensuring explicit driver dependency.

Modified `DriverImpl.QueueAnsiRequest` to pass `this` to `SendOrSchedule`. Updated `AnsiRequestSchedulerTests` to reflect new method signatures, passing `null` for the driver parameter where applicable.

Added `<param>` documentation for new parameters to improve clarity. These changes enhance flexibility, maintainability, and testability by reducing reliance on global state and allowing driver substitution in tests.
Tig 1 month ago
parent
commit
a41843102b

+ 2 - 2
Terminal.Gui/Drivers/AnsiHandling/AnsiEscapeSequenceRequest.cs

@@ -18,12 +18,12 @@ public class AnsiEscapeSequenceRequest : AnsiEscapeSequence
     /// </summary>
     /// </summary>
     public Action? Abandoned { get; init; }
     public Action? Abandoned { get; init; }
 
 
-
     /// <summary>
     /// <summary>
     ///     Sends the <see cref="AnsiEscapeSequence.Request"/> to the raw output stream of the current <see cref="IDriver"/>.
     ///     Sends the <see cref="AnsiEscapeSequence.Request"/> to the raw output stream of the current <see cref="IDriver"/>.
     ///     Only call this method from the main UI thread. You should use <see cref="AnsiRequestScheduler"/> if
     ///     Only call this method from the main UI thread. You should use <see cref="AnsiRequestScheduler"/> if
     ///     sending many requests.
     ///     sending many requests.
     /// </summary>
     /// </summary>
-    public void Send () { Application.Driver?.WriteRaw (Request); }
+    /// <param name="driver"></param>
+    public void Send (IDriver? driver) { driver?.WriteRaw (Request); }
 
 
 }
 }

+ 11 - 8
Terminal.Gui/Drivers/AnsiHandling/AnsiRequestScheduler.cs

@@ -69,15 +69,17 @@ public class AnsiRequestScheduler
     ///     Sends the <paramref name="request"/> immediately or queues it if there is already
     ///     Sends the <paramref name="request"/> immediately or queues it if there is already
     ///     an outstanding request for the given <see cref="AnsiEscapeSequence.Terminator"/>.
     ///     an outstanding request for the given <see cref="AnsiEscapeSequence.Terminator"/>.
     /// </summary>
     /// </summary>
+    /// <param name="driverImpl"></param>
+    /// <param name="driver"></param>
     /// <param name="request"></param>
     /// <param name="request"></param>
     /// <returns><see langword="true"/> if request was sent immediately. <see langword="false"/> if it was queued.</returns>
     /// <returns><see langword="true"/> if request was sent immediately. <see langword="false"/> if it was queued.</returns>
-    public bool SendOrSchedule (AnsiEscapeSequenceRequest request) { return SendOrSchedule (request, true); }
+    public bool SendOrSchedule (IDriver? driver, AnsiEscapeSequenceRequest request) { return SendOrSchedule (driver, request, true); }
 
 
-    private bool SendOrSchedule (AnsiEscapeSequenceRequest request, bool addToQueue)
+    private bool SendOrSchedule (IDriver? driver, AnsiEscapeSequenceRequest request, bool addToQueue)
     {
     {
         if (CanSend (request, out ReasonCannotSend reason))
         if (CanSend (request, out ReasonCannotSend reason))
         {
         {
-            Send (request);
+            Send (driver, request);
 
 
             return true;
             return true;
         }
         }
@@ -90,7 +92,7 @@ public class AnsiRequestScheduler
                 // Try again after evicting
                 // Try again after evicting
                 if (CanSend (request, out _))
                 if (CanSend (request, out _))
                 {
                 {
-                    Send (request);
+                    Send (driver, request);
 
 
                     return true;
                     return true;
                 }
                 }
@@ -141,6 +143,7 @@ public class AnsiRequestScheduler
     ///     Identifies and runs any <see cref="_queuedRequests"/> that can be sent based on the
     ///     Identifies and runs any <see cref="_queuedRequests"/> that can be sent based on the
     ///     current outstanding requests of the parser.
     ///     current outstanding requests of the parser.
     /// </summary>
     /// </summary>
+    /// <param name="driver"></param>
     /// <param name="force">
     /// <param name="force">
     ///     Repeated requests to run the schedule over short period of time will be ignored.
     ///     Repeated requests to run the schedule over short period of time will be ignored.
     ///     Pass <see langword="true"/> to override this behaviour and force evaluation of outstanding requests.
     ///     Pass <see langword="true"/> to override this behaviour and force evaluation of outstanding requests.
@@ -149,7 +152,7 @@ public class AnsiRequestScheduler
     ///     <see langword="true"/> if a request was found and run. <see langword="false"/>
     ///     <see langword="true"/> if a request was found and run. <see langword="false"/>
     ///     if no outstanding requests or all have existing outstanding requests underway in parser.
     ///     if no outstanding requests or all have existing outstanding requests underway in parser.
     /// </returns>
     /// </returns>
-    public bool RunSchedule (bool force = false)
+    public bool RunSchedule (IDriver? driver, bool force = false)
     {
     {
         if (!force && Now () - _lastRun < _runScheduleThrottle)
         if (!force && Now () - _lastRun < _runScheduleThrottle)
         {
         {
@@ -162,7 +165,7 @@ public class AnsiRequestScheduler
         if (opportunity != null)
         if (opportunity != null)
         {
         {
             // Give it another go
             // Give it another go
-            if (SendOrSchedule (opportunity.Item1, false))
+            if (SendOrSchedule (driver, opportunity.Item1, false))
             {
             {
                 _queuedRequests.Remove (opportunity);
                 _queuedRequests.Remove (opportunity);
 
 
@@ -175,11 +178,11 @@ public class AnsiRequestScheduler
         return false;
         return false;
     }
     }
 
 
-    private void Send (AnsiEscapeSequenceRequest r)
+    private void Send (IDriver? driver, AnsiEscapeSequenceRequest r)
     {
     {
         _lastSend.AddOrUpdate (r.Terminator!, _ => Now (), (_, _) => Now ());
         _lastSend.AddOrUpdate (r.Terminator!, _ => Now (), (_, _) => Now ());
         _parser.ExpectResponse (r.Terminator, r.ResponseReceived, r.Abandoned, false);
         _parser.ExpectResponse (r.Terminator, r.ResponseReceived, r.Abandoned, false);
-        r.Send ();
+        r.Send (driver);
     }
     }
 
 
     private bool CanSend (AnsiEscapeSequenceRequest r, out ReasonCannotSend reason)
     private bool CanSend (AnsiEscapeSequenceRequest r, out ReasonCannotSend reason)

+ 1 - 1
Terminal.Gui/Drivers/DriverImpl.cs

@@ -485,7 +485,7 @@ internal class DriverImpl : IDriver
     ///     The request is sent immediately if possible, or queued for later execution
     ///     The request is sent immediately if possible, or queued for later execution
     ///     by the <see cref="AnsiRequestScheduler"/> to prevent overwhelming the console.
     ///     by the <see cref="AnsiRequestScheduler"/> to prevent overwhelming the console.
     /// </remarks>
     /// </remarks>
-    public void QueueAnsiRequest (AnsiEscapeSequenceRequest request) { _ansiRequestScheduler.SendOrSchedule (request); }
+    public void QueueAnsiRequest (AnsiEscapeSequenceRequest request) { _ansiRequestScheduler.SendOrSchedule (this, request); }
 
 
     /// <summary>
     /// <summary>
     ///     Gets the <see cref="AnsiRequestScheduler"/> instance used by this driver.
     ///     Gets the <see cref="AnsiRequestScheduler"/> instance used by this driver.

+ 15 - 15
Tests/UnitTestsParallelizable/Drivers/AnsiRequestSchedulerTests.cs

@@ -34,7 +34,7 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Once);
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Once);
 
 
         // Act
         // Act
-        bool result = _scheduler.SendOrSchedule (request);
+        bool result = _scheduler.SendOrSchedule (null, request);
 
 
         // Assert
         // Assert
         Assert.Empty (_scheduler.QueuedRequests); // We sent it i.e. we did not queue it for later
         Assert.Empty (_scheduler.QueuedRequests); // We sent it i.e. we did not queue it for later
@@ -57,7 +57,7 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Once);
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Once);
 
 
         // Act
         // Act
-        bool result = _scheduler.SendOrSchedule (request1);
+        bool result = _scheduler.SendOrSchedule (null, request1);
 
 
         // Assert
         // Assert
         Assert.Single (_scheduler.QueuedRequests); // Ensure only one request is in the queue
         Assert.Single (_scheduler.QueuedRequests); // Ensure only one request is in the queue
@@ -80,7 +80,7 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
 
 
-        _scheduler.SendOrSchedule (request);
+        _scheduler.SendOrSchedule (null, request);
 
 
         // Simulate time passing beyond throttle
         // Simulate time passing beyond throttle
         SetTime (101); // Exceed throttle limit
         SetTime (101); // Exceed throttle limit
@@ -88,7 +88,7 @@ public class AnsiRequestSchedulerTests
         // Act
         // Act
 
 
         // Send another request after the throttled time limit
         // Send another request after the throttled time limit
-        bool result = _scheduler.SendOrSchedule (request);
+        bool result = _scheduler.SendOrSchedule (null, request);
 
 
         // Assert
         // Assert
         Assert.Empty (_scheduler.QueuedRequests); // Should send and clear the request
         Assert.Empty (_scheduler.QueuedRequests); // Should send and clear the request
@@ -111,7 +111,7 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
 
 
-        _scheduler.SendOrSchedule (request);
+        _scheduler.SendOrSchedule (null, request);
 
 
         // Simulate time passing
         // Simulate time passing
         SetTime (55); // Does not exceed throttle limit
         SetTime (55); // Does not exceed throttle limit
@@ -119,24 +119,24 @@ public class AnsiRequestSchedulerTests
         // Act
         // Act
 
 
         // Send another request after the throttled time limit
         // Send another request after the throttled time limit
-        bool result = _scheduler.SendOrSchedule (request);
+        bool result = _scheduler.SendOrSchedule (null, request);
 
 
         // Assert
         // Assert
         Assert.Single (_scheduler.QueuedRequests); // Should have been queued
         Assert.Single (_scheduler.QueuedRequests); // Should have been queued
         Assert.False (result); // Should have been queued
         Assert.False (result); // Should have been queued
 
 
         // Throttle still not exceeded
         // Throttle still not exceeded
-        Assert.False (_scheduler.RunSchedule ());
+        Assert.False (_scheduler.RunSchedule (null));
 
 
         SetTime (90);
         SetTime (90);
 
 
         // Throttle still not exceeded
         // Throttle still not exceeded
-        Assert.False (_scheduler.RunSchedule ());
+        Assert.False (_scheduler.RunSchedule (null));
 
 
         SetTime (105);
         SetTime (105);
 
 
         // Throttle exceeded - so send the request
         // Throttle exceeded - so send the request
-        Assert.True (_scheduler.RunSchedule ());
+        Assert.True (_scheduler.RunSchedule (null));
 
 
         _parserMock.Verify ();
         _parserMock.Verify ();
     }
     }
@@ -156,13 +156,13 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once);
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (false).Verifiable (Times.Once);
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.ExpectResponse ("c", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Exactly (2));
 
 
-        Assert.True (_scheduler.SendOrSchedule (request1));
+        Assert.True (_scheduler.SendOrSchedule (null, request1));
 
 
         // Parser already has an ongoing request for "c"
         // Parser already has an ongoing request for "c"
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Exactly (2));
         _parserMock.Setup (p => p.IsExpecting ("c")).Returns (true).Verifiable (Times.Exactly (2));
 
 
         // Cannot send because there is already outstanding request
         // Cannot send because there is already outstanding request
-        Assert.False (_scheduler.SendOrSchedule (request1));
+        Assert.False (_scheduler.SendOrSchedule (null, request1));
         Assert.Single (_scheduler.QueuedRequests);
         Assert.Single (_scheduler.QueuedRequests);
 
 
         // Simulate request going stale
         // Simulate request going stale
@@ -178,7 +178,7 @@ public class AnsiRequestSchedulerTests
                    .Verifiable ();
                    .Verifiable ();
 
 
         // When we send again the evicted one should be
         // When we send again the evicted one should be
-        bool evicted = _scheduler.RunSchedule ();
+        bool evicted = _scheduler.RunSchedule (null);
 
 
         Assert.True (evicted); // Stale request should be evicted
         Assert.True (evicted); // Stale request should be evicted
         Assert.Empty (_scheduler.QueuedRequests);
         Assert.Empty (_scheduler.QueuedRequests);
@@ -191,7 +191,7 @@ public class AnsiRequestSchedulerTests
     public void RunSchedule_DoesNothing_WhenQueueIsEmpty ()
     public void RunSchedule_DoesNothing_WhenQueueIsEmpty ()
     {
     {
         // Act
         // Act
-        bool result = _scheduler.RunSchedule ();
+        bool result = _scheduler.RunSchedule (null);
 
 
         // Assert
         // Assert
         Assert.False (result); // No requests to process
         Assert.False (result); // No requests to process
@@ -213,8 +213,8 @@ public class AnsiRequestSchedulerTests
         _parserMock.Setup (p => p.ExpectResponse ("x", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Once);
         _parserMock.Setup (p => p.ExpectResponse ("x", It.IsAny<Action<string>> (), null, false)).Verifiable (Times.Once);
 
 
         // Act
         // Act
-        bool a = _scheduler.SendOrSchedule (request1);
-        bool b = _scheduler.SendOrSchedule (request2);
+        bool a = _scheduler.SendOrSchedule (null, request1);
+        bool b = _scheduler.SendOrSchedule (null, request2);
 
 
         // Assert
         // Assert
         Assert.False (a);
         Assert.False (a);