-
-
Notifications
You must be signed in to change notification settings - Fork 110
fix: add support for [GenerateGenericTest] on generic classes (#4431) #4439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
## Summary - Fix generic class handling in reflection mode by supporting [GenerateGenericTest] attribute - Fix init-only property setters for generic types (UnsafeAccessor doesn't work with open generics) ## Changes - **ReflectionTestDataCollector.cs**: Handle [GenerateGenericTest] attributes that explicitly specify type arguments for generic test classes - **TestMetadataGenerator.cs**: Use reflection-based setter for init-only properties on generic types instead of UnsafeAccessor ## Test cases Added test files in TUnit.TestProject/Bugs/4431/ demonstrating: - Generic class with [GenerateGenericTest] attribute - Generic class with [ClassDataSource] property injection - Various combinations of generic classes/methods with data sources Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SummaryAdds support for [GenerateGenericTest] attribute on generic test classes in both source-gen and reflection modes, fixing issue #4431. Critical Issues1. AOT Compatibility Violation - Generic Type Init-Only Property SettersLocation: TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs:1538-1549 The PR introduces runtime reflection in source-generated code for generic types with init-only properties. This code uses GetType().GetField() without DynamicallyAccessedMembers annotations, which will be problematic with Native AOT and IL trimming. Problems:
Suggested fix: Generate a clear exception for generic types with init-only properties and data sources, directing users to use regular properties or constructors instead. 2. Missing Test CoverageThe PR description shows unchecked test plan item. Please run the full test suite to verify no regressions. 3. Snapshot Tests Not RunPer CLAUDE.md Rule #2, changes to source generator output require running snapshot tests in TUnit.Core.SourceGenerator.Tests. Suggestions
Verdict |
- Replace reflection-based setter for init-only properties on generic types with NotSupportedException (AOT-compatible) - Add missing verified files for GenericPropertyInjection tests - Add source generator test for generic method with data source Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
SummaryAdds support for Critical IssuesNone found ✅ Suggestions1. Consider Performance Optimization - Cartesian Product ComputationThe new cartesian product logic for class + method generic combinations is implemented correctly, but there's repeated attribute iteration. Consider caching 2. Documentation for Init-Only Property LimitationThe PR correctly identifies and handles the limitation that init-only properties don't work with generic types (since UnsafeAccessor doesn't support open generics). The error message is clear and helpful. No action needed, but this is good defensive programming. 3. Snapshot Test VerificationSeveral new
Assuming these were generated by running the snapshot tests as required by CLAUDE.md Rule #2. ✅ Previous Review StatusThe previous review comment (from PR author) raised concerns about:
The critical AOT issue has been properly addressed by avoiding the problematic runtime reflection path entirely. Verdict✅ APPROVE - No critical issues. The implementation correctly:
The "Known limitation" mentioned in the PR description about generic methods with both |
Summary
Fixes #4431 - Adds support for
[GenerateGenericTest]attribute on generic test classes.Changes:
[GenerateGenericTest]attributeDetails
ReflectionTestDataCollector.cs
Handle
[GenerateGenericTest]attributes that explicitly specify type arguments for generic test classes. Previously, the reflection mode only worked with data sources to determine generic type arguments.TestMetadataGenerator.cs
Use reflection-based setter for init-only properties on generic types instead of UnsafeAccessor, which doesn't work with open generic types.
Test plan
TUnit.TestProject/Bugs/4431/demonstrating generic class scenariosKnown limitation
There is still an issue with generic methods (not classes) that have both
[GenerateGenericTest]AND[MethodDataSource]- these tests may not be discovered correctly. A separate issue will be filed for this case.🤖 Generated with Claude Code