IgnoredFailures Status and Fix Engine Cancellation Token (#99)

* Fix Cancellation Token being cancelled when Ignore Failure is true

* IgnoredFailure Status

* Update ProgressPrinter.cs

* ReleaseNotes.md

* Failure tests

Author: thomhurst

src/ModularPipelines.Build/Modules/LocalMachine/CreateLocalNugetFolderModule.cs

@@ -7,8 +7,6 @@
 
 namespace ModularPipelines.Build.Modules.LocalMachine;
 
-[DependsOn<RunUnitTestsModule>]
-[DependsOn<PackagePathsParserModule>]
 public class CreateLocalNugetFolderModule : Module<Folder>
 {
     protected override async Task<ModuleResult<Folder>?> ExecuteAsync(IModuleContext context, CancellationToken cancellationToken)

src/ModularPipelines.Build/Modules/UploadPackagesToNugetModule.cs

@@ -67,7 +67,6 @@ protected override async Task<bool> ShouldSkip(IModuleContext context)
             .UploadPackages(new NuGetUploadOptions(packagePaths.Value!.AsPaths(), new Uri("https://api.nuget.org/v3/index.json"))
             {
                 ApiKey = _nugetSettings.Value.ApiKey!,
-                NoSymbols = true
             });
     }
 }

src/ModularPipelines.Build/ReleaseNotes.md

@@ -1 +1,3 @@
--   README and License added to NuGet packages 
+-   README and License added to NuGet packages
+-   Fix issue with the pipeline terminating even if IgnoreFailure was true on a module  
+-   Added an IgnoredFailure status type for modules 

src/ModularPipelines/Enums/Status.cs

@@ -6,6 +6,7 @@ public enum Status
     Processing,
     Successful,
     Failed,
+    IgnoredFailure,
     PipelineTerminated,
     TimedOut,
     Skipped,

src/ModularPipelines/Exceptions/ModuleFailedException.cs

@@ -1,22 +1,13 @@
-using System.Runtime.Serialization;
+using ModularPipelines.Modules;
 
 namespace ModularPipelines.Exceptions;
 
 public class ModuleFailedException : PipelineException
 {
-    public ModuleFailedException()
-    {
-    }
-
-    protected ModuleFailedException(SerializationInfo info, StreamingContext context) : base(info, context)
-    {
-    }
-
-    public ModuleFailedException(string? message) : base(message)
-    {
-    }
+    public ModuleBase Module { get; }
 
-    public ModuleFailedException(string? message, Exception? innerException) : base(message, innerException)
+    public ModuleFailedException(ModuleBase module, Exception exception) : base($"The module {module.GetType().Name} has failed", exception)
     {
+        Module = module;
     }
 }

src/ModularPipelines/Extensions/HostExtensions.cs

@@ -18,7 +18,14 @@ public static async Task<IReadOnlyList<ModuleBase>> ExecuteAsync(this IHost host
         }
         finally
         {
-            await ((ServiceProvider) host.Services).DisposeAsync();
+            if (!IsRunningFromNUnit)
+            {
+                await ((ServiceProvider) host.Services).DisposeAsync();
+            }
         }
     }
+
+    private static readonly bool IsRunningFromNUnit = 
+        AppDomain.CurrentDomain.GetAssemblies().Any(
+            a => a.FullName?.ToLowerInvariant().StartsWith("nunit.framework") == true);
 }

src/ModularPipelines/Helpers/ProgressPrinter.cs

@@ -3,6 +3,7 @@
 using ModularPipelines.Modules;
 using ModularPipelines.Options;
 using Spectre.Console;
+using Status = ModularPipelines.Enums.Status;
 
 namespace ModularPipelines.Helpers;
 
@@ -118,29 +119,58 @@ private static void RegisterModules(IReadOnlyList<RunnableModule> modulesToProce
                     progressTask.Increment(ticksPerSecond);
                 }
             }, cancellationToken);
+            
+            SetupSkippedCallback(cancellationToken, moduleToProcess, progressTask, moduleName);
+            SetupFinishedSuccessfullyCallback(modulesToProcess, totalTask, cancellationToken, moduleToProcess, progressTask, moduleName);
+            
+            RegisterSubModules(moduleToProcess, progressContext, cancellationToken);
+        }
+    }
 
-            // Callback for Module has finished
-            _ = moduleToProcess.Module.ResultTaskInternal.ContinueWith(t =>
+    private static void SetupSkippedCallback(CancellationToken cancellationToken, RunnableModule moduleToProcess,
+        ProgressTask progressTask, string moduleName)
+    {
+        // Callback for Module has been ignored
+        _ = moduleToProcess.Module.SkippedTask.ContinueWith(t =>
+        {
+            lock (moduleToProcess)
+            {
+                progressTask.Description = $"[yellow][[Skipped]] {moduleName}[/]";
+                progressTask.StopTask();
+            }
+        }, cancellationToken);
+    }
+
+    private static void SetupFinishedSuccessfullyCallback(IReadOnlyList<RunnableModule> modulesToProcess, ProgressTask totalTask,
+        CancellationToken cancellationToken, RunnableModule moduleToProcess, ProgressTask progressTask, string moduleName)
+    {
+        // Callback for Module has finished
+        _ = moduleToProcess.Module.ResultTaskInternal.ContinueWith(t =>
+        {
+            lock (moduleToProcess)
             {
+                if (progressTask.IsFinished)
+                {
+                    return;
+                }
+
                 if (t.IsCompletedSuccessfully)
                 {
                     progressTask.Increment(100);
                 }
 
-                progressTask.Description = t.IsCompletedSuccessfully ? $"[green]{moduleName}[/]" : $"[red][[Failed]] {moduleName}[/]";
+                progressTask.Description =
+                    t.IsCompletedSuccessfully ? $"{GetColour()}{moduleName}[/]" : $"[red][[Failed]] {moduleName}[/]";
 
                 progressTask.StopTask();
-                totalTask.Increment(100.0 / modulesToProcess.Count);
-            }, cancellationToken);
 
-            // Callback for Module has been ignored
-            _ = moduleToProcess.Module.IgnoreTask.ContinueWith(t =>
-            {
-                progressTask.Description = $"[yellow][[Ignored]] {moduleName}[/]";
-                progressTask.StopTask();
-            }, cancellationToken);
-
-            RegisterSubModules(moduleToProcess, progressContext, cancellationToken);
+                totalTask.Increment(100.0 / modulesToProcess.Count);
+            }
+        }, cancellationToken);
+        
+        string GetColour()
+        {
+            return moduleToProcess.Module.Status == Status.Successful ? "[green]" : "[orange3]";
         }
     }
 

src/ModularPipelines/Modules/Module.cs

@@ -160,25 +160,27 @@ internal override async Task StartAsync()
 
             Exception = exception;
 
-            Context.EngineCancellationToken.Cancel();
-
             if (await ShouldIgnoreFailures(Context, exception))
             {
                 var moduleResult = ModuleResult.FromException<T>(exception);
                 moduleResult.ModuleName = GetType().Name;
 
+                Status = Status.IgnoredFailure;
+
                 await Context.ModuleResultRepository.SaveResultAsync(this, moduleResult);
 
                 TaskCompletionSource.SetResult(moduleResult);
             }
             else
             {
+                Context.EngineCancellationToken.Cancel();
+
                 // Give time for Engine to request cancellation
                 await Task.Delay(300);
 
                 TaskCompletionSource.SetException(exception);
 
-                throw;
+                throw new ModuleFailedException(this, exception);
             }
         }
         finally
@@ -315,7 +317,7 @@ internal override void SetSkipped()
     {
         Status = Status.Skipped;
 
-        IgnoreTask.Start(TaskScheduler.Default);
+        SkippedTask.Start(TaskScheduler.Default);
 
         TaskCompletionSource.SetResult(new SkippedModuleResult<T>());
 

src/ModularPipelines/Modules/ModuleBase.cs

@@ -12,7 +12,7 @@ public abstract class ModuleBase
     protected internal IModuleContext Context = null!; // Late Initialisation
 
     internal readonly Task StartTask = new(() => { });
-    internal readonly Task IgnoreTask = new(() => { });
+    internal readonly Task SkippedTask = new(() => { });
     internal abstract Task<object> ResultTaskInternal { get; }
 
     internal readonly CancellationTokenSource ModuleCancellationTokenSource = new();

test/ModularPipelines.UnitTests/IgnoredFailureTests.cs

@@ -0,0 +1,42 @@
+using Microsoft.Extensions.DependencyInjection;
+using ModularPipelines.Context;
+using ModularPipelines.Engine;
+using ModularPipelines.Models;
+using ModularPipelines.Modules;
+
+namespace ModularPipelines.UnitTests;
+
+public class IgnoredFailureTests : TestBase
+{
+    private class IgnoredFailureModule : Module<CommandResult>
+    {
+        protected override Task<bool> ShouldIgnoreFailures(IModuleContext context, Exception exception)
+        {
+            return Task.FromResult(true);
+        }
+
+        protected override async Task<ModuleResult<CommandResult>?> ExecuteAsync(IModuleContext context, CancellationToken cancellationToken)
+        {
+            await Task.Yield();
+            throw new Exception();
+        }
+    }
+    
+    [Test]
+    public async Task Has_Not_Thrown_Or_Cancelled_Pipeline()
+    {
+        var module = await RunModule<IgnoredFailureModule>();
+
+        var serviceProvider = module.Context.Get<IServiceProvider>()!;
+        var engineCancellationToken = serviceProvider.GetRequiredService<EngineCancellationToken>();
+
+        var moduleResult = await module;
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(moduleResult.ModuleResultType, Is.EqualTo(ModuleResultType.Failure));
+            Assert.That(moduleResult.Exception, Is.Not.Null);
+            Assert.That(engineCancellationToken.IsCancellationRequested, Is.False);
+        });
+    }
+}

test/ModularPipelines.UnitTests/ModuleTimeoutTests.cs

@@ -1,4 +1,5 @@
 using ModularPipelines.Context;
+using ModularPipelines.Exceptions;
 using ModularPipelines.Models;
 using ModularPipelines.Modules;
 
@@ -20,6 +21,7 @@ private class Module : Module<string>
     [Test]
     public void Throws_Timeout_Exception()
     {
-        Assert.ThrowsAsync<TaskCanceledException>(RunModule<Module>);
+        var exception = Assert.ThrowsAsync<ModuleFailedException>(RunModule<Module>);
+        Assert.That(exception!.InnerException, Is.TypeOf<TaskCanceledException>());
     }
 }

test/ModularPipelines.UnitTests/NonIgnoredFailureTests.cs

@@ -0,0 +1,31 @@
+using Microsoft.Extensions.DependencyInjection;
+using ModularPipelines.Context;
+using ModularPipelines.Engine;
+using ModularPipelines.Exceptions;
+using ModularPipelines.Models;
+using ModularPipelines.Modules;
+
+namespace ModularPipelines.UnitTests;
+
+public class NonIgnoredFailureTests : TestBase
+{
+    private class NonIgnoredFailureModule : Module<CommandResult>
+    {
+        protected override async Task<ModuleResult<CommandResult>?> ExecuteAsync(IModuleContext context, CancellationToken cancellationToken)
+        {
+            await Task.Yield();
+            throw new Exception();
+        }
+    }
+    
+    [Test]
+    public void Has_Thrown_And_Cancelled_Pipeline()
+    {
+        var exception = Assert.ThrowsAsync<ModuleFailedException>(async () => await RunModule<NonIgnoredFailureModule>());
+
+        var serviceProvider = exception!.Module.Context.Get<IServiceProvider>()!;
+        var engineCancellationToken = serviceProvider.GetRequiredService<EngineCancellationToken>();
+        
+        Assert.That(engineCancellationToken.IsCancellationRequested, Is.True);
+    }
+}