From b1bfc85ae0857c43841b582b8c5909d6630c715d Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Tue, 2 Oct 2018 14:08:45 -0400 Subject: [PATCH 1/5] Cleanup Watchdog disposables initialization --- .../Components/Watchdog/Watchdog.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs index 756b009c02..8e294c17f1 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs @@ -171,16 +171,24 @@ namespace Tgstation.Server.Host.Components.Watchdog if (serverControl == null) throw new ArgumentNullException(nameof(serverControl)); - - restartRegistration = serverControl.RegisterForRestart(this); - + chat.RegisterCommandHandler(this); AlphaIsActive = true; ActiveLaunchParameters = initialLaunchParameters; releaseServers = false; - semaphore = new SemaphoreSlim(1); activeParametersUpdated = new TaskCompletionSource(); + + restartRegistration = serverControl.RegisterForRestart(this); + try + { + semaphore = new SemaphoreSlim(1); + } + catch + { + restartRegistration.Dispose(); + throw; + } } /// From 840387b0adcb268004a834eb2ee196a5a539415e Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Tue, 2 Oct 2018 15:00:42 -0400 Subject: [PATCH 2/5] Add missing ArgumentNullException throw --- src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs index 8e294c17f1..e6c1cb4fc5 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs @@ -166,6 +166,7 @@ namespace Tgstation.Server.Host.Components.Watchdog this.byondTopicSender = byondTopicSender ?? throw new ArgumentNullException(nameof(byondTopicSender)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); + ActiveLaunchParameters = initialLaunchParameters ?? throw new ArgumentNullException(nameof(initialLaunchParameters)); this.instance = instance ?? throw new ArgumentNullException(nameof(instance)); this.autoStart = autoStart; From d175142d46ea7c13f33ee97ac7c238c292821d7c Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Tue, 2 Oct 2018 15:03:07 -0400 Subject: [PATCH 3/5] Add Watchdog construction test --- src/Tgstation.Server.Host/AssemblyInfo.cs | 1 + .../Components/Watchdog/TestWatchdog.cs | 66 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs diff --git a/src/Tgstation.Server.Host/AssemblyInfo.cs b/src/Tgstation.Server.Host/AssemblyInfo.cs index d2a28919a4..8c17c3b557 100644 --- a/src/Tgstation.Server.Host/AssemblyInfo.cs +++ b/src/Tgstation.Server.Host/AssemblyInfo.cs @@ -1,3 +1,4 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Tgstation.Server.Host.Tests")] +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] diff --git a/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs b/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs new file mode 100644 index 0000000000..563cca9587 --- /dev/null +++ b/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs @@ -0,0 +1,66 @@ +using Byond.TopicSender; +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System; +using Tgstation.Server.Api.Models.Internal; +using Tgstation.Server.Host.Components.Chat; +using Tgstation.Server.Host.Components.Compiler; +using Tgstation.Server.Host.Core; + +namespace Tgstation.Server.Host.Components.Watchdog.Tests +{ + [TestClass] + public sealed class TestWatchdog + { + [TestMethod] + public void TestConstruction() + { + Assert.ThrowsException(() => new Watchdog(null, null, null, null, null, null, null, null, null, null, null, null, default)); + + var mockChat = new Mock(); + mockChat.Setup(x => x.RegisterCommandHandler(It.IsNotNull())).Verifiable(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, null, null, null, null, null, null, null, null, null, null, null, default)); + + var mockSessionControllerFactory = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, null, null, null, null, null, null, null, null, null, null, default)); + + var mockDmbFactory = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, null, null, null, null, null, null, null, null, null, null, default)); + + var mockLogger = new Mock>(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, null, null, null, null, null, null, null, null, default)); + + var mockReattachInfoHandler = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, null, null, null, null, null, null, null, default)); + + var mockDatabaseContextFactory = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, null, null, null, null, null, null, default)); + + var mockByondTopicSender = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, null, null, null, null, null, default)); + + var mockEventConsumer = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, null, null, null, null, default)); + + var mockJobManager = new Mock(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, mockJobManager.Object, null, null, null, default)); + + var mockRestartRegistration = new Mock(); + mockRestartRegistration.Setup(x => x.Dispose()).Verifiable(); + var mockServerControl = new Mock(); + mockServerControl.Setup(x => x.RegisterForRestart(It.IsNotNull())).Returns(mockRestartRegistration.Object).Verifiable(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, mockJobManager.Object, mockServerControl.Object, null, null, default)); + + var mockLaunchParameters = new DreamDaemonLaunchParameters(); + Assert.ThrowsException(() => new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, mockJobManager.Object, mockServerControl.Object, mockLaunchParameters, null, default)); + + var mockInstance = new Models.Instance(); + new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, mockJobManager.Object, mockServerControl.Object, mockLaunchParameters, mockInstance, default).Dispose(); + + mockRestartRegistration.VerifyAll(); + mockServerControl.VerifyAll(); + mockChat.VerifyAll(); + } + } +} From b4ef4ae13da14949e898861f045dd2d5866267c7 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Tue, 2 Oct 2018 15:26:57 -0400 Subject: [PATCH 4/5] Add a missing task await --- src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs index e6c1cb4fc5..b79b9f5363 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs @@ -816,6 +816,8 @@ namespace Tgstation.Server.Host.Components.Watchdog await Task.WhenAny(allTask, cancelTcs.Task).ConfigureAwait(false); cancellationToken.ThrowIfCancellationRequested(); + await allTask.ConfigureAwait(false); + //both servers are now running, alpha is the active server(unless reattach), huzzah AlphaIsActive = reattachInfo?.AlphaIsActive ?? true; From 3596383532f4635d2e3bc3bfc79031190e0e7e02 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Tue, 2 Oct 2018 15:40:54 -0400 Subject: [PATCH 5/5] Add a basic watchdog test --- .../Components/Watchdog/TestWatchdog.cs | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs b/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs index 563cca9587..3326386343 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Watchdog/TestWatchdog.cs @@ -3,6 +3,9 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Components.Chat; using Tgstation.Server.Host.Components.Compiler; @@ -62,5 +65,79 @@ namespace Tgstation.Server.Host.Components.Watchdog.Tests mockServerControl.VerifyAll(); mockChat.VerifyAll(); } + + [TestMethod] + public async Task TestSuccessfulLaunchAndShutdown() + { + var mockChat = new Mock(); + mockChat.Setup(x => x.RegisterCommandHandler(It.IsNotNull())).Verifiable(); + var mockSessionControllerFactory = new Mock(); + var mockDmbFactory = new Mock(); + var mockLogger = new Mock>(); + var mockReattachInfoHandler = new Mock(); + var mockDatabaseContextFactory = new Mock(); + var mockByondTopicSender = new Mock(); + var mockEventConsumer = new Mock(); + var mockJobManager = new Mock(); + var mockRestartRegistration = new Mock(); + mockRestartRegistration.Setup(x => x.Dispose()).Verifiable(); + var mockServerControl = new Mock(); + mockServerControl.Setup(x => x.RegisterForRestart(It.IsNotNull())).Returns(mockRestartRegistration.Object).Verifiable(); + var mockLaunchParameters = new DreamDaemonLaunchParameters(); + var mockInstance = new Models.Instance(); + + using (var wd = new Watchdog(mockChat.Object, mockSessionControllerFactory.Object, mockDmbFactory.Object, mockLogger.Object, mockReattachInfoHandler.Object, mockDatabaseContextFactory.Object, mockByondTopicSender.Object, mockEventConsumer.Object, mockJobManager.Object, mockServerControl.Object, mockLaunchParameters, mockInstance, default)) + using (var cts = new CancellationTokenSource()) + { + var mockCompileJob = new Models.CompileJob(); + var mockDmbProvider = new Mock(); + mockDmbProvider.SetupGet(x => x.CompileJob).Returns(mockCompileJob).Verifiable(); + var mDmbP = mockDmbProvider.Object; + + var infiniteTask = new TaskCompletionSource().Task; + + mockDmbFactory.SetupGet(x => x.OnNewerDmb).Returns(infiniteTask); + mockDmbFactory.Setup(x => x.LockNextDmb(2)).Returns(mDmbP).Verifiable(); + + var sessionsToVerify = new List>(); + + var cancellationToken = cts.Token; + mockSessionControllerFactory.Setup(x => x.LaunchNew(mockLaunchParameters, mDmbP, null, It.IsAny(), It.IsAny(), false, cancellationToken)).Returns(() => + { + var mockSession = new Mock(); + mockSession.SetupGet(x => x.Lifetime).Returns(infiniteTask).Verifiable(); + mockSession.SetupGet(x => x.OnReboot).Returns(infiniteTask).Verifiable(); + mockSession.SetupGet(x => x.Dmb).Returns(mDmbP).Verifiable(); + mockSession.SetupGet(x => x.LaunchResult).Returns(Task.FromResult(new LaunchResult + { + StartupTime = TimeSpan.FromSeconds(1) + })).Verifiable(); + sessionsToVerify.Add(mockSession); + return Task.FromResult(mockSession.Object); + }).Verifiable(); + + cts.CancelAfter(TimeSpan.FromSeconds(15)); + + try + { + await wd.Launch(cancellationToken).ConfigureAwait(false); + await wd.Terminate(false, cancellationToken).ConfigureAwait(false); + } + finally + { + cts.Cancel(); + } + Assert.AreEqual(2, sessionsToVerify.Count); + foreach (var I in sessionsToVerify) + I.VerifyAll(); + mockDmbProvider.VerifyAll(); + } + + mockSessionControllerFactory.VerifyAll(); + mockDmbFactory.VerifyAll(); + mockRestartRegistration.VerifyAll(); + mockServerControl.VerifyAll(); + mockChat.VerifyAll(); + } } }