diff --git a/TUnit.Core/AbstractExecutableTest.cs b/TUnit.Core/AbstractExecutableTest.cs index cb467d1c12..232115a618 100644 --- a/TUnit.Core/AbstractExecutableTest.cs +++ b/TUnit.Core/AbstractExecutableTest.cs @@ -30,6 +30,11 @@ public abstract class AbstractExecutableTest // avoiding a per-test scan over ParallelConstraints in the hot path. internal bool RequiresGlobalNotInParallelLock { get; set; } + // Set by the scheduler for tests that either have dependencies or are the target of + // another test's dependency. These tests can be reached from both the scheduler and + // dependency recursion, so TestRunner must keep using its execution dedup ledger. + internal bool RequiresExecutionDedup { get; set; } + public required TestContext Context { get; diff --git a/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs b/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs index e3a3788e4b..d845cb82b6 100644 --- a/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs +++ b/TUnit.Engine/Scheduling/ConstraintKeyScheduler.cs @@ -145,7 +145,7 @@ private async Task ExecuteTestAndReleaseKeysAsync( { try { - await _testRunner.ExecuteTestAsync(test, cancellationToken).ConfigureAwait(false); + await _testRunner.ExecuteTestWithoutExecutionTaskAsync(test, cancellationToken).ConfigureAwait(false); } finally { diff --git a/TUnit.Engine/Scheduling/TestRunner.cs b/TUnit.Engine/Scheduling/TestRunner.cs index 0ddbf8e632..09980ddaee 100644 --- a/TUnit.Engine/Scheduling/TestRunner.cs +++ b/TUnit.Engine/Scheduling/TestRunner.cs @@ -42,19 +42,48 @@ internal TestRunner( _notInParallelLock = notInParallelLock; } - // Dedup ledger for re-entrant ExecuteTestAsync calls (dependency recursion, scheduler races). - // Entries are intentionally retained for the session: a late dependency lookup must still - // observe the in-flight or completed TCS. Session-scoped lifetime bounds growth to O(test count). + // Dedup ledger for tests involved in dependency relationships. Entries are intentionally + // retained for the session: a late dependency lookup must still observe the in-flight or + // completed TCS. Session-scoped lifetime bounds growth to O(test count). private readonly ConcurrentDictionary> _executingTests = new(); private Exception? _firstFailFastException; + internal int ExecutingTestsCount => _executingTests.Count; + public ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken) + { + if (!test.RequiresExecutionDedup) + { + if (test.ExecutionTask is { } executionTask) + { + return new ValueTask(executionTask); + } + + return ExecuteTestInternalAsync(test, cancellationToken); + } + + return ExecuteTestWithDependenciesAsync(test, cancellationToken); + } + + internal ValueTask ExecuteTestWithoutExecutionTaskAsync(AbstractExecutableTest test, CancellationToken cancellationToken) + { + if (!test.RequiresExecutionDedup) + { + return ExecuteTestInternalAsync(test, cancellationToken); + } + + return ExecuteTestWithDependenciesAsync(test, cancellationToken); + } + + private ValueTask ExecuteTestWithDependenciesAsync(AbstractExecutableTest test, CancellationToken cancellationToken) { if (_executingTests.TryGetValue(test.TestId, out var existingTcs)) { return new ValueTask(existingTcs.Task); } - var tcs = new TaskCompletionSource(); + // Continuations may re-enter scheduling paths; keep them off the thread + // that publishes ledger completion. + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); existingTcs = _executingTests.GetOrAdd(test.TestId, tcs); if (existingTcs != tcs) diff --git a/TUnit.Engine/Scheduling/TestScheduler.cs b/TUnit.Engine/Scheduling/TestScheduler.cs index 0c855235ff..6376421290 100644 --- a/TUnit.Engine/Scheduling/TestScheduler.cs +++ b/TUnit.Engine/Scheduling/TestScheduler.cs @@ -144,6 +144,8 @@ public async Task ScheduleAndExecuteAsync( // Group tests by their parallel constraints var groupedTests = await _groupingService.GroupTestsByConstraintsAsync(executableTests).ConfigureAwait(false); + MarkDependencyRelatedTestsForExecutionDedup(executableTests); + // Suites with no global [NotInParallel] tests skip the runtime exclusion // lock entirely. Once enabled, the flag is monotonic — dynamic batches // that introduce NIP later (see ExecuteDynamicBatchAsync) keep it on. @@ -317,10 +319,54 @@ private async Task ExecuteDynamicBatchAsync(List dynamic // tests with [NotInParallel(key)], [ParallelGroup(...)] etc. honour their constraints // instead of being silently dropped. var groupedDynamicTests = await _groupingService.GroupTestsByConstraintsAsync(dynamicTests.ToArray()).ConfigureAwait(false); + MarkDependencyRelatedTestsForExecutionDedup(dynamicTests); MarkGlobalNotInParallelTests(groupedDynamicTests); await ExecuteAllPhasesAsync(groupedDynamicTests, cancellationToken).ConfigureAwait(false); } + internal static void MarkDependencyRelatedTestsForExecutionDedup(IEnumerable tests) + { + HashSet? visitedDependencyTargets = null; + Stack? pendingDependencyTargets = null; + + foreach (var test in tests) + { + if (test.Dependencies.Length == 0) + { + continue; + } + + test.RequiresExecutionDedup = true; + + visitedDependencyTargets ??= []; + // Shared across roots so a common dependency target is marked once even + // when multiple scheduled tests reference it. + pendingDependencyTargets ??= []; + + foreach (var dependency in test.Dependencies) + { + pendingDependencyTargets.Push(dependency.Test); + } + + while (pendingDependencyTargets.Count > 0) + { + var dependencyTarget = pendingDependencyTargets.Pop(); + + if (!visitedDependencyTargets.Add(dependencyTarget)) + { + continue; + } + + dependencyTarget.RequiresExecutionDedup = true; + + foreach (var nestedDependency in dependencyTarget.Dependencies) + { + pendingDependencyTargets.Push(nestedDependency.Test); + } + } + } + } + private void MarkGlobalNotInParallelTests(GroupedTests grouped) { if (grouped.NotInParallel.Length == 0) @@ -362,8 +408,7 @@ private async Task ExecuteSequentiallyAsync( { foreach (var test in tests) { - test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken).AsTask(); - await test.ExecutionTask.ConfigureAwait(false); + await ExecuteScheduledTestAsync(test, cancellationToken).ConfigureAwait(false); } } @@ -381,8 +426,7 @@ private Task ExecuteWithGlobalLimitAsync( }, async (test, ct) => { - test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, ct).AsTask(); - await test.ExecutionTask.ConfigureAwait(false); + await ExecuteScheduledTestAsync(test, ct).ConfigureAwait(false); } ); } @@ -402,8 +446,7 @@ private async Task ExecuteWithGlobalLimitAsync( await globalSemaphore.WaitAsync(cancellationToken).ConfigureAwait(false); try { - test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken).AsTask(); - await test.ExecutionTask.ConfigureAwait(false); + await ExecuteScheduledTestAsync(test, cancellationToken).ConfigureAwait(false); } finally { @@ -415,6 +458,20 @@ private async Task ExecuteWithGlobalLimitAsync( } #endif + private async ValueTask ExecuteScheduledTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken) + { + if (!test.RequiresExecutionDedup) + { + await _testRunner.ExecuteTestAsync(test, cancellationToken).ConfigureAwait(false); + return; + } + + // Dependency recursion is the re-entrant path. Only those tests reuse the + // scheduler task slot; independent tests may also use it for constraint wrappers. + test.ExecutionTask ??= _testRunner.ExecuteTestAsync(test, cancellationToken).AsTask(); + await test.ExecutionTask.ConfigureAwait(false); + } + // The cancellation token is forwarded only so WaitForTasksWithFailFastHandling can // distinguish a fail-fast cancellation from a normal task fault when both phases run. // Tasks `a` and `b` are already started and carry their own token internally, so the diff --git a/TUnit.UnitTests/TestRunnerTests.cs b/TUnit.UnitTests/TestRunnerTests.cs new file mode 100644 index 0000000000..0f5b05da56 --- /dev/null +++ b/TUnit.UnitTests/TestRunnerTests.cs @@ -0,0 +1,334 @@ +using TUnit.Core; +using TUnit.Engine.Interfaces; +using TUnit.Engine.Scheduling; +using TUnit.Engine.Services.TestExecution; + +namespace TUnit.UnitTests; + +public class TestRunnerTests +{ + [Test] + public async Task ExecuteTestAsync_WithNoDependencies_DoesNotUseExecutingTestsLedger() + { + var runner = CreateRunner(out var coordinator); + var test = CreateTest("no-dependencies"); + + await runner.ExecuteTestAsync(test, CancellationToken.None); + + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(0); + await Assert.That(coordinator.GetCallCount(test)).IsEqualTo(1); + } + + [Test] + public async Task ExecuteTestAsync_WithNoDependenciesAndExistingExecutionTask_AwaitsExistingTask() + { + var runner = CreateRunner(out var coordinator); + var test = CreateTest("existing-execution-task"); + var existingExecution = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + test.ExecutionTask = existingExecution.Task; + + var execution = runner.ExecuteTestAsync(test, CancellationToken.None); + + await Assert.That(execution.IsCompleted).IsFalse(); + + existingExecution.SetResult(); + await execution; + + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(0); + await Assert.That(coordinator.GetCallCount(test)).IsEqualTo(0); + } + + [Test] + public async Task ExecuteTestWithoutExecutionTaskAsync_WithNoDependenciesAndExistingExecutionTask_ExecutesTest() + { + var runner = CreateRunner(out var coordinator); + var test = CreateTest("scheduler-wrapper"); + var existingExecution = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + test.ExecutionTask = existingExecution.Task; + + await runner.ExecuteTestWithoutExecutionTaskAsync(test, CancellationToken.None); + + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(0); + await Assert.That(coordinator.GetCallCount(test)).IsEqualTo(1); + } + + [Test] + public async Task ExecuteTestAsync_WithDependencies_UsesExecutingTestsLedger() + { + var runner = CreateRunner(out _); + var dependency = CreateTest("dependency"); + var test = CreateTest("with-dependencies", [dependency]); + TestScheduler.MarkDependencyRelatedTestsForExecutionDedup([test]); + + await runner.ExecuteTestAsync(test, CancellationToken.None); + + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(2); + } + + [Test] + public async Task ExecuteTestAsync_WithNoDependenciesButDependencyTarget_UsesExecutingTestsLedger() + { + var runner = CreateRunner(out var coordinator); + var test = CreateTest("dependency-target"); + test.RequiresExecutionDedup = true; + + await runner.ExecuteTestAsync(test, CancellationToken.None); + + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(1); + await Assert.That(coordinator.GetCallCount(test)).IsEqualTo(1); + } + + [Test] + public async Task ExecuteTestAsync_WithDependencies_DeduplicatesConcurrentAttempts() + { + var runner = CreateRunner(out var coordinator); + var dependency = CreateTest("dependency"); + var test = CreateTest("with-dependencies", [dependency]); + TestScheduler.MarkDependencyRelatedTestsForExecutionDedup([test]); + var releaseExecution = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + coordinator.SetExecutionTask(test, releaseExecution.Task); + + var first = runner.ExecuteTestAsync(test, CancellationToken.None); + var second = runner.ExecuteTestAsync(test, CancellationToken.None); + + releaseExecution.SetResult(); + await Task.WhenAll(first.AsTask(), second.AsTask()); + + await Assert.That(coordinator.GetCallCount(test)).IsEqualTo(1); + await Assert.That(runner.ExecutingTestsCount).IsEqualTo(2); + } + + [Test] + public async Task MarkDependencyRelatedTestsForExecutionDedup_MarksTransitiveDependencyTargetsOutsideBatch() + { + var leaf = CreateTest("leaf"); + var middle = CreateTest("middle", [leaf]); + var root = CreateTest("root", [middle]); + + TestScheduler.MarkDependencyRelatedTestsForExecutionDedup([root]); + + await Assert.That(root.RequiresExecutionDedup).IsTrue(); + await Assert.That(middle.RequiresExecutionDedup).IsTrue(); + await Assert.That(leaf.RequiresExecutionDedup).IsTrue(); + } + + private static TestRunner CreateRunner(out FakeTestCoordinator coordinator) + { + coordinator = new FakeTestCoordinator(); + + return new TestRunner( + coordinator, + new FakeMessageBus(), + isFailFastEnabled: false, + new CancellationTokenSource(), + logger: null!, + new TestStateManager(), + new ParallelLimitLockProvider(), + new NotInParallelLock()); + } + + private static AbstractExecutableTest CreateTest( + string testId, + AbstractExecutableTest[]? dependencies = null) + { + var metadata = CreateMetadata(testId); + var beforeDiscoveryContext = new BeforeTestDiscoveryContext { TestFilter = null }; + var discoveryContext = new TestDiscoveryContext(beforeDiscoveryContext) { TestFilter = null }; + var sessionContext = new TestSessionContext(discoveryContext) + { + Id = Guid.NewGuid().ToString(), + TestFilter = null + }; + var assemblyContext = new AssemblyHookContext(sessionContext) + { + Assembly = typeof(TestRunnerTests).Assembly + }; + var classContext = new ClassHookContext(assemblyContext) + { + ClassType = typeof(TestRunnerTests) + }; + var builderContext = new TestBuilderContext + { + TestMetadata = metadata.MethodMetadata + }; + var context = new TestContext(testId, new FakeServiceProvider(), classContext, builderContext, CancellationToken.None); + + var test = new StubExecutableTest + { + TestId = testId, + Metadata = metadata, + Arguments = [], + Context = context, + Dependencies = dependencies?.Select(dependency => new ResolvedDependency + { + Test = dependency, + Metadata = TestDependency.FromMethodName(dependency.Metadata.TestMethodName) + }).ToArray() ?? [] + }; + + return test; + } + + private static TestMetadata CreateMetadata(string testId) + { + var classMetadata = new ClassMetadata + { + Type = typeof(TestRunnerTests), + TypeInfo = new ConcreteType(typeof(TestRunnerTests)), + Name = nameof(TestRunnerTests), + Namespace = typeof(TestRunnerTests).Namespace ?? string.Empty, + Assembly = new AssemblyMetadata + { + Name = typeof(TestRunnerTests).Assembly.GetName().Name ?? string.Empty + }, + Parent = null, + Parameters = [], + Properties = [] + }; + + return new TestMetadata + { + TestClassType = typeof(TestRunnerTests), + TestMethodName = testId, + TestName = testId, + FilePath = "Unknown", + LineNumber = 0, + AttributeFactory = () => [], + MethodMetadata = new MethodMetadata + { + Type = typeof(TestRunnerTests), + TypeInfo = new ConcreteType(typeof(TestRunnerTests)), + Name = testId, + GenericTypeCount = 0, + ReturnType = typeof(void), + ReturnTypeInfo = new ConcreteType(typeof(void)), + Parameters = [], + Class = classMetadata + }, + DataSources = [], + ClassDataSources = [], + PropertyDataSources = [] + }; + } + + private sealed class StubExecutableTest : AbstractExecutableTest + { + public override Task CreateInstanceAsync() => Task.FromResult(new object()); + + public override Task InvokeTestAsync(object instance, CancellationToken cancellationToken) + { + _ = instance; + _ = cancellationToken; + return Task.CompletedTask; + } + } + + private sealed class FakeTestCoordinator : ITestCoordinator + { + private readonly Lock _lock = new(); + private readonly Dictionary _callCounts = []; + private readonly Dictionary _executionTasks = []; + + public int GetCallCount(AbstractExecutableTest test) + { + lock (_lock) + { + return _callCounts.GetValueOrDefault(test.TestId); + } + } + + public void SetExecutionTask(AbstractExecutableTest test, Task executionTask) + { + lock (_lock) + { + _executionTasks[test.TestId] = executionTask; + } + } + + public ValueTask ExecuteTestAsync(AbstractExecutableTest test, CancellationToken cancellationToken) + { + _ = cancellationToken; + + Task? executionTask; + lock (_lock) + { + _callCounts[test.TestId] = _callCounts.GetValueOrDefault(test.TestId) + 1; + _executionTasks.TryGetValue(test.TestId, out executionTask); + } + + if (executionTask is not null) + { + return CompleteAfterAsync(test, executionTask); + } + + test.SetResult(TestState.Passed); + return default; + } + + private static async ValueTask CompleteAfterAsync(AbstractExecutableTest test, Task executionTask) + { + await executionTask.ConfigureAwait(false); + test.SetResult(TestState.Passed); + } + } + + private sealed class FakeMessageBus : ITUnitMessageBus + { + public ValueTask Discovered(TestContext testContext) + { + _ = testContext; + return default; + } + + public ValueTask InProgress(TestContext testContext) + { + _ = testContext; + return default; + } + + public ValueTask Passed(TestContext testContext, DateTimeOffset start) + { + _ = testContext; + _ = start; + return default; + } + + public ValueTask Failed(TestContext testContext, Exception exception, DateTimeOffset start) + { + _ = testContext; + _ = exception; + _ = start; + return default; + } + + public ValueTask Skipped(TestContext testContext, string reason) + { + _ = testContext; + _ = reason; + return default; + } + + public ValueTask Cancelled(TestContext testContext, DateTimeOffset start) + { + _ = testContext; + _ = start; + return default; + } + + public ValueTask SessionArtifact(Artifact artifact) + { + _ = artifact; + return default; + } + } + + private sealed class FakeServiceProvider : IServiceProvider + { + public object? GetService(Type serviceType) + { + _ = serviceType; + return null; + } + } +}