-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/add-fail-comparer-options #14
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
This comment has been minimized.
This comment has been minimized.
Test Summary196 tests 196 ✅ 0s ⏱️ Results for commit 907438d. ♻️ This comment has been updated with latest results. |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🚀 Stryker report generated 🚀 |
|
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 adds two new error-handling CLI options to the compare command, enabling configurable exit codes for warnings and threshold hits, updates the command implementation, tests, and documentation accordingly.
- Introduce
--fail-on-warningsand--fail-on-threshold-hitoptions and wire them intoComparerCommand.Execute - Update existing tests and add new tests for the error-handling behavior
- Refresh README and constants to reflect new exit codes and usage
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Options/FailOnWarningsOption.cs & FailOnThresholdHitOption.cs | New option classes defining the two error-handling flags |
| src/Commands/ComparerCommand.cs | Extend Execute signature and implement exit-code logic |
| src/Models/Constants.cs | Define new exit-code constants for warnings and threshold hits |
| tests/**/*ToolCommandsTest.cs & OptionsTests.cs/etc. | Update expected option counts, invoke new flags in tests |
| tests/**/*ComparerCommandTests.cs | Extend command invocation in existing tests; missing combined scenario |
| README.md | Add documentation for new error-handling options and exit codes |
Comments suppressed due to low confidence (4)
README.md:186
- The description for
--fail-on-warningsis incorrect: it should mention exiting on warnings rather than thresholds.
* (`-fw`, `--fail-on-warnings`): Exit with error code when any threshold is hit during comparison. **[default: disabled]**
README.md:255
- The exit-code values listed here (1 and 2) don’t match the constants (
WARNING = 2,THRESHOLD_HIT = 3). Update to align with the actual codes.
* **1**: Warnings detected (when `--fail-on-warnings` is enabled)
tests/PowerUtils.BenchmarkDotnet.Reporter.Tests/Commands/ComparerCommandTests.cs:933
- Add a test covering the scenario where both
--fail-on-warningsand--fail-on-threshold-hitare enabled and both warnings and threshold hits occur, to verify that warnings take priority.
public void When_Comparison_Generate_Warning_And_FailOnWarnings_Is_False_Should_Return_Success_ExitCode()
tests/PowerUtils.BenchmarkDotnet.Reporter.Tests/Options/OptionsTests.cs:136
- [nitpick] Two statements are on the same line—split into separate lines for readability.
var compareCommand = toolCommands.Subcommands.Single(c => c.Name == "compare"); var option = compareCommand.Options.Single(o => o.Name == "--fail-on-warnings");
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



Added error handling options to improve the experience in CI/CD
Checklist before requesting a review