From 7eaacc8c6d48589a0f73198cb013d9bf804e9807 Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Sun, 10 May 2026 14:16:28 +0100 Subject: [PATCH 1/2] fix(cli): propagate handler exit code to process exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Top-level `await builder.RunAsync()` in Program.cs discarded the int the CLI host returned, so the program compiled as `Task Main` and the OS always saw exit code 0 — even when a handler returned non-zero after the API responded with 400/409/422. Scripts and E2E tests silently passed when they should have failed. Surgical fix: return the int from RunAsync. Defensive fix in CliHost so future consumers can't reintroduce the same mistake: every code path now mirrors the exit code into Environment.ExitCode before returning, which is honored even when the caller's Main returns void/Task. Regression tests in CliExitCodeTests launch the real CLI binary and assert the process exits non-zero for: missing required option, HTTP 400 HttpValidationProblemDetails, HTTP 409 ProblemDetails, HTTP 422 ProblemDetails. The test project gets a RepositoryRoot AssemblyMetadata attribute so the tests can locate the CLI csproj — same pattern E2E uses. --- src/GroundControl.Cli/Program.cs | 2 +- src/GroundControl.Host.Cli/CliHost.cs | 16 +- .../CliExitCodeTests.cs | 259 ++++++++++++++++++ .../GroundControl.Cli.Tests.csproj | 7 + 4 files changed, 278 insertions(+), 6 deletions(-) create mode 100644 tests/GroundControl.Cli.Tests/CliExitCodeTests.cs diff --git a/src/GroundControl.Cli/Program.cs b/src/GroundControl.Cli/Program.cs index e481f95b..84f09439 100644 --- a/src/GroundControl.Cli/Program.cs +++ b/src/GroundControl.Cli/Program.cs @@ -12,4 +12,4 @@ builder.UseDependencyModule(); builder.Services.AddSingleton(new CredentialStore(CredentialStore.DefaultPath)); -await builder.RunAsync(); +return await builder.RunAsync(); diff --git a/src/GroundControl.Host.Cli/CliHost.cs b/src/GroundControl.Host.Cli/CliHost.cs index 55e82079..dd1a897c 100644 --- a/src/GroundControl.Host.Cli/CliHost.cs +++ b/src/GroundControl.Host.Cli/CliHost.cs @@ -40,14 +40,14 @@ public async Task RunAsync() if (_error is not null) { AnsiConsole.MarkupLine($":thumbs_down: {_error}"); - return 1; + return SetExitCode(1); } Debug.Assert(_parseResult != null, nameof(_parseResult) + " != null"); if (_applicationHost is null) { - return await _parseResult.InvokeAsync(); + return SetExitCode(await _parseResult.InvokeAsync()); } try @@ -60,11 +60,11 @@ public async Task RunAsync() providerField?.SetValue(_parseResult.CommandResult.Command, _applicationHost.Services); var invocationConfiguration = new InvocationConfiguration { EnableDefaultExceptionHandler = false }; - return await _parseResult.InvokeAsync(invocationConfiguration); + return SetExitCode(await _parseResult.InvokeAsync(invocationConfiguration)); } catch (Exception ex) when (ex is TaskCanceledException or OperationCanceledException) { - return 0; + return SetExitCode(0); } catch (Exception ex) { @@ -84,9 +84,15 @@ public async Task RunAsync() } shell.DisplayEmptyLine(); - return 1; + return SetExitCode(1); } } + private static int SetExitCode(int exitCode) + { + Environment.ExitCode = exitCode; + return exitCode; + } + internal static CliHost CreateError(string error) => new(error); } \ No newline at end of file diff --git a/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs b/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs new file mode 100644 index 00000000..187deeca --- /dev/null +++ b/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs @@ -0,0 +1,259 @@ +using System.Diagnostics; +using System.Net; +using System.Reflection; +using System.Text; +using System.Text.Json; + +namespace GroundControl.Cli.Tests; + +/// +/// Locks in the contract that a CLI handler returning a non-zero exit code surfaces +/// to the OS as a non-zero process exit code. Regression coverage for the bug where +/// `await builder.RunAsync()` in Program.cs silently dropped the int, leaving the +/// process exit at 0 even when the API returned 400/409/422 and the handler returned 1. +/// +public sealed class CliExitCodeTests +{ + private static readonly string RepositoryRoot = ResolveRepositoryRoot(); + private static readonly string CliProjectPath = Path.Combine(RepositoryRoot, "src", "GroundControl.Cli", "GroundControl.Cli.csproj"); + + [Fact] + public async Task Cli_HandlerReturnsNonZero_ProcessExitCodeIsNonZero() + { + // Arrange: scope create with --no-interactive but no --dimension/--values forces + // CreateScopeHandler to print an error and return 1 without any HTTP traffic. + var args = new[] { "scope", "create", "--no-interactive", "--output", "json" }; + + // Act + var result = await RunCliAsync(args, serverUrl: null, TestContext.Current.CancellationToken); + + // Assert + result.ExitCode.ShouldBe(1, $"stdout:\n{result.Stdout}\nstderr:\n{result.Stderr}"); + (result.Stdout + result.Stderr).ShouldContain("Missing required option"); + } + + [Fact] + public async Task Cli_ApiReturns400_ProcessExitCodeIsOne() + { + // Arrange + using var server = StubHttpServer.Start(StubHttpServer.BadRequestValidationProblem); + var args = new[] { "scope", "create", "--dimension", "tier", "--values", "dev,prod", "--no-interactive", "--output", "json" }; + + // Act + var result = await RunCliAsync(args, server.BaseUrl, TestContext.Current.CancellationToken); + + // Assert + result.ExitCode.ShouldBe(1, $"stdout:\n{result.Stdout}\nstderr:\n{result.Stderr}"); + } + + [Fact] + public async Task Cli_ApiReturns409_ProcessExitCodeIsOne() + { + // Arrange + using var server = StubHttpServer.Start(StubHttpServer.ConflictProblem); + var args = new[] { "scope", "create", "--dimension", "tier", "--values", "dev,prod", "--no-interactive", "--output", "json" }; + + // Act + var result = await RunCliAsync(args, server.BaseUrl, TestContext.Current.CancellationToken); + + // Assert + result.ExitCode.ShouldBe(1, $"stdout:\n{result.Stdout}\nstderr:\n{result.Stderr}"); + } + + [Fact] + public async Task Cli_ApiReturns422_ProcessExitCodeIsOne() + { + // Arrange + using var server = StubHttpServer.Start(StubHttpServer.UnprocessableEntityProblem); + var args = new[] { "scope", "create", "--dimension", "tier", "--values", "dev,prod", "--no-interactive", "--output", "json" }; + + // Act + var result = await RunCliAsync(args, server.BaseUrl, TestContext.Current.CancellationToken); + + // Assert + result.ExitCode.ShouldBe(1, $"stdout:\n{result.Stdout}\nstderr:\n{result.Stderr}"); + } + + private static async Task RunCliAsync(string[] args, string? serverUrl, CancellationToken cancellationToken) + { + var startInfo = new ProcessStartInfo("dotnet") + { + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true + }; + + startInfo.ArgumentList.Add("run"); + startInfo.ArgumentList.Add("--no-build"); + startInfo.ArgumentList.Add("--project"); + startInfo.ArgumentList.Add(CliProjectPath); + startInfo.ArgumentList.Add("--"); + foreach (var arg in args) + { + startInfo.ArgumentList.Add(arg); + } + + if (serverUrl is not null) + { + startInfo.Environment["GroundControl__ServerUrl"] = serverUrl; + } + + using var process = Process.Start(startInfo) ?? throw new InvalidOperationException("Failed to start CLI process."); + + var stdoutTask = process.StandardOutput.ReadToEndAsync(cancellationToken); + var stderrTask = process.StandardError.ReadToEndAsync(cancellationToken); + + await process.WaitForExitAsync(cancellationToken); + var stdout = await stdoutTask; + var stderr = await stderrTask; + + return new ProcessOutcome(process.ExitCode, stdout, stderr); + } + + private static string ResolveRepositoryRoot() + { + var value = typeof(CliExitCodeTests).Assembly + .GetCustomAttributes() + .FirstOrDefault(a => a.Key == "RepositoryRoot")?.Value; + + return value ?? throw new InvalidOperationException("RepositoryRoot assembly metadata not found."); + } + + private sealed record ProcessOutcome(int ExitCode, string Stdout, string Stderr); + + private sealed class StubHttpServer : IDisposable + { + private readonly HttpListener _listener; + private readonly CancellationTokenSource _cts = new(); + private readonly Task _loop; + + public string BaseUrl { get; } + + private StubHttpServer(HttpListener listener, string baseUrl, Func handler) + { + _listener = listener; + BaseUrl = baseUrl; + _loop = Task.Run(async () => + { + while (!_cts.IsCancellationRequested && _listener.IsListening) + { + HttpListenerContext context; + try + { + context = await _listener.GetContextAsync().WaitAsync(_cts.Token); + } + catch (OperationCanceledException) + { + return; + } + catch (HttpListenerException) + { + return; + } + + try + { + await handler(context); + } + finally + { + context.Response.Close(); + } + } + }); + } + + public static StubHttpServer Start(Func handler) + { + // Pick a free port via a transient TcpListener so we don't collide. + var port = GetFreeTcpPort(); + var prefix = $"http://127.0.0.1:{port}/"; + var listener = new HttpListener(); + listener.Prefixes.Add(prefix); + listener.Start(); + return new StubHttpServer(listener, prefix.TrimEnd('/'), handler); + } + + public static Func BadRequestValidationProblem { get; } = WriteProblemAsync( + statusCode: HttpStatusCode.BadRequest, + contentType: "application/problem+json", + body: JsonSerializer.Serialize(new + { + type = "https://tools.ietf.org/html/rfc9110#section-15.5.1", + title = "One or more validation errors occurred.", + status = 400, + detail = "Validation failed.", + errors = new Dictionary + { + ["Dimension"] = ["A scope with dimension 'tier' already exists."] + } + })); + + public static Func ConflictProblem { get; } = WriteProblemAsync( + statusCode: HttpStatusCode.Conflict, + contentType: "application/problem+json", + body: JsonSerializer.Serialize(new + { + type = "https://tools.ietf.org/html/rfc9110#section-15.5.10", + title = "Conflict", + status = 409, + detail = "The resource was modified by another user." + })); + + public static Func UnprocessableEntityProblem { get; } = WriteProblemAsync( + statusCode: HttpStatusCode.UnprocessableEntity, + contentType: "application/problem+json", + body: JsonSerializer.Serialize(new + { + type = "https://tools.ietf.org/html/rfc4918#section-11.2", + title = "Unprocessable Entity", + status = 422, + detail = "The request could not be processed due to a business rule violation." + })); + + private static Func WriteProblemAsync(HttpStatusCode statusCode, string contentType, string body) + { + return async context => + { + context.Response.StatusCode = (int)statusCode; + context.Response.ContentType = contentType; + var bytes = Encoding.UTF8.GetBytes(body); + context.Response.ContentLength64 = bytes.Length; + await context.Response.OutputStream.WriteAsync(bytes); + }; + } + + private static int GetFreeTcpPort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + return ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + } + + public void Dispose() + { + _cts.Cancel(); + try + { + _listener.Stop(); + _listener.Close(); + } + catch (ObjectDisposedException) + { + // Already disposed. + } + + try + { + _loop.Wait(TimeSpan.FromSeconds(2)); + } + catch (AggregateException) + { + // Background loop exited via cancellation. + } + + _cts.Dispose(); + } + } +} diff --git a/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj b/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj index 0e0049fe..9a5565d1 100644 --- a/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj +++ b/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj @@ -15,4 +15,11 @@ + + + <_Parameter1>RepositoryRoot + <_Parameter2>$(RepositoryRoot) + + + \ No newline at end of file From 6cef0feb0cf5c3c410586ac538e05a50fce4e4ec Mon Sep 17 00:00:00 2001 From: Mohammadreza Taikandi Date: Sun, 10 May 2026 14:35:30 +0100 Subject: [PATCH 2/2] fix(tests): launch CLI dll directly so exit-code tests work in Release CI builds Release while `dotnet run --no-build` defaults to Debug, so the test was launching `dotnet run` against a missing Debug binary and failing with "An error occurred trying to start process". Locally everyone's already built Debug, so the test passed. Fix: derive the CLI dll path from the test assembly's location (sibling output directories under artifacts/bin/{Project}/{config}/) and invoke `dotnet .dll`. Config-agnostic and avoids the dotnet-run startup overhead. Drops the now-unneeded RepositoryRoot AssemblyMetadata. --- .../CliExitCodeTests.cs | 35 ++++++++++++------- .../GroundControl.Cli.Tests.csproj | 7 ---- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs b/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs index 187deeca..7e672174 100644 --- a/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs +++ b/tests/GroundControl.Cli.Tests/CliExitCodeTests.cs @@ -1,6 +1,5 @@ using System.Diagnostics; using System.Net; -using System.Reflection; using System.Text; using System.Text.Json; @@ -14,8 +13,7 @@ namespace GroundControl.Cli.Tests; /// public sealed class CliExitCodeTests { - private static readonly string RepositoryRoot = ResolveRepositoryRoot(); - private static readonly string CliProjectPath = Path.Combine(RepositoryRoot, "src", "GroundControl.Cli", "GroundControl.Cli.csproj"); + private static readonly string CliBinaryPath = ResolveCliBinaryPath(); [Fact] public async Task Cli_HandlerReturnsNonZero_ProcessExitCodeIsNonZero() @@ -84,11 +82,7 @@ private static async Task RunCliAsync(string[] args, string? ser CreateNoWindow = true }; - startInfo.ArgumentList.Add("run"); - startInfo.ArgumentList.Add("--no-build"); - startInfo.ArgumentList.Add("--project"); - startInfo.ArgumentList.Add(CliProjectPath); - startInfo.ArgumentList.Add("--"); + startInfo.ArgumentList.Add(CliBinaryPath); foreach (var arg in args) { startInfo.ArgumentList.Add(arg); @@ -111,13 +105,28 @@ private static async Task RunCliAsync(string[] args, string? ser return new ProcessOutcome(process.ExitCode, stdout, stderr); } - private static string ResolveRepositoryRoot() + // Why: locate the CLI dll relative to the test assembly so the test runs in any + // configuration (Debug/Release). Output layout is `artifacts/bin/{Project}/{config}/` + // for both projects, so swapping the project segment yields the CLI's dll path + // without depending on MSBuild metadata or `dotnet run` (which assumes Debug). + private static string ResolveCliBinaryPath() { - var value = typeof(CliExitCodeTests).Assembly - .GetCustomAttributes() - .FirstOrDefault(a => a.Key == "RepositoryRoot")?.Value; + var testAssemblyPath = typeof(CliExitCodeTests).Assembly.Location; + var configDirectory = Path.GetDirectoryName(testAssemblyPath) + ?? throw new InvalidOperationException("Could not determine test assembly directory."); + var configName = Path.GetFileName(configDirectory); + var artifactsBin = Path.GetDirectoryName(Path.GetDirectoryName(configDirectory)!) + ?? throw new InvalidOperationException("Could not determine artifacts/bin directory."); + + var path = Path.Combine(artifactsBin, "GroundControl.Cli", configName, "GroundControl.Cli.dll"); + if (!File.Exists(path)) + { + throw new FileNotFoundException( + $"CLI binary not found at {path}. Ensure GroundControl.Cli has been built in the same configuration as the test project.", + path); + } - return value ?? throw new InvalidOperationException("RepositoryRoot assembly metadata not found."); + return path; } private sealed record ProcessOutcome(int ExitCode, string Stdout, string Stderr); diff --git a/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj b/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj index 9a5565d1..0e0049fe 100644 --- a/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj +++ b/tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj @@ -15,11 +15,4 @@ - - - <_Parameter1>RepositoryRoot - <_Parameter2>$(RepositoryRoot) - - - \ No newline at end of file