Allow overriding test source location

[thomhurst]

Fixes #3330

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Allow overriding test source location

Todo List:

• Read and analyze the modified TestMetadataGenerator.cs file
• Review repository guidelines from CLAUDE.md
• Check for related test files and coverage
• Analyze code quality and best practices
• Review for potential bugs and issues
• Assess performance considerations
• Check security concerns
• Provide comprehensive feedback

---

🔍 Code Review Summary

This PR modifies the GetTestMethodSourceLocation methods in TestMetadataGenerator.cs to prioritize file path and line number information from the TestAttribute's [CallerFilePath] and [CallerLineNumber] parameters over the syntax-based location detection.

✅ Strengths

1. Clear Intent: The change properly implements priority-based source location detection, which aligns with the PR title "Allow overriding test source location"

2. Consistent Implementation: Both overloads of GetTestMethodSourceLocation are updated with identical logic, maintaining consistency

3. Fallback Strategy: The implementation maintains proper fallback hierarchy:

   • Primary: TestAttribute's CallerFilePath/CallerLineNumber values
   • Secondary: Method syntax/symbol location
   • Tertiary: Final fallback to syntax tree location

4. Code Style: Follows the repository's coding standards (braces, var usage, etc.)

⚠️ Areas of Concern

Critical Issue: Missing Reflection Mode Implementation

Severity: High 🚨

Per the repository's CLAUDE.md mandatory rules:

"Dual-mode implementation required: ALL changes must work identically in both source-generated and reflection modes"

This change only modifies the source generator path but does not include corresponding changes in TUnit.Engine (reflection mode). This violates the fundamental requirement that all features must work in both execution modes.

Test Coverage Gap

Severity: Medium ⚠️

No test files were modified to cover the new behavior. The repository emphasizes the importance of tests, and this feature should have tests demonstrating:

• Override behavior when CallerFilePath/CallerLineNumber are provided
• Fallback behavior when they're not provided or invalid
• Both source-gen and reflection mode validation

Performance Consideration

Severity: Low

The change adds additional constructor argument access calls (testAttribute.ConstructorArguments.ElementAtOrDefault(0/1).Value) in the hot path. While minimal, this could be micro-optimized by caching these values if performance becomes a concern.

🔧 Required Actions

1. Implement reflection mode support in TUnit.Engine to mirror this logic
2. Add comprehensive tests covering both execution paths
3. Run snapshot tests: dotnet test TUnit.Core.SourceGenerator.Tests and accept any changed snapshots
4. Verify no breaking changes with dotnet test TUnit.PublicAPI

📋 Security & Safety

• No security concerns identified - the changes only affect test metadata generation
• Thread safety: Not applicable for this change
• AOT/Trimming compatibility: ✅ Uses compile-time attributes, no reflection concerns

🎯 Recommendation

Cannot approve in current state due to missing reflection mode implementation. This is a critical requirement that must be addressed before merge.

The code quality of the source generator changes is good, but the incomplete implementation violates the repository's core architectural principles.

---