From 21c29d6446f82974df9756904bbf77b7c08790a2 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Mon, 2 Nov 2020 11:59:39 +0100 Subject: [PATCH] Fixes failing unit test due to null reference. Amended signatures of method under test to support testing without the previously observed behaviour of an erroring test passing but crashing the test runner process. --- .../HostedServices/HealthCheckNotifier.cs | 4 +-- .../HostedServices/KeepAlive.cs | 5 +-- .../HostedServices/LogScrubber.cs | 3 +- .../RecurringHostedServiceBase.cs | 11 ++++++- .../HostedServices/TempFileCleanup.cs | 5 ++- .../HealthCheckNotifierTests.cs | 33 ++++++++++--------- .../HostedServices/KeepAliveTests.cs | 20 +++++------ .../HostedServices/LogScrubberTests.cs | 23 ++++++++----- .../HostedServices/TempFileCleanupTests.cs | 9 ++--- 9 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs index af0a0f335c..c42892101a 100644 --- a/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs +++ b/src/Umbraco.Infrastructure/HostedServices/HealthCheckNotifier.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Core; @@ -52,9 +53,8 @@ namespace Umbraco.Infrastructure.HostedServices _logger = logger; _profilingLogger = profilingLogger; } - - public override async void ExecuteAsync(object state) + internal override async Task PerformExecuteAsync(object state) { if (_healthChecksSettings.Notification.Enabled == false) { diff --git a/src/Umbraco.Infrastructure/HostedServices/KeepAlive.cs b/src/Umbraco.Infrastructure/HostedServices/KeepAlive.cs index 337890c799..39ac0f3d87 100644 --- a/src/Umbraco.Infrastructure/HostedServices/KeepAlive.cs +++ b/src/Umbraco.Infrastructure/HostedServices/KeepAlive.cs @@ -1,5 +1,6 @@ using System; using System.Net.Http; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Core; @@ -35,7 +36,7 @@ namespace Umbraco.Infrastructure.HostedServices _httpClientFactory = httpClientFactory; } - public override async void ExecuteAsync(object state) + internal override async Task PerformExecuteAsync(object state) { if (_keepAliveSettings.DisableKeepAliveTask) { @@ -50,7 +51,7 @@ namespace Umbraco.Infrastructure.HostedServices return; case ServerRole.Unknown: _logger.LogDebug("Does not run on servers with unknown role."); - return; + return; } // Ensure we do not run if not main domain, but do NOT lock it diff --git a/src/Umbraco.Infrastructure/HostedServices/LogScrubber.cs b/src/Umbraco.Infrastructure/HostedServices/LogScrubber.cs index d01609e9a3..1cf2da05f9 100644 --- a/src/Umbraco.Infrastructure/HostedServices/LogScrubber.cs +++ b/src/Umbraco.Infrastructure/HostedServices/LogScrubber.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Umbraco.Core; @@ -38,7 +39,7 @@ namespace Umbraco.Infrastructure.HostedServices _profilingLogger = profilingLogger; } - public override async void ExecuteAsync(object state) + internal override async Task PerformExecuteAsync(object state) { switch (_serverRegistrar.GetCurrentServerRole()) { diff --git a/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs b/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs index 071ada0b62..ee77326115 100644 --- a/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs +++ b/src/Umbraco.Infrastructure/HostedServices/RecurringHostedServiceBase.cs @@ -31,7 +31,16 @@ namespace Umbraco.Infrastructure.HostedServices return Task.CompletedTask; } - public abstract void ExecuteAsync(object state); + public async void ExecuteAsync(object state) + { + // Delegate work to method returning a task, that can be called and asserted in a unit test. + // Without this there can be behaviour where tests pass, but an error within them causes the test + // running process to crash. + // Hat-tip: https://stackoverflow.com/a/14207615/489433 + await PerformExecuteAsync(state); + } + + internal abstract Task PerformExecuteAsync(object state); public Task StopAsync(CancellationToken cancellationToken) { diff --git a/src/Umbraco.Infrastructure/HostedServices/TempFileCleanup.cs b/src/Umbraco.Infrastructure/HostedServices/TempFileCleanup.cs index 3e565b0112..e27b83c8f6 100644 --- a/src/Umbraco.Infrastructure/HostedServices/TempFileCleanup.cs +++ b/src/Umbraco.Infrastructure/HostedServices/TempFileCleanup.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Umbraco.Core; using Umbraco.Core.IO; @@ -32,7 +33,7 @@ namespace Umbraco.Infrastructure.HostedServices _tempFolders = _ioHelper.GetTempFolders(); } - public override async void ExecuteAsync(object state) + internal override async Task PerformExecuteAsync(object state) { // Ensure we do not run if not main domain if (_mainDom.IsMainDom == false) @@ -45,6 +46,8 @@ namespace Umbraco.Infrastructure.HostedServices { CleanupFolder(folder); } + + return; } private void CleanupFolder(DirectoryInfo folder) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs index 372b94d9dd..bbbb4035c5 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/HealthCheckNotifierTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -28,66 +29,66 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices private const string _check3Id = "00000000-0000-0000-0000-000000000003"; [Test] - public void Does_Not_Execute_When_Not_Enabled() + public async Task Does_Not_Execute_When_Not_Enabled() { var sut = CreateHealthCheckNotifier(enabled: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Does_Not_Execute_When_Runtime_State_Is_Not_Run() + public async Task Does_Not_Execute_When_Runtime_State_Is_Not_Run() { var sut = CreateHealthCheckNotifier(runtimeLevel: RuntimeLevel.Boot); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Does_Not_Execute_When_Server_Role_Is_Replica() + public async Task Does_Not_Execute_When_Server_Role_Is_Replica() { var sut = CreateHealthCheckNotifier(serverRole: ServerRole.Replica); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Does_Not_Execute_When_Server_Role_Is_Unknown() + public async Task Does_Not_Execute_When_Server_Role_Is_Unknown() { var sut = CreateHealthCheckNotifier(serverRole: ServerRole.Unknown); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Does_Not_Execute_When_Not_Main_Dom() + public async Task Does_Not_Execute_When_Not_Main_Dom() { var sut = CreateHealthCheckNotifier(isMainDom: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Does_Not_Execute_With_No_Enabled_Notification_Methods() + public async Task Does_Not_Execute_With_No_Enabled_Notification_Methods() { var sut = CreateHealthCheckNotifier(notificationEnabled: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsNotSent(); } [Test] - public void Executes_With_Enabled_Notification_Methods() + public async Task Executes_With_Enabled_Notification_Methods() { var sut = CreateHealthCheckNotifier(); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyNotificationsSent(); } [Test] - public void Executes_Only_Enabled_Checks() + public async Task Executes_Only_Enabled_Checks() { var sut = CreateHealthCheckNotifier(); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); _mockNotificationMethod.Verify(x => x.SendAsync(It.Is( y => y.ResultsAsDictionary.Count == 1 && y.ResultsAsDictionary.ContainsKey("Check1"))), Times.Once); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/KeepAliveTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/KeepAliveTests.cs index 82ecffc683..9fc1454b6d 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/KeepAliveTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/KeepAliveTests.cs @@ -26,42 +26,42 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices private const string _applicationUrl = "https://mysite.com"; [Test] - public void Does_Not_Execute_When_Not_Enabled() + public async Task Does_Not_Execute_When_Not_Enabled() { var sut = CreateKeepAlive(enabled: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyKeepAliveRequestNotSent(); } [Test] - public void Does_Not_Execute_When_Server_Role_Is_Replica() + public async Task Does_Not_Execute_When_Server_Role_Is_Replica() { var sut = CreateKeepAlive(serverRole: ServerRole.Replica); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyKeepAliveRequestNotSent(); } [Test] - public void Does_Not_Execute_When_Server_Role_Is_Unknown() + public async Task Does_Not_Execute_When_Server_Role_Is_Unknown() { var sut = CreateKeepAlive(serverRole: ServerRole.Unknown); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyKeepAliveRequestNotSent(); } [Test] - public void Does_Not_Execute_When_Not_Main_Dom() + public async Task Does_Not_Execute_When_Not_Main_Dom() { var sut = CreateKeepAlive(isMainDom: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyKeepAliveRequestNotSent(); } [Test] - public void Executes_And_Calls_Ping_Url() + public async Task Executes_And_Calls_Ping_Url() { var sut = CreateKeepAlive(); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyKeepAliveRequestSent(); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/LogScrubberTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/LogScrubberTests.cs index 2903a656df..d30c83c545 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/LogScrubberTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/LogScrubberTests.cs @@ -1,10 +1,13 @@ using System; +using System.Data; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Configuration.Models; +using Umbraco.Core.Events; using Umbraco.Core.Logging; using Umbraco.Core.Scoping; using Umbraco.Core.Services; @@ -21,34 +24,34 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices const int _maxLogAgeInMinutes = 60; [Test] - public void Does_Not_Execute_When_Server_Role_Is_Replica() + public async Task Does_Not_Execute_When_Server_Role_Is_Replica() { var sut = CreateLogScrubber(serverRole: ServerRole.Replica); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyLogsNotScrubbed(); } [Test] - public void Does_Not_Execute_When_Server_Role_Is_Unknown() + public async Task Does_Not_Execute_When_Server_Role_Is_Unknown() { var sut = CreateLogScrubber(serverRole: ServerRole.Unknown); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyLogsNotScrubbed(); } [Test] - public void Does_Not_Execute_When_Not_Main_Dom() + public async Task Does_Not_Execute_When_Not_Main_Dom() { var sut = CreateLogScrubber(isMainDom: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyLogsNotScrubbed(); } [Test] - public void Executes_And_Scrubs_Logs() + public async Task Executes_And_Scrubs_Logs() { var sut = CreateLogScrubber(); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyLogsScrubbed(); } @@ -67,7 +70,11 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices var mockMainDom = new Mock(); mockMainDom.SetupGet(x => x.IsMainDom).Returns(isMainDom); + var mockScope = new Mock(); var mockScopeProvider = new Mock(); + mockScopeProvider + .Setup(x => x.CreateScope(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(mockScope.Object); var mockLogger = new Mock>(); var mockProfilingLogger = new Mock(); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/TempFileCleanupTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/TempFileCleanupTests.cs index 827a4f83de..7feda1e9da 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/TempFileCleanupTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/TempFileCleanupTests.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; @@ -17,18 +18,18 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.HostedServices private string _testPath = @"c:\test\temp\path"; [Test] - public void Does_Not_Execute_When_Not_Main_Dom() + public async Task Does_Not_Execute_When_Not_Main_Dom() { var sut = CreateTempFileCleanup(isMainDom: false); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyFilesNotCleaned(); } [Test] - public void Executes_And_Cleans_Files() + public async Task Executes_And_Cleans_Files() { var sut = CreateTempFileCleanup(); - sut.ExecuteAsync(null); + await sut.PerformExecuteAsync(null); VerifyFilesCleaned(); }