Skip to content
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

feat(HostBuilderExtensions): Code deduplication and added/fixed tests #415

Merged
merged 5 commits into from
Jan 9, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,5 @@ The library also includes other utilities for interaction with the console. Thes
```

And more! See the [documentation](https://natemcmaster.github.io/CommandLineUtils/) for more API, such as `IConsole`, `IReporter`, and others.


106 changes: 54 additions & 52 deletions src/Hosting.CommandLine/HostBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Runtime.ExceptionServices;
using System.Threading;
using System.Threading.Tasks;
using McMaster.Extensions.CommandLineUtils;
Expand Down Expand Up @@ -57,40 +56,18 @@ public static async Task<int> RunCommandLineApplicationAsync<TApp>(
CancellationToken cancellationToken = default)
where TApp : class
{
configure ??= app => { };
var exceptionHandler = new StoreExceptionHandler();
var state = new CommandLineState(args);
hostBuilder.Properties[typeof(CommandLineState)] = state;
hostBuilder.ConfigureServices(
(context, services)
=>
(context, services) =>
{
services
.TryAddSingleton<IUnhandledExceptionHandler>(exceptionHandler);
services
.AddSingleton<IHostLifetime, CommandLineLifetime>()
.TryAddSingleton(PhysicalConsole.Singleton);
services
.AddSingleton(provider =>
{
state.SetConsole(provider.GetService<IConsole>());
return state;
})
.AddSingleton<CommandLineContext>(state)
.AddSingleton<ICommandLineService, CommandLineService<TApp>>();
services
.AddSingleton(configure);
services.AddCommonServices(state);
services.AddSingleton<ICommandLineService, CommandLineService<TApp>>();
services.AddSingleton(configure);
});

using var host = hostBuilder.Build();
await host.RunAsync(cancellationToken);

if (exceptionHandler.StoredException != null)
{
ExceptionDispatchInfo.Capture(exceptionHandler.StoredException).Throw();
}

return state.ExitCode;
return await host.RunCommandLineApplicationAsync(cancellationToken);
}

/// <summary>
Expand All @@ -110,40 +87,65 @@ public static async Task<int> RunCommandLineApplicationAsync(
Action<CommandLineApplication> configure,
CancellationToken cancellationToken = default)
{
var exceptionHandler = new StoreExceptionHandler();
var state = new CommandLineState(args);
hostBuilder.Properties[typeof(CommandLineState)] = state;
hostBuilder.ConfigureServices(
(context, services)
=>
(context, services) =>
{
services
.TryAddSingleton<IUnhandledExceptionHandler>(exceptionHandler);
services
.AddSingleton<IHostLifetime, CommandLineLifetime>()
.TryAddSingleton(PhysicalConsole.Singleton);
services
.AddSingleton(provider =>
{
state.SetConsole(provider.GetService<IConsole>());
return state;
})
.AddSingleton<CommandLineContext>(state)
.AddSingleton<ICommandLineService, CommandLineService>();
services
.AddSingleton(configure);
services.AddCommonServices(state);
services.AddSingleton<ICommandLineService, CommandLineService>();
services.AddSingleton(configure);
});

using var host = hostBuilder.Build();
return await host.RunCommandLineApplicationAsync(cancellationToken);
}

await host.RunAsync(cancellationToken);
/// <summary>
/// Configures an instance of <typeparamref name="TApp" /> using <see cref="CommandLineApplication" /> to provide
/// command line parsing on the given <paramref name="args" />.
/// </summary>
/// <typeparam name="TApp">The type of the command line application implementation</typeparam>
/// <param name="hostBuilder">This instance</param>
/// <param name="args">The command line arguments</param>
/// <param name="configure">The delegate to configure the application</param>
/// <returns><see cref="IHostBuilder"/></returns>
public static IHostBuilder UseCommandLineApplication<TApp>(
this IHostBuilder hostBuilder,
string[] args,
Action<CommandLineApplication<TApp>> configure)
where TApp : class
{
var state = new CommandLineState(args);
hostBuilder.Properties[typeof(CommandLineState)] = state;
hostBuilder.ConfigureServices(
(context, services)
=>
{
services.AddCommonServices(state);
services.AddSingleton<ICommandLineService, CommandLineService<TApp>>();
services.AddSingleton(configure);
});

if (exceptionHandler.StoredException != null)
{
ExceptionDispatchInfo.Capture(exceptionHandler.StoredException).Throw();
}
return hostBuilder;
}

return state.ExitCode;
private static void AddCommonServices(this IServiceCollection services, CommandLineState state)
{
services
.TryAddSingleton<StoreExceptionHandler>();
services
.TryAddSingleton<IUnhandledExceptionHandler>(provider => provider.GetRequiredService<StoreExceptionHandler>());
services
.AddSingleton<IHostLifetime, CommandLineLifetime>()
.TryAddSingleton(PhysicalConsole.Singleton);
services
.AddSingleton(provider =>
{
state.SetConsole(provider.GetService<IConsole>());
return state;
})
.AddSingleton<CommandLineContext>(state);
}
}
}
40 changes: 40 additions & 0 deletions src/Hosting.CommandLine/HostExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) Nate McMaster.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.Extensions.Hosting
{
using System;
using System.Runtime.ExceptionServices;
using System.Threading;
using System.Threading.Tasks;
using McMaster.Extensions.CommandLineUtils;
using McMaster.Extensions.Hosting.CommandLine.Internal;
using Microsoft.Extensions.DependencyInjection;

/// <summary>
/// Extension methods for <see cref="IHost" /> support.
/// </summary>
public static class HostExtensions
{
/// <summary>
/// Runs the app using the <see cref="CommandLineApplication" /> previously configured in
/// <see cref="HostBuilderExtensions.UseCommandLineApplication{TApp}(IHostBuilder, string[], Action{CommandLineApplication{TApp}})"/>.
/// </summary>
/// <param name="host">A program abstraction.</param>
/// <param name="cancellationToken">Propagates notification that operations should be canceled.</param>
public static async Task<int> RunCommandLineApplicationAsync(this IHost host, CancellationToken cancellationToken = default)
{
var exceptionHandler = host.Services.GetService<StoreExceptionHandler>();
var state = host.Services.GetRequiredService<CommandLineState>();

await host.RunAsync(cancellationToken);

if (exceptionHandler?.StoredException != null)
{
ExceptionDispatchInfo.Capture(exceptionHandler.StoredException).Throw();
}

return state.ExitCode;
}
}
}
3 changes: 3 additions & 0 deletions src/Hosting.CommandLine/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
#nullable enable
Microsoft.Extensions.Hosting.HostExtensions
static Microsoft.Extensions.Hosting.HostBuilderExtensions.UseCommandLineApplication<TApp>(this Microsoft.Extensions.Hosting.IHostBuilder! hostBuilder, string![]! args, System.Action<McMaster.Extensions.CommandLineUtils.CommandLineApplication<TApp!>!>! configure) -> Microsoft.Extensions.Hosting.IHostBuilder!
static Microsoft.Extensions.Hosting.HostExtensions.RunCommandLineApplicationAsync(this Microsoft.Extensions.Hosting.IHost! host, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<int>!
9 changes: 5 additions & 4 deletions test/Hosting.CommandLine.Tests/CustomValueParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ public async Task ItParsesUsingCustomParserFromConfigAction()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output)))
.RunCommandLineApplicationAsync<CustomOptionTypeCommand>(
new[] { "--custom-type", DemoOptionValue },
app => app.ValueParsers.AddOrReplace(
new CustomValueParser()));
app => app.ValueParsers.AddOrReplace(new CustomValueParser()));
Assert.Equal(0, exitCode);
}

Expand All @@ -46,7 +45,8 @@ public async Task ItParsesUsingCustomParserFromInjectedConvention()
collection.AddSingleton<IConvention, CustomValueParserConvention>();
})
.RunCommandLineApplicationAsync<CustomOptionTypeCommand>(
new[] { "--custom-type", DemoOptionValue });
new[] { "--custom-type", DemoOptionValue },
app => { });
Assert.Equal(0, exitCode);
}

Expand All @@ -56,7 +56,8 @@ public async Task ItParsesUsingCustomParserFromAttribute()
var exitCode = await new HostBuilder()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output)))
.RunCommandLineApplicationAsync<CustomOptionTypeCommandWithAttribute>(
new[] { "--custom-type", DemoOptionValue });
new[] { "--custom-type", DemoOptionValue },
app => { });
Assert.Equal(0, exitCode);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public async Task TestReturnCode()
{
var exitCode = await new HostBuilder()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output)))
.RunCommandLineApplicationAsync<Return42Command>(new string[0]);
.RunCommandLineApplicationAsync<Return42Command>(new string[0], app => { });
Assert.Equal(42, exitCode);
}

Expand All @@ -39,7 +39,7 @@ public async Task TestConsoleInjection()
console.SetupGet(c => c.Out).Returns(textWriter.Object);
await new HostBuilder()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(console.Object))
.RunCommandLineApplicationAsync<Write42Command>(new string[0]);
.RunCommandLineApplicationAsync<Write42Command>(new string[0], app => { });
Mock.Verify(console, textWriter);
}

Expand All @@ -58,7 +58,7 @@ public async Task TestConventionInjection()
.AddSingleton<IConsole>(new TestConsole(_output))
.AddSingleton(valueHolder)
.AddSingleton(convention.Object))
.RunCommandLineApplicationAsync<CaptureRemainingArgsCommand>(args);
.RunCommandLineApplicationAsync<CaptureRemainingArgsCommand>(args, app => { });
Assert.Equal(args, valueHolder.Value);
Mock.Verify(convention);
}
Expand All @@ -74,13 +74,36 @@ public async Task TestConfigureCommandLineApplication()
Assert.NotNull(commandLineApp);
}

[Fact]
public async Task TestUseCommandLineApplication()
{
CommandLineApplication<Return42Command> commandLineApp = default;
var hostBuilder = new HostBuilder();
hostBuilder.UseCommandLineApplication<Return42Command>(new string[0], app => commandLineApp = app);
var host = hostBuilder.Build();
await host.RunCommandLineApplicationAsync();
Assert.NotNull(commandLineApp);
}

[Fact]
public async Task UseCommandLineApplicationReThrowsExceptions()
{
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => new HostBuilder()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output)))
.UseCommandLineApplication<ThrowsExceptionCommand>(new string[0], app => { })
.Build()
.RunCommandLineApplicationAsync());
Assert.Equal("A test", ex.Message);
}

[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" }));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did so many of existing tests change to add app => { } to the RunCommandLineApplicationAsync call?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you've realized this too and already made the necessary changes to fix this behavior. Thanks.

.RunCommandLineApplicationAsync<ParentCommand>(new string[] { "return41" }, app => { }));
Assert.Equal(new string[] { "return42" }, ex.NearestMatches);
}

Expand All @@ -90,7 +113,7 @@ public async Task ItRethrowsThrownExceptions()
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => new HostBuilder()
.ConfigureServices(collection => collection.AddSingleton<IConsole>(new TestConsole(_output)))
.RunCommandLineApplicationAsync<ThrowsExceptionCommand>(new string[0]));
.RunCommandLineApplicationAsync<ThrowsExceptionCommand>(new string[0], app => { }));
Assert.Equal("A test", ex.Message);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public async Task TestUsingServiceProvider()
public async Task TestCommandLineContextFromNonDIContexts()
{
CommandLineContext configureServicesContext = null;
CommandLineContext confgureAppContext = null;
CommandLineContext configureAppContext = null;
await new HostBuilder()
.ConfigureServices((context, collection) =>
{
Expand All @@ -117,14 +117,14 @@ public async Task TestCommandLineContextFromNonDIContexts()
})
.ConfigureAppConfiguration((context, builder) =>
{
confgureAppContext = context.GetCommandLineContext();
configureAppContext = context.GetCommandLineContext();
})
.RunCommandLineApplicationAsync(new string[0], app => app.OnExecute(() =>
{
}));

Assert.NotNull(configureServicesContext);
Assert.NotNull(confgureAppContext);
Assert.NotNull(configureAppContext);
}
}
}