-
-
Notifications
You must be signed in to change notification settings - Fork 108
fix: Include class-level DependsOn attributes in generated test metadata #4499
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
|
test |
|
Summary: Fixes source generator to collect class-level [DependsOn] attributes, bringing it into parity with reflection mode. Critical Issues: None found ✅ Suggestions:
If snapshot tests fail, review the changes and commit the updated .verified.txt files (never .received.txt).
Analysis Notes: Dual-Mode Compliance (Rule 1): ✅ Correct - Reflection mode (ReflectionAttributeExtractor.GetAttributes at TUnit.Engine/Discovery/ReflectionAttributeExtractor.cs:107-134) already collects class-level attributes (line 123). This PR fixes source generator mode to match reflection mode behavior. The change brings the two modes into parity rather than creating a disparity. Performance (Rule 4): ✅ Good - Uses .Concat() which is appropriate for this code path (not a hot path during execution). Test metadata generation happens once during discovery. Test Coverage: ✅ Good - Added comprehensive test cases covering class-level [DependsOn] with multiple test classes. Tests verify the actual dependency behavior (invocation counts). Verdict: ✅ APPROVE - No critical issues. The fix correctly addresses the disparity between source generator and reflection modes. Just verify snapshot tests pass before merging. |
The GenerateDependencies method in TestMetadataGenerator was only collecting [DependsOn] attributes from the test method. This caused class-level [DependsOn] attributes to be ignored, meaning dependencies declared at the class level were not being pulled in when filtering tests. This fix adds methodSymbol.ContainingType.GetAttributes() to also capture class-level DependsOn attributes, ensuring tests with class-level dependencies are properly resolved and included when filtered.
efafe21 to
083d106
Compare
SummaryFixes source generator to collect class-level Critical IssuesNone found ✅ Suggestions1. Snapshot Tests Required (Rule 2) Please run: dotnet test TUnit.Core.SourceGenerator.TestsIf any
2. Consider Adding Explicit Source Generator Test Analysis NotesDual-Mode Compliance (Rule 1): ✅ Correct
The change correctly brings source generator mode into parity with reflection mode. Performance (Rule 4): ✅ Good Test Coverage: ✅ Good Previous Review StatusA previous review comment exists noting similar findings. The PR has been updated but snapshot test verification remains pending. Verdict |
Summary
GenerateDependenciesinTestMetadataGeneratorto also check class-level[DependsOn]attributes[DependsOn]dependenciesProblem
When using
[DependsOn]at the class level (rather than method level), the dependency was not being captured in the generated test metadata. This meant that when filtering tests, the dependencies declared at class level were not being pulled in.For example:
Root Cause
The
GenerateDependenciesmethod only collected[DependsOn]attributes frommethodSymbol.GetAttributes(), missing attributes on the containing type.Fix
Added
.Concat(methodSymbol.ContainingType.GetAttributes())to also capture class-level attributes.Test plan
/*/*/*DependsOn_AsyncTest*/*now correctly pulls inMyAsyncTest.Testdependency