fix(engine): invoke OnTestRegistered on property-injected data sources

docs/docs/writing-tests/event-subscribing.md

@@ -21,6 +21,12 @@ The interfaces they can implement are:
 
 This can be useful especially when generating data that you need to track and maybe dispose later. By hooking into these events, we can do things like track and dispose our objects when we need.
 
+## Registration callbacks for injected properties
+
+TUnit creates injected property values after the first `OnTestRegistered` pass, so that a receiver such as a `SkipAttribute` can skip the test before expensive data sources are created. An injected property value that implements `ITestRegisteredEventReceiver` receives its callback in a second pass, after the property values are resolved and before the test runs. The second pass uses the same `Order` sorting, but it always runs after every receiver from the first pass.
+
+A property value can still call `SetParallelLimiter`, `SetSkipped` and the other `TestRegisteredContext` methods. If it skips the test, its shared data source objects have already been created.
+
 ## Registration callbacks for executors
 
 An executor that implements `ITestRegisteredEventReceiver` receives `OnTestRegistered` when a registration receiver installs it through `SetTestExecutor` or `SetHookExecutor`, unless it has already received the callback for that test. Its callback runs after the installing receiver returns. Dynamically installed executors are not globally sorted by their own `Order`.

src/TUnit.Core/Interfaces/ITestRegisteredEventReceiver.cs

@@ -28,6 +28,12 @@ namespace TUnit.Core.Interfaces;
 /// Each receiver instance is invoked at most once per test, even when it is both an eligible
 /// event object and an installed executor. Distinct instances are compared by reference identity.
 /// </para>
+/// <para>
+/// Injected property values are resolved after the first registration pass, so that receivers
+/// such as <see cref="SkipAttribute"/> can skip a test before expensive data sources are created.
+/// An injected property value that implements this interface is invoked in a second pass, after
+/// every receiver from the first pass and before the test executes.
+/// </para>
 /// </remarks>
 public interface ITestRegisteredEventReceiver : IEventReceiver
 {

src/TUnit.Engine/Services/TestFilterService.cs

@@ -81,9 +81,42 @@ private async Task InvokeTestRegisteredReceiver(ITestRegisteredEventReceiver rec
         }
     }
 
+    private async Task InvokeTestRegisteredReceivers(ITestRegisteredEventReceiver[] receivers, TestRegisteredContext context,
+        HashSet<ITestRegisteredEventReceiver> invokedReceivers)
+    {
+        foreach (var receiver in receivers)
+        {
+            await InvokeTestRegisteredReceiver(receiver, context, invokedReceivers).ConfigureAwait(false);
+
+            // Dynamically installed executor callbacks run after their installer, regardless of their
+            // own Order. The shared identity set also covers executors in the original receiver list.
+            while (context.TryDequeueExecutorEventReceiver(out var executorReceiver))
+            {
+                await InvokeTestRegisteredReceiver(executorReceiver, context, invokedReceivers).ConfigureAwait(false);
+            }
+        }
+    }
+
+    private static TestRegisteredContext CreateTestRegisteredContext(TestContext testContext)
+    {
+        var discoveredTest = new DiscoveredTest<object>
+        {
+            TestContext = testContext
+        };
+
+        testContext.InternalDiscoveredTest = discoveredTest;
+
+        return new TestRegisteredContext(testContext)
+        {
+            DiscoveredTest = discoveredTest
+        };
+    }
+
     private async Task RegisterTest(AbstractExecutableTest test, bool isForExecution)
     {
         var registeredReceivers = test.Context.GetTestRegisteredReceivers();
+        TestRegisteredContext? registeredContext = null;
+        HashSet<ITestRegisteredEventReceiver>? invokedReceivers = null;
 
         // Invoke event receivers BEFORE argument registration so that SkipAttribute
         // and other ITestRegisteredEventReceiver implementations can set SkipReason
@@ -92,30 +125,10 @@ private async Task RegisterTest(AbstractExecutableTest test, bool isForExecution
         // that need to prevent ClassDataSource initialization when the test should be skipped.
         if (registeredReceivers.Length > 0)
         {
-            var discoveredTest = new DiscoveredTest<object>
-            {
-                TestContext = test.Context
-            };
-
-            var registeredContext = new TestRegisteredContext(test.Context)
-            {
-                DiscoveredTest = discoveredTest
-            };
-
-            test.Context.InternalDiscoveredTest = discoveredTest;
-            var invokedReceivers = new HashSet<ITestRegisteredEventReceiver>(Core.Helpers.ReferenceEqualityComparer.Instance);
-
-            foreach (var receiver in registeredReceivers)
-            {
-                await InvokeTestRegisteredReceiver(receiver, registeredContext, invokedReceivers).ConfigureAwait(false);
+            registeredContext = CreateTestRegisteredContext(test.Context);
+            invokedReceivers = new HashSet<ITestRegisteredEventReceiver>(Core.Helpers.ReferenceEqualityComparer.Instance);
 
-                // Dynamically installed executor callbacks run after their installer, regardless of their
-                // own Order. The shared identity set also covers executors in the original receiver list.
-                while (registeredContext.TryDequeueExecutorEventReceiver(out var executorReceiver))
-                {
-                    await InvokeTestRegisteredReceiver(executorReceiver, registeredContext, invokedReceivers).ConfigureAwait(false);
-                }
-            }
+            await InvokeTestRegisteredReceivers(registeredReceivers, registeredContext, invokedReceivers).ConfigureAwait(false);
         }
 
         // If the test was marked as skipped by an event receiver, skip argument registration entirely.
@@ -153,15 +166,31 @@ private async Task RegisterTest(AbstractExecutableTest test, bool isForExecution
         // requests would create fixtures as a side effect and leak them forever (#6151).
         if (isForExecution)
         {
+            var argumentsRegistered = false;
+
             try
             {
                 await testArgumentRegistrationService.RegisterTestArgumentsAsync(test.Context).ConfigureAwait(false);
+                argumentsRegistered = true;
             }
             catch (Exception ex)
             {
                 // Mark the test as failed - event receivers have already run above
                 test.SetResult(TestState.Failed, ex);
             }
+
+            // Injected property values only exist now, after argument registration resolved them,
+            // and those values can themselves be registered receivers (e.g. a shared fixture that
+            // calls SetParallelLimiter). Argument registration dropped the receiver caches, so the
+            // rebuilt list includes them; the shared identity set skips receivers that already ran (#7290).
+            if (argumentsRegistered &&
+                test.Context.Metadata.TestDetails.TestClassInjectedPropertyArguments.Count > 0)
+            {
+                registeredContext ??= CreateTestRegisteredContext(test.Context);
+                invokedReceivers ??= new HashSet<ITestRegisteredEventReceiver>(Core.Helpers.ReferenceEqualityComparer.Instance);
+
+                await InvokeTestRegisteredReceivers(test.Context.GetTestRegisteredReceivers(), registeredContext, invokedReceivers).ConfigureAwait(false);
+            }
         }
 
         // Clear the cached display name after registration events

tests/TUnit.Engine.Tests/RegisteredReceiverInjectedProperty7290Tests.cs

@@ -0,0 +1,27 @@
+using Shouldly;
+using TUnit.Engine.Tests.Enums;
+
+namespace TUnit.Engine.Tests;
+
+/// <summary>
+/// Verifies issue #7290: a property-injected data source that implements
+/// ITestRegisteredEventReceiver receives OnTestRegistered, so it can set a parallel limiter or
+/// skip the test. Runs every class in Bugs/_7290/Tests.cs: four repeated property consumers, one
+/// constructor-plus-property consumer and one test skipped by its injected fixture.
+/// </summary>
+public class RegisteredReceiverInjectedProperty7290Tests(TestMode testMode) : InvokableTestBase(testMode)
+{
+    [Test]
+    public async Task Test()
+    {
+        await RunTestsWithFilter(
+            "/*/TUnit.TestProject.Bugs._7290/*/*",
+            [
+                result => result.ResultSummary.Outcome.ShouldBe("Completed"),
+                result => result.ResultSummary.Counters.Total.ShouldBe(6),
+                result => result.ResultSummary.Counters.Passed.ShouldBe(5),
+                result => result.ResultSummary.Counters.Failed.ShouldBe(0),
+                result => result.ResultSummary.Counters.NotExecuted.ShouldBe(1)
+            ]);
+    }
+}

tests/TUnit.TestProject/Bugs/_7290/Tests.cs

@@ -0,0 +1,110 @@
+using System.Collections.Concurrent;
+using TUnit.Core.Interfaces;
+
+namespace TUnit.TestProject.Bugs._7290;
+
+/// <summary>
+/// Reproduction for issue #7290: "ITestRegisteredEventReceiver is not invoked on property-injected
+/// data source objects."
+/// https://github.com/thomhurst/TUnit/issues/7290
+///
+/// Registered receivers ran once, before argument registration resolved injected property values,
+/// so a shared fixture received OnTestRegistered as a constructor or method argument but never as
+/// an injected property. The fixture below limits its consumers to one test at a time.
+/// </summary>
+public sealed class OneAtATime : IParallelLimit
+{
+    public int Limit => 1;
+}
+
+public sealed class LimitedFixture : ITestRegisteredEventReceiver
+{
+    private readonly ConcurrentDictionary<string, int> _registrations = new();
+    private int _running;
+    private int _maxRunning;
+
+    public int Order => 0;
+
+    public int MaxRunning => Volatile.Read(ref _maxRunning);
+
+    public ValueTask OnTestRegistered(TestRegisteredContext context)
+    {
+        _registrations.AddOrUpdate(context.TestContext.Metadata.TestDetails.TestId, 1, static (_, count) => count + 1);
+        context.SetParallelLimiter(new OneAtATime());
+        return default;
+    }
+
+    public int RegistrationsFor(TestContext context) =>
+        _registrations.TryGetValue(context.Metadata.TestDetails.TestId, out var count) ? count : 0;
+
+    public async Task RunAsync()
+    {
+        var running = Interlocked.Increment(ref _running);
+        int max;
+        while (running > (max = Volatile.Read(ref _maxRunning)) &&
+               Interlocked.CompareExchange(ref _maxRunning, running, max) != max)
+        {
+        }
+
+        await Task.Delay(50);
+        Interlocked.Decrement(ref _running);
+    }
+}
+
+public sealed class SkippingFixture : ITestRegisteredEventReceiver
+{
+    public int Order => 0;
+
+    public ValueTask OnTestRegistered(TestRegisteredContext context)
+    {
+        context.SetSkipped("Skipped by property-injected fixture");
+        return default;
+    }
+}
+
+public sealed class PropertyConsumerTests
+{
+    [ClassDataSource<LimitedFixture>(Shared = SharedType.PerClass)]
+    public required LimitedFixture Fixture { get; init; }
+
+    [Test, Repeat(3)]
+    public async Task Property_Injected_Fixture_Receives_OnTestRegistered()
+    {
+        var context = TestContext.Current!;
+
+        await Fixture.RunAsync();
+
+        await Assert.That(context.Parallelism.Limiter).IsTypeOf<OneAtATime>();
+        await Assert.That(Fixture.RegistrationsFor(context)).IsEqualTo(1);
+        await Assert.That(Fixture.MaxRunning).IsEqualTo(1);
+    }
+}
+
+[ClassDataSource<LimitedFixture>(Shared = SharedType.PerClass)]
+public sealed class ConstructorAndPropertyConsumerTests(LimitedFixture constructorFixture)
+{
+    [ClassDataSource<LimitedFixture>(Shared = SharedType.PerClass)]
+    public required LimitedFixture Fixture { get; init; }
+
+    [Test]
+    public async Task Shared_Instance_Receives_OnTestRegistered_Once()
+    {
+        var context = TestContext.Current!;
+
+        await Assert.That(Fixture).IsSameReferenceAs(constructorFixture);
+        await Assert.That(context.Parallelism.Limiter).IsTypeOf<OneAtATime>();
+        await Assert.That(Fixture.RegistrationsFor(context)).IsEqualTo(1);
+    }
+}
+
+public sealed class PropertySkipTests
+{
+    [ClassDataSource<SkippingFixture>(Shared = SharedType.PerClass)]
+    public required SkippingFixture Fixture { get; init; }
+
+    [Test]
+    public void Property_Injected_Fixture_Can_Skip_The_Test()
+    {
+        throw new InvalidOperationException("This test should have been skipped by its injected fixture.");
+    }
+}