From f0965fbece06df4b1f2b102c984d8f1ddc13de55 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Mon, 4 Aug 2025 11:46:28 +0100 Subject: [PATCH 1/2] Guarantee before hooks execute bottom-up and after hooks execute top-down --- .../Services/HookCollectionService.cs | 134 +++++++++++----- TUnit.TestProject/HookExecutionOrderTest.cs | 145 ++++++++++++++++++ 2 files changed, 243 insertions(+), 36 deletions(-) create mode 100644 TUnit.TestProject/HookExecutionOrderTest.cs diff --git a/TUnit.Engine/Services/HookCollectionService.cs b/TUnit.Engine/Services/HookCollectionService.cs index b69175238a..30df1fa399 100644 --- a/TUnit.Engine/Services/HookCollectionService.cs +++ b/TUnit.Engine/Services/HookCollectionService.cs @@ -19,17 +19,20 @@ public ValueTask>> Coll { var hooks = _beforeTestHooksCache.GetOrAdd(testClassType, type => { - var allHooks = new List<(int order, Func hook)>(); + var hooksByType = new List<(Type type, List<(int order, Func hook)> hooks)>(); + // Collect hooks for each type in the hierarchy var currentType = type; while (currentType != null) { - if (Sources.BeforeTestHooks.TryGetValue(currentType, out var typeHooks)) + var typeHooks = new List<(int order, Func hook)>(); + + if (Sources.BeforeTestHooks.TryGetValue(currentType, out var sourceHooks)) { - foreach (var hook in typeHooks) + foreach (var hook in sourceHooks) { var hookFunc = CreateInstanceHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } @@ -42,18 +45,31 @@ public ValueTask>> Coll foreach (var hook in openTypeHooks) { var hookFunc = CreateInstanceHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } } + if (typeHooks.Count > 0) + { + hooksByType.Add((currentType, typeHooks)); + } + currentType = currentType.BaseType; } - return allHooks - .OrderBy(h => h.order) - .Select(h => h.hook) - .ToList(); + // For Before hooks: base class hooks run first + // Reverse the list since we collected from derived to base + hooksByType.Reverse(); + + var finalHooks = new List>(); + foreach (var (_, typeHooks) in hooksByType) + { + // Within each type level, sort by Order + finalHooks.AddRange(typeHooks.OrderBy(h => h.order).Select(h => h.hook)); + } + + return finalHooks; }); return new ValueTask>>(hooks); @@ -63,17 +79,20 @@ public ValueTask>> Coll { var hooks = _afterTestHooksCache.GetOrAdd(testClassType, type => { - var allHooks = new List<(int order, Func hook)>(); + var hooksByType = new List<(Type type, List<(int order, Func hook)> hooks)>(); + // Collect hooks for each type in the hierarchy var currentType = type; while (currentType != null) { - if (Sources.AfterTestHooks.TryGetValue(currentType, out var typeHooks)) + var typeHooks = new List<(int order, Func hook)>(); + + if (Sources.AfterTestHooks.TryGetValue(currentType, out var sourceHooks)) { - foreach (var hook in typeHooks) + foreach (var hook in sourceHooks) { var hookFunc = CreateInstanceHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } @@ -86,18 +105,30 @@ public ValueTask>> Coll foreach (var hook in openTypeHooks) { var hookFunc = CreateInstanceHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } } + if (typeHooks.Count > 0) + { + hooksByType.Add((currentType, typeHooks)); + } + currentType = currentType.BaseType; } - return allHooks - .OrderBy(h => h.order) - .Select(h => h.hook) - .ToList(); + // For After hooks: derived class hooks run first + // No need to reverse since we collected from derived to base + + var finalHooks = new List>(); + foreach (var (_, typeHooks) in hooksByType) + { + // Within each type level, sort by Order + finalHooks.AddRange(typeHooks.OrderBy(h => h.order).Select(h => h.hook)); + } + + return finalHooks; }); return new ValueTask>>(hooks); @@ -151,17 +182,20 @@ public ValueTask>> { var hooks = _beforeClassHooksCache.GetOrAdd(testClassType, type => { - var allHooks = new List<(int order, Func hook)>(); + var hooksByType = new List<(Type type, List<(int order, Func hook)> hooks)>(); + // Collect hooks for each type in the hierarchy var currentType = type; while (currentType != null) { - if (Sources.BeforeClassHooks.TryGetValue(currentType, out var typeHooks)) + var typeHooks = new List<(int order, Func hook)>(); + + if (Sources.BeforeClassHooks.TryGetValue(currentType, out var sourceHooks)) { - foreach (var hook in typeHooks) + foreach (var hook in sourceHooks) { var hookFunc = CreateClassHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } @@ -174,18 +208,31 @@ public ValueTask>> foreach (var hook in openTypeHooks) { var hookFunc = CreateClassHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } } + if (typeHooks.Count > 0) + { + hooksByType.Add((currentType, typeHooks)); + } + currentType = currentType.BaseType; } - return allHooks - .OrderBy(h => h.order) - .Select(h => h.hook) - .ToList(); + // For Before hooks: base class hooks run first + // Reverse the list since we collected from derived to base + hooksByType.Reverse(); + + var finalHooks = new List>(); + foreach (var (_, typeHooks) in hooksByType) + { + // Within each type level, sort by Order + finalHooks.AddRange(typeHooks.OrderBy(h => h.order).Select(h => h.hook)); + } + + return finalHooks; }); return new ValueTask>>(hooks); @@ -195,17 +242,20 @@ public ValueTask>> { var hooks = _afterClassHooksCache.GetOrAdd(testClassType, type => { - var allHooks = new List<(int order, Func hook)>(); + var hooksByType = new List<(Type type, List<(int order, Func hook)> hooks)>(); + // Collect hooks for each type in the hierarchy var currentType = type; while (currentType != null) { - if (Sources.AfterClassHooks.TryGetValue(currentType, out var typeHooks)) + var typeHooks = new List<(int order, Func hook)>(); + + if (Sources.AfterClassHooks.TryGetValue(currentType, out var sourceHooks)) { - foreach (var hook in typeHooks) + foreach (var hook in sourceHooks) { var hookFunc = CreateClassHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } @@ -218,18 +268,30 @@ public ValueTask>> foreach (var hook in openTypeHooks) { var hookFunc = CreateClassHookDelegate(hook); - allHooks.Add((hook.Order, hookFunc)); + typeHooks.Add((hook.Order, hookFunc)); } } } + if (typeHooks.Count > 0) + { + hooksByType.Add((currentType, typeHooks)); + } + currentType = currentType.BaseType; } - return allHooks - .OrderBy(h => h.order) - .Select(h => h.hook) - .ToList(); + // For After hooks: derived class hooks run first + // No need to reverse since we collected from derived to base + + var finalHooks = new List>(); + foreach (var (_, typeHooks) in hooksByType) + { + // Within each type level, sort by Order + finalHooks.AddRange(typeHooks.OrderBy(h => h.order).Select(h => h.hook)); + } + + return finalHooks; }); return new ValueTask>>(hooks); diff --git a/TUnit.TestProject/HookExecutionOrderTest.cs b/TUnit.TestProject/HookExecutionOrderTest.cs new file mode 100644 index 0000000000..9ce99a881b --- /dev/null +++ b/TUnit.TestProject/HookExecutionOrderTest.cs @@ -0,0 +1,145 @@ +using System.Collections.Generic; +using TUnit.Core; + +namespace TUnit.TestProject; + +public class HookExecutionOrderTest +{ + private static readonly List ExecutionOrder = new(); + + public class BaseTest + { + [Before(Test, Order = 10)] // High order, but should still run first due to hierarchy + public void BaseBeforeTest() + { + // Clear on the very first before hook to handle multiple test runs + if (ExecutionOrder.Count > 0 && ExecutionOrder[^1].StartsWith("BaseAfterTest")) + { + ExecutionOrder.Clear(); + } + ExecutionOrder.Add("BaseBeforeTest"); + } + + [Before(Test, Order = -5)] // Negative order, should run before BaseBeforeTest at same level + public void BaseBeforeTest2() + { + ExecutionOrder.Add("BaseBeforeTest2"); + } + + [After(Test, Order = -10)] // Negative order, but should still run last due to hierarchy + public void BaseAfterTest() + { + ExecutionOrder.Add("BaseAfterTest"); + } + + [After(Test, Order = 5)] // Higher order, should run after BaseAfterTest at same level + public void BaseAfterTest2() + { + ExecutionOrder.Add("BaseAfterTest2"); + } + } + + public class MiddleTest : BaseTest + { + [Before(Test, Order = 100)] // Very high order, but still runs after base + public void MiddleBeforeTest() + { + ExecutionOrder.Add("MiddleBeforeTest"); + } + + [Before(Test, Order = 0)] // Default order + public void MiddleBeforeTest2() + { + ExecutionOrder.Add("MiddleBeforeTest2"); + } + + [After(Test, Order = -100)] // Very low order, but still runs before base + public void MiddleAfterTest() + { + ExecutionOrder.Add("MiddleAfterTest"); + } + + [After(Test, Order = 0)] // Default order + public void MiddleAfterTest2() + { + ExecutionOrder.Add("MiddleAfterTest2"); + } + } + + public class DerivedTest : MiddleTest + { + [Before(Test, Order = -1000)] // Extremely low order, but still runs after middle + public void DerivedBeforeTest() + { + ExecutionOrder.Add("DerivedBeforeTest"); + } + + [Before(Test, Order = 3)] // Positive order + public void DerivedBeforeTest2() + { + ExecutionOrder.Add("DerivedBeforeTest2"); + } + + [After(Test, Order = 1000)] // Extremely high order, but still runs before middle + public void DerivedAfterTest() + { + ExecutionOrder.Add("DerivedAfterTest"); + } + + [After(Test, Order = -3)] // Negative order + public void DerivedAfterTest2() + { + ExecutionOrder.Add("DerivedAfterTest2"); + } + + [Test] + public void TestHookExecutionOrder() + { + ExecutionOrder.Add("TestMethod"); + } + + [After(Test, Order = 2000)] // Use very high order to run after all other after hooks + public void VerifyExecutionOrder() + { + // Expected order: + // Before hooks (base to derived, with Order respected within each level): + // - Base: BaseBeforeTest2 (-5), then BaseBeforeTest (10) + // - Middle: MiddleBeforeTest2 (0), then MiddleBeforeTest (100) + // - Derived: DerivedBeforeTest (-1000), then DerivedBeforeTest2 (3) + // Test method + // After hooks (derived to base, with Order respected within each level): + // - Derived: DerivedAfterTest2 (-3), then DerivedAfterTest (1000) + // - Middle: MiddleAfterTest (-100), then MiddleAfterTest2 (0) + // - Base: BaseAfterTest (-10), then BaseAfterTest2 (5) + + var expected = new List + { + // Before hooks - base to derived + "BaseBeforeTest2", // Base level, Order = -5 + "BaseBeforeTest", // Base level, Order = 10 + "MiddleBeforeTest2", // Middle level, Order = 0 + "MiddleBeforeTest", // Middle level, Order = 100 + "DerivedBeforeTest", // Derived level, Order = -1000 + "DerivedBeforeTest2", // Derived level, Order = 3 + + "TestMethod", + + // After hooks - derived to base + "DerivedAfterTest2", // Derived level, Order = -3 + "DerivedAfterTest", // Derived level, Order = 1000 + "MiddleAfterTest", // Middle level, Order = -100 + "MiddleAfterTest2", // Middle level, Order = 0 + "BaseAfterTest", // Base level, Order = -10 + "BaseAfterTest2" // Base level, Order = 5 + }; + + for (int i = 0; i < expected.Count; i++) + { + if (i >= ExecutionOrder.Count || ExecutionOrder[i] != expected[i]) + { + throw new Exception($"Hook execution order is incorrect at index {i}. Expected: {expected[i]}, Actual: {(i < ExecutionOrder.Count ? ExecutionOrder[i] : "missing")}. Full order - Expected: [{string.Join(", ", expected)}], Actual: [{string.Join(", ", ExecutionOrder)}]"); + } + } + } + } +} From 621124676798a039881ace4e4fbebc4bd6213a59 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Mon, 4 Aug 2025 11:51:08 +0100 Subject: [PATCH 2/2] test: add verification for hook execution order in HookExecutionOrderTest --- TUnit.TestProject/HookExecutionOrderTest.cs | 88 ++++++++++----------- TUnit.TestProject/TypedDataSourceTests.cs | 4 +- 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/TUnit.TestProject/HookExecutionOrderTest.cs b/TUnit.TestProject/HookExecutionOrderTest.cs index 9ce99a881b..f6e84d000f 100644 --- a/TUnit.TestProject/HookExecutionOrderTest.cs +++ b/TUnit.TestProject/HookExecutionOrderTest.cs @@ -20,6 +20,50 @@ public void BaseBeforeTest() ExecutionOrder.Add("BaseBeforeTest"); } + [After(Test, Order = 2000)] // Use very high order to run after all other after hooks + public void VerifyExecutionOrder() + { + // Expected order: + // Before hooks (base to derived, with Order respected within each level): + // - Base: BaseBeforeTest2 (-5), then BaseBeforeTest (10) + // - Middle: MiddleBeforeTest2 (0), then MiddleBeforeTest (100) + // - Derived: DerivedBeforeTest (-1000), then DerivedBeforeTest2 (3) + // Test method + // After hooks (derived to base, with Order respected within each level): + // - Derived: DerivedAfterTest2 (-3), then DerivedAfterTest (1000) + // - Middle: MiddleAfterTest (-100), then MiddleAfterTest2 (0) + // - Base: BaseAfterTest (-10), then BaseAfterTest2 (5) + + var expected = new List + { + // Before hooks - base to derived + "BaseBeforeTest2", // Base level, Order = -5 + "BaseBeforeTest", // Base level, Order = 10 + "MiddleBeforeTest2", // Middle level, Order = 0 + "MiddleBeforeTest", // Middle level, Order = 100 + "DerivedBeforeTest", // Derived level, Order = -1000 + "DerivedBeforeTest2", // Derived level, Order = 3 + + "TestMethod", + + // After hooks - derived to base + "DerivedAfterTest2", // Derived level, Order = -3 + "DerivedAfterTest", // Derived level, Order = 1000 + "MiddleAfterTest", // Middle level, Order = -100 + "MiddleAfterTest2", // Middle level, Order = 0 + "BaseAfterTest", // Base level, Order = -10 + "BaseAfterTest2" // Base level, Order = 5 + }; + + for (var i = 0; i < expected.Count; i++) + { + if (i >= ExecutionOrder.Count || ExecutionOrder[i] != expected[i]) + { + throw new Exception($"Hook execution order is incorrect at index {i}. Expected: {expected[i]}, Actual: {(i < ExecutionOrder.Count ? ExecutionOrder[i] : "missing")}. Full order - Expected: [{string.Join(", ", expected)}], Actual: [{string.Join(", ", ExecutionOrder)}]"); + } + } + } + [Before(Test, Order = -5)] // Negative order, should run before BaseBeforeTest at same level public void BaseBeforeTest2() { @@ -97,49 +141,5 @@ public void TestHookExecutionOrder() { ExecutionOrder.Add("TestMethod"); } - - [After(Test, Order = 2000)] // Use very high order to run after all other after hooks - public void VerifyExecutionOrder() - { - // Expected order: - // Before hooks (base to derived, with Order respected within each level): - // - Base: BaseBeforeTest2 (-5), then BaseBeforeTest (10) - // - Middle: MiddleBeforeTest2 (0), then MiddleBeforeTest (100) - // - Derived: DerivedBeforeTest (-1000), then DerivedBeforeTest2 (3) - // Test method - // After hooks (derived to base, with Order respected within each level): - // - Derived: DerivedAfterTest2 (-3), then DerivedAfterTest (1000) - // - Middle: MiddleAfterTest (-100), then MiddleAfterTest2 (0) - // - Base: BaseAfterTest (-10), then BaseAfterTest2 (5) - - var expected = new List - { - // Before hooks - base to derived - "BaseBeforeTest2", // Base level, Order = -5 - "BaseBeforeTest", // Base level, Order = 10 - "MiddleBeforeTest2", // Middle level, Order = 0 - "MiddleBeforeTest", // Middle level, Order = 100 - "DerivedBeforeTest", // Derived level, Order = -1000 - "DerivedBeforeTest2", // Derived level, Order = 3 - - "TestMethod", - - // After hooks - derived to base - "DerivedAfterTest2", // Derived level, Order = -3 - "DerivedAfterTest", // Derived level, Order = 1000 - "MiddleAfterTest", // Middle level, Order = -100 - "MiddleAfterTest2", // Middle level, Order = 0 - "BaseAfterTest", // Base level, Order = -10 - "BaseAfterTest2" // Base level, Order = 5 - }; - - for (int i = 0; i < expected.Count; i++) - { - if (i >= ExecutionOrder.Count || ExecutionOrder[i] != expected[i]) - { - throw new Exception($"Hook execution order is incorrect at index {i}. Expected: {expected[i]}, Actual: {(i < ExecutionOrder.Count ? ExecutionOrder[i] : "missing")}. Full order - Expected: [{string.Join(", ", expected)}], Actual: [{string.Join(", ", ExecutionOrder)}]"); - } - } - } } } diff --git a/TUnit.TestProject/TypedDataSourceTests.cs b/TUnit.TestProject/TypedDataSourceTests.cs index ee95216376..ee67ee5c12 100644 --- a/TUnit.TestProject/TypedDataSourceTests.cs +++ b/TUnit.TestProject/TypedDataSourceTests.cs @@ -42,7 +42,7 @@ public IntRangeDataSource(int start, int end) public override async IAsyncEnumerable>> GetTypedDataRowsAsync(DataGeneratorMetadata dataGeneratorMetadata) { - for (int i = _start; i <= _end; i++) + for (var i = _start; i <= _end; i++) { var value = i; yield return () => Task.FromResult(value); @@ -92,7 +92,7 @@ public AsyncIntDataSource(int count) protected override async IAsyncEnumerable>> GenerateDataSourcesAsync(DataGeneratorMetadata dataGeneratorMetadata) { - for (int i = 0; i < _count; i++) + for (var i = 0; i < _count; i++) { await Task.Delay(1); // Simulate async work var value = i * 10;