Skip to content

Conversation

@jbrinkman
Copy link
Owner

Problem

The CLI was failing with the error:

Could not resolve type 'DotNetApiDiff.Commands.CompareCommandSettings'.
System.MissingMethodException: Cannot dynamically create an instance of type 'DotNetApiDiff.Commands.CompareCommandSettings'. Reason: No parameterless constructor defined.

This was caused by the required keyword on properties in CompareCommandSettings, which prevents Spectre.Console.Cli from creating instances via the parameterless constructor.

Root Cause Analysis

The issue wasn't caught by existing tests because:

  1. Unit tests bypassed the CLI layer - They created CompareCommandSettings objects directly using object initializers
  2. Integration tests were silently skipping - They had extensive skip logic that caused them to pass without actually testing the CLI
  3. Integration tests used incorrect syntax - They called the CLI without the required compare command

Solution

Fixed the Constructor Issue

  • Removed required keyword from CompareCommandSettings properties
  • Made properties nullable and added proper validation in the Validate method
  • Added null-forgiving operators in Execute method since validation ensures non-null values

Improved Integration Tests

  • Fixed all CLI calls to use correct compare command syntax
  • Added proper test categories [Trait("Category", "Integration")]
  • Fixed path resolution in FindExecutablePath method (5 levels up instead of 4)
  • Replaced silent test skipping with proper assertions that fail when prerequisites aren't met
  • Tests now properly detect issues instead of silently passing

Testing

  • ✅ All 345 tests pass (318 unit + 27 integration)
  • ✅ Integration tests can be run with task test:integration
  • ✅ CLI now works correctly: dotnet run -- compare source.dll target.dll
  • ✅ Original failing command now works properly

Files Changed

  • src/DotNetApiDiff/Commands/CompareCommand.cs - Fixed constructor issue
  • tests/DotNetApiDiff.Tests/Integration/CliWorkflowTests.cs - Fixed CLI syntax and test robustness
  • tests/DotNetApiDiff.Tests/Integration/ConfigurationWorkflowTests.cs - Added test category

This fix ensures the CLI works correctly and that integration tests will catch similar issues in the future.

…e integration tests

- Remove 'required' keyword from CompareCommandSettings properties to allow Spectre.Console.Cli to instantiate the class
- Add proper null validation in the Validate method instead
- Fix integration tests to use correct 'compare' command syntax
- Add proper test categories [Trait("Category", "Integration")] to integration tests
- Fix path resolution in FindExecutablePath method (5 levels up instead of 4)
- Replace silent test skipping with proper assertions that fail when prerequisites aren't met
- All 345 tests now pass including 27 integration tests

Fixes the CLI error: 'Could not resolve type CompareCommandSettings - No parameterless constructor defined'
- Replace async process output handling with synchronous ReadToEnd() to avoid StringBuilder issues in CI
- Fix path resolution for dotnet run command (5 levels up instead of 4)
- Update test assertions to expect exit code 2 (breaking changes detected) as normal for test assemblies
- Combine stdout and stderr for error message assertions since validation errors may go to either stream
- Add format-specific validation for JSON and Markdown output tests
- All 27 integration tests now pass both locally and should pass in CI

Fixes CI pipeline failures:
- CliWorkflow_WithDifferentOutputFormats_ShouldSucceed JSON test
- CliWorkflow_WithNamespaceFiltering_ShouldApplyFilters StringBuilder exception
- CliWorkflow_WithNonExistentTargetAssembly_ShouldFail error message assertion
- CliWorkflow_WithInvalidOutputFormat_ShouldFail error message assertion
@jbrinkman jbrinkman merged commit de6ffa4 into main Jul 25, 2025
7 checks passed
@jbrinkman jbrinkman deleted the bug/CompareCommandSettings-constructor branch July 25, 2025 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants