Skip to content

Fix: --junit-output-path CLI argument not being respected #4096

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.

Sign up for GitHub

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

Merged
thomhurst merged 3 commits into from
Dec 17, 2025
Merged

Fix: --junit-output-path CLI argument not being respected #4096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09d8306
Initial plan
Copilot Dec 17, 2025
ff6ab6a
Initial plan for fixing junit-output-path command line argument
Copilot Dec 17, 2025
77c3678
Fix: Check if _outputPath is set before assigning in IsEnabledAsync
Copilot Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions TUnit.Engine/Reporters/JUnitReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ public async Task<bool> IsEnabledAsync()
return false;
}

// Determine output path
_outputPath = Environment.GetEnvironmentVariable("JUNIT_XML_OUTPUT_PATH")
?? GetDefaultOutputPath();
// Determine output path (only if not already set via command-line argument)
if (string.IsNullOrEmpty(_outputPath))
{
_outputPath = Environment.GetEnvironmentVariable("JUNIT_XML_OUTPUT_PATH")
?? GetDefaultOutputPath();
}
Comment on lines +35 to +40
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix correctly preserves CLI argument values, but there's no test coverage for this priority ordering behavior. The existing tests in TUnit.Engine.Tests/JUnitReporterTests.cs verify environment variable behavior but don't test the interaction between SetOutputPath() and IsEnabledAsync().

Consider adding a test case that verifies:

  1. CLI argument (via SetOutputPath) takes precedence over environment variable
  2. CLI argument takes precedence over default path
  3. Environment variable takes precedence over default path when CLI argument is not set

This would ensure the priority order (CLI > env var > default) is maintained in future changes.

Copilot uses AI. Check for mistakes.

_isEnabled = true;
return await extension.IsEnabledAsync();
Expand Down
2 changes: 1 addition & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"sdk": {
"version": "10.0.101",
"version": "10.0.100",
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SDK version has been changed from "10.0.101" to "10.0.100", but this change is not mentioned or explained in the PR description. This appears to be a downgrade from patch version 101 to 100.

If this change is intentional and related to the fix, please document why it's necessary. If it's unrelated to the bug fix, consider reverting this change or explaining the rationale in the PR description to ensure reviewers understand why it's included.

Copilot uses AI. Check for mistakes.
"rollForward": "latestMajor",
"allowPrerelease": true
},
Expand Down
Loading