Add support for TUNIT_DISABLE_GITHUB_REPORTER environment variable

[Copilot]

This PR adds support for the new TUNIT_DISABLE_GITHUB_REPORTER environment variable while maintaining full backward compatibility with the existing DISABLE_GITHUB_REPORTER variable.

Problem

The existing DISABLE_GITHUB_REPORTER environment variable lacks the TUNIT_ prefix that follows the established naming convention used by other frameworks:

• GitHub environment variables are prefixed with GITHUB_
• .NET environment variables are prefixed with DOTNET_
• Go environment variables are prefixed with GO

Solution

Modified the GitHubReporter.IsEnabledAsync() method to check for both environment variables:

if (Environment.GetEnvironmentVariable("TUNIT_DISABLE_GITHUB_REPORTER") is not null ||
    Environment.GetEnvironmentVariable("DISABLE_GITHUB_REPORTER") is not null)
{
    return false;
}

Changes Made

• Core Implementation: Updated GitHubReporter.cs to support both environment variables with equal priority
• Test Coverage: Added comprehensive unit tests in GitHubReporterTests.cs covering all scenarios:
  • Only TUNIT_DISABLE_GITHUB_REPORTER set
  • Only DISABLE_GITHUB_REPORTER set
  • Both variables set
  • Neither variable set
• Pipeline Updates: Updated 3 pipeline test modules to use the new variable name as examples of the preferred convention

Backward Compatibility

Existing workflows using DISABLE_GITHUB_REPORTER will continue to work without any changes. Both variables have equal priority - if either is set, the GitHubReporter will be disabled.

Testing

All 4 new unit tests pass successfully, confirming the implementation works correctly for all combinations of environment variable settings.

Fixes #2599.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[claude]

Claude finished @Copilot's task —— View job

---

PR Review Complete ✅

I've thoroughly reviewed this PR and it looks excellent overall. The implementation is solid with good test coverage and follows established patterns.

🟢 Strengths

Core Implementation (GitHubReporter.cs:20-21)

• Clean logic: Simple and clear environment variable checking with both variables having equal priority
• Backward compatibility: Maintains support for existing DISABLE_GITHUB_REPORTER while adding the prefixed variant
• Consistent patterns: Follows the same pattern as other environment variable checks in the method

Test Coverage (GitHubReporterTests.cs)

• Comprehensive scenarios: Covers all combinations - new variable only, old variable only, both set, neither set
• Proper isolation: Each test properly sets up and cleans up environment variables
• Good naming: Test method names clearly describe the expected behavior
• Resource management: Proper cleanup of temporary files and environment variables

Pipeline Updates (3 modules)

• Consistent adoption: All three pipeline modules updated to use the new prefixed variable name
• Documentation by example: Shows the preferred convention for future usage

🟡 Minor Considerations

Test Cleanup Pattern

The tests use try/finally blocks for environment variable cleanup. While functional, consider using using with IDisposable wrapper for cleaner resource management:

// Current approach works but could be more elegant
Environment.SetEnvironmentVariable("TUNIT_DISABLE_GITHUB_REPORTER", "true");
try { /* test */ }
finally { Environment.SetEnvironmentVariable("TUNIT_DISABLE_GITHUB_REPORTER", null); }

Test Coverage Gap

Missing test case: What happens when GITHUB_ACTIONS is not set but the disable variables are not set? Current tests either set GITHUB_ACTIONS=null with disable variables, or don't explicitly test the normal enabled path.

🟢 Security & Performance

• No security concerns: Environment variable reading is safe
• No performance impact: Simple string null checks are negligible
• No resource leaks: Environment variables are properly cleaned up in tests

🟢 Code Quality & Best Practices

• Follows C# conventions: Proper null checking patterns with is not null
• Maintains existing patterns: Consistent with other environment variable checks in the same method
• Good documentation: PR description clearly explains the rationale and implementation
• Backward compatibility: Excellent approach to introducing the new variable without breaking existing usage

Overall Assessment

This is a well-implemented feature that properly addresses the naming convention concern while maintaining full backward compatibility. The test coverage is thorough and the implementation is clean and straightforward.

Recommendation: ✅ APPROVE - Ready to merge with no blocking issues.

---