diff --git a/src/TUnit.Engine/Models/GroupedTests.cs b/src/TUnit.Engine/Models/GroupedTests.cs index a2eb24869cb..8dc631530b3 100644 --- a/src/TUnit.Engine/Models/GroupedTests.cs +++ b/src/TUnit.Engine/Models/GroupedTests.cs @@ -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 ConstraintKeys, int Priority)[] KeyedNotInParallel { get; init; } // Array of groups with nested arrays for maximum iteration performance @@ -36,7 +37,8 @@ internal record GroupedConstrainedTests /// /// 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 . /// public required (AbstractExecutableTest Test, IReadOnlyList ConstraintKeys, int Priority)[] KeyedTests { get; init; } } diff --git a/src/TUnit.Engine/Services/TestGroupingService.cs b/src/TUnit.Engine/Services/TestGroupingService.cs index 9fc2519283e..fd9213dddfe 100644 --- a/src/TUnit.Engine/Services/TestGroupingService.cs +++ b/src/TUnit.Engine/Services/TestGroupingService.cs @@ -178,22 +178,21 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable - { - 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, 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 @@ -204,22 +203,19 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable - { - 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, 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 @@ -252,6 +248,17 @@ private static GroupedTests GroupTestsByConstraintsCore(IEnumerable + /// Orders keyed NotInParallel tests by (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. + /// + 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, diff --git a/tests/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelOrderTests.cs b/tests/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelOrderTests.cs new file mode 100644 index 00000000000..fd464d2eeef --- /dev/null +++ b/tests/TUnit.Engine.Tests/Scheduling/KeyedNotInParallelOrderTests.cs @@ -0,0 +1,26 @@ +using Shouldly; +using TUnit.Engine.Tests.Enums; + +namespace TUnit.Engine.Tests.Scheduling; + +/// +/// 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. +/// +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), + ]); + } +} diff --git a/tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs b/tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs new file mode 100644 index 00000000000..b8c7fd92eda --- /dev/null +++ b/tests/TUnit.TestProject/Bugs/7088/KeyedNotInParallelOrderTests.cs @@ -0,0 +1,96 @@ +using TUnit.Core.Enums; +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._7088; + +/// +/// 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. +/// +internal sealed class StartOrderProbe(params string[] expectedOrder) +{ + private readonly List _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(); +} + +/// +/// Same as , but inside a [ParallelGroup], which +/// routes the keyed tests through the constrained-parallel-group path. +/// +[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(); +}