From 1000fe992933f3d872faf60a9675fbf0af2d03f8 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:43:00 +0000 Subject: [PATCH 1/2] Fix #7525: isolate test registration callback failures --- .../Services/TestFilterService.cs | 25 +++++++--- .../RegistrationFailure7525Tests.cs | 23 +++++++++ tests/TUnit.TestProject/Bugs/7525/Tests.cs | 48 +++++++++++++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs create mode 100644 tests/TUnit.TestProject/Bugs/7525/Tests.cs diff --git a/src/TUnit.Engine/Services/TestFilterService.cs b/src/TUnit.Engine/Services/TestFilterService.cs index 820a1dca823..fa463e16885 100644 --- a/src/TUnit.Engine/Services/TestFilterService.cs +++ b/src/TUnit.Engine/Services/TestFilterService.cs @@ -113,6 +113,25 @@ private static TestRegisteredContext CreateTestRegisteredContext(TestContext tes } private async Task RegisterTest(AbstractExecutableTest test, bool isForExecution) + { + try + { + await RegisterTestCore(test, isForExecution).ConfigureAwait(false); + } + catch (Exception ex) + { + // Registration callbacks are user code. Preserve the failure on this test so + // neither sequential nor parallel registration aborts the rest of the request. + test.SetResult(TestState.Failed, ex); + } + finally + { + // Callbacks may have changed display formatting before failing. + test.Context.InvalidateDisplayNameCache(); + } + } + + private async Task RegisterTestCore(AbstractExecutableTest test, bool isForExecution) { var registeredReceivers = test.Context.GetTestRegisteredReceivers(); TestRegisteredContext? registeredContext = null; @@ -178,7 +197,6 @@ async Task InvokeRegistrationDelegatesAsync() } } - test.Context.InvalidateDisplayNameCache(); return; } @@ -219,11 +237,6 @@ async Task InvokeRegistrationDelegatesAsync() await InvokeRegistrationDelegatesAsync().ConfigureAwait(false); } } - - // Clear the cached display name after registration events - // This ensures that ArgumentDisplayFormatterAttribute and similar attributes - // have a chance to register their formatters before the display name is finalized - test.Context.InvalidateDisplayNameCache(); } /// diff --git a/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs b/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs new file mode 100644 index 00000000000..389c5ed1263 --- /dev/null +++ b/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs @@ -0,0 +1,23 @@ +using Shouldly; +using TUnit.Engine.Tests.Enums; + +namespace TUnit.Engine.Tests; + +public class RegistrationFailure7525Tests(TestMode testMode) : InvokableTestBase(testMode) +{ + [Test] + [Arguments("RegistrationFailureTests", 3, 1)] + [Arguments("ParallelRegistrationFailureTests", 8, 6)] + public async Task RegistrationFailureDoesNotAbortRequest(string className, int total, int passed) + { + await RunTestsWithFilter($"/*/TUnit.TestProject.Bugs._7525/{className}/*", + [ + result => result.ResultSummary.Counters.Total.ShouldBe(total), + result => result.ResultSummary.Counters.Failed.ShouldBe(2), + result => result.ResultSummary.Counters.Passed.ShouldBe(passed), + result => result.Results.Where(x => x.Outcome == "Failed") + .ShouldAllBe(x => x.Output!.ErrorInfo!.Message!.Contains("Registration ") + && x.Output.ErrorInfo.Message.Contains("failed (#7525)")) + ]); + } +} diff --git a/tests/TUnit.TestProject/Bugs/7525/Tests.cs b/tests/TUnit.TestProject/Bugs/7525/Tests.cs new file mode 100644 index 00000000000..4c2443a2197 --- /dev/null +++ b/tests/TUnit.TestProject/Bugs/7525/Tests.cs @@ -0,0 +1,48 @@ +using TUnit.Core.Interfaces; +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._7525; + +public class RegistrationFailureTests +{ + public sealed class ThrowingReceiverAttribute : Attribute, ITestRegisteredEventReceiver + { + public int Order => 0; + + public ValueTask OnTestRegistered(TestRegisteredContext context) + => throw new InvalidOperationException("Registration receiver failed (#7525)"); + } + + public sealed class ThrowingDelegateAttribute : DataSourceGeneratorAttribute + { + protected override IEnumerable> GenerateDataSources(DataGeneratorMetadata metadata) + { + yield return () => + { + metadata.TestBuilderContext.Current.Events.OnTestRegistered += async (_, _) => + { + await Task.Yield(); + throw new InvalidOperationException("Registration delegate failed (#7525)"); + }; + return 1; + }; + } + } + + [Test, ThrowingReceiver, EngineTest(ExpectedResult.Failure)] + public void ReceiverFailure() => throw new Exception("Test body must not execute"); + + [Test, ThrowingDelegate, EngineTest(ExpectedResult.Failure)] + public void DelegateFailure(int value) => throw new Exception("Test body must not execute"); + + [Test, EngineTest(ExpectedResult.Pass)] + public void UnrelatedTest() { } +} + +// Eight or more tests take the parallel registration path. +[InheritsTests] +public class ParallelRegistrationFailureTests : RegistrationFailureTests +{ + [Test, Arguments(1), Arguments(2), Arguments(3), Arguments(4), Arguments(5), EngineTest(ExpectedResult.Pass)] + public void OtherTests(int value) { } +} From c10f713e260c79870b1b4a7552f272003faaef88 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:53:09 +0000 Subject: [PATCH 2/2] Fix #7525: preserve registration failure disposal and skip precedence --- .../Services/TestFilterService.cs | 16 +++++++++ .../RegistrationFailure7525Tests.cs | 12 +++++++ tests/TUnit.TestProject/Bugs/7525/Tests.cs | 33 ++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/TUnit.Engine/Services/TestFilterService.cs b/src/TUnit.Engine/Services/TestFilterService.cs index fa463e16885..d2a307627a5 100644 --- a/src/TUnit.Engine/Services/TestFilterService.cs +++ b/src/TUnit.Engine/Services/TestFilterService.cs @@ -122,6 +122,22 @@ private async Task RegisterTest(AbstractExecutableTest test, bool isForExecution { // Registration callbacks are user code. Preserve the failure on this test so // neither sequential nor parallel registration aborts the rest of the request. + if (isForExecution) + { + try + { + // Track only objects that already exist. Tracking is idempotent if + // argument registration completed before a later callback failed. + testArgumentRegistrationService.TrackSkippedTestArguments(test.Context); + } + catch (Exception trackingException) + { + logger.LogDebug($"Could not track the objects of registration-failed test '{test.TestId}' for disposal: {trackingException.Message}"); + } + } + + // A callback failure takes precedence over an earlier request to skip. + test.Context.SkipReason = null; test.SetResult(TestState.Failed, ex); } finally diff --git a/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs b/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs index 389c5ed1263..ed9c0298e06 100644 --- a/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs +++ b/tests/TUnit.Engine.Tests/RegistrationFailure7525Tests.cs @@ -20,4 +20,16 @@ await RunTestsWithFilter($"/*/TUnit.TestProject.Bugs._7525/{className}/*", && x.Output.ErrorInfo.Message.Contains("failed (#7525)")) ]); } + [Test] + public async Task RegistrationFailureDisposesExistingArguments() + { + await RunTestsWithFilter("/*/TUnit.TestProject.Bugs._7525/RegistrationDisposalTests/*", + [ + result => result.ResultSummary.Counters.Total.ShouldBe(2), + result => result.ResultSummary.Counters.Failed.ShouldBe(1), + result => result.ResultSummary.Counters.Passed.ShouldBe(1), + result => result.Results.Single(x => x.Outcome == "Failed").Output!.ErrorInfo!.Message! + .ShouldContain("Registration receiver failed (#7525)") + ]); + } } diff --git a/tests/TUnit.TestProject/Bugs/7525/Tests.cs b/tests/TUnit.TestProject/Bugs/7525/Tests.cs index 4c2443a2197..88fb5ec214c 100644 --- a/tests/TUnit.TestProject/Bugs/7525/Tests.cs +++ b/tests/TUnit.TestProject/Bugs/7525/Tests.cs @@ -10,7 +10,10 @@ public sealed class ThrowingReceiverAttribute : Attribute, ITestRegisteredEventR public int Order => 0; public ValueTask OnTestRegistered(TestRegisteredContext context) - => throw new InvalidOperationException("Registration receiver failed (#7525)"); + { + context.SetSkipped("skip before registration failure"); + throw new InvalidOperationException("Registration receiver failed (#7525)"); + } } public sealed class ThrowingDelegateAttribute : DataSourceGeneratorAttribute @@ -46,3 +49,31 @@ public class ParallelRegistrationFailureTests : RegistrationFailureTests [Test, Arguments(1), Arguments(2), Arguments(3), Arguments(4), Arguments(5), EngineTest(ExpectedResult.Pass)] public void OtherTests(int value) { } } + +public sealed class RegistrationDisposable : IDisposable +{ + public static List Created { get; } = []; + public bool IsDisposed { get; private set; } + public RegistrationDisposable() => Created.Add(this); + public IAsyncInitializer Child => throw new InvalidOperationException("tracking getter failed"); + public void Dispose() => IsDisposed = true; +} + +[EngineTest(ExpectedResult.Failure)] +public class RegistrationDisposalTests +{ + public static IEnumerable Values() + { + yield return new RegistrationDisposable(); + } + + [Test, RegistrationFailureTests.ThrowingReceiver, MethodDataSource(nameof(Values))] + public void Fails(RegistrationDisposable value) => throw new Exception("Test body must not execute"); + + [Test, DependsOn(nameof(Fails), ProceedOnFailure = true)] + public async Task ArgumentWasDisposed() + { + await Assert.That(RegistrationDisposable.Created.Count).IsEqualTo(1); + await Assert.That(RegistrationDisposable.Created[0].IsDisposed).IsTrue(); + } +}