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
6 changes: 4 additions & 2 deletions src/TUnit.Engine/Models/GroupedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ internal record GroupedTests
public required AbstractExecutableTest[] NotInParallel { get; init; }

// Array of tests with their constraint keys - no duplication
// Tests are pre-sorted by priority
// Tests are pre-sorted by priority. Priority is a dense rank (0 runs first) derived from
// TestPriority's ordering, not a hash, so ConstraintKeyScheduler can sort on it directly.
public required (AbstractExecutableTest Test, IReadOnlyList<string> ConstraintKeys, int Priority)[] KeyedNotInParallel { get; init; }

// Array of groups with nested arrays for maximum iteration performance
Expand All @@ -36,7 +37,8 @@ internal record GroupedConstrainedTests

/// <summary>
/// Tests that have both ParallelGroup and NotInParallel constraints
/// These must respect their constraint keys even within the parallel group
/// These must respect their constraint keys even within the parallel group.
/// Priority is a dense rank (0 runs first), as for <see cref="GroupedTests.KeyedNotInParallel"/>.
/// </summary>
public required (AbstractExecutableTest Test, IReadOnlyList<string> ConstraintKeys, int Priority)[] KeyedTests { get; init; }
}
51 changes: 29 additions & 22 deletions src/TUnit.Engine/Services/TestGroupingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,22 +178,21 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable<AbstractExec
sortedNotInParallel[i] = notInParallelList[i].Test;
}

keyedNotInParallelList.Sort((a, b) =>
{
var classCompare = string.CompareOrdinal(a.ClassName, b.ClassName);
if (classCompare != 0) return classCompare;

var priorityCompare = b.Priority.Priority.CompareTo(a.Priority.Priority);
if (priorityCompare != 0) return priorityCompare;

return a.Priority.Order.CompareTo(b.Priority.Order);
});
// Keyed tests are handed to ConstraintKeyScheduler with an int sort key. It must be a rank
// derived from TestPriority's ordering: a hash code does not preserve that ordering (#7088).
keyedNotInParallelList.Sort(static (a, b) => CompareKeyed(a.Priority, a.ClassName, b.Priority, b.ClassName));

var keyedArrays = new (AbstractExecutableTest, IReadOnlyList<string>, int)[keyedNotInParallelList.Count];
var keyedRank = 0;
for (int i = 0; i < keyedNotInParallelList.Count; i++)
{
var item = keyedNotInParallelList[i];
keyedArrays[i] = (item.Test, item.ConstraintKeys, item.Priority.GetHashCode());
if (i > 0 && item.Priority != keyedNotInParallelList[i - 1].Priority)
{
keyedRank++;
}

keyedArrays[i] = (item.Test, item.ConstraintKeys, keyedRank);
}

// Convert constrained parallel groups to the final format
Expand All @@ -204,22 +203,19 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable<AbstractExec
var unconstrained = kvp.Value.Unconstrained;
var keyed = kvp.Value.Keyed;

keyed.Sort((a, b) =>
{
var classCompare = string.CompareOrdinal(a.Item2, b.Item2);
if (classCompare != 0) return classCompare;

var priorityCompare = b.Item4.Priority.CompareTo(a.Item4.Priority);
if (priorityCompare != 0) return priorityCompare;

return a.Item4.Order.CompareTo(b.Item4.Order);
});
keyed.Sort(static (a, b) => CompareKeyed(a.Item4, a.Item2, b.Item4, b.Item2));

var sortedKeyed = new (AbstractExecutableTest, IReadOnlyList<string>, int)[keyed.Count];
var groupRank = 0;
for (int i = 0; i < keyed.Count; i++)
{
var item = keyed[i];
sortedKeyed[i] = (item.Item1, item.Item3, item.Item4.GetHashCode());
if (i > 0 && item.Item4 != keyed[i - 1].Item4)
{
groupRank++;
}

sortedKeyed[i] = (item.Item1, item.Item3, groupRank);
}

finalConstrainedGroups[groupName] = new GroupedConstrainedTests
Expand Down Expand Up @@ -252,6 +248,17 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable<AbstractExec
return result;
}

/// <summary>
/// Orders keyed NotInParallel tests by <see cref="TestPriority"/> (highest priority first, then
/// ascending Order), with class name as a deterministic tie-break. Tests that compare equal on
/// priority share a scheduler rank, so ConstraintKeyScheduler's stable sort keeps this order.
/// </summary>
private static int CompareKeyed(TestPriority a, string aClassName, TestPriority b, string bClassName)
{
var priorityCompare = a.CompareTo(b);
return priorityCompare != 0 ? priorityCompare : string.CompareOrdinal(aClassName, bClassName);
}

private static void ProcessNotInParallelConstraint(
AbstractExecutableTest test,
string className,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Shouldly;
using TUnit.Engine.Tests.Enums;

namespace TUnit.Engine.Tests.Scheduling;

/// <summary>
/// Regression for https://github.com/thomhurst/TUnit/issues/7088.
/// Keyed [NotInParallel] tests must start in [ExecutionPriority] order, then ascending Order.
/// Each test in the test project fails if it starts out of order.
/// </summary>
public class KeyedNotInParallelOrderTests(TestMode testMode) : InvokableTestBase(testMode)
{
[Test]
[Arguments("KeyedNotInParallelOrderTests")]
[Arguments("GroupedKeyedNotInParallelOrderTests")]
public async Task KeyedNotInParallel_RunsInPriorityThenOrder(string className)
{
await RunTestsWithFilter($"/*/TUnit.TestProject.Bugs._7088/{className}/*",
[
result => result.ResultSummary.Outcome.ShouldBe("Completed"),
result => result.ResultSummary.Counters.Total.ShouldBe(6),
result => result.ResultSummary.Counters.Passed.ShouldBe(6),
result => result.ResultSummary.Counters.Failed.ShouldBe(0),
]);
}
}
96 changes: 96 additions & 0 deletions tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
using TUnit.Core.Enums;
using TUnit.TestProject.Attributes;

namespace TUnit.TestProject.Bugs._7088;

/// <summary>
/// Regression for https://github.com/thomhurst/TUnit/issues/7088.
/// Keyed [NotInParallel] tests must start in [ExecutionPriority] order (highest first),
/// then in ascending NotInParallel Order. Each test checks that the tests that started
/// before it are exactly the expected prefix, so a test that starts out of order fails.
/// </summary>
internal sealed class StartOrderProbe(params string[] expectedOrder)
{
private readonly List<string> _started = [];
private readonly object _lock = new();

public async Task RecordAndVerifyAsync()
{
var testName = TestContext.Current!.Metadata.TestDetails.TestName;

string[] startedSoFar;
lock (_lock)
{
_started.Add(testName);
startedSoFar = [.. _started];
}

await Assert.That(string.Join(", ", startedSoFar))
.IsEqualTo(string.Join(", ", expectedOrder.Take(startedSoFar.Length)));
}
}

[EngineTest(ExpectedResult.Pass)]
public class KeyedNotInParallelOrderTests
{
private static readonly StartOrderProbe Probe = new(
nameof(HighPriority_Order50),
nameof(Order1),
nameof(Order2),
nameof(Order3),
nameof(Order1000000000),
nameof(DefaultOrder));

[Test, NotInParallel("KeyedNotInParallelOrder", Order = 3)]
public Task Order3() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("KeyedNotInParallelOrder")]
public Task DefaultOrder() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("KeyedNotInParallelOrder", Order = 1_000_000_000)]
public Task Order1000000000() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("KeyedNotInParallelOrder", Order = 1)]
public Task Order1() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("KeyedNotInParallelOrder", Order = 50), ExecutionPriority(Priority.High)]
public Task HighPriority_Order50() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("KeyedNotInParallelOrder", Order = 2)]
public Task Order2() => Probe.RecordAndVerifyAsync();
}

/// <summary>
/// Same as <see cref="KeyedNotInParallelOrderTests"/>, but inside a [ParallelGroup], which
/// routes the keyed tests through the constrained-parallel-group path.
/// </summary>
[EngineTest(ExpectedResult.Pass)]
[ParallelGroup("KeyedNotInParallelOrder.Group")]
public class GroupedKeyedNotInParallelOrderTests
{
private static readonly StartOrderProbe Probe = new(
nameof(HighPriority_Order50),
nameof(Order1),
nameof(Order2),
nameof(Order3),
nameof(Order1000000000),
nameof(DefaultOrder));

[Test, NotInParallel("GroupedKeyedNotInParallelOrder", Order = 3)]
public Task Order3() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("GroupedKeyedNotInParallelOrder")]
public Task DefaultOrder() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("GroupedKeyedNotInParallelOrder", Order = 1_000_000_000)]
public Task Order1000000000() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("GroupedKeyedNotInParallelOrder", Order = 1)]
public Task Order1() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("GroupedKeyedNotInParallelOrder", Order = 50), ExecutionPriority(Priority.High)]
public Task HighPriority_Order50() => Probe.RecordAndVerifyAsync();

[Test, NotInParallel("GroupedKeyedNotInParallelOrder", Order = 2)]
public Task Order2() => Probe.RecordAndVerifyAsync();
}
Loading