Fix GitHub reporter test state handling by including ErrorTestNodeStateProperty in status checks

[thomhurst]

Fixes #2967

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Progress

• Read and analyze the changed file
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage implications
• Provide comprehensive feedback

Code Review Findings

This PR fixes GitHub reporter test state handling by properly including ErrorTestNodeStateProperty in status checks. The changes are focused and well-implemented.

✅ Code Quality & Best Practices

• Clean, consistent use of pattern matching with or expressions
• Proper consolidation of similar property checks
• Good simplification of the state property selection logic (TUnit.Engine/Reporters/GitHubReporter.cs:183)
• Follows existing code patterns and conventions

✅ Bug Fix Analysis

The fix addresses a clear gap where ErrorTestNodeStateProperty was inconsistently handled:

• Lines 107-108: Now correctly includes ErrorTestNodeStateProperty in failed test count
• Lines 241-242: Properly handles error states in details formatting
• Lines 269-270: Adds missing error case in exception extraction
• Line 282: Ensures error states show as "Failed" in status

✅ Performance Considerations

• Improved efficiency: The simplified state property lookup (TUnit.Engine/Reporters/GitHubReporter.cs:183) using TestNodeStateProperty base type is more efficient than multiple || checks
• No performance regression: Changes maintain O(1) property checks
• Memory neutral: No additional allocations introduced

✅ Security Assessment

• No security concerns identified
• Exception handling remains safe with null-coalescing operators
• No exposure of sensitive information

⚠️ Test Coverage Considerations

• Missing test coverage: This fix suggests there may be gaps in test coverage for ErrorTestNodeStateProperty scenarios
• Recommendation: Add unit tests covering error state reporting to prevent regression
• Integration tests: Consider testing the complete GitHub reporter flow with error states

✅ Implementation Correctness

The fix ensures behavioral consistency across all error handling paths. Previously, error states might have been:

• Excluded from failed count (TUnit.Engine/Reporters/GitHubReporter.cs:107)
• Missing from detailed error formatting (TUnit.Engine/Reporters/GitHubReporter.cs:241)
• Lacking proper exception extraction (TUnit.Engine/Reporters/GitHubReporter.cs:266)
• Not showing correct status (TUnit.Engine/Reporters/GitHubReporter.cs:279)

🎯 Overall Assessment

This is a high-quality fix that addresses a real bug with minimal, targeted changes. The code improvements actually enhance both correctness and performance.

Recommendation: ✅ Approve - This PR effectively fixes the issue and improves the codebase.

---