From 4aacd03d3187358db7a1b72926f73253a52f5f06 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sat, 11 Apr 2026 13:00:24 +0100 Subject: [PATCH] fix: cascade HookExecutorAttribute from class/assembly to hooks (#5462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HookExecutorAttribute was only honored when applied directly to a hook method. ReflectionHookDiscoveryService.GetHookExecutor and HookMetadataGenerator.GetHookExecutorType both only consulted method-level attributes, so [HookExecutor] on the containing class or the assembly was silently ignored and the hook ran on DefaultExecutor. Mirror the pattern #5463 introduced for CultureAttribute / STAThreadExecutorAttribute / TestExecutorAttribute: implement IHookRegisteredEventReceiver and IScopedAttribute on the non-generic HookExecutorAttribute base. The generic HookExecutorAttribute inherits the implementation. Cascading is delivered for free by the existing pipeline: - MethodMetadata.GetCustomAttributes() already aggregates method + class + assembly attributes in that order. - EventReceiverOrchestrator runs them through ScopedAttributeFilter (first wins per ScopeType), so method-level beats class-level beats assembly-level for ScopeType = typeof(IHookExecutor). - HookMethod.SetHookExecutor leaves _hookExecutorIsExplicit = false, so cascaded executors remain overridable by per-test CustomHookExecutor (#2666 path), while a method-level [HookExecutor] still wins via the init-time _hookExecutorIsExplicit = true branch in ResolveEffectiveExecutor. No changes to discovery services — both source-gen and reflection modes are fixed via the receiver pipeline alone. Also tightens the base attribute with [AttributeUsage(Assembly|Class| Method)] (it had no AttributeUsage previously and defaulted to All) and adds [DynamicallyAccessedMembers(PublicConstructors)] to the constructor parameter and to the generic T parameter for AOT/trim correctness. Tests: HookExecutorHookTests.cs adds three regression fixtures mirroring CultureHookTests — class-level cascading, method-level override, and inherits-class — each using its own RecordingHookExecutor subclass so parallel runs stay isolated. Verified passing in both source-gen and reflection modes. --- .../Executors/HookExecutorAttribute.cs | 31 +++- ...Has_No_API_Changes.DotNet10_0.verified.txt | 10 +- ..._Has_No_API_Changes.DotNet8_0.verified.txt | 10 +- ..._Has_No_API_Changes.DotNet9_0.verified.txt | 10 +- ...ary_Has_No_API_Changes.Net4_7.verified.txt | 6 +- TUnit.TestProject/HookExecutorHookTests.cs | 164 ++++++++++++++++++ 6 files changed, 218 insertions(+), 13 deletions(-) create mode 100644 TUnit.TestProject/HookExecutorHookTests.cs diff --git a/TUnit.Core/Attributes/Executors/HookExecutorAttribute.cs b/TUnit.Core/Attributes/Executors/HookExecutorAttribute.cs index 5cdd78c4e9..fc8171c928 100644 --- a/TUnit.Core/Attributes/Executors/HookExecutorAttribute.cs +++ b/TUnit.Core/Attributes/Executors/HookExecutorAttribute.cs @@ -1,11 +1,36 @@ +using System.Diagnostics.CodeAnalysis; using TUnit.Core.Interfaces; namespace TUnit.Core.Executors; -public class HookExecutorAttribute(Type type) : TUnitAttribute +[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method)] +public class HookExecutorAttribute : TUnitAttribute, IHookRegisteredEventReceiver, IScopedAttribute { - public Type HookExecutorType { get; } = type; + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + private readonly Type _hookExecutorType; + + private IHookExecutor? _executor; + + public HookExecutorAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type) + { + _hookExecutorType = type; + } + + public Type HookExecutorType => _hookExecutorType; + + /// + public int Order => 0; + + /// + public Type ScopeType => typeof(IHookExecutor); + + /// + public ValueTask OnHookRegistered(HookRegisteredContext context) + { + context.HookExecutor = _executor ??= (IHookExecutor)Activator.CreateInstance(_hookExecutorType)!; + return default(ValueTask); + } } [AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method)] -public sealed class HookExecutorAttribute() : HookExecutorAttribute(typeof(T)) where T : IHookExecutor, new(); +public sealed class HookExecutorAttribute<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>() : HookExecutorAttribute(typeof(T)) where T : IHookExecutor, new(); diff --git a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet10_0.verified.txt b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet10_0.verified.txt index 09c9145279..8b52caf557 100644 --- a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet10_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet10_0.verified.txt @@ -2035,13 +2035,17 @@ namespace .Executors public . OnHookRegistered(.HookRegisteredContext context) { } public . OnTestRegistered(.TestRegisteredContext context) { } } - public class HookExecutorAttribute : .TUnitAttribute + [(.Assembly | .Class | .Method)] + public class HookExecutorAttribute : .TUnitAttribute, .IScopedAttribute, ., . { - public HookExecutorAttribute( type) { } + public HookExecutorAttribute([.(..PublicConstructors)] type) { } public HookExecutorType { get; } + public int Order { get; } + public ScopeType { get; } + public . OnHookRegistered(.HookRegisteredContext context) { } } [(.Assembly | .Class | .Method)] - public sealed class HookExecutorAttribute : . + public sealed class HookExecutorAttribute<[.(..PublicConstructors)] T> : . where T : ., new () { public HookExecutorAttribute() { } diff --git a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet8_0.verified.txt b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet8_0.verified.txt index 7ca7bd632b..0622541817 100644 --- a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet8_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet8_0.verified.txt @@ -2035,13 +2035,17 @@ namespace .Executors public . OnHookRegistered(.HookRegisteredContext context) { } public . OnTestRegistered(.TestRegisteredContext context) { } } - public class HookExecutorAttribute : .TUnitAttribute + [(.Assembly | .Class | .Method)] + public class HookExecutorAttribute : .TUnitAttribute, .IScopedAttribute, ., . { - public HookExecutorAttribute( type) { } + public HookExecutorAttribute([.(..PublicConstructors)] type) { } public HookExecutorType { get; } + public int Order { get; } + public ScopeType { get; } + public . OnHookRegistered(.HookRegisteredContext context) { } } [(.Assembly | .Class | .Method)] - public sealed class HookExecutorAttribute : . + public sealed class HookExecutorAttribute<[.(..PublicConstructors)] T> : . where T : ., new () { public HookExecutorAttribute() { } diff --git a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet9_0.verified.txt b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet9_0.verified.txt index aabe70fe99..0a1d3fcbe5 100644 --- a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet9_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.DotNet9_0.verified.txt @@ -2035,13 +2035,17 @@ namespace .Executors public . OnHookRegistered(.HookRegisteredContext context) { } public . OnTestRegistered(.TestRegisteredContext context) { } } - public class HookExecutorAttribute : .TUnitAttribute + [(.Assembly | .Class | .Method)] + public class HookExecutorAttribute : .TUnitAttribute, .IScopedAttribute, ., . { - public HookExecutorAttribute( type) { } + public HookExecutorAttribute([.(..PublicConstructors)] type) { } public HookExecutorType { get; } + public int Order { get; } + public ScopeType { get; } + public . OnHookRegistered(.HookRegisteredContext context) { } } [(.Assembly | .Class | .Method)] - public sealed class HookExecutorAttribute : . + public sealed class HookExecutorAttribute<[.(..PublicConstructors)] T> : . where T : ., new () { public HookExecutorAttribute() { } diff --git a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.Net4_7.verified.txt b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.Net4_7.verified.txt index 6efe709542..0266604c25 100644 --- a/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.Net4_7.verified.txt +++ b/TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.Net4_7.verified.txt @@ -1977,10 +1977,14 @@ namespace .Executors public . OnHookRegistered(.HookRegisteredContext context) { } public . OnTestRegistered(.TestRegisteredContext context) { } } - public class HookExecutorAttribute : .TUnitAttribute + [(.Assembly | .Class | .Method)] + public class HookExecutorAttribute : .TUnitAttribute, .IScopedAttribute, ., . { public HookExecutorAttribute( type) { } public HookExecutorType { get; } + public int Order { get; } + public ScopeType { get; } + public . OnHookRegistered(.HookRegisteredContext context) { } } [(.Assembly | .Class | .Method)] public sealed class HookExecutorAttribute : . diff --git a/TUnit.TestProject/HookExecutorHookTests.cs b/TUnit.TestProject/HookExecutorHookTests.cs new file mode 100644 index 0000000000..e448cff67d --- /dev/null +++ b/TUnit.TestProject/HookExecutorHookTests.cs @@ -0,0 +1,164 @@ +using System.Collections.Concurrent; +using TUnit.Core; +using TUnit.Core.Executors; +using TUnit.Core.Interfaces; +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject; + +/// +/// Regression tests for https://github.com/thomhurst/TUnit/issues/5462 — HookExecutorAttribute +/// applied at class (and assembly) level must cascade to hooks declared in the scope, not just +/// to hooks where the attribute sits directly on the method. Mirrors CultureHookTests, which is +/// the analogous coverage for #5452. +/// +internal static class RecordingHookExecutorState +{ + // Each fixture uses its own executor type (and therefore its own bucket) so that + // assertions are not affected by parallel-running fixtures. + private static readonly ConcurrentDictionary> _invocations = new(); + + public static void Record(string bucket, string hookName) + { + _invocations.GetOrAdd(bucket, _ => new ConcurrentBag()).Add(hookName); + } + + public static int Count(string bucket) + { + return _invocations.TryGetValue(bucket, out var bag) ? bag.Count : 0; + } +} + +public sealed class RecordingHookExecutor_F1ClassLevel : GenericAbstractExecutor +{ + protected override async ValueTask ExecuteAsync(Func action) + { + RecordingHookExecutorState.Record(nameof(RecordingHookExecutor_F1ClassLevel), "executed"); + await action(); + } +} + +public sealed class RecordingHookExecutor_F2ClassLevel : GenericAbstractExecutor +{ + protected override async ValueTask ExecuteAsync(Func action) + { + RecordingHookExecutorState.Record(nameof(RecordingHookExecutor_F2ClassLevel), "executed"); + await action(); + } +} + +public sealed class RecordingHookExecutor_F2MethodOverride : GenericAbstractExecutor +{ + protected override async ValueTask ExecuteAsync(Func action) + { + RecordingHookExecutorState.Record(nameof(RecordingHookExecutor_F2MethodOverride), "executed"); + await action(); + } +} + +public sealed class RecordingHookExecutor_F3Inherits : GenericAbstractExecutor +{ + protected override async ValueTask ExecuteAsync(Func action) + { + RecordingHookExecutorState.Record(nameof(RecordingHookExecutor_F3Inherits), "executed"); + await action(); + } +} + +[EngineTest(ExpectedResult.Pass)] +[HookExecutor] +public class HookExecutorHookTests_ClassLevel +{ + [Before(Class)] + public static Task BeforeClass() + { + return Task.CompletedTask; + } + + [After(Class)] + public static async Task AfterClass() + { + // Runs after the last test in this class — assert directly. By the end of the + // class lifecycle the class-level [HookExecutor] must have wrapped at least + // Before(Class), the per-test Before(Test)/After(Test) for both tests, and this + // After(Class) hook itself. We just need ≥ 1 to confirm cascading worked at all. + var count = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F1ClassLevel)); + await Assert.That(count).IsGreaterThanOrEqualTo(1); + } + + [Before(Test)] + public Task BeforeTest() + { + return Task.CompletedTask; + } + + [After(Test)] + public Task AfterTest() + { + return Task.CompletedTask; + } + + [Test] + public async Task ClassLevelHookExecutor_RanForBeforeClassHook() + { + // Before(Class) ran before this test. With the class-level [HookExecutor<...>] + // cascading, the recording executor must have been invoked at least once. + var count = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F1ClassLevel)); + await Assert.That(count).IsGreaterThanOrEqualTo(1); + } + + [Test] + public async Task ClassLevelHookExecutor_RanForBeforeTestHook() + { + // By the time this test body runs, Before(Test) has executed for it. The recording + // executor should have been invoked at least twice — once for Before(Class) (shared + // across both tests) and once for the most recent Before(Test). + var count = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F1ClassLevel)); + await Assert.That(count).IsGreaterThanOrEqualTo(2); + } +} + +[EngineTest(ExpectedResult.Pass)] +[HookExecutor] +public class HookExecutorHookTests_MethodLevelOverride +{ + [Before(Test), HookExecutor] + public Task BeforeTest() + { + return Task.CompletedTask; + } + + [Test] + public async Task MethodLevel_HookExecutor_OverridesClassLevel() + { + // The Before(Test) hook above carries its own [HookExecutor], + // which must beat the class-level [HookExecutor] for this hook. + // Method-override executor must be invoked, class-level executor must not be — + // there is no other hook in this fixture for the class-level executor to wrap. + var methodOverrideCount = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F2MethodOverride)); + await Assert.That(methodOverrideCount).IsGreaterThanOrEqualTo(1); + + var classLevelCount = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F2ClassLevel)); + await Assert.That(classLevelCount).IsEqualTo(0); + } +} + +[EngineTest(ExpectedResult.Pass)] +[HookExecutor] +public class HookExecutorHookTests_InheritsClassLevel +{ + // No method-level [HookExecutor] override — class-level RecordingHookExecutor_F3Inherits + // applies to the Before(Test) hook. + [Before(Test)] + public Task BeforeTest() + { + return Task.CompletedTask; + } + + [Test] + public async Task InheritsClassHookExecutor_ForBeforeTest() + { + var count = RecordingHookExecutorState.Count(nameof(RecordingHookExecutor_F3Inherits)); + await Assert.That(count).IsGreaterThanOrEqualTo(1); + } +}