Fix configuration precedence for OutputFormat and OutputPath (#38) #43
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #38: Configuration values for OutputFormat and OutputPath are now properly used when included in configuration files. Changes: - Modified CompareCommand.cs to fix configuration precedence logic - Changed OutputFormat property to nullable to detect explicit user input - Fixed JSON enum serialization in ComparisonConfiguration.cs - Added comprehensive unit tests for JSON enum parsing - Added integration tests for configuration precedence scenarios The fix ensures: - Config file values are used when no CLI override is provided - Explicit CLI arguments properly override config values - JSON enum parsing works with both 'Html' and 'html' formats - Round-trip serialization maintains enum values correctly All 425 tests passing with no regressions.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a configuration precedence issue where command line default values were incorrectly overriding configuration file values for OutputFormat and OutputPath parameters, even when users didn't explicitly provide command line arguments.
- Changed OutputFormat property to nullable to distinguish between explicit user input and automatic defaults
- Fixed JSON enum serialization to ensure consistent handling between save and load operations
- Added comprehensive test coverage for configuration precedence scenarios
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/DotNetApiDiff/Commands/CompareCommand.cs |
Fixed configuration precedence logic by making OutputFormat nullable and only overriding config values when explicitly provided |
src/DotNetApiDiff/Models/Configuration/ComparisonConfiguration.cs |
Added JsonStringEnumConverter to SaveToJsonFile method for consistent enum serialization |
tests/DotNetApiDiff.Tests/Unit/JsonEnumSerializationTests.cs |
Added comprehensive unit tests for JSON enum parsing with case variations |
tests/DotNetApiDiff.Tests/Integration/CliWorkflowTests.cs |
Added integration tests to verify configuration precedence and CLI override behavior |
tests/DotNetApiDiff.Tests/TestData/config-output-format-and-path.json |
Test configuration file with output format and path settings |
Signed-off-by: jbrinkman <[email protected]>
Fixes Windows CI failure where file paths with backslashes in JSON strings
were not properly escaped, causing JSON parsing errors.
Changes:
- Escape backslashes in outputPath interpolation for integration tests
- Use .Replace('\', '\\') to convert single backslashes to double backslashes
- Fixes both CliWorkflow tests that create JSON config files with paths
This resolves the System.Text.Json.JsonException on Windows CI builds
while maintaining compatibility with Unix-style paths on macOS/Linux.
- Fixed CliWorkflow_WithConfigFileOutputSettings_ShouldUseConfigurationValues test
- Added proper JSON escaping when replacing paths in config file content
- Use .Replace('\', '\\') to escape backslashes for Windows paths
- Resolves JSON parsing error: 'U' is an invalid escapable character
This complements the previous Windows CI fix and ensures all integration
tests work correctly on Windows by properly escaping file paths when
they are inserted into JSON configuration files.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #38: Configuration values for OutputFormat and OutputPath are now properly used when included in configuration files.
Problem
The original issue was that configuration values from JSON files were being ignored due to incorrect precedence logic. Command line default values were unconditionally overriding configuration file values, even when users didn't explicitly provide command line arguments.
Solution
Core Changes
Fixed Configuration Precedence Logic (
CompareCommand.cs):OutputFormatproperty from non-nullable with default value to nullableFixed JSON Enum Serialization (
ComparisonConfiguration.cs):JsonStringEnumConvertertoSaveToJsonFilemethodTest Coverage
Added Comprehensive Unit Tests (
JsonEnumSerializationTests.cs):LoadFromJsonFilemethod functionalityAdded Integration Tests (
CliWorkflowTests.cs):CliWorkflow_ConfigurationJsonEnumParsing_ShouldParseOutputFormatCorrectly: Verifies config values are used when no CLI overrideCliWorkflow_CommandLineOverridesConfig_ShouldUseExplicitCommandLineFormat: Verifies CLI values override config when explicitly providedBehavior
✅ Before Fix
✅ After Fix
--outputargument, it correctly overrides config file valuesTesting
Files Changed
src/DotNetApiDiff/Commands/CompareCommand.cs- Fixed precedence logicsrc/DotNetApiDiff/Models/Configuration/ComparisonConfiguration.cs- Fixed JSON serializationtests/DotNetApiDiff.Tests/Integration/CliWorkflowTests.cs- Added integration teststests/DotNetApiDiff.Tests/Unit/JsonEnumSerializationTests.cs- Added unit teststests/DotNetApiDiff.Tests/TestData/config-output-format-and-path.json- Test data