fix: TestExecutorAttribute respects scope hierarchy (method > class > assembly)

[thomhurst]

Summary

• Adds IScopedAttribute to TestExecutorAttribute, STAThreadExecutorAttribute, and CultureAttribute
• Uses typeof(ITestExecutor) as the scope type to group all executor-setting attributes together
• Enables the existing ScopedAttributeFilter mechanism to properly filter executors by scope precedence

Root Cause

TestExecutorAttribute did not implement IScopedAttribute, so all executor attributes from all scopes (assembly, class, method) were being invoked. Each would call SetTestExecutor(), overwriting the previous one. Since attributes are collected in method → class → assembly order, the assembly-level executor would be invoked last and incorrectly win.

Solution

By implementing IScopedAttribute with ScopeType => typeof(ITestExecutor), all executor attributes are now properly filtered by the ScopedAttributeFilter which uses TryAdd semantics (first occurrence wins). Since attributes are collected method-first, method-level executors now correctly take precedence.

Test plan

• Build passes
• Public API snapshot tests updated and pass
• Manual verification with the reproduction case from the issue

Fixes #4351

🤖 Generated with Claude Code

[thomhurst]

Summary

Fixes executor attribute scope precedence by implementing IScopedAttribute on TestExecutorAttribute, STAThreadExecutorAttribute, and CultureAttribute.

Critical Issues

None found ✅

Analysis

This PR correctly addresses issue #4351 by implementing the IScopedAttribute interface on all executor-related attributes. The fix is sound:

Root Cause: Without IScopedAttribute, all executor attributes from assembly/class/method scopes were being collected and processed. Since each would call SetTestExecutor(), they would overwrite each other non-deterministically based on processing order.

Solution: By implementing IScopedAttribute with ScopeType = typeof(ITestExecutor), the existing ScopedAttributeFilter.FilterScopedAttributes() mechanism now correctly filters executors using TryAdd semantics (first occurrence wins). Since attributes are collected method → class → assembly, method-level executors now correctly take precedence.

Dual-Mode Compatibility: ✅ The source generator copies attributes verbatim (AttributeWriter.cs:TUnit.Core.SourceGenerator/CodeGenerators/Writers/AttributeWriter.cs:73), so the IScopedAttribute interface implementation is preserved in generated code. The reflection-based engine handles filtering via ScopedAttributeFilter (TUnit.Engine/Extensions/TestContextExtensions.cs:154).

AOT Compatibility: ✅ The existing [DynamicallyAccessedMembers] annotation on the non-generic TestExecutorAttribute is preserved (TUnit.Core/Attributes/Executors/TestExecutorAttribute.cs:19 in the diff).

Snapshot Tests: ✅ All .verified.txt files correctly updated, no .received.txt files committed.

Consistency: ✅ All three executor-related attributes (TestExecutorAttribute, STAThreadExecutorAttribute, CultureAttribute) now implement IScopedAttribute consistently.

Suggestions

Consider adding a regression test to verify executor scope precedence works correctly (method > class > assembly). The issue #4351 includes a complete reproduction case that could be adapted into a test in TUnit.TestProject.

Verdict

✅ APPROVE - No critical issues. The implementation is correct, follows TUnit's dual-mode architecture, maintains AOT compatibility, and properly updates public API snapshots.

[thomhurst]

Summary

Fixes executor attribute scope hierarchy by implementing IScopedAttribute on TestExecutorAttribute, STAThreadExecutorAttribute, and CultureAttribute to ensure method > class > assembly precedence.

Critical Issues

None found ✅

Analysis

This PR correctly addresses issue #4351 by implementing the IScopedAttribute interface on all executor-related attributes. The implementation is sound:

Root Cause Confirmed: Without IScopedAttribute, all executor attributes from assembly/class/method scopes were collected and each called SetTestExecutor(), overwriting each other. Since attributes are collected in method → class → assembly order (TUnit.Engine/Discovery/ReflectionAttributeExtractor.cs:195-197) and processed sequentially, the last one processed (assembly-level) would win.

Solution Verified: By implementing IScopedAttribute with ScopeType = typeof(ITestExecutor), the existing ScopedAttributeFilter.FilterScopedAttributes() mechanism (TUnit.Engine/Utilities/ScopedAttributeFilter.cs:33) now correctly filters executors using TryAdd semantics (first occurrence wins). Since attributes are collected method → class → assembly, method-level executors now correctly take precedence.

Dual-Mode Compatibility: ✅ Both reflection (TUnit.Engine) and source generation (TUnit.Core.SourceGenerator) paths work correctly:

• Reflection: Uses ScopedAttributeFilter.FilterScopedAttributes in TUnit.Engine/Extensions/TestContextExtensions.cs:154
• Source Gen: Copies attribute implementations verbatim, preserving interface implementations

AOT Compatibility: ✅ The existing [DynamicallyAccessedMembers] annotation on the non-generic TestExecutorAttribute is preserved.

Snapshot Tests: ✅ All .verified.txt files correctly updated for all target frameworks, no .received.txt files committed.

Test Coverage: ✅ Comprehensive tests added in TUnit.TestProject/TestExecutorScopeHierarchyTests.cs covering:

• Generic and non-generic TestExecutorAttribute overloads
• Method > class > assembly precedence
• Multiple test classes to verify isolation

Consistency: ✅ All three executor-related attributes (TestExecutorAttribute, STAThreadExecutorAttribute, CultureAttribute) now consistently implement IScopedAttribute.

Previous Review Status

Previous comment from thomhurst (2026-01-13) already approved this PR with similar findings. The author has addressed all concerns by adding comprehensive tests.

Verdict

✅ APPROVE - No critical issues. The implementation correctly fixes the scope hierarchy bug, maintains dual-mode compatibility, preserves AOT compatibility, properly updates public API snapshots, and includes excellent test coverage.