From de31dfb11eecd804f7e284c01a4fa37ec0a3ddbc Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Fri, 16 Aug 2019 22:04:34 +0200 Subject: [PATCH] Fixed several tests and test infrastructure issues --- .../DiskTests/DiskProviderFixtureBase.cs | 2 + .../ProcessProviderFixture.cs | 5 +- .../ServiceProviderTests.cs | 17 ++++- src/NzbDrone.Core.Test/Framework/DbTest.cs | 22 +++---- .../Sonarr.Core.Test.csproj | 3 + .../Framework/MigrationController.cs | 3 +- .../IntegrationTestBase.cs | 14 +++- src/NzbDrone.Test.Common/NzbDroneRunner.cs | 64 ++++++++++--------- src/NzbDrone.Test.Common/TestBase.cs | 41 ++++++++++-- .../StartNzbDroneService.cs | 7 +- 10 files changed, 120 insertions(+), 58 deletions(-) diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs index b15dab431..c81aefbd8 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs @@ -248,12 +248,14 @@ public void should_be_able_to_rename_open_hardlinks_with_fileshare_delete() } [Test] + [Ignore("No longer behaving this way in a Windows 10 Feature Update")] public void should_not_be_able_to_rename_open_hardlinks_with_fileshare_none() { Assert.Throws(() => DoHardLinkRename(FileShare.None)); } [Test] + [Ignore("No longer behaving this way in a Windows 10 Feature Update")] public void should_not_be_able_to_rename_open_hardlinks_with_fileshare_write() { Assert.Throws(() => DoHardLinkRename(FileShare.Read)); diff --git a/src/NzbDrone.Common.Test/ProcessProviderFixture.cs b/src/NzbDrone.Common.Test/ProcessProviderFixture.cs index 84b1deb6c..3d872bf78 100644 --- a/src/NzbDrone.Common.Test/ProcessProviderFixture.cs +++ b/src/NzbDrone.Common.Test/ProcessProviderFixture.cs @@ -66,7 +66,7 @@ public void GetProcessById_should_return_null_for_invalid_process(int processId) [Test] public void Should_be_able_to_start_process() { - var process = Subject.Start(Path.Combine(Directory.GetCurrentDirectory(), DummyApp.DUMMY_PROCCESS_NAME + ".exe")); + var process = StartDummyProcess(); Subject.Exists(DummyApp.DUMMY_PROCCESS_NAME).Should() .BeTrue("excepted one dummy process to be already running"); @@ -141,7 +141,8 @@ public void kill_all_should_kill_all_process_with_name() private Process StartDummyProcess() { - return Subject.Start(DummyApp.DUMMY_PROCCESS_NAME + ".exe"); + var path = Path.Combine(TestContext.CurrentContext.TestDirectory, DummyApp.DUMMY_PROCCESS_NAME + ".exe"); + return Subject.Start(path); } [Test] diff --git a/src/NzbDrone.Common.Test/ServiceProviderTests.cs b/src/NzbDrone.Common.Test/ServiceProviderTests.cs index 4ebf15c93..23645a2d1 100644 --- a/src/NzbDrone.Common.Test/ServiceProviderTests.cs +++ b/src/NzbDrone.Common.Test/ServiceProviderTests.cs @@ -1,4 +1,5 @@ using System; +using System.Security.Principal; using System.ServiceProcess; using FluentAssertions; using NUnit.Framework; @@ -61,6 +62,10 @@ public void Exists_should_not_find_random_service() [Test] public void Service_should_be_installed_and_then_uninstalled() { + if (!IsAnAdministrator()) + { + Assert.Inconclusive("Can't run test without Administrator rights"); + } Subject.ServiceExist(TEMP_SERVICE_NAME).Should().BeFalse("Service already installed"); Subject.Install(TEMP_SERVICE_NAME); @@ -100,8 +105,13 @@ public void Should_be_able_to_start_and_stop_service() } [Test] - public void should_throw_if_starting_a_running_serivce() + public void should_throw_if_starting_a_running_service() { + if (!IsAnAdministrator()) + { + Assert.Inconclusive("Can't run test without Administrator rights"); + } + Subject.GetService(ALWAYS_INSTALLED_SERVICE).Status .Should().NotBe(ServiceControllerStatus.Running); @@ -127,5 +137,10 @@ public void Should_log_warn_if_on_stop_if_service_is_already_stopped() ExceptionVerification.ExpectedWarns(1); } + private static bool IsAnAdministrator() + { + var principal = new WindowsPrincipal(WindowsIdentity.GetCurrent()); + return principal.IsInRole(WindowsBuiltInRole.Administrator); + } } } diff --git a/src/NzbDrone.Core.Test/Framework/DbTest.cs b/src/NzbDrone.Core.Test/Framework/DbTest.cs index 7ae33e059..1eb68a55f 100644 --- a/src/NzbDrone.Core.Test/Framework/DbTest.cs +++ b/src/NzbDrone.Core.Test/Framework/DbTest.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Data.SQLite; using System.IO; using System.Linq; using FluentMigrator.Runner; @@ -115,21 +116,14 @@ public virtual void SetupDb() [TearDown] public void TearDown() { - if (TestFolderInfo != null && Directory.Exists(TestFolderInfo.AppDataFolder)) + // Make sure there are no lingering connections. (When this happens it means we haven't disposed something properly) + GC.Collect(); + GC.WaitForPendingFinalizers(); + SQLiteConnection.ClearAllPools(); + + if (TestFolderInfo != null) { - var files = Directory.GetFiles(TestFolderInfo.AppDataFolder); - - foreach (var file in files) - { - try - { - File.Delete(file); - } - catch (Exception) - { - - } - } + DeleteTempFolder(TestFolderInfo.AppDataFolder); } } } diff --git a/src/NzbDrone.Core.Test/Sonarr.Core.Test.csproj b/src/NzbDrone.Core.Test/Sonarr.Core.Test.csproj index 96ee152ea..fd85bfb2c 100644 --- a/src/NzbDrone.Core.Test/Sonarr.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/Sonarr.Core.Test.csproj @@ -15,6 +15,9 @@ Files\1024.png PreserveNewest + + ..\Libraries\Sqlite\System.Data.SQLite.dll + PreserveNewest diff --git a/src/NzbDrone.Core/Datastore/Migration/Framework/MigrationController.cs b/src/NzbDrone.Core/Datastore/Migration/Framework/MigrationController.cs index 793725e9f..69d33e7e6 100644 --- a/src/NzbDrone.Core/Datastore/Migration/Framework/MigrationController.cs +++ b/src/NzbDrone.Core/Datastore/Migration/Framework/MigrationController.cs @@ -50,6 +50,8 @@ public void Migrate(string connectionString, MigrationContext migrationContext) { runner.MigrateUp(true); } + + processor.Dispose(); } catch (SQLiteException) { @@ -57,7 +59,6 @@ public void Migrate(string connectionString, MigrationContext migrationContext) SQLiteConnection.ClearAllPools(); throw; } - sw.Stop(); diff --git a/src/NzbDrone.Integration.Test/IntegrationTestBase.cs b/src/NzbDrone.Integration.Test/IntegrationTestBase.cs index 050fac466..a6a078ab9 100644 --- a/src/NzbDrone.Integration.Test/IntegrationTestBase.cs +++ b/src/NzbDrone.Integration.Test/IntegrationTestBase.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Threading; @@ -126,7 +127,7 @@ public void SmokeTestTearDown() [SetUp] public void IntegrationSetUp() { - TempDirectory = Path.Combine(TestContext.CurrentContext.TestDirectory, "_test_" + DateTime.UtcNow.Ticks); + TempDirectory = Path.Combine(TestContext.CurrentContext.TestDirectory, "_test_" + Process.GetCurrentProcess().Id + "_" + DateTime.UtcNow.Ticks); // Wait for things to get quiet, otherwise the previous test might influence the current one. Commands.WaitAll(); @@ -150,6 +151,17 @@ public void IntegrationTearDown() _signalrConnection = null; _signalRReceived = new List(); } + + if (Directory.Exists(TempDirectory)) + { + try + { + Directory.Delete(TempDirectory, true); + } + catch + { + } + } } public string GetTempDirectory(params string[] args) diff --git a/src/NzbDrone.Test.Common/NzbDroneRunner.cs b/src/NzbDrone.Test.Common/NzbDroneRunner.cs index 7029de422..134ef10af 100644 --- a/src/NzbDrone.Test.Common/NzbDroneRunner.cs +++ b/src/NzbDrone.Test.Common/NzbDroneRunner.cs @@ -9,6 +9,7 @@ using NUnit.Framework; using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Processes; +using NzbDrone.Core.Configuration; using RestSharp; namespace NzbDrone.Test.Common @@ -30,8 +31,11 @@ public NzbDroneRunner(Logger logger, int port = 8989) public void Start() { - AppData = Path.Combine(TestContext.CurrentContext.TestDirectory, "_intg_" + DateTime.Now.Ticks); + AppData = Path.Combine(TestContext.CurrentContext.TestDirectory, "_intg_" + TestBase.GetUID()); + Directory.CreateDirectory(AppData); + GenerateApiKey(); + var sonarrConsoleExe = OsInfo.IsWindows ? "Sonarr.Console.exe" : "Sonarr.exe"; if (BuildInfo.IsDebug) @@ -52,8 +56,6 @@ public void Start() Assert.Fail("Process has exited"); } - SetApiKey(); - var request = new RestRequest("system/status"); request.AddHeader("Authorization", ApiKey); request.AddHeader("X-Api-Key", ApiKey); @@ -74,13 +76,23 @@ public void Start() public void KillAll() { - if (_nzbDroneProcess != null) + try { - _processProvider.Kill(_nzbDroneProcess.Id); + if (_nzbDroneProcess != null) + { + _processProvider.Kill(_nzbDroneProcess.Id); + } + + _processProvider.KillAll(ProcessProvider.SONARR_CONSOLE_PROCESS_NAME); + _processProvider.KillAll(ProcessProvider.SONARR_PROCESS_NAME); + } + catch (InvalidOperationException) + { + // May happen if the process closes while being closed } - _processProvider.KillAll(ProcessProvider.SONARR_CONSOLE_PROCESS_NAME); - _processProvider.KillAll(ProcessProvider.SONARR_PROCESS_NAME); + + TestBase.DeleteTempFolder(AppData); } private void Start(string outputNzbdroneConsoleExe) @@ -100,33 +112,25 @@ private void OnOutputDataReceived(string data) } } - private void SetApiKey() + private void GenerateApiKey() { var configFile = Path.Combine(AppData, "config.xml"); - var attempts = 0; - while (ApiKey == null && attempts < 50) - { - try - { - if (File.Exists(configFile)) - { - var apiKeyElement = XDocument.Load(configFile) - .XPathSelectElement("Config/ApiKey"); - if (apiKeyElement != null) - { - ApiKey = apiKeyElement.Value; - } - } - } - catch (XmlException ex) - { - Console.WriteLine("Error getting API Key from XML file: " + ex.Message, ex); - } + // Generate and set the api key so we don't have to poll the config file + var apiKey = Guid.NewGuid().ToString().Replace("-", ""); - attempts++; - Thread.Sleep(1000); - } + var xDoc = new XDocument( + new XDeclaration("1.0", "utf-8", "yes"), + new XElement(ConfigFileProvider.CONFIG_ELEMENT_NAME, + new XElement(nameof(ConfigFileProvider.ApiKey), apiKey) + ) + ); + + var data = xDoc.ToString(); + + File.WriteAllText(configFile, data); + + ApiKey = apiKey; } } } diff --git a/src/NzbDrone.Test.Common/TestBase.cs b/src/NzbDrone.Test.Common/TestBase.cs index df23f75c6..811756aa3 100644 --- a/src/NzbDrone.Test.Common/TestBase.cs +++ b/src/NzbDrone.Test.Common/TestBase.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.IO; using System.Threading; using FluentAssertions; @@ -43,8 +44,8 @@ protected TSubject Subject public abstract class TestBase : LoggingTest { - private static readonly Random _random = new Random(); + private static int _nextUid; private AutoMoqer _mocker; protected AutoMoqer Mocker @@ -84,7 +85,21 @@ private string VirtualPath } } - protected string TempFolder { get; private set; } + private string _tempFolder; + protected string TempFolder + { + get + { + if (_tempFolder == null) + { + _tempFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, "_temp_" + GetUID()); + + Directory.CreateDirectory(_tempFolder); + } + + return _tempFolder; + } + } [SetUp] public void TestBaseSetup() @@ -93,9 +108,7 @@ public void TestBaseSetup() LogManager.ReconfigExistingLoggers(); - TempFolder = Path.Combine(TestContext.CurrentContext.TestDirectory, "_temp_" + DateTime.Now.Ticks); - - Directory.CreateDirectory(TempFolder); + _tempFolder = null; } [TearDown] @@ -103,9 +116,25 @@ public void TestBaseTearDown() { _mocker = null; + DeleteTempFolder(_tempFolder); + } + + + public static string GetUID() + { + return Process.GetCurrentProcess().Id + "_" + DateTime.Now.Ticks + "_" + Interlocked.Increment(ref _nextUid); + } + + public static void DeleteTempFolder(string folder) + { + if (folder == null) + { + return; + } + try { - var tempFolder = new DirectoryInfo(TempFolder); + var tempFolder = new DirectoryInfo(folder); if (tempFolder.Exists) { foreach (var file in tempFolder.GetFiles("*", SearchOption.AllDirectories)) diff --git a/src/NzbDrone.Update.Test/StartNzbDroneService.cs b/src/NzbDrone.Update.Test/StartNzbDroneService.cs index e4a283cda..e61ba0a73 100644 --- a/src/NzbDrone.Update.Test/StartNzbDroneService.cs +++ b/src/NzbDrone.Update.Test/StartNzbDroneService.cs @@ -16,7 +16,7 @@ public class StartNzbDroneServiceFixture : TestBase [Test] public void should_start_service_if_app_type_was_serivce() { - const string targetFolder = "c:\\Sonarr\\"; + string targetFolder = "c:\\Sonarr\\".AsOsAgnostic(); Subject.Start(AppType.Service, targetFolder); @@ -26,13 +26,14 @@ public void should_start_service_if_app_type_was_serivce() [Test] public void should_start_console_if_app_type_was_service_but_start_failed_because_of_permissions() { - const string targetFolder = "c:\\Sonarr\\"; + string targetFolder = "c:\\Sonarr\\".AsOsAgnostic(); + string targetProcess = "c:\\Sonarr\\Sonarr.Console.exe".AsOsAgnostic(); Mocker.GetMock().Setup(c => c.Start(ServiceProvider.SERVICE_NAME)).Throws(new InvalidOperationException()); Subject.Start(AppType.Service, targetFolder); - Mocker.GetMock().Verify(c => c.SpawnNewProcess("c:\\Sonarr\\Sonarr.Console.exe", "/" + StartupContext.NO_BROWSER, null, false), Times.Once()); + Mocker.GetMock().Verify(c => c.SpawnNewProcess(targetProcess, "/" + StartupContext.NO_BROWSER, null, false), Times.Once()); ExceptionVerification.ExpectedWarns(1); }