From 45af8dcbd6bd5e5ed4d299aff0a93140eb42c377 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 14 Apr 2026 01:07:48 +0100 Subject: [PATCH] fix: start session activity before discovery so discovery spans parent correctly (#5244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "test discovery" span was orphaned because the session activity didn't exist yet during discovery — it was only created after Before(TestSession) hooks, which run later. Extract session activity creation into an idempotent TryStartSessionActivity() and call it after Before(TestDiscovery) hooks (for execution requests) so users who set up their TracerProvider early get a single unified trace. --- TUnit.Engine/Services/HookExecutor.cs | 49 ++- TUnit.Engine/TestDiscoveryService.cs | 8 + TUnit.Engine/TestExecutor.cs | 5 + .../SessionActivityLifecycleTests.cs | 405 ++++++++++++++++++ 4 files changed, 450 insertions(+), 17 deletions(-) create mode 100644 TUnit.UnitTests/SessionActivityLifecycleTests.cs diff --git a/TUnit.Engine/Services/HookExecutor.cs b/TUnit.Engine/Services/HookExecutor.cs index dc90566995..a18801bca8 100644 --- a/TUnit.Engine/Services/HookExecutor.cs +++ b/TUnit.Engine/Services/HookExecutor.cs @@ -59,24 +59,13 @@ public async ValueTask ExecuteBeforeTestSessionHooksAsync(CancellationToken canc } } - // Start the session activity AFTER hooks have run, because user hooks - // typically set up the TracerProvider / ActivityListener. If we started - // the activity before hooks, the ActivitySource would have no listeners - // and StartActivity would return null - producing no root span. + // Try to start the session activity now. When the user sets up their + // TracerProvider in Before(TestSession), this is the first opportunity + // where HasListeners() returns true. When they set it up earlier (e.g. + // in Before(TestDiscovery)), the activity was already started by + // TryStartSessionActivity() before discovery — this call is a no-op. #if NET - var sessionContext = _contextProvider.TestSessionContext; - - if (TUnitActivitySource.Source.HasListeners()) - { - sessionContext.Activity = TUnitActivitySource.StartActivity( - TUnitActivitySource.SpanTestSession, - System.Diagnostics.ActivityKind.Internal, - default, - [ - new("tunit.session.id", sessionContext.Id), - new("tunit.filter", sessionContext.TestFilter) - ]); - } + TryStartSessionActivity(); #endif } @@ -122,6 +111,32 @@ public async ValueTask> ExecuteAfterTestSessionHooksAsync(Cancel } #if NET + /// + /// Lazily starts the session activity once an ActivityListener is registered, + /// so discovery and execution spans can parent under it. + /// + internal void TryStartSessionActivity() + { + var sessionContext = _contextProvider.TestSessionContext; + + if (sessionContext.Activity is not null) + { + return; + } + + if (TUnitActivitySource.Source.HasListeners()) + { + sessionContext.Activity = TUnitActivitySource.StartActivity( + TUnitActivitySource.SpanTestSession, + System.Diagnostics.ActivityKind.Internal, + default, + [ + new("tunit.session.id", sessionContext.Id), + new("tunit.filter", sessionContext.TestFilter) + ]); + } + } + private void FinishSessionActivity(bool hasErrors) { var sessionContext = _contextProvider.TestSessionContext; diff --git a/TUnit.Engine/TestDiscoveryService.cs b/TUnit.Engine/TestDiscoveryService.cs index 758911f759..c6221b1181 100644 --- a/TUnit.Engine/TestDiscoveryService.cs +++ b/TUnit.Engine/TestDiscoveryService.cs @@ -57,6 +57,14 @@ public async Task DiscoverTests(string testSessionId, ITest { await _testExecutor.ExecuteBeforeTestDiscoveryHooksAsync(cancellationToken).ConfigureAwait(false); +#if NET + // Start session activity early so discovery spans can parent under it. + if (isForExecution) + { + _testExecutor.TryStartSessionActivity(); + } +#endif + var contextProvider = _testExecutor.GetContextProvider(); contextProvider.BeforeTestDiscoveryContext.RestoreExecutionContext(); diff --git a/TUnit.Engine/TestExecutor.cs b/TUnit.Engine/TestExecutor.cs index 3030f9514a..aac6a56144 100644 --- a/TUnit.Engine/TestExecutor.cs +++ b/TUnit.Engine/TestExecutor.cs @@ -411,6 +411,11 @@ public ValueTask ExecuteAfterTestDiscoveryHooksAsync(CancellationToken cancellat return _hookExecutor.ExecuteAfterTestDiscoveryHooksAsync(cancellationToken); } +#if NET + /// + internal void TryStartSessionActivity() => _hookExecutor.TryStartSessionActivity(); +#endif + /// /// Get the context provider for accessing test contexts. /// diff --git a/TUnit.UnitTests/SessionActivityLifecycleTests.cs b/TUnit.UnitTests/SessionActivityLifecycleTests.cs new file mode 100644 index 0000000000..9c761abec7 --- /dev/null +++ b/TUnit.UnitTests/SessionActivityLifecycleTests.cs @@ -0,0 +1,405 @@ +#if NET + +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using TUnit.Assertions.Extensions; +using TUnit.Core; +using TUnit.Core.Services; +using TUnit.Engine; +using TUnit.Engine.Interfaces; +using TUnit.Engine.Services; + +namespace TUnit.UnitTests; + +/// +/// Tests for the session activity lifecycle, verifying that the "test session" +/// activity is created at the right time so discovery and execution spans +/// can parent under it — fixing orphaned root spans in traces. +/// See https://github.com/thomhurst/TUnit/issues/5244 +/// +public class SessionActivityLifecycleTests +{ + /// + /// Creates a minimal with stubbed dependencies. + /// Only is used by + /// ; the rest are + /// no-op stubs that will never be called. + /// + private static (HookExecutor Executor, TestSessionContext SessionContext) CreateHookExecutor( + string? testFilter = null) + { + var beforeDiscovery = new BeforeTestDiscoveryContext { TestFilter = testFilter }; + var discoveryContext = new TestDiscoveryContext(beforeDiscovery) { TestFilter = testFilter }; + var sessionContext = new TestSessionContext(discoveryContext) + { + Id = Guid.NewGuid().ToString(), + TestFilter = testFilter + }; + + var contextProvider = new StubContextProvider(sessionContext); + var hookDelegateBuilder = new StubHookDelegateBuilder(); + + // EventReceiverOrchestrator is sealed and not used by TryStartSessionActivity, + // so we pass null — the executor will never call it in these tests. + var executor = new HookExecutor(hookDelegateBuilder, contextProvider, null!); + + return (executor, sessionContext); + } + + [Test] + public async Task TryStartSessionActivity_WithListeners_CreatesSessionActivity() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + + await Assert.That(sessionContext.Activity).IsNotNull(); + await Assert.That(sessionContext.Activity!.OperationName).IsEqualTo("test session"); + await Assert.That(sessionContext.Activity.Kind).IsEqualTo(ActivityKind.Internal); + } + + [Test] + public async Task TryStartSessionActivity_CreatesActivityWithCorrectSource() + { + var (executor, sessionContext) = CreateHookExecutor(); + + // TUnit's own HTML reporter listener is active during test execution, + // so HasListeners() is always true here. + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + + await Assert.That(sessionContext.Activity).IsNotNull(); + await Assert.That(sessionContext.Activity!.Source.Name).IsEqualTo("TUnit"); + } + + [Test] + public async Task TryStartSessionActivity_CalledTwice_IsIdempotent() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var firstActivity = sessionContext.Activity; + + executor.TryStartSessionActivity(); + var secondActivity = sessionContext.Activity; + + await Assert.That(firstActivity).IsNotNull(); + await Assert.That(secondActivity).IsSameReferenceAs(firstActivity!); + } + + [Test] + public async Task TryStartSessionActivity_SetsSessionIdTag() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + + var tags = sessionContext.Activity!.Tags.ToList(); + await Assert.That(tags).Contains( + new KeyValuePair("tunit.session.id", sessionContext.Id)); + } + + [Test] + public async Task TryStartSessionActivity_SetsTestFilterTag() + { + var (executor, sessionContext) = CreateHookExecutor(testFilter: "/*/*/MyClass/*"); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + + var tags = sessionContext.Activity!.Tags.ToList(); + await Assert.That(tags).Contains( + new KeyValuePair("tunit.filter", "/*/*/MyClass/*")); + } + + [Test] + public async Task DiscoveryAndAssembly_ShareSameTrace_WhenBothParentedUnderSession() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var sessionActivity = sessionContext.Activity!; + + // Discovery and assembly spans both parent under the session activity + using var discoveryActivity = TUnitActivitySource.StartActivity( + "test discovery", + ActivityKind.Internal, + sessionActivity.Context); + + using var assemblyActivity = TUnitActivitySource.StartActivity( + TUnitActivitySource.SpanTestAssembly, + ActivityKind.Internal, + sessionActivity.Context, + [new("tunit.assembly.name", "TestAssembly")]); + + // Both should be in the same trace (single unified trace — the fix for #5244) + await Assert.That(discoveryActivity).IsNotNull(); + await Assert.That(assemblyActivity).IsNotNull(); + await Assert.That(discoveryActivity!.TraceId).IsEqualTo(sessionActivity.TraceId); + await Assert.That(assemblyActivity!.TraceId).IsEqualTo(sessionActivity.TraceId); + + // Both should be direct children of the session + await Assert.That(discoveryActivity.ParentSpanId).IsEqualTo(sessionActivity.SpanId); + await Assert.That(assemblyActivity.ParentSpanId).IsEqualTo(sessionActivity.SpanId); + } + + [Test] + public async Task FullSpanHierarchy_SessionParentsAllChildren() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var sessionActivity = sessionContext.Activity!; + + // Discovery span + var discoveryActivity = TUnitActivitySource.StartActivity( + "test discovery", + ActivityKind.Internal, + sessionActivity.Context); + + // Stop discovery before execution (mirrors real flow) + TUnitActivitySource.StopActivity(discoveryActivity); + + // Assembly span + var assemblyActivity = TUnitActivitySource.StartActivity( + TUnitActivitySource.SpanTestAssembly, + ActivityKind.Internal, + sessionActivity.Context, + [new("tunit.assembly.name", "TestAssembly")]); + + // Class span + var classActivity = TUnitActivitySource.StartActivity( + TUnitActivitySource.SpanTestSuite, + ActivityKind.Internal, + assemblyActivity?.Context ?? default, + [new("test.suite.name", "TestClass")]); + + // Test span + var testActivity = TUnitActivitySource.StartActivity( + TUnitActivitySource.SpanTestCase, + ActivityKind.Internal, + classActivity?.Context ?? default, + [new("test.case.name", "TestMethod")]); + + // Verify hierarchy: session → discovery + await Assert.That(discoveryActivity).IsNotNull(); + await Assert.That(discoveryActivity!.ParentId).IsEqualTo(sessionActivity.Id); + + // Verify hierarchy: session → assembly → class → test + await Assert.That(assemblyActivity).IsNotNull(); + await Assert.That(assemblyActivity!.ParentId).IsEqualTo(sessionActivity.Id); + + await Assert.That(classActivity).IsNotNull(); + await Assert.That(classActivity!.ParentId).IsEqualTo(assemblyActivity.Id); + + await Assert.That(testActivity).IsNotNull(); + await Assert.That(testActivity!.ParentId).IsEqualTo(classActivity.Id); + + // Cleanup + TUnitActivitySource.StopActivity(testActivity); + TUnitActivitySource.StopActivity(classActivity); + TUnitActivitySource.StopActivity(assemblyActivity); + } + + [Test] + public async Task DiscoverySpan_InSameTrace_WhenParentedUnderSession() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var sessionActivity = sessionContext.Activity!; + + // Create a discovery span parented under session + using var discoveryActivity = TUnitActivitySource.StartActivity( + "test discovery", + ActivityKind.Internal, + sessionActivity.Context); + + // Both spans should share the same trace ID + await Assert.That(discoveryActivity).IsNotNull(); + await Assert.That(discoveryActivity!.TraceId).IsEqualTo(sessionActivity.TraceId); + await Assert.That(discoveryActivity.ParentSpanId).IsEqualTo(sessionActivity.SpanId); + } + + [Test] + public async Task SessionActivity_StoppedAndCleared_AfterFinish() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var activity = sessionContext.Activity!; + + // Verify activity is running + await Assert.That(activity.IsStopped).IsFalse(); + + // Stop via the same pattern used in ExecuteAfterTestSessionHooksAsync + TUnitActivitySource.StopActivity(activity); + sessionContext.Activity = null; + + await Assert.That(activity.IsStopped).IsTrue(); + await Assert.That(sessionContext.Activity).IsNull(); + } + + [Test] + public async Task HookSpan_ParentsUnderSession_WhenSessionActivityExists() + { + var (executor, sessionContext) = CreateHookExecutor(); + + using var scope = new ActivityListenerScope(); + + executor.TryStartSessionActivity(); + var sessionActivity = sessionContext.Activity!; + + // Simulate a Before(TestSession) hook span — these use context.Activity as parent + using var hookActivity = TUnitActivitySource.StartActivity( + "BeforeTestSession: SetupDatabase", + ActivityKind.Internal, + sessionActivity.Context); + + await Assert.That(hookActivity).IsNotNull(); + await Assert.That(hookActivity!.ParentId).IsEqualTo(sessionActivity.Id); + } + + #region Stubs + + /// + /// Minimal stub that returns a fixed + /// . Other members throw if called. + /// + private sealed class StubContextProvider(TestSessionContext sessionContext) : IContextProvider + { + public BeforeTestDiscoveryContext BeforeTestDiscoveryContext => + throw new NotSupportedException(); + + public TestDiscoveryContext TestDiscoveryContext => + throw new NotSupportedException(); + + public TestSessionContext TestSessionContext => sessionContext; + + public AssemblyHookContext GetOrCreateAssemblyContext(Assembly assembly) => + throw new NotSupportedException(); + + public ClassHookContext GetOrCreateClassContext( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | + DynamicallyAccessedMemberTypes.PublicProperties | + DynamicallyAccessedMemberTypes.PublicMethods)] + Type classType) => + throw new NotSupportedException(); + + public TestContext CreateTestContext( + string testName, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | + DynamicallyAccessedMemberTypes.PublicProperties | + DynamicallyAccessedMemberTypes.PublicMethods)] + Type classType, + TestBuilderContext testBuilderContext, + CancellationToken cancellationToken) => + throw new NotSupportedException(); + } + + /// + /// No-op stub. All collection methods + /// return empty lists — no hooks are registered in these unit tests. + /// + private sealed class StubHookDelegateBuilder : IHookDelegateBuilder + { + public ValueTask InitializeAsync() => default; + + public ValueTask>> CollectBeforeTestHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectAfterTestHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectBeforeEveryTestHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectAfterEveryTestHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectBeforeClassHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectAfterClassHooksAsync(Type testClassType) => + new([]); + public ValueTask>> CollectBeforeEveryClassHooksAsync() => + new([]); + public ValueTask>> CollectAfterEveryClassHooksAsync() => + new([]); + public ValueTask>> CollectBeforeAssemblyHooksAsync(Assembly assembly) => + new([]); + public ValueTask>> CollectAfterAssemblyHooksAsync(Assembly assembly) => + new([]); + public ValueTask>> CollectBeforeEveryAssemblyHooksAsync() => + new([]); + public ValueTask>> CollectAfterEveryAssemblyHooksAsync() => + new([]); + public ValueTask>> CollectBeforeTestSessionHooksAsync() => + new([]); + public ValueTask>> CollectAfterTestSessionHooksAsync() => + new([]); + public ValueTask>> CollectBeforeTestDiscoveryHooksAsync() => + new([]); + public ValueTask>> CollectAfterTestDiscoveryHooksAsync() => + new([]); + } + + #endregion + + #region Activity listener helper + + /// + /// Manages an scoped to a test, ensuring + /// cleanup even if the test fails. + /// + private sealed class ActivityListenerScope : IDisposable + { + private readonly ActivityListener _listener; + private readonly ConcurrentBag _activities = []; + + public ActivityListenerScope() + { + _listener = new ActivityListener + { + ShouldListenTo = static source => source.Name == "TUnit", + Sample = static (ref ActivityCreationOptions _) => + ActivitySamplingResult.AllDataAndRecorded, + ActivityStarted = activity => _activities.Add(activity), + }; + + ActivitySource.AddActivityListener(_listener); + } + + public void Dispose() + { + // Stop any activities that are still running to prevent leaks + foreach (var activity in _activities) + { + if (!activity.IsStopped) + { + activity.Stop(); + activity.Dispose(); + } + } + + _listener.Dispose(); + } + } + + #endregion +} + +#endif