diff --git a/docs/combinations.md b/docs/combinations.md index a524e03aa..fc5a98d29 100644 --- a/docs/combinations.md +++ b/docs/combinations.md @@ -175,7 +175,7 @@ public Task BuildAddressExceptionsDisabledTest() city); } ``` -snippet source | anchor +snippet source | anchor @@ -401,7 +401,7 @@ class CustomCombinationConverter : string.Join(", ", keys.Select(_ => _.Value)); } ``` -snippet source | anchor +snippet source | anchor Full control of serialization can be achieved by inheriting from `WriteOnlyJsonConverter`. @@ -420,7 +420,7 @@ static CustomCombinationConverter customConverter = new(); public static void Init() => VerifierSettings.AddExtraSettings(_ => _.Converters.Insert(0, customConverter)); ``` -snippet source | anchor +snippet source | anchor @@ -551,5 +551,5 @@ Headers can be enabled globally: public static void EnableIncludeHeaders() => CombinationSettings.IncludeHeaders(); ``` -snippet source | anchor +snippet source | anchor diff --git a/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt b/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt new file mode 100644 index 000000000..f70460ad2 --- /dev/null +++ b/src/StaticSettingsTests/CombinationTests.EveryRegisteredCallbackIsAwaited_run.verified.txt @@ -0,0 +1,4 @@ +{ + list: Result, + 1: Exception: from the first before callback +} \ No newline at end of file diff --git a/src/StaticSettingsTests/CombinationTests.cs b/src/StaticSettingsTests/CombinationTests.cs index 71eea270f..e5a67405d 100644 --- a/src/StaticSettingsTests/CombinationTests.cs +++ b/src/StaticSettingsTests/CombinationTests.cs @@ -79,6 +79,43 @@ await Verify(new .UseMethodName("CallbackResults"); } + // Two registrations, for example an extension package plus user code. A multicast + // delegate would return only the last target's Task, leaving the first to run + // unawaited: its exceptions unobserved and its work racing the combination method. + [Fact] + public async Task EveryRegisteredCallbackIsAwaited() + { + var exceptionMessages = new List(); + + // Fails only after an await, so the failure is carried by the returned Task rather + // than thrown synchronously. Registered first, so a multicast delegate would return + // the second registration's Task instead and this one would never be observed. + CombinationSettings.UseCallbacks( + async _ => + { + await Task.Yield(); + throw new("from the first before callback"); + }, + (_, _) => Task.CompletedTask, + (_, exception) => + { + exceptionMessages.Add(exception.Message); + return Task.CompletedTask; + }); + + CombinationSettings.UseCallbacks( + _ => Task.CompletedTask, + (_, _) => Task.CompletedTask, + (_, _) => Task.CompletedTask); + + int[] list = [1]; + await Combination() + .Verify((int param1) => param1, list) + .UseMethodName("EveryRegisteredCallbackIsAwaited_run"); + + Assert.Equal(["from the first before callback"], exceptionMessages); + } + [Fact] public async Task AfterCallbackRawValueWhenRecording() { diff --git a/src/Verify/Combinations/CombinationSettings.cs b/src/Verify/Combinations/CombinationSettings.cs index 0ef3ab9d1..2f0372107 100644 --- a/src/Verify/Combinations/CombinationSettings.cs +++ b/src/Verify/Combinations/CombinationSettings.cs @@ -1,4 +1,4 @@ -namespace VerifyTests; +namespace VerifyTests; public delegate Task BeforeCombination(IReadOnlyList keys); public delegate Task AfterCombination(IReadOnlyList keys, object? result); @@ -16,7 +16,11 @@ public static void IncludeHeaders() => public static void CaptureExceptions() => CaptureExceptionsEnabled = true; - static BeforeCombination? before; + // Held as lists rather than combined with `+=`. Invoking a multicast Task returning + // delegate returns only the last target's Task, so every earlier callback would run + // unawaited: its exceptions unobserved, and its async work racing the combination + // method. Same hazard the Then extension documents for Func. + static List? before; internal static Task RunBeforeCallbacks(IReadOnlyList keys) { @@ -25,10 +29,18 @@ internal static Task RunBeforeCallbacks(IReadOnlyList keys) return Task.CompletedTask; } - return before(keys); + return RunAll(before, keys); } - static AfterCombination? after; + static async Task RunAll(List callbacks, IReadOnlyList keys) + { + foreach (var callback in callbacks) + { + await callback(keys); + } + } + + static List? after; internal static Task RunAfterCallbacks(IReadOnlyList keys, object? result) { @@ -37,10 +49,18 @@ internal static Task RunAfterCallbacks(IReadOnlyList keys, object? resu return Task.CompletedTask; } - return after(keys, result); + return RunAll(after, keys, result); } - static CombinationException? combinationException; + static async Task RunAll(List callbacks, IReadOnlyList keys, object? result) + { + foreach (var callback in callbacks) + { + await callback(keys, result); + } + } + + static List? combinationException; internal static Task RunExceptionCallbacks(IReadOnlyList keys, Exception exception) { @@ -49,14 +69,25 @@ internal static Task RunExceptionCallbacks(IReadOnlyList keys, Exceptio return Task.CompletedTask; } - return combinationException(keys, exception); + return RunAll(combinationException, keys, exception); + } + + static async Task RunAll(List callbacks, IReadOnlyList keys, Exception exception) + { + foreach (var callback in callbacks) + { + await callback(keys, exception); + } } public static void UseCallbacks(BeforeCombination before, AfterCombination after, CombinationException exception) { - CombinationSettings.before += before; - CombinationSettings.after += after; - combinationException += exception; + CombinationSettings.before ??= []; + CombinationSettings.before.Add(before); + CombinationSettings.after ??= []; + CombinationSettings.after.Add(after); + combinationException ??= []; + combinationException.Add(exception); } public static void Reset() @@ -67,4 +98,4 @@ public static void Reset() after = null; before = null; } -} \ No newline at end of file +}