feat: introduce TestBuildingContext for optimized test building and filtering

[thomhurst]

Fixes #3307

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: TestBuildingContext Implementation

Reviewing the TestBuildingContext implementation for optimized test building and filtering.

Review Checklist:

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Review TestBuildingContext implementation
• Check code quality and best practices
• Assess performance considerations
• Review for potential bugs
• Check security concerns
• Evaluate test coverage needs
• Provide comprehensive feedback

---

Code Quality & Architecture ✅

Strengths:

• Clean, well-documented record type for TestBuildingContext with clear purpose
• Good separation of concerns between discovery and execution contexts
• Consistent naming and follows modern C# patterns (collection expressions, record types)
• Proper use of nullable annotations for the Filter parameter

Minor Issues:

• Line 16 in ITestBuilder.cs has an inconsistent parameter name testBuilderContext vs buildingContext used elsewhere

Performance Considerations ⭐

Excellent Optimizations:

• Early filtering optimization in TestBuilder.cs:122-129: Pre-filters tests before expensive building operations when in execution mode - this is a significant performance win
• Conservative filter matching in CouldTestMatchFilter(): Uses lightweight path-based filtering without building full test objects
• Streaming support maintained in TestBuilderPipeline.cs:76: Passes context through streaming methods

Performance Impact:

• The optimization should provide substantial performance improvements for filtered test execution
• Memory usage reduced by skipping test building for non-matching tests
• No performance regression for discovery mode (filter is null)

Implementation Analysis 🔍

TestBuildingContext Design:

• Simple, focused record with two clear responsibilities
• Boolean flag clearly separates discovery vs execution modes
• Optional filter allows for both scenarios

Integration Points:

• Well-integrated across the pipeline: TestDiscoveryService → TestBuilderPipeline → TestBuilder
• Consistent parameter passing through all layers
• Proper handling in both streaming and non-streaming paths

Potential Issues & Concerns ⚠️

Filter Type Coupling:

• CouldTestMatchFilter() method has knowledge of specific filter types (TreeNodeFilter, NopFilter, etc.)
• Uses #pragma warning disable TPEXP suggesting experimental API usage
• Consider abstracting filter compatibility checking to avoid tight coupling

Path Building Logic:

• BuildPathFromMetadata() duplicates path format logic that may exist elsewhere
• Path format: /AssemblyName/Namespace/ClassName/MethodName - ensure this matches other parts of the system

Error Handling:

• Early return with empty array in line 127 could mask configuration issues
• Consider logging when tests are filtered out for debugging purposes

Security Concerns 🔒

No significant security issues identified:

• No direct user input handling
• No reflection or dynamic code generation in the new context
• Proper separation between execution and discovery contexts

Test Coverage Recommendations 📋

Critical Test Cases Needed:

1. Filter optimization behavior: Test that CouldTestMatchFilter correctly identifies matching/non-matching tests
2. Discovery vs execution modes: Verify behavior differences between IsForExecution: true/false
3. Filter type compatibility: Test with different filter implementations
4. Edge cases: Null filters, empty test sets, complex filter expressions
5. Performance regression tests: Measure time difference with/without optimization

Specific Scenarios:

// Test early filtering works
[Test] async Task TestEarlyFilteringSkipsNonMatchingTests()

// Test discovery mode doesn't apply filter optimization  
[Test] async Task DiscoveryModeDoesNotApplyFiltering()

// Test path building matches expected format
[Test] void TestPathBuildingFormat()

Dual-Mode Compliance ✅

Good news: This change appears to be engine-only optimization that shouldn't require source generator changes. The optimization is in the reflection execution path, which is appropriate.

Verification needed: Ensure identical behavior in both modes when tests are actually built and executed.

Recommendations 📝

1. Add logging for filtered-out tests in debug builds to aid troubleshooting
2. Extract path building logic to a shared utility if used elsewhere
3. Consider filter interface abstraction to reduce coupling with specific filter types
4. Add performance benchmarks to validate the optimization impact
5. Update documentation explaining when early filtering applies

Overall Assessment ⭐⭐⭐⭐⭐

This is a well-designed performance optimization that:

• ✅ Follows TUnit architecture patterns
• ✅ Maintains backward compatibility
• ✅ Provides clear performance benefits
• ✅ Has minimal risk of introducing bugs
• ✅ Follows coding standards

The implementation is clean, focused, and addresses a real performance need during test execution with filters.

---

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: TestBuildingContext for Optimized Test Building and Filtering

Reviewing the TestBuildingContext implementation for optimized test building and filtering.

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Review TestBuildingContext implementation
• Check code quality and best practices
• Assess performance considerations
• Review for potential bugs
• Check security concerns
• Evaluate test coverage needs
• Provide comprehensive feedback

---

Code Quality & Architecture ✅

Strengths:

• Clean, well-documented record type for TestBuildingContext with clear purpose
• Good separation of concerns between discovery and execution contexts
• Consistent naming and follows modern C# patterns (collection expressions, record types)
• Proper use of nullable annotations for the Filter parameter

Minor Issues:

• Line 16 in ITestBuilder.cs has an inconsistent parameter name testBuilderContext vs buildingContext used elsewhere

Performance Considerations ⭐

Excellent Optimizations:

• Early filtering optimization in TestBuilder.cs:122-129: Pre-filters tests before expensive building operations when in execution mode - this is a significant performance win
• Conservative filter matching in CouldTestMatchFilter(): Uses lightweight path-based filtering without building full test objects
• Streaming support maintained in TestBuilderPipeline.cs:76: Passes context through streaming methods

Performance Impact:

• The optimization should provide substantial performance improvements for filtered test execution
• Memory usage reduced by skipping test building for non-matching tests
• No performance regression for discovery mode (filter is null)

Implementation Analysis 🔍

TestBuildingContext Design:

• Simple, focused record with two clear responsibilities
• Boolean flag clearly separates discovery vs execution modes
• Optional filter allows for both scenarios

Integration Points:

• Well-integrated across the pipeline: TestDiscoveryService → TestBuilderPipeline → TestBuilder
• Consistent parameter passing through all layers
• Proper handling in both streaming and non-streaming paths

Potential Issues & Concerns ⚠️

Filter Type Coupling:

• CouldTestMatchFilter() method has knowledge of specific filter types (TreeNodeFilter, NopFilter, etc.)
• Uses #pragma warning disable TPEXP suggesting experimental API usage
• Consider abstracting filter compatibility checking to avoid tight coupling

Path Building Logic:

• BuildPathFromMetadata() duplicates path format logic that may exist elsewhere
• Path format: /AssemblyName/Namespace/ClassName/MethodName - ensure this matches other parts of the system

Error Handling:

• Early return with empty array in line 127 could mask configuration issues
• Consider logging when tests are filtered out for debugging purposes

Security Concerns 🔒

No significant security issues identified:

• No direct user input handling
• No reflection or dynamic code generation in the new context
• Proper separation between execution and discovery contexts

Test Coverage Recommendations 📋

Critical Test Cases Needed:

1. Filter optimization behavior: Test that CouldTestMatchFilter correctly identifies matching/non-matching tests
2. Discovery vs execution modes: Verify behavior differences between IsForExecution: true/false
3. Filter type compatibility: Test with different filter implementations
4. Edge cases: Null filters, empty test sets, complex filter expressions
5. Performance regression tests: Measure time difference with/without optimization

Specific Scenarios:

// Test early filtering works
[Test] async Task TestEarlyFilteringSkipsNonMatchingTests()

// Test discovery mode doesn't apply filter optimization  
[Test] async Task DiscoveryModeDoesNotApplyFiltering()

// Test path building matches expected format
[Test] void TestPathBuildingFormat()

Dual-Mode Compliance ✅

Good news: This change appears to be engine-only optimization that shouldn't require source generator changes. The optimization is in the reflection execution path, which is appropriate.

Verification needed: Ensure identical behavior in both modes when tests are actually built and executed.

Recommendations 📝

1. Add logging for filtered-out tests in debug builds to aid troubleshooting
2. Extract path building logic to a shared utility if used elsewhere
3. Consider filter interface abstraction to reduce coupling with specific filter types
4. Add performance benchmarks to validate the optimization impact
5. Update documentation explaining when early filtering applies

Overall Assessment ⭐⭐⭐⭐⭐

This is a well-designed performance optimization that:

• ✅ Follows TUnit architecture patterns
• ✅ Maintains backward compatibility
• ✅ Provides clear performance benefits
• ✅ Has minimal risk of introducing bugs
• ✅ Follows coding standards

The implementation is clean, focused, and addresses a real performance need during test execution with filters.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: TestBuildingContext for Optimized Test Building and Filtering

Reviewing the TestBuildingContext implementation for optimized test building and filtering.

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Review TestBuildingContext implementation
• Check code quality and best practices
• Assess performance considerations
• Review for potential bugs
• Check security concerns
• Evaluate test coverage needs
• Provide comprehensive feedback

---

Code Quality & Architecture ✅

Strengths:

• Clean, well-documented record type for TestBuildingContext with clear purpose and XML documentation
• Good separation of concerns between discovery and execution contexts
• Consistent naming and follows modern C# patterns (collection expressions, record types, nullable annotations)
• Proper integration across the pipeline: TestDiscoveryService → TestBuilderPipeline → TestBuilder

Minor Issues:

• Line 16 in ITestBuilder.cs has an inconsistent parameter name testBuilderContext vs buildingContext used elsewhere
• Inconsistent parameter naming in interface vs implementation could cause confusion

Performance Considerations ⭐⭐⭐⭐⭐

Excellent Optimizations:

• Early filtering optimization in TestBuilder.cs:122-129: Pre-filters tests before expensive building operations when in execution mode - this is a significant performance win for filtered test runs
• Conservative filter matching in CouldTestMatchFilter(): Uses lightweight path-based filtering without building full test objects
• Streaming support maintained: Passes context through streaming methods correctly
• Memory efficiency: Avoids building tests that won't match the filter, reducing memory usage

Performance Impact Analysis:

• Should provide substantial performance improvements for filtered test execution (e.g., running single test methods)
• Memory usage reduced by skipping test building for non-matching tests
• No performance regression for discovery mode (filter is null)
• Smart path-based filtering allows early exit without expensive test object creation

Implementation Analysis 🔍

TestBuildingContext Design:

• Simple, focused record with two clear responsibilities
• Boolean flag clearly separates discovery vs execution modes
• Optional filter allows for both scenarios
• Immutable record type promotes thread safety

Integration Points:

• Well-integrated across all layers of the test building pipeline
• Consistent parameter passing through all methods
• Proper handling in both streaming and non-streaming paths
• Correctly used in both reflection and source-gen scenarios

Filter Implementation Review 🔍

Path Building Logic (TestBuilder.cs:1649-1658):

• Correctly builds paths matching TestFilterService format: /AssemblyName/Namespace/ClassName/MethodName
• Handles null cases appropriately with fallback values
• Path format matches what the framework expects

Filter Matching Logic (TestBuilder.cs:1583-1643):

• Conservative approach - returns true unless definitively can rule out test
• Handles different filter types (TreeNodeFilter, NopFilter, etc.)
• Smart handling of property conditions by stripping them for path-only matching
• Uses reflection to create TreeNodeFilter instances (necessary due to internal constructor)

Potential Issues & Concerns ⚠️

Filter Type Coupling:

• CouldTestMatchFilter() method has knowledge of specific filter types
• Uses #pragma warning disable TPEXP suggesting experimental API usage
• Consider creating an interface abstraction to reduce coupling with platform-specific filter types

Property Condition Handling:

• Regex-based stripping of property conditions ([key=value]) in line 1618 is functional but brittle
• Could break if Microsoft changes the filter format
• Consider more robust parsing

Error Handling:

• Early return with empty array in line 127 could mask configuration issues
• Consider adding debug logging when tests are filtered out for troubleshooting

Security Concerns 🔒

No significant security issues identified:

• No direct user input handling in the optimization paths
• No reflection or dynamic code generation in the new context logic
• Proper separation between execution and discovery contexts
• Filter handling uses existing platform APIs safely

Dual-Mode Compliance ✅

Good compliance with TUnit's dual-mode requirements:

• This appears to be an engine-only optimization that works in reflection mode
• Source generator mode likely has its own optimizations
• The optimization is in the test building path, which is appropriate for reflection mode
• No changes needed to source generator since this is runtime optimization

Test Coverage Recommendations 📋

Critical Test Cases Needed:

1. Filter optimization behavior: Test that CouldTestMatchFilter correctly identifies matching/non-matching tests
2. Discovery vs execution modes: Verify behavior differences between IsForExecution: true/false
3. Filter type compatibility: Test with different filter implementations (TreeNodeFilter, NopFilter)
4. Path building accuracy: Test that BuildPathFromMetadata produces correct paths
5. Performance regression tests: Measure time difference with/without optimization

Specific Test Scenarios:

// Test early filtering works correctly
[Test] async Task TestEarlyFilteringSkipsNonMatchingTests()

// Test discovery mode doesn't apply filter optimization  
[Test] async Task DiscoveryModeDoesNotApplyFiltering()

// Test path building matches expected format
[Test] void TestPathBuildingMatchesExpectedFormat()

// Test property condition stripping
[Test] void TestPropertyConditionStripping()

Minor Code Quality Issues 📝

1. Parameter naming inconsistency: testBuilderContext vs buildingContext in interface
2. Hardcoded regex pattern: Property condition regex could be extracted to a constant
3. Warning suppression: Consider if TPEXP usage is necessary or if there's a better approach

Overall Assessment ⭐⭐⭐⭐⭐

This is a well-designed performance optimization that:

• ✅ Follows TUnit architecture patterns and guidelines
• ✅ Maintains backward compatibility
• ✅ Provides clear performance benefits for filtered execution
• ✅ Has minimal risk of introducing bugs
• ✅ Follows modern C# coding standards
• ✅ Integrates cleanly with existing pipeline architecture

Recommendation: APPROVE - This is a solid optimization that addresses a real performance need during filtered test execution. The implementation is clean, focused, and should provide measurable performance improvements for users running specific test filters.

The early filtering optimization is particularly valuable for large test suites where users frequently run individual tests or small subsets of tests during development.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: TestBuildingContext for Optimized Test Building and Filtering

Reviewing the TestBuildingContext implementation for optimized test building and filtering.

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Review TestBuildingContext implementation
• Check code quality and best practices
• Assess performance considerations
• Review for potential bugs
• Check security concerns
• Evaluate test coverage needs
• Provide comprehensive feedback

---

Code Quality & Architecture ✅

Strengths:

• Clean, well-documented record type for TestBuildingContext with clear purpose and XML documentation
• Good separation of concerns between discovery and execution contexts
• Consistent naming and follows modern C# patterns (collection expressions, record types, nullable annotations)
• Proper integration across the pipeline: TestDiscoveryService → TestBuilderPipeline → TestBuilder

Minor Issues:

• Line 16 in ITestBuilder.cs has an inconsistent parameter name testBuilderContext vs buildingContext used elsewhere
• Inconsistent parameter naming in interface vs implementation could cause confusion

Performance Considerations ⭐⭐⭐⭐⭐

Excellent Optimizations:

• Early filtering optimization in TestBuilder.cs:122-129: Pre-filters tests before expensive building operations when in execution mode - this is a significant performance win for filtered test runs
• Conservative filter matching in CouldTestMatchFilter(): Uses lightweight path-based filtering without building full test objects
• Streaming support maintained: Passes context through streaming methods correctly
• Memory efficiency: Avoids building tests that won't match the filter, reducing memory usage

Performance Impact Analysis:

• Should provide substantial performance improvements for filtered test execution (e.g., running single test methods)
• Memory usage reduced by skipping test building for non-matching tests
• No performance regression for discovery mode (filter is null)
• Smart path-based filtering allows early exit without expensive test object creation

Implementation Analysis 🔍

TestBuildingContext Design:

• Simple, focused record with two clear responsibilities
• Boolean flag clearly separates discovery vs execution modes
• Optional filter allows for both scenarios
• Immutable record type promotes thread safety

Integration Points:

• Well-integrated across all layers of the test building pipeline
• Consistent parameter passing through all methods
• Proper handling in both streaming and non-streaming paths
• Correctly used in both reflection and source-gen scenarios

Filter Implementation Review 🔍

Path Building Logic (TestBuilder.cs:1649-1658):

• Correctly builds paths matching TestFilterService format: /AssemblyName/Namespace/ClassName/MethodName
• Handles null cases appropriately with fallback values
• Path format matches what the framework expects

Filter Matching Logic (TestBuilder.cs:1583-1643):

• Conservative approach - returns true unless definitively can rule out test
• Handles different filter types (TreeNodeFilter, NopFilter, etc.)
• Smart handling of property conditions by stripping them for path-only matching
• Uses reflection to create TreeNodeFilter instances (necessary due to internal constructor)

Potential Issues & Concerns ⚠️

Filter Type Coupling:

• CouldTestMatchFilter() method has knowledge of specific filter types
• Uses #pragma warning disable TPEXP suggesting experimental API usage
• Consider creating an interface abstraction to reduce coupling with platform-specific filter types

Property Condition Handling:

• Regex-based stripping of property conditions ([key=value]) in line 1618 is functional but brittle
• Could break if Microsoft changes the filter format
• Consider more robust parsing

Error Handling:

• Early return with empty array in line 127 could mask configuration issues
• Consider adding debug logging when tests are filtered out for troubleshooting

Security Concerns 🔒

No significant security issues identified:

• No direct user input handling in the optimization paths
• No reflection or dynamic code generation in the new context logic
• Proper separation between execution and discovery contexts
• Filter handling uses existing platform APIs safely

Dual-Mode Compliance ✅

Good compliance with TUnit's dual-mode requirements:

• This appears to be an engine-only optimization that works in reflection mode
• Source generator mode likely has its own optimizations
• The optimization is in the test building path, which is appropriate for reflection mode
• No changes needed to source generator since this is runtime optimization

Test Coverage Recommendations 📋

Critical Test Cases Needed:

1. Filter optimization behavior: Test that CouldTestMatchFilter correctly identifies matching/non-matching tests
2. Discovery vs execution modes: Verify behavior differences between IsForExecution: true/false
3. Filter type compatibility: Test with different filter implementations (TreeNodeFilter, NopFilter)
4. Path building accuracy: Test that BuildPathFromMetadata produces correct paths
5. Performance regression tests: Measure time difference with/without optimization

Specific Test Scenarios:

// Test early filtering works correctly
[Test] async Task TestEarlyFilteringSkipsNonMatchingTests()

// Test discovery mode doesn't apply filter optimization  
[Test] async Task DiscoveryModeDoesNotApplyFiltering()

// Test path building matches expected format
[Test] void TestPathBuildingMatchesExpectedFormat()

// Test property condition stripping
[Test] void TestPropertyConditionStripping()

Minor Code Quality Issues 📝

1. Parameter naming inconsistency: testBuilderContext vs buildingContext in interface
2. Hardcoded regex pattern: Property condition regex could be extracted to a constant
3. Warning suppression: Consider if TPEXP usage is necessary or if there's a better approach

Overall Assessment ⭐⭐⭐⭐⭐

This is a well-designed performance optimization that:

• ✅ Follows TUnit architecture patterns and guidelines
• ✅ Maintains backward compatibility
• ✅ Provides clear performance benefits for filtered execution
• ✅ Has minimal risk of introducing bugs
• ✅ Follows modern C# coding standards
• ✅ Integrates cleanly with existing pipeline architecture

Recommendation: APPROVE - This is a solid optimization that addresses a real performance need during filtered test execution. The implementation is clean, focused, and should provide measurable performance improvements for users running specific test filters.

The early filtering optimization is particularly valuable for large test suites where users frequently run individual tests or small subsets of tests during development.

---