Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
118 changes: 81 additions & 37 deletions src/TUnit.Engine/Scheduling/TestScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,42 +92,10 @@ public async Task<bool> ScheduleAndExecuteAsync(
if (_logger.IsDebugEnabled)
await _logger.LogDebugAsync($"Scheduling execution of {testList.Count} tests").ConfigureAwait(false);

var circularDependencies = _circularDependencyDetector.DetectCircularDependencies(testList);

var testsInCircularDependencies = new HashSet<AbstractExecutableTest>();

foreach (var (test, dependencyChain) in circularDependencies)
{
// Format the error message to match the expected format
var simpleNames = new List<string>(dependencyChain.Count);
foreach (var t in dependencyChain)
{
simpleNames.Add($"{t.Metadata.TestClassType.Name}.{t.Metadata.TestMethodName}");
}

var errorMessage = $"DependsOn Conflict: {string.Join(" > ", simpleNames)}";
var exception = new CircularDependencyException(errorMessage);

// Mark all tests in the dependency chain as failed
foreach (var chainTest in dependencyChain)
{
if (testsInCircularDependencies.Add(chainTest))
{
_testStateManager.MarkCircularDependencyFailed(chainTest, exception);
TestSessionContext.Current?.MarkFailure();
await _messageBus.Failed(chainTest.Context, exception, DateTimeOffset.UtcNow).ConfigureAwait(false);
}
}
}

var executableTests = new List<AbstractExecutableTest>(testList.Count);
foreach (var test in testList)
{
if (!testsInCircularDependencies.Contains(test))
{
executableTests.Add(test);
}
}
var hasDependencies = HasDependencies(testList);
var executableTests = hasDependencies
? await RemoveCircularDependenciesAsync(testList, cancellationToken).ConfigureAwait(false)
: testList;

if (executableTests.Count == 0)
{
Expand All @@ -144,7 +112,10 @@ public async Task<bool> ScheduleAndExecuteAsync(
// Group tests by their parallel constraints
var groupedTests = await _groupingService.GroupTestsByConstraintsAsync(executableTests).ConfigureAwait(false);

MarkDependencyRelatedTestsForExecutionDedup(executableTests);
if (hasDependencies)
{
MarkDependencyRelatedTestsForExecutionDedup(executableTests);
}

// Suites with no global [NotInParallel] tests skip the runtime exclusion
// lock entirely. Once enabled, the flag is monotonic — dynamic batches
Expand All @@ -171,6 +142,72 @@ public async Task<bool> ScheduleAndExecuteAsync(
return true;
}

private static bool HasDependencies(List<AbstractExecutableTest> tests)
{
foreach (var test in tests)
{
if (test.Dependencies.Length != 0)
{
return true;
}
}

return false;
}

private async Task<List<AbstractExecutableTest>> RemoveCircularDependenciesAsync(
List<AbstractExecutableTest> tests,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
var circularDependencies = _circularDependencyDetector.DetectCircularDependencies(tests, cancellationToken);
HashSet<AbstractExecutableTest>? testsInCircularDependencies = null;

foreach (var (_, dependencyChain) in circularDependencies)
{
cancellationToken.ThrowIfCancellationRequested();
testsInCircularDependencies ??= [];

var simpleNames = new List<string>(dependencyChain.Count);
foreach (var test in dependencyChain)
{
cancellationToken.ThrowIfCancellationRequested();
simpleNames.Add($"{test.Metadata.TestClassType.Name}.{test.Metadata.TestMethodName}");
}

var exception = new CircularDependencyException(
$"DependsOn Conflict: {string.Join(" > ", simpleNames)}");

foreach (var test in dependencyChain)
{
cancellationToken.ThrowIfCancellationRequested();
if (testsInCircularDependencies.Add(test))
{
_testStateManager.MarkCircularDependencyFailed(test, exception);
TestSessionContext.Current?.MarkFailure();
await _messageBus.Failed(test.Context, exception, DateTimeOffset.UtcNow).ConfigureAwait(false);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
}

if (testsInCircularDependencies is null)
{
return tests;
}

var executableTests = new List<AbstractExecutableTest>(tests.Count - testsInCircularDependencies.Count);
foreach (var test in tests)
{
cancellationToken.ThrowIfCancellationRequested();
if (!testsInCircularDependencies.Contains(test))
{
executableTests.Add(test);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return executableTests;
}

#if NET
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCode("Test execution involves reflection for hooks and initialization")]
#endif
Expand Down Expand Up @@ -357,6 +394,13 @@ internal static void MarkDependencyRelatedTestsForExecutionDedup(IEnumerable<Abs
continue;
}

if (dependencyTarget.State == TestState.Failed &&
dependencyTarget.Result?.Exception is CircularDependencyException)
{
dependencyTarget.ExecutionTask ??= Task.CompletedTask;
continue;
}

dependencyTarget.RequiresExecutionDedup = true;

foreach (var nestedDependency in dependencyTarget.Dependencies)
Expand Down
16 changes: 11 additions & 5 deletions src/TUnit.Engine/Services/CircularDependencyDetector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@ internal sealed class CircularDependencyDetector
/// Detects circular dependencies in the given collection of tests
/// </summary>
/// <param name="tests">Tests to analyze for circular dependencies</param>
/// <param name="cancellationToken">Token used to cancel graph traversal</param>
/// <returns>List of tests with circular dependencies and their dependency chains</returns>
public List<(AbstractExecutableTest Test, List<AbstractExecutableTest> DependencyChain)> DetectCircularDependencies(
IEnumerable<AbstractExecutableTest> tests)
IEnumerable<AbstractExecutableTest> tests,
CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
var testList = tests as IList<AbstractExecutableTest> ?? tests.ToList();
var circularDependencies = new List<(AbstractExecutableTest Test, List<AbstractExecutableTest> DependencyChain)>();
var visitedStates = new Dictionary<string, VisitState>(capacity: testList.Count);
var pathBuffer = new List<AbstractExecutableTest>(4);

foreach (var test in testList)
{
cancellationToken.ThrowIfCancellationRequested();
if (visitedStates.ContainsKey(test.TestId))
{
continue;
Expand All @@ -32,7 +36,7 @@ internal sealed class CircularDependencyDetector
pathBuffer.Clear();

// Typical cycle depth is small (2-5 tests), pre-size to 4
if (HasCycleDfs(test, testList, visitedStates, pathBuffer))
if (HasCycleDfs(test, visitedStates, pathBuffer, cancellationToken))
{
// Found a cycle - add all tests in the cycle to circular dependencies
var cycle = new List<AbstractExecutableTest>(pathBuffer);
Expand All @@ -52,10 +56,11 @@ private enum VisitState

private bool HasCycleDfs(
AbstractExecutableTest test,
IList<AbstractExecutableTest> allTests,
Dictionary<string, VisitState> visitedStates,
List<AbstractExecutableTest> currentPath)
List<AbstractExecutableTest> currentPath,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
if (visitedStates.TryGetValue(test.TestId, out var state))
{
if (state == VisitState.Visiting)
Expand All @@ -78,7 +83,8 @@ private bool HasCycleDfs(
// Check all dependencies
foreach (var dependency in test.Dependencies)
{
if (HasCycleDfs(dependency.Test, allTests, visitedStates, currentPath))
cancellationToken.ThrowIfCancellationRequested();
if (HasCycleDfs(dependency.Test, visitedStates, currentPath, cancellationToken))
{
return true;
}
Expand Down
20 changes: 20 additions & 0 deletions src/TUnit.Engine/Services/TestGroupingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable<AbstractExec
{
var testCount = tests is ICollection<AbstractExecutableTest> collection ? collection.Count : 0;
var testsWithKeys = new List<(AbstractExecutableTest Test, TestSortKey Key)>(testCount > 0 ? testCount : 16);
var hasParallelConstraints = false;
foreach (var test in tests)
{
NotInParallelConstraint? notInParallelConstraint = null;
foreach (var constraint in test.Context.ParallelConstraints)
{
hasParallelConstraints = true;
if (constraint is NotInParallelConstraint nip)
{
notInParallelConstraint = nip;
Expand Down Expand Up @@ -94,6 +96,24 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable<AbstractExec
return a.Key.NotInParallelOrder.CompareTo(b.Key.NotInParallelOrder);
});

if (!hasParallelConstraints && traceMessages is null)
{
var orderedTests = new AbstractExecutableTest[testsWithKeys.Count];
for (var i = 0; i < testsWithKeys.Count; i++)
{
orderedTests[i] = testsWithKeys[i].Test;
}

return new GroupedTests
{
Parallel = orderedTests,
NotInParallel = [],
KeyedNotInParallel = [],
ParallelGroups = [],
ConstrainedParallelGroups = []
};
}

var estimatedCount = testsWithKeys.Count;
var notInParallelList = new List<(AbstractExecutableTest Test, string ClassName, TestPriority Priority)>(estimatedCount / 4);
var keyedNotInParallelList = new List<(AbstractExecutableTest Test, string ClassName, IReadOnlyList<string> ConstraintKeys, TestPriority Priority)>(estimatedCount / 8);
Expand Down
48 changes: 43 additions & 5 deletions tests/TUnit.UnitTests/TestRunnerTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using TUnit.Core;
using TUnit.Core.Exceptions;
using TUnit.Engine.Interfaces;
using TUnit.Engine.Scheduling;
using TUnit.Engine.Services;
using TUnit.Engine.Services.TestExecution;

namespace TUnit.UnitTests;
Expand Down Expand Up @@ -113,6 +115,40 @@ public async Task MarkDependencyRelatedTestsForExecutionDedup_MarksTransitiveDep
await Assert.That(leaf.RequiresExecutionDedup).IsTrue();
}

[Test, Timeout(5_000)]
public async Task ExecuteTestAsync_WithFailedCircularDependency_SkipsDependentWithoutReentry(
CancellationToken cancellationToken)
{
var runner = CreateRunner(out var coordinator);
var firstCycleTest = CreateTest("cycle-a");
var secondCycleTest = CreateTest("cycle-b", [firstCycleTest]);
firstCycleTest.Dependencies = [CreateResolvedDependency(secondCycleTest)];
var dependent = CreateTest("dependent", [firstCycleTest]);
var exception = new CircularDependencyException("cycle");
var stateManager = new TestStateManager();
stateManager.MarkCircularDependencyFailed(firstCycleTest, exception);
stateManager.MarkCircularDependencyFailed(secondCycleTest, exception);
TestScheduler.MarkDependencyRelatedTestsForExecutionDedup([dependent]);

await runner.ExecuteTestAsync(dependent, cancellationToken);

await Assert.That(dependent.State).IsEqualTo(TestState.Skipped);
await Assert.That(coordinator.GetCallCount(firstCycleTest)).IsEqualTo(0);
await Assert.That(coordinator.GetCallCount(secondCycleTest)).IsEqualTo(0);
}

[Test]
public async Task DetectCircularDependencies_WithCancelledToken_Throws()
{
var detector = new CircularDependencyDetector();
var test = CreateTest("test");
using var cancellationTokenSource = new CancellationTokenSource();
cancellationTokenSource.Cancel();

await Assert.That(() => detector.DetectCircularDependencies([test], cancellationTokenSource.Token))
.ThrowsExactly<OperationCanceledException>();
}

private static TestRunner CreateRunner(out FakeTestCoordinator coordinator)
{
coordinator = new FakeTestCoordinator();
Expand Down Expand Up @@ -160,16 +196,18 @@ private static AbstractExecutableTest CreateTest(
Metadata = metadata,
Arguments = [],
Context = context,
Dependencies = dependencies?.Select(dependency => new ResolvedDependency
{
Test = dependency,
Metadata = TestDependency.FromMethodName(dependency.Metadata.TestMethodName)
}).ToArray() ?? []
Dependencies = dependencies?.Select(CreateResolvedDependency).ToArray() ?? []
};

return test;
}

private static ResolvedDependency CreateResolvedDependency(AbstractExecutableTest dependency) => new()
{
Test = dependency,
Metadata = TestDependency.FromMethodName(dependency.Metadata.TestMethodName)
};

private static TestMetadata<TestRunnerTests> CreateMetadata(string testId)
{
var classMetadata = new ClassMetadata
Expand Down
Loading