feat(HostBuilderExtensions): Code deduplication and added/fixed tests #415
Conversation
Issues of test coverage and code duplication have been addressed. Closes #408
| [Fact] | ||
| public async Task ItThrowsOnUnknownSubCommand() | ||
| { | ||
| var ex = await Assert.ThrowsAsync<UnrecognizedCommandParsingException>( | ||
| () => new HostBuilder() | ||
| .ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output))) | ||
| .RunCommandLineApplicationAsync<ParentCommand>(new string[] { "return41" })); |
There was a problem hiding this comment.
Why did so many of existing tests change to add app => { } to the RunCommandLineApplicationAsync call?
There was a problem hiding this comment.
I'm going to add a change to this PR to preserve the existing behavior. While not technically a binary breaking change, requiring this extra parameter would probably some users upgrading.
There was a problem hiding this comment.
Hey @natemcmaster - Thanks for taking the time to review this PR.
This was because there was code flagged as unreachable that I removed in several of the extension methods. Here's an example.
After reviewing, I am now realizing that this is seemingly a breaking change and I shouldn't have made it.
There was a problem hiding this comment.
It looks like you've realized this too and already made the necessary changes to fix this behavior. Thanks.
|
Thanks for the contribution, @cbcrouse ! |
|
Of course @natemcmaster, very much looking forward to this change. Thanks @hellfirehd for getting it started! |
|
Thanks for seeing this through! I really appreciate it! |
Continuation of Issue 408
There's another PR out there, but I'm continuing this work since they are not able to continue it.