diff --git a/src/Nitro/CommandLine/src/CommandLine/Extensions/RootCommandExtensions.cs b/src/Nitro/CommandLine/src/CommandLine/Extensions/RootCommandExtensions.cs index 2335200d14d..0fef396c734 100644 --- a/src/Nitro/CommandLine/src/CommandLine/Extensions/RootCommandExtensions.cs +++ b/src/Nitro/CommandLine/src/CommandLine/Extensions/RootCommandExtensions.cs @@ -21,6 +21,13 @@ public static async Task ExecuteAsync( // Parse command var parseResult = rootCommand.Parse(args); + // Short-circuit on parse errors: let InvokeAsync report them and return the + // corresponding exit code. + if (parseResult.Errors.Count > 0) + { + return await parseResult.InvokeAsync(invocationConfiguration, cancellationToken); + } + var format = parseResult.GetValue(Opt.Instance); if (format.HasValue) diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/GlobalOptionsCommandTests.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/GlobalOptionsCommandTests.cs new file mode 100644 index 00000000000..e0e56efd2cf --- /dev/null +++ b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/GlobalOptionsCommandTests.cs @@ -0,0 +1,22 @@ +namespace ChilliCream.Nitro.CommandLine.Tests.Commands; + +public sealed class GlobalOptionsCommandTests(NitroCommandFixture fixture) : CommandTestBase(fixture) +{ + [Fact] + public async Task ExecuteAsync_InvalidOutputValue_ReturnsError() + { + // arrange + SetupNoAuthentication(); + + // act + var result = await ExecuteCommandAsync("api", "list", "--output", "wtf"); + + // assert + Assert.Equal(1, result.ExitCode); + result.StdErr.MatchInlineSnapshot( + """ + Argument 'wtf' not recognized. Must be one of: + 'json' + """); + } +} diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/InteractionMode.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/InteractionMode.cs similarity index 100% rename from src/Nitro/CommandLine/test/CommandLine.Tests/InteractionMode.cs rename to src/Nitro/CommandLine/test/CommandLine.Tests/Commands/InteractionMode.cs diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/MockErrorFactory.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/MockErrorFactory.cs similarity index 100% rename from src/Nitro/CommandLine/test/CommandLine.Tests/MockErrorFactory.cs rename to src/Nitro/CommandLine/test/CommandLine.Tests/Commands/MockErrorFactory.cs diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/NitroCommandFixture.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/NitroCommandFixture.cs similarity index 100% rename from src/Nitro/CommandLine/test/CommandLine.Tests/NitroCommandFixture.cs rename to src/Nitro/CommandLine/test/CommandLine.Tests/Commands/NitroCommandFixture.cs diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/README.md b/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/README.md deleted file mode 100644 index 2a7e048f2db..00000000000 --- a/src/Nitro/CommandLine/test/CommandLine.Tests/Commands/README.md +++ /dev/null @@ -1,227 +0,0 @@ -# Command Test Migration Guide - -Reference for migrating Nitro CLI command tests to the shared base class pattern. - -## Class Hierarchy - -``` -CommandTestBase (root - mocks, auth, file system, execution) - | - +-- SchemasCommandTestBase (schema/fusion domain) - | +-- FusionCommandTestBase - | | +-- FusionValidateCommandTests - | | +-- FusionPublishCommandTests - | +-- UploadSchemaCommandTests - | +-- PublishSchemaCommandTests - | - +-- ClientsCommandTestBase (client domain) - | +-- ShowClientCommandTests - | +-- UploadClientCommandTests - | +-- ValidateClientCommandTests - | ... -``` - -Each concrete test class inherits from a **domain base** (not `IClassFixture` directly). - -## What Goes Where - -### `CommandTestBase` (root) - -- Mock declarations for all API clients (`SchemasClientMock`, `ClientsClientMock`, `ApisClientMock`, etc.) -- File system and environment variable mocks -- Auth helpers: `SetupNoAuthentication()`, `SetupSession()`, `SetupSessionWithWorkspace()` -- `SetupInteractionMode(InteractionMode mode)` -- Command execution: `ExecuteCommandAsync(params string[] args)`, `StartInteractiveCommand(params string[] args)` -- File helpers: `SetupFile()`, `SetupDirectory()`, `SetupCreateFile()`, `SetupOpenReadStream()` -- Auto-verification of all strict mocks in `DisposeAsync()` - -### Domain Base (e.g. `ClientsCommandTestBase`) - -- **Constants** shared across all commands in the domain -- **Setup methods** for each GraphQL operation, organized in `#region` blocks -- **Payload factories** (private static) that create mock response objects -- **Error factories** (protected static) that create typed error mocks -- **Subscription event factories** (protected static) for subscription-based commands - -### Concrete Test Class - -- Test methods only -- `TheoryData` provider methods (e.g. `GetUploadClientErrors()`) -- Node/entity factory helpers specific to one command (e.g. `CreateShowClientNode()`) -- No `ClientsClientMock.Setup(...)` calls -- use base class setup methods - -## Constants - -Define shared test values as `protected const` in the domain base: - -```csharp -// CommandTestBase -protected const string ApiId = "api-1"; -protected const string Stage = "dev"; -protected const string Tag = "v1"; - -// ClientsCommandTestBase -protected const string ClientId = "client-1"; -protected const string ClientName = "web-client"; -protected const string ApiName = "products"; -protected const string RequestId = "request-1"; -protected const string OperationsFile = "operations.json"; -``` - -Use these instead of hardcoded strings in test arguments and setup calls. - -## Setup Method Naming - -Follow these conventions: - -| Pattern | Purpose | -|---|---| -| `SetupMutation(params errors)` | Set up a mutation mock. No errors = success payload. | -| `SetupMutationException()` | Mutation throws `InvalidOperationException`. | -| `SetupMutationNull()` | Mutation returns null for a normally-present field. | -| `SetupQuery(result)` | Set up a query mock with a specific result. | -| `SetupQueryException()` | Query throws `InvalidOperationException`. | -| `SetupSubscription(params events)` | Set up a subscription event stream. | - -Examples from `ClientsCommandTestBase`: - -```csharp -// Mutation: success (no errors) or failure (with errors) -protected void SetupUploadClientMutation( - params IUploadClient_UploadClient_Errors[] errors) { ... } - -// Mutation: throws -protected void SetupUploadClientMutationException() { ... } - -// Mutation: null result -protected void SetupUploadClientMutationNullClientVersion() { ... } - -// Query -protected void SetupShowClientQuery(IShowClientCommandQuery_Node? result) { ... } - -// Subscription -protected void SetupValidateClientSubscription( - params IOnClientVersionValidationUpdated_OnClientVersionValidationUpdate[] events) { ... } -``` - -Exception setup methods always throw `InvalidOperationException("Something unexpected happened.")`. - -## Test Naming - -Every test class should have these standard tests: - -| Test Name | Type | Purpose | -|---|---|---| -| `Help_ReturnsSuccess` | `[Fact]` | Validates `--help` output | -| `NoSession_Or_ApiKey_ReturnsError(mode)` | `[Theory]` all 3 modes | Auth required | -| `DoesNotExist_ReturnsError(mode)` | `[Theory]` all 3 modes | Input file validation | -| `Throws_ReturnsError` | `[Fact]` | Exception handling (default NonInteractive) | -| `HasErrors_ReturnsError(error, msg)` | `[Theory]` with `TheoryData` | Typed GraphQL errors | -| `ReturnsNull_ReturnsError` | `[Fact]` | Null result handling | -| `_ReturnsSuccess` | `[Fact]` | Happy path | - -### Key Rules - -- **One `Throws` test per operation.** Don't test GraphQLException, AuthorizationException, HttpRequestException separately. One `[Fact]` with `InvalidOperationException` covers the error-handling path. -- **One `HasErrors` Theory per operation.** Don't split by interaction mode. Default NonInteractive mode, assert both `StdOut` and `StdErr`. Only include errors whose output is a simple string message. Rich errors with structured output (e.g. persisted query validation errors, breaking change trees) should be tested as individual `[Fact]` tests with their own snapshot assertions, not as TheoryData rows. -- **One test for null results.** No NonInteractive/Interactive variants. -- **`*DoesNotExist` tests are Theories** with all 3 interaction modes. -- **No interaction mode variants** for error/throw/null tests. Only test the default mode. - -## Error TheoryData - -Use `TheoryData` with base class error factory methods: - -```csharp -public static TheoryData - GetUploadClientErrors() => new() -{ - { CreateUploadClientUnauthorizedError(), "Unauthorized." }, - { CreateUploadClientClientNotFoundError(), $"Client '{ClientId}' was not found." }, - { CreateUploadClientDuplicatedTagError(), $"Tag '{Tag}' already exists." }, - { CreateUploadClientConcurrentOperationError(), "A concurrent operation is in progress." }, -}; -``` - -Error factory methods live in the domain base class, organized in `#region Error Factories -- `. - -## Subscription Tests - -For commands with subscriptions (validate, publish), set up mutation and subscription separately: - -```csharp -// arrange -SetupValidateClientMutation(); // success payload with RequestId -SetupValidateClientSubscription( - CreateClientVersionValidationOperationInProgressEvent(), - CreateClientVersionValidationInProgressEvent(), - CreateClientVersionValidationSuccessEvent()); - -// act -var result = await ExecuteCommandAsync("client", "validate", ...); -``` - -Event factory methods are in the domain base (`CreateClientVersionValidationSuccessEvent()`, etc.). - -Subscription setup uses `SetupSequence` internally for replay support. - -## Interactive Tests - -For commands with prompts (create, delete), use `StartInteractiveCommand()`: - -```csharp -SetupSessionWithWorkspace(); -SetupInteractionMode(InteractionMode.Interactive); -SetupSelectApisPrompt((ApiId, ApiName)); - -var command = StartInteractiveCommand("client", "create"); -command.SelectOption(0); // select API -command.Input(ClientName); // enter name -var result = await command.RunToCompletionAsync(); - -result.AssertSuccess(); -``` - -## Region Organization in Domain Base - -```csharp -// Setup methods grouped by operation -#region Create -#region Delete -#region Show -#region Upload -#region Validate -#region Publish -#region Unpublish -#region List -#region Download - -// Subscription event factories -#region Subscription Event Factories -- Client Version Publish -#region Subscription Event Factories -- Client Version Validation - -// Error factories per operation -#region Error Factories -- UploadClient -#region Error Factories -- PublishClientVersion -#region Error Factories -- ValidateClientVersion - -// Payload factories (private) -#region Payload Factories -``` - -## Migration Checklist - -When migrating a command's tests: - -1. **Create or extend the domain base class** inheriting from `CommandTestBase` -2. **Change the test class** from `IClassFixture` to `(fixture)` -3. **Add constants** for shared values to the domain base -4. **Move all `*ClientMock.Setup(...)` calls** into base class setup methods -5. **Move payload/error/event factories** into the base class -6. **Replace `new CommandBuilder(fixture)...`** with `ExecuteCommandAsync(...)` or `StartInteractiveCommand(...)` -7. **Collapse `ClientThrows*` test pairs** into one `Throws_ReturnsError` `[Fact]` -8. **Collapse error test pairs** (NonInteractive + Interactive) into one `[Theory]` with `TheoryData` -9. **Collapse null-result test pairs** into one `[Fact]` -10. **Remove `*.VerifyAll()` calls** -- base `DisposeAsync()` handles this -11. **Remove local helpers** that are now in the base -12. **Clean up using directives** diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/HttpClient/NitroClientRegistrationTests.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/GlobalOptionsTests.cs similarity index 78% rename from src/Nitro/CommandLine/test/CommandLine.Tests/HttpClient/NitroClientRegistrationTests.cs rename to src/Nitro/CommandLine/test/CommandLine.Tests/GlobalOptionsTests.cs index e6e30a3c6b1..39e4253a6c9 100644 --- a/src/Nitro/CommandLine/test/CommandLine.Tests/HttpClient/NitroClientRegistrationTests.cs +++ b/src/Nitro/CommandLine/test/CommandLine.Tests/GlobalOptionsTests.cs @@ -20,18 +20,18 @@ using Moq; using Spectre.Console.Testing; -namespace ChilliCream.Nitro.CommandLine.Tests.HttpClient; +namespace ChilliCream.Nitro.CommandLine.Tests; -public class NitroClientRegistrationTests +public class GlobalOptionsTests { [Fact] public async Task ExecuteAsync_Should_ConfigureApiKeyAuth_When_ApiKeyOptionProvided() { - // Act + // act await using var provider = await BuildAndExecuteAsync(["--api-key", "my-key"]); using var client = CreateApiClient(provider); - // Assert + // assert var apiKeyHeader = Assert.Single(client.DefaultRequestHeaders.GetValues("CCC-api-key")); Assert.Equal("my-key", apiKeyHeader); } @@ -39,14 +39,14 @@ public async Task ExecuteAsync_Should_ConfigureApiKeyAuth_When_ApiKeyOptionProvi [Fact] public async Task ExecuteAsync_Should_ConfigureBearerAuth_When_SessionPresent() { - // Arrange + // arrange var session = CreateSessionWithTokens(accessToken: "my-token"); - // Act + // act await using var provider = await BuildAndExecuteAsync([], session); using var client = CreateApiClient(provider); - // Assert + // assert var authHeader = Assert.Single(client.DefaultRequestHeaders.GetValues("Authorization")); Assert.Equal("Bearer my-token", authHeader); } @@ -54,14 +54,14 @@ public async Task ExecuteAsync_Should_ConfigureBearerAuth_When_SessionPresent() [Fact] public async Task ExecuteAsync_Should_PreferApiKey_Over_SessionToken() { - // Arrange + // arrange var session = CreateSessionWithTokens(accessToken: "session-token"); - // Act + // act await using var provider = await BuildAndExecuteAsync(["--api-key", "cli-key"], session); using var client = CreateApiClient(provider); - // Assert + // assert var apiKeyHeader = Assert.Single(client.DefaultRequestHeaders.GetValues("CCC-api-key")); Assert.Equal("cli-key", apiKeyHeader); Assert.False(client.DefaultRequestHeaders.Contains("Authorization")); @@ -78,26 +78,26 @@ public async Task ExecuteAsync_Should_PreferApiKey_Over_SessionToken() [InlineData("https://custom.host.com/graphql?foo=bar", "https://custom.host.com/graphql")] public async Task ExecuteAsync_Should_NormalizeCloudUrl(string input, string expected) { - // Act + // act await using var provider = await BuildAndExecuteAsync( ["--api-key", "x", "--cloud-url", input]); using var client = CreateApiClient(provider); - // Assert + // assert Assert.Equal(new Uri(expected), client.BaseAddress); } [Fact] public async Task ExecuteAsync_Should_UseSessionUrl_When_NoExplicitUrl() { - // Arrange + // arrange var session = CreateSessionWithTokens(apiUrl: "session-api.chillicream.com"); - // Act + // act await using var provider = await BuildAndExecuteAsync([], session); using var client = CreateApiClient(provider); - // Assert + // assert Assert.Equal( new Uri("https://session-api.chillicream.com/graphql"), client.BaseAddress); @@ -106,11 +106,11 @@ public async Task ExecuteAsync_Should_UseSessionUrl_When_NoExplicitUrl() [Fact] public async Task ExecuteAsync_Should_UseDefaultUrl_When_NoSessionAndNoExplicitUrl() { - // Act + // act await using var provider = await BuildAndExecuteAsync(["--api-key", "x"]); using var client = CreateApiClient(provider); - // Assert + // assert Assert.Equal( new Uri("https://api.chillicream.com/graphql"), client.BaseAddress); @@ -119,18 +119,55 @@ public async Task ExecuteAsync_Should_UseDefaultUrl_When_NoSessionAndNoExplicitU [Fact] public async Task ExecuteAsync_Should_NotSetAuth_When_NoAuthAvailable() { - // Act + // act await using var provider = await BuildAndExecuteAsync([]); using var client = CreateApiClient(provider); - // Assert + // assert Assert.False(client.DefaultRequestHeaders.Contains("CCC-api-key")); Assert.False(client.DefaultRequestHeaders.Contains("Authorization")); } + [Theory] + [InlineData("NITRO_TEST_VAR")] + [InlineData("BARISTA_TEST_VAR")] + public async Task DefaultFromEnvironmentValue_Should_ReadFromProvider_When_EnvironmentVariableIsSet( + string environmentVariableName) + { + // arrange + const string expectedValue = "my-test-value"; + var envProviderMock = new Mock(); + envProviderMock + .Setup(x => x.GetEnvironmentVariable(environmentVariableName)) + .Returns(expectedValue); + + string? capturedValue = null; + var testOption = new Option("--test-var"); + testOption.DefaultFromEnvironmentValue("TEST_VAR"); + + // act + await using var provider = await BuildAndExecuteAsync( + [], + environmentVariables: envProviderMock.Object, + configureProbeCommand: command => + { + command.Options.Add(testOption); + command.SetAction((parseResult, _) => + { + capturedValue = parseResult.GetValue(testOption); + return Task.FromResult(0); + }); + }); + + // assert + Assert.Equal(expectedValue, capturedValue); + } + private static async Task BuildAndExecuteAsync( string[] args, - Session? session = null) + Session? session = null, + IEnvironmentVariableProvider? environmentVariables = null, + Action? configureProbeCommand = null) { var services = new ServiceCollection(); services.AddNitroServices(); @@ -142,6 +179,11 @@ private static async Task BuildAndExecuteAsync( sessionMock.SetupGet(x => x.Session).Returns(session); services.Replace(ServiceDescriptor.Singleton(sessionMock.Object)); + if (environmentVariables is not null) + { + services.Replace(ServiceDescriptor.Singleton(environmentVariables)); + } + services .AddSingleton(Mock.Of()) .AddSingleton(Mock.Of()) @@ -167,7 +209,7 @@ private static async Task BuildAndExecuteAsync( new NitroConsole( testConsole, errorConsole, - new EnvironmentVariableProvider(), + environmentVariables ?? new EnvironmentVariableProvider(), new SnapshotActivitySinkFactory())); var provider = services.BuildServiceProvider(); @@ -176,6 +218,7 @@ private static async Task BuildAndExecuteAsync( var probeCommand = new Command("__probe"); probeCommand.AddGlobalNitroOptions(); probeCommand.SetAction((_, _) => Task.FromResult(0)); + configureProbeCommand?.Invoke(probeCommand); rootCommand.Add(probeCommand); var invocationConfig = new InvocationConfiguration diff --git a/src/Nitro/CommandLine/test/CommandLine.Tests/Options/EnvironmentVariableDefaultTests.cs b/src/Nitro/CommandLine/test/CommandLine.Tests/Options/EnvironmentVariableDefaultTests.cs deleted file mode 100644 index 2b03b9078b6..00000000000 --- a/src/Nitro/CommandLine/test/CommandLine.Tests/Options/EnvironmentVariableDefaultTests.cs +++ /dev/null @@ -1,80 +0,0 @@ -using System.CommandLine; -using ChilliCream.Nitro.Client; -using ChilliCream.Nitro.CommandLine.Services; -using ChilliCream.Nitro.CommandLine.Services.Sessions; -using ChilliCream.Nitro.CommandLine.Tests.Console; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.DependencyInjection.Extensions; -using Moq; -using Spectre.Console.Testing; - -namespace ChilliCream.Nitro.CommandLine.Tests.Options; - -public class EnvironmentVariableDefaultTests -{ - [Fact] - public async Task DefaultFromEnvironmentValue_Should_ReadFromProvider_When_EnvironmentVariableIsSet() - { - // Arrange - const string expectedValue = "my-test-value"; - var envProviderMock = new Mock(); - envProviderMock - .Setup(x => x.GetEnvironmentVariable("NITRO_TEST_VAR")) - .Returns(expectedValue); - - string? capturedValue = null; - - var services = new ServiceCollection(); - services.AddNitroServices(); - - var sessionMock = new Mock(); - sessionMock - .Setup(x => x.LoadSessionAsync(It.IsAny())) - .ReturnsAsync((Session?)null); - sessionMock.SetupGet(x => x.Session).Returns((Session?)null); - services.Replace(ServiceDescriptor.Singleton(sessionMock.Object)); - services.Replace(ServiceDescriptor.Singleton(envProviderMock.Object)); - - services.AddSingleton(); - services.AddSingleton( - sp => sp.GetRequiredService()); - - var testConsole = new TestConsole(); - var errorConsole = new TestConsole(); - services.AddSingleton( - new NitroConsole( - testConsole, - errorConsole, - envProviderMock.Object, - new SnapshotActivitySinkFactory())); - - await using var provider = services.BuildServiceProvider(); - - var testOption = new Option("--test-var"); - testOption.DefaultFromEnvironmentValue("TEST_VAR"); - - var rootCommand = new NitroRootCommand(); - var probeCommand = new Command("__probe"); - probeCommand.Options.Add(testOption); - probeCommand.AddGlobalNitroOptions(); - probeCommand.SetAction((parseResult, _) => - { - capturedValue = parseResult.GetValue(testOption); - return Task.FromResult(0); - }); - rootCommand.Add(probeCommand); - - var invocationConfig = new InvocationConfiguration - { - Output = TextWriter.Null, - Error = TextWriter.Null - }; - - // Act - await rootCommand.ExecuteAsync( - ["__probe"], provider, invocationConfig, CancellationToken.None); - - // Assert - Assert.Equal(expectedValue, capturedValue); - } -}