-
-
Notifications
You must be signed in to change notification settings - Fork 109
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.
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
Conversation
Co-authored-by: thomhurst <[email protected]>
Co-authored-by: thomhurst <[email protected]>
--junit-output-path CLI argument not being respected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where the --junit-output-path CLI argument was being ignored by the JUnit reporter. The issue occurred because IsEnabledAsync() unconditionally overwrote _outputPath after SetOutputPath() had already been called with the CLI argument value.
Key Changes:
- Added a guard condition in
JUnitReporter.IsEnabledAsync()to only set_outputPathfrom environment variable or default when it hasn't been already set via CLI argument - Changed SDK version in global.json from 10.0.101 to 10.0.100
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| TUnit.Engine/Reporters/JUnitReporter.cs | Added string.IsNullOrEmpty(_outputPath) guard to preserve CLI argument value and establish correct priority order: CLI arg → env var → default path |
| global.json | Downgraded .NET SDK version from 10.0.101 to 10.0.100 (not mentioned in PR description) |
| // Determine output path (only if not already set via command-line argument) | ||
| if (string.IsNullOrEmpty(_outputPath)) | ||
| { | ||
| _outputPath = Environment.GetEnvironmentVariable("JUNIT_XML_OUTPUT_PATH") | ||
| ?? GetDefaultOutputPath(); | ||
| } |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
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:
- CLI argument (via SetOutputPath) takes precedence over environment variable
- CLI argument takes precedence over default path
- 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.
| { | ||
| "sdk": { | ||
| "version": "10.0.101", | ||
| "version": "10.0.100", |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
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.
The
--junit-output-pathcommand-line argument was being ignored.IsEnabledAsync()unconditionally overwrote_outputPathafterSetOutputPath()had already been called with the CLI argument value.Changes
TUnit.Engine/Reporters/JUnitReporter.cs: Guard_outputPathassignment inIsEnabledAsync()to only set when emptyPriority order now correct:
--junit-output-path)JUNIT_XML_OUTPUT_PATH)TestResults/{AssemblyName}-junit.xml)Original prompt
junit-output-pathnot working properly #4095💡 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.