Make the tests that use the Heartbeat more deterministic. (#1724)

- Added Start to Heartbeat and made OnHeartbeat internal
- Explicitly call start in tests

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/Heartbeat.cs

@@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
         private readonly TimeSpan _interval;
         private readonly ISystemClock _systemClock;
         private readonly IKestrelTrace _trace;
-        private readonly Timer _timer;
+        private Timer _timer;
         private int _executingOnHeartbeat;
 
         public Heartbeat(IHeartbeatHandler[] callbacks, ISystemClock systemClock, IKestrelTrace trace)
@@ -30,6 +30,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
             _interval = interval;
             _systemClock = systemClock;
             _trace = trace;
+        }
+
+        public void Start()
+        {
             _timer = new Timer(OnHeartbeat, state: this, dueTime: _interval, period: _interval);
         }
 
@@ -37,9 +41,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
         {
             ((Heartbeat)state).OnHeartbeat();
         }
-        
+
         // Called by the Timer (background) thread
-        private void OnHeartbeat()
+        internal void OnHeartbeat()
         {
             var now = _systemClock.UtcNow;
 
@@ -69,7 +73,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
 
         public void Dispose()
         {
-            _timer.Dispose();
+            _timer?.Dispose();
         }
     }
 }

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs

@@ -121,6 +121,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
                     throw new InvalidOperationException("Server has already started.");
                 }
                 _hasStarted = true;
+                _heartbeat.Start();
 
                 async Task OnBind(ListenOptions endpoint)
                 {
@@ -172,7 +173,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
             }
             await Task.WhenAll(tasks).ConfigureAwait(false);
 
-            _heartbeat?.Dispose();
+            _heartbeat.Dispose();
         }
 
         // Ungraceful shutdown

test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/DateHeaderValueManagerTests.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Threading.Tasks;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.AspNetCore.Testing;
@@ -48,7 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
-            using (new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
+            using (var heartbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
             {
                 Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);
                 systemClock.UtcNow = future;
@@ -59,7 +58,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         }
 
         [Fact]
-        public async Task GetDateHeaderValue_ReturnsUpdatedValueAfterHeartbeat()
+        public void GetDateHeaderValue_ReturnsUpdatedValueAfterHeartbeat()
         {
             var now = DateTimeOffset.UtcNow;
             var future = now.AddSeconds(10);
@@ -72,18 +71,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
-            var heartbeatTcs = new TaskCompletionSource<object>();
             var mockHeartbeatHandler = new Mock<IHeartbeatHandler>();
 
-            mockHeartbeatHandler.Setup(h => h.OnHeartbeat(future)).Callback(() => heartbeatTcs.TrySetResult(null));
-
-            using (new Heartbeat(new[] { dateHeaderValueManager, mockHeartbeatHandler.Object }, systemClock, testKestrelTrace, timerInterval))
+            using (var heartbeat = new Heartbeat(new[] { dateHeaderValueManager, mockHeartbeatHandler.Object }, systemClock, testKestrelTrace, timerInterval))
             {
+                heartbeat.OnHeartbeat();
+
                 Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);
 
                 // Wait for the next heartbeat before verifying GetDateHeaderValues picks up new time.
                 systemClock.UtcNow = future;
-                await heartbeatTcs.Task;
+
+                heartbeat.OnHeartbeat();
 
                 Assert.Equal(future.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);
                 Assert.True(systemClock.UtcNowCalled >= 2);
@@ -104,8 +103,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
-            using (new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
+            using (var heatbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
             {
+                heatbeat.OnHeartbeat();
                 Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);
             }
 

test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Linq;
 using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Logging;
@@ -27,8 +28,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             heartbeatHandler.Setup(h => h.OnHeartbeat(systemClock.UtcNow)).Callback(() => handlerMre.Wait());
             kestrelTrace.Setup(t => t.TimerSlow(heartbeatInterval, systemClock.UtcNow)).Callback(() => traceMre.Set());
 
-            using (new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace.Object, heartbeatInterval))
+            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace.Object, heartbeatInterval))
             {
+                heartbeat.Start();
                 Assert.True(traceMre.Wait(TimeSpan.FromSeconds(10)));
             }
 
@@ -49,12 +51,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             heartbeatHandler.Setup(h => h.OnHeartbeat(systemClock.UtcNow)).Throws(ex);
 
-            using (new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace, heartbeatInterval))
+            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace, heartbeatInterval))
             {
-                Assert.True(kestrelTrace.Logger.MessageLoggedTask.Wait(TimeSpan.FromSeconds(10)));
+                heartbeat.OnHeartbeat();
             }
 
-            Assert.Equal(ex, kestrelTrace.Logger.Messages.First(message => message.LogLevel == LogLevel.Error).Exception);
+            Assert.Equal(ex, kestrelTrace.Logger.Messages.Single(message => message.LogLevel == LogLevel.Error).Exception);
         }
     }
 }