Remove a sync Wait that can be async (#740)

Additional test logging to try and diagnose flakey behavour.
OctopusDeploy · Dec 14, 2023 · be322a0 · be322a0
1 parent 4b5049d
commit be322a0
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 28 deletions.
diff --git a/source/Octopus.Tentacle.Tests.Integration/Support/ClientAndTentacle.cs b/source/Octopus.Tentacle.Tests.Integration/Support/ClientAndTentacle.cs
@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Text;
 using System.Threading.Tasks;
 using Halibut;
 using Octopus.Tentacle.Client;
@@ -53,10 +54,16 @@ public async ValueTask DisposeAsync()
         {
             SafelyMoveTentacleLogFileToSharedLocation();
 
-            logger.Information("****** ****** ****** ****** ****** ****** ******");
-            logger.Information("****** CLIENT AND TENTACLE DISPOSE CALLED  *****");
-            logger.Information("*     Subsequent errors should be ignored      *");
-            logger.Information("****** ****** ****** ****** ****** ****** ******");
+            var banner = new StringBuilder();
+            banner.AppendLine("");
+            banner.AppendLine("");
+
+            banner.AppendLine("****** ****** ****** ****** ****** ****** ******");
+            banner.AppendLine("****** CLIENT AND TENTACLE DISPOSE CALLED  *****");
+            banner.AppendLine("*     Subsequent errors should be ignored      *");
+            banner.AppendLine("****** ****** ****** ****** ****** ****** ******");
+
+            logger.Information(banner.ToString());
 
             logger.Information("Starting DisposeAsync");
 

diff --git a/...tacle.Tests.Integration/Support/ExtensionMethods/ScriptExecutionResultExtensionMethods.cs b/...tacle.Tests.Integration/Support/ExtensionMethods/ScriptExecutionResultExtensionMethods.cs
@@ -0,0 +1,34 @@
+using System.Collections.Generic;
+using System.Text;
+using Octopus.Tentacle.Client.Scripts;
+using Octopus.Tentacle.Contracts;
+using Serilog;
+
+namespace Octopus.Tentacle.Tests.Integration.Support.ExtensionMethods
+{
+    public static class ScriptExecutionResultExtensionMethods
+    {
+        public static void LogExecuteScriptOutput(this (ScriptExecutionResult ScriptExecutionResult, List<ProcessOutput> ProcessOutput) result, ILogger logger)
+        {
+            var scriptOutput = new StringBuilder();
+
+            scriptOutput.AppendLine("");
+            scriptOutput.AppendLine("");
+            scriptOutput.AppendLine("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+            scriptOutput.AppendLine("~~ Start Execute Script Output ~~");
+            scriptOutput.AppendLine("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+
+            scriptOutput.AppendLine($"ScriptExecutionResult.State: {result.ScriptExecutionResult.State}");
+            scriptOutput.AppendLine($"ScriptExecutionResult.ExitCode: {result.ScriptExecutionResult.ExitCode}");
+            scriptOutput.AppendLine("");
+
+            result.ProcessOutput.ForEach(x => scriptOutput.AppendLine($"{x.Source} {x.Occurred} {x.Text}"));
+
+            scriptOutput.AppendLine($@"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+            scriptOutput.AppendLine($@"~~~ End Execute Script Output ~~~");
+            scriptOutput.AppendLine($@"~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+
+            logger.Information(scriptOutput.ToString());
+        }
+    }
+}
diff --git a/source/Octopus.Tentacle.Tests.Integration/TentacleStartupAndShutdownTests.cs b/source/Octopus.Tentacle.Tests.Integration/TentacleStartupAndShutdownTests.cs
@@ -1,7 +1,6 @@
 #nullable enable
 using System.Collections.Generic;
 using System.Linq;
-using System.Text;
 using System.Threading.Tasks;
 using FluentAssertions;
 using Halibut.Exceptions;
@@ -10,6 +9,7 @@
 using Octopus.Tentacle.CommonTestUtils.Builders;
 using Octopus.Tentacle.Contracts;
 using Octopus.Tentacle.Tests.Integration.Support;
+using Octopus.Tentacle.Tests.Integration.Support.ExtensionMethods;
 using Octopus.Tentacle.Tests.Integration.Support.TestAttributes;
 using Octopus.Tentacle.Tests.Integration.Util.Builders;
 
@@ -53,11 +53,7 @@ public async Task WhenRunningTentacleAsAServiceItShouldBeAbleToRestartItself(Ten
                     result = await clientAndTentacle.TentacleClient.ExecuteScript(startScriptCommand, CancellationToken);
                 }
 
-                var scriptOutput = new StringBuilder();
-                result.ProcessOutput.ForEach(x => scriptOutput.AppendLine($"{x.Source} {x.Occurred} {x.Text}"));
-
-                Logger.Information($@"Script Output:
-{scriptOutput}");
+                result.LogExecuteScriptOutput(Logger);
 
                 result.ProcessOutput.Any(x => x.Text.Contains("Stopping service")).Should().BeTrue("Stopping service should be logged");
                 result.ScriptExecutionResult.State.Should().Be(ProcessState.Complete);

diff --git a/source/Octopus.Tentacle.Tests.Integration/WorkspaceCleanerTests.cs b/source/Octopus.Tentacle.Tests.Integration/WorkspaceCleanerTests.cs
@@ -4,9 +4,11 @@
 using FluentAssertions;
 using NUnit.Framework;
 using Octopus.Tentacle.CommonTestUtils.Builders;
+using Octopus.Tentacle.Contracts;
 using Octopus.Tentacle.Contracts.ClientServices;
 using Octopus.Tentacle.Scripts;
 using Octopus.Tentacle.Tests.Integration.Support;
+using Octopus.Tentacle.Tests.Integration.Support.ExtensionMethods;
 using Octopus.Tentacle.Tests.Integration.Util;
 using Octopus.Tentacle.Tests.Integration.Util.Builders;
 using Octopus.Tentacle.Tests.Integration.Util.Builders.Decorators;
@@ -49,9 +51,13 @@ public async Task WhenScriptServiceIsRunning_ThenWorkspaceIsNotDeleted(TentacleC
             Directory.Exists(startScriptWorkspaceDirectory).Should().BeTrue("Workspace should not have been cleaned up");
 
             File.WriteAllText(waitBeforeCompletingScriptFile, "Write file that makes script continue executing");
-            await runningScriptTask;
+            var runningScriptResult = await runningScriptTask;
 
-            Directory.Exists(startScriptWorkspaceDirectory).Should().BeFalse("Workspace should be naturally cleaned up after completion");
+            runningScriptResult.LogExecuteScriptOutput(Logger);
+
+            runningScriptResult.ScriptExecutionResult.ExitCode.Should().Be(0, "Script should have completed successfully");
+            runningScriptResult.ScriptExecutionResult.State.Should().Be(ProcessState.Complete, "Script should have completed successfully");
+            Directory.Exists(startScriptWorkspaceDirectory).Should().BeFalse($"Workspace {startScriptWorkspaceDirectory} should have been cleaned up when CompleteScript was called");
         }
 
         [Test]

diff --git a/source/Octopus.Tentacle.Tests/Integration/ScriptServiceV2Fixture.cs b/source/Octopus.Tentacle.Tests/Integration/ScriptServiceV2Fixture.cs
@@ -366,7 +366,7 @@ public async Task GetStatusShouldReturnAnExitCodeOf46ForAScriptWithAnUnknownResu
 
             response.ExitCode.Should().Be(-46);
 
-            CleanupWorkspace(ticket);
+            await CleanupWorkspace(ticket, CancellationToken.None);
         }
 
         [Test]
@@ -380,7 +380,7 @@ public async Task CancelScriptShouldReturnAnExitCodeOf46ForAScriptWithAnUnknownR
 
             response.ExitCode.Should().Be(-46);
 
-            CleanupWorkspace(ticket);
+            await CleanupWorkspace(ticket, CancellationToken.None);
         }
 
         [Test]
@@ -472,10 +472,10 @@ private ScriptStateStore SetupScriptStateStore(ScriptTicket ticket)
             return stateWorkspace;
         }
 
-        private void CleanupWorkspace(ScriptTicket ticket)
+        private async Task CleanupWorkspace(ScriptTicket ticket, CancellationToken cancellationToken)
         {
             var workspace = workspaceFactory.GetWorkspace(ticket);
-            workspace.Delete();
+            await workspace.Delete(cancellationToken);
         }
 
         (StartScriptCommandV2 StartScriptCommand, FileInfo FileScriptWillCreate) GetStartScriptCommandForScriptThatCreatesAFile(ScriptTicket scriptTicket, int? scriptDelayInSeconds = null)

diff --git a/source/Octopus.Tentacle.Tests/Integration/ScriptServiceV3AlphaFixture.cs b/source/Octopus.Tentacle.Tests/Integration/ScriptServiceV3AlphaFixture.cs
@@ -362,7 +362,7 @@ public async Task GetStatusShouldReturnAnExitCodeOf46ForAScriptWithAnUnknownResu
 
             response.ExitCode.Should().Be(-46);
 
-            CleanupWorkspace(ticket);
+            await CleanupWorkspace(ticket, CancellationToken.None);
         }
 
         [Test]
@@ -376,7 +376,7 @@ public async Task CancelScriptShouldReturnAnExitCodeOf46ForAScriptWithAnUnknownR
 
             response.ExitCode.Should().Be(-46);
 
-            CleanupWorkspace(ticket);
+            await CleanupWorkspace(ticket, CancellationToken.None);
         }
 
         [Test]
@@ -468,10 +468,10 @@ private ScriptStateStore SetupScriptStateStore(ScriptTicket ticket)
             return stateWorkspace;
         }
 
-        private void CleanupWorkspace(ScriptTicket ticket)
+        private async Task CleanupWorkspace(ScriptTicket ticket, CancellationToken cancellationToken)
         {
             var workspace = workspaceFactory.GetWorkspace(ticket);
-            workspace.Delete();
+            await workspace.Delete(cancellationToken);
         }
 
         (StartScriptCommandV3Alpha StartScriptCommand, FileInfo FileScriptWillCreate) GetStartScriptCommandForScriptThatCreatesAFile(ScriptTicket scriptTicket, int? scriptDelayInSeconds = null)

diff --git a/source/Octopus.Tentacle/Scripts/IScriptWorkspace.cs b/source/Octopus.Tentacle/Scripts/IScriptWorkspace.cs
@@ -17,7 +17,6 @@ public interface IScriptWorkspace
         string? ScriptMutexName { get; set; }
         void BootstrapScript(string scriptBody);
         string ResolvePath(string fileName);
-        void Delete();
         Task Delete(CancellationToken cancellationToken);
         IScriptLog CreateLog();
         string LogFilePath { get; }

diff --git a/source/Octopus.Tentacle/Scripts/ScriptWorkspace.cs b/source/Octopus.Tentacle/Scripts/ScriptWorkspace.cs
@@ -82,11 +82,6 @@ public string ResolvePath(string fileName)
             return path;
         }
 
-        public void Delete()
-        {
-            FileSystem.DeleteDirectory(WorkingDirectory, DeletionOptions.TryThreeTimesIgnoreFailure);
-        }
-
         public async Task Delete(CancellationToken cancellationToken)
         {
             await FileSystem.DeleteDirectory(WorkingDirectory, cancellationToken, DeletionOptions.TryThreeTimesIgnoreFailure);

diff --git a/source/Octopus.Tentacle/Services/Scripts/ScriptService.cs b/source/Octopus.Tentacle/Services/Scripts/ScriptService.cs
@@ -76,7 +76,7 @@ public async Task<ScriptStatusResponse> CompleteScriptAsync(CompleteScriptComman
             cancellationTokens.TryRemove(command.Ticket.TaskId, out _);
             var response = GetResponse(command.Ticket, script, command.LastLogSequence);
             var workspace = workspaceFactory.GetWorkspace(command.Ticket);
-            workspace.Delete();
+            await workspace.Delete(cancellationToken);
             return response;
         }
 

diff --git a/source/Octopus.Tentacle/Services/Scripts/ScriptServiceV2.cs b/source/Octopus.Tentacle/Services/Scripts/ScriptServiceV2.cs
@@ -121,7 +121,7 @@ public async Task CompleteScriptAsync(CompleteScriptCommandV2 command, Cancellat
             }
 
             var workspace = workspaceFactory.GetWorkspace(command.Ticket);
-            workspace.Delete();
+            await workspace.Delete(cancellationToken);
         }
 
         RunningScript LaunchShell(ScriptTicket ticket, string serverTaskId, IScriptWorkspace workspace, IScriptStateStore stateStore, CancellationToken cancellationToken)