Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
perf(engine): address review feedback on per-test cleanups
- Add Debug.Assert guards on the three hot-path receiver getters
  (GetTestStartReceivers, GetTestEndReceivers, GetTestSkippedReceivers)
  to catch invariant violations in Debug/test builds without cost in Release.
- Unify dictionary access in ObjectGraphDiscoverer: both ShouldSkipType and
  GetFlattenedInitializerProperties now use GetOrAdd with a static factory.
- Document the RegisterReceivers -> EnsureEventReceiversCached ordering
  dependency that makes the early skip-path safe on ExecuteTestLifecycleAsync.
  • Loading branch information
thomhurst committed Apr 24, 2026
commit 05f1046c73032ec568c629c0b41f8773ce725ee4
42 changes: 18 additions & 24 deletions TUnit.Core/Discovery/ObjectGraphDiscoverer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -442,39 +442,33 @@ private static void TraverseInitializerProperties(
/// source-gen registration exists in the type's inheritance chain.
/// </summary>
private static InitializerPropertyInfo[]? GetFlattenedInitializerProperties(Type type)
{
if (FlattenedInitializerPropertiesCache.TryGetValue(type, out var cached))
=> FlattenedInitializerPropertiesCache.GetOrAdd(type, static t =>
{
return cached;
}
List<InitializerPropertyInfo>? merged = null;
HashSet<string>? seen = null;

List<InitializerPropertyInfo>? merged = null;
HashSet<string>? seen = null;

for (var currentType = type; currentType != null && currentType != typeof(object); currentType = currentType.BaseType)
{
var registered = InitializerPropertyRegistry.GetProperties(currentType);
if (registered == null)
for (var currentType = t; currentType != null && currentType != typeof(object); currentType = currentType.BaseType)
{
continue;
}
var registered = InitializerPropertyRegistry.GetProperties(currentType);
if (registered == null)
{
continue;
}

merged ??= new List<InitializerPropertyInfo>(registered.Length);
seen ??= new HashSet<string>(StringComparer.Ordinal);
merged ??= new List<InitializerPropertyInfo>(registered.Length);
seen ??= new HashSet<string>(StringComparer.Ordinal);

foreach (var p in registered)
{
if (seen.Add(p.PropertyName))
foreach (var p in registered)
{
merged.Add(p);
if (seen.Add(p.PropertyName))
{
merged.Add(p);
}
}
}
}

var result = merged?.ToArray();
FlattenedInitializerPropertiesCache[type] = result;
return result;
}
return merged?.ToArray();
});

/// <summary>
/// Traverses the pre-flattened source-generated IAsyncInitializer properties.
Expand Down
8 changes: 7 additions & 1 deletion TUnit.Engine/Extensions/TestContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using TUnit.Core;
using System.Diagnostics;
using TUnit.Core;
using TUnit.Core.Enums;
using TUnit.Core.Interfaces;
using TUnit.Engine.Utilities;
Expand Down Expand Up @@ -264,13 +265,15 @@ private static int CountNonNullValues(IDictionary<string, object?> props)
#if NET
public static ITestStartEventReceiver[] GetTestStartReceivers(this TestContext testContext, EventReceiverStage stage)
{
Debug.Assert(testContext.EventReceiversBuilt, "EnsureEventReceiversCached must run before GetTestStartReceivers — caller is on the hot path and skips the guard.");
return stage == EventReceiverStage.Early
? testContext.CachedTestStartReceiversEarly!
: testContext.CachedTestStartReceiversLate!;
}
#else
public static ITestStartEventReceiver[] GetTestStartReceivers(this TestContext testContext)
{
Debug.Assert(testContext.EventReceiversBuilt, "EnsureEventReceiversCached must run before GetTestStartReceivers — caller is on the hot path and skips the guard.");
return testContext.CachedTestStartReceivers!;
}
#endif
Expand All @@ -281,13 +284,15 @@ public static ITestStartEventReceiver[] GetTestStartReceivers(this TestContext t
#if NET
public static ITestEndEventReceiver[] GetTestEndReceivers(this TestContext testContext, EventReceiverStage stage)
{
Debug.Assert(testContext.EventReceiversBuilt, "EnsureEventReceiversCached must run before GetTestEndReceivers — caller is on the hot path and skips the guard.");
return stage == EventReceiverStage.Early
? testContext.CachedTestEndReceiversEarly!
: testContext.CachedTestEndReceiversLate!;
}
#else
public static ITestEndEventReceiver[] GetTestEndReceivers(this TestContext testContext)
{
Debug.Assert(testContext.EventReceiversBuilt, "EnsureEventReceiversCached must run before GetTestEndReceivers — caller is on the hot path and skips the guard.");
return testContext.CachedTestEndReceivers!;
}
#endif
Expand All @@ -297,6 +302,7 @@ public static ITestEndEventReceiver[] GetTestEndReceivers(this TestContext testC
/// </summary>
public static ITestSkippedEventReceiver[] GetTestSkippedReceivers(this TestContext testContext)
{
Debug.Assert(testContext.EventReceiversBuilt, "EnsureEventReceiversCached must run before GetTestSkippedReceivers — caller is on the hot path and skips the guard.");
return testContext.CachedTestSkippedReceivers!;
}

Expand Down
10 changes: 10 additions & 0 deletions TUnit.Engine/Services/TestExecution/TestCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,15 @@ await _eventReceiverOrchestrator.InvokeLastTestInSessionEventReceiversAsync(
/// Timeout is passed through to TestExecutor.ExecuteAsync, which applies it only to the test
/// body — hooks and data source initialization run outside the timeout scope (fixes #4772).
/// </summary>
/// <remarks>
/// Ordering invariant: <see cref="ExecuteTestInternalAsync"/> calls
/// <c>_eventReceiverOrchestrator.RegisterReceivers</c> (which invokes
/// <c>EnsureEventReceiversCached</c>) before entering this method. The hot-path
/// receiver getters used by <c>InvokeTest{Skipped,End}EventReceiversAsync</c> therefore
/// skip their own guards and rely on that upstream call to have populated the caches —
/// the skip-path branch below is safe because it cannot be reached without that
/// prerequisite having run.
/// </remarks>
private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
{
// Check if this test should be skipped before creating the class instance.
Expand All @@ -287,6 +296,7 @@ private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, C
{
_stateManager.MarkSkipped(test, test.Context.SkipReason!);

// Receiver caches are populated upstream (see <remarks> on this method).
await _eventReceiverOrchestrator.InvokeTestSkippedEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);

await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);
Expand Down
Loading