Skip to content

feat: support assembly-level [assembly: Repeat(N)]#4753

Merged
thomhurst merged 1 commit intomainfrom
feat/assembly-level-repeat
Feb 12, 2026
Merged

feat: support assembly-level [assembly: Repeat(N)]#4753
thomhurst merged 1 commit intomainfrom
feat/assembly-level-repeat

Conversation

@thomhurst
Copy link
Owner

Summary

  • Add assembly-level fallback for RepeatAttribute in both source generator ExtractRepeatCount overloads and the reflection engine's ReflectionTestDataCollector
  • Precedence order: method > class > assembly (most specific wins)
  • Add snapshot test using inline source to verify assembly-level repeat works and method-level overrides it correctly

Test plan

  • Source generator builds without errors
  • Reflection engine builds without errors
  • RepeatTests.Test snapshot test passes (existing behavior unchanged)
  • RepeatTests.Assembly_Level_Repeat snapshot test passes, verifying RepeatCount = 3 for inherited and RepeatCount = 1 for method-level override
  • CI passes

🤖 Generated with Claude Code

@claude
Copy link
Contributor

claude bot commented Feb 12, 2026

Code review

Performance Issue: Uncached Assembly Attribute in Hot Path

File: TUnit.Engine/Discovery/ReflectionTestDataCollector.cs:1064-1066

The assembly-level repeat count lookup at line 1066 is not cached, violating the Performance First rule from CLAUDE.md:

Performance First - Minimize allocations in hot paths. Cache reflection. Use ValueTask for potentially-sync operations.

Why this matters:

BuildTestMetadata is called once for every test method during discovery (a hot path). For assemblies with hundreds or thousands of tests, this means testClass.Assembly.GetCustomAttribute<RepeatAttribute>() is called repeatedly for the same assembly, even though assembly attributes are immutable.

Suggested approach:

Add a cache for assembly-level repeat counts following the existing pattern in the file:

// At class level (around line 32-35)
private static readonly ConcurrentDictionary<Assembly, int?> _assemblyRepeatCountCache = new();

// In BuildTestMetadata (around line 1064-1066)
RepeatCount = testMethod.GetCustomAttribute<RepeatAttribute>()?.Times
    ?? testClass.GetCustomAttribute<RepeatAttribute>()?.Times
    ?? _assemblyRepeatCountCache.GetOrAdd(
        testClass.Assembly,
        static assembly => assembly.GetCustomAttribute<RepeatAttribute>()?.Times),

Why this is better:

  1. Performance: Reduces redundant reflection calls from O(n) to O(1) per assembly, where n = number of tests
  2. Consistency: Follows the established caching pattern already used in this file for _assemblyTypesCache and _typeMethodsCache
  3. Thread-safe: ConcurrentDictionary.GetOrAdd handles concurrent access correctly
  4. Maintainability: Makes the performance optimization explicit and easier to understand

This is the only issue found. The rest of the implementation correctly follows the dual-mode architecture and maintains proper precedence ordering.

RepeatAttribute declares AttributeTargets.Assembly but neither the source
generator nor the reflection engine checked assembly-level attributes.
This adds assembly as the lowest-precedence fallback (method > class > assembly)
in both ExtractRepeatCount overloads and the reflection discovery path.

Also tighten all RepeatAttribute checks in the source generator to verify
the attribute's containing namespace is TUnit.Core, preventing false matches
against user-defined attributes with the same name.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thomhurst thomhurst force-pushed the feat/assembly-level-repeat branch from b2b3b09 to 3153980 Compare February 12, 2026 20:44
@thomhurst thomhurst merged commit d771c3f into main Feb 12, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the feat/assembly-level-repeat branch February 12, 2026 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant