From 7ca1544e570b310941fb08e693277ae89eb55db2 Mon Sep 17 00:00:00 2001 From: Tim Pohlmann Date: Sat, 20 Jun 2026 05:28:01 +0000 Subject: [PATCH 1/3] feat: add --fail-on option to control failing severity Adds a --fail-on option to `dolphin check` that configures the lowest severity (error, warning, info) that causes a non-zero exit. Defaults to error, preserving existing behavior. Co-Authored-By: Claude Opus 4.8 --- src/Dolphin/Cli/CheckCommand.cs | 30 +++++++++++++---- tests/Dolphin.Tests/CheckCommandTests.cs | 43 ++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/Dolphin/Cli/CheckCommand.cs b/src/Dolphin/Cli/CheckCommand.cs index f46a42a..ad061f9 100644 --- a/src/Dolphin/Cli/CheckCommand.cs +++ b/src/Dolphin/Cli/CheckCommand.cs @@ -31,23 +31,32 @@ public static Command Build() description: "Scan only this file instead of the entire project" ); + var failOnOption = new Option( + "--fail-on", + description: "Lowest severity that causes a non-zero exit: error, warning, or info", + getDefaultValue: () => "error" + ); + failOnOption.FromAmong("error", "warning", "info"); + var cmd = new Command("check", "Run static analysis rules against the codebase") { cwdOption, ruleOption, formatOption, - fileOption + fileOption, + failOnOption }; cmd.SetHandler( - async (cwd, ruleId, format, file) => - Environment.Exit(await HandleAsync(cwd, ruleId, format, file)), - cwdOption, ruleOption, formatOption, fileOption); + async (cwd, ruleId, format, file, failOn) => + Environment.Exit(await HandleAsync(cwd, ruleId, format, file, failOn)), + cwdOption, ruleOption, formatOption, fileOption, failOnOption); return cmd; } - internal static async Task HandleAsync(string cwd, string? ruleId, string format, string? file) + internal static async Task HandleAsync( + string cwd, string? ruleId, string format, string? file, string failOn = "error") { cwd = Path.GetFullPath(cwd); if (!Directory.Exists(cwd)) @@ -113,6 +122,15 @@ internal static async Task HandleAsync(string cwd, string? ruleId, string f Formatter.Print(result.Findings, format); - return result.Findings.Any(f => f.Severity == Severity.Error) ? 1 : 0; + // Lower enum value == higher severity (Error=0, Warning=1, Info=2). + // Fail if any finding is at least as severe as the configured threshold. + var threshold = failOn switch + { + "warning" => Severity.Warning, + "info" => Severity.Info, + _ => Severity.Error + }; + + return result.Findings.Any(f => f.Severity <= threshold) ? 1 : 0; } } diff --git a/tests/Dolphin.Tests/CheckCommandTests.cs b/tests/Dolphin.Tests/CheckCommandTests.cs index f3024ec..bc7fc5a 100644 --- a/tests/Dolphin.Tests/CheckCommandTests.cs +++ b/tests/Dolphin.Tests/CheckCommandTests.cs @@ -53,6 +53,49 @@ public async Task HandleAsync_FakeScanner_NoFindings_Returns0() } } + private const string WarningFindingJson = + """{"results":[{"check_id":"r","path":"f.ts","start":{"line":1,"col":1},"end":{"line":1,"col":1},"extra":{"message":"m","severity":"WARNING","lines":"x"}}]}"""; + + [TestMethod] + public async Task HandleAsync_WarningFinding_DefaultFailOn_Returns0() + { + if (OperatingSystem.IsWindows()) Assert.Inconclusive("Fake scanner uses a shell script; Unix-only"); + var (tmpDir, fakeBinDir) = CreateFakeOpengrepEnv(exitCode: 1, stdout: WarningFindingJson); + var savedPath = Environment.GetEnvironmentVariable("PATH"); + Environment.SetEnvironmentVariable("PATH", fakeBinDir + ":" + savedPath); + try + { + var result = await CheckCommand.HandleAsync(tmpDir, null, "text", null); + Assert.AreEqual(0, result); + } + finally + { + Environment.SetEnvironmentVariable("PATH", savedPath); + Directory.Delete(tmpDir, recursive: true); + Directory.Delete(fakeBinDir, recursive: true); + } + } + + [TestMethod] + public async Task HandleAsync_WarningFinding_FailOnWarning_Returns1() + { + if (OperatingSystem.IsWindows()) Assert.Inconclusive("Fake scanner uses a shell script; Unix-only"); + var (tmpDir, fakeBinDir) = CreateFakeOpengrepEnv(exitCode: 1, stdout: WarningFindingJson); + var savedPath = Environment.GetEnvironmentVariable("PATH"); + Environment.SetEnvironmentVariable("PATH", fakeBinDir + ":" + savedPath); + try + { + var result = await CheckCommand.HandleAsync(tmpDir, null, "text", null, "warning"); + Assert.AreEqual(1, result); + } + finally + { + Environment.SetEnvironmentVariable("PATH", savedPath); + Directory.Delete(tmpDir, recursive: true); + Directory.Delete(fakeBinDir, recursive: true); + } + } + [TestMethod] public async Task HandleAsync_FakeScanner_ScannerWarning_Returns0() { From 13821c80a2808f23cce8a0de14f614de0f67845a Mon Sep 17 00:00:00 2001 From: Tim Pohlmann Date: Sat, 20 Jun 2026 05:35:33 +0000 Subject: [PATCH 2/3] refactor: make fail-on threshold independent of enum order; add tests Address review feedback: compute the failing severities explicitly rather than relying on the Severity enum's ordinal ordering, and normalize the --fail-on input case-insensitively for direct callers. Add coverage for the info threshold and the option wiring. Co-Authored-By: Claude Opus 4.8 --- src/Dolphin/Cli/CheckCommand.cs | 20 ++++++----- tests/Dolphin.Tests/CheckCommandTests.cs | 44 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/Dolphin/Cli/CheckCommand.cs b/src/Dolphin/Cli/CheckCommand.cs index ad061f9..d349a75 100644 --- a/src/Dolphin/Cli/CheckCommand.cs +++ b/src/Dolphin/Cli/CheckCommand.cs @@ -122,15 +122,17 @@ internal static async Task HandleAsync( Formatter.Print(result.Findings, format); - // Lower enum value == higher severity (Error=0, Warning=1, Info=2). - // Fail if any finding is at least as severe as the configured threshold. - var threshold = failOn switch + return result.Findings.Any(f => Fails(f.Severity, failOn)) ? 1 : 0; + } + + // Whether a finding of the given severity should cause a non-zero exit under the + // configured threshold. Listed explicitly so behavior does not depend on the + // Severity enum's declaration order. + private static bool Fails(Severity severity, string failOn) => + failOn.ToLowerInvariant() switch { - "warning" => Severity.Warning, - "info" => Severity.Info, - _ => Severity.Error + "info" => true, + "warning" => severity is Severity.Error or Severity.Warning, + _ => severity == Severity.Error }; - - return result.Findings.Any(f => f.Severity <= threshold) ? 1 : 0; - } } diff --git a/tests/Dolphin.Tests/CheckCommandTests.cs b/tests/Dolphin.Tests/CheckCommandTests.cs index bc7fc5a..4007f2d 100644 --- a/tests/Dolphin.Tests/CheckCommandTests.cs +++ b/tests/Dolphin.Tests/CheckCommandTests.cs @@ -1,3 +1,5 @@ +using System.CommandLine; +using System.CommandLine.Parsing; using System.Diagnostics; using System.Text.Json; using Dolphin.Cli; @@ -10,6 +12,25 @@ public class CheckCommandTests { // ── Unit tests ───────────────────────────────────────────────────────────── + [TestMethod] + public void Build_FailOnOption_DefaultsToError() + { + var cmd = CheckCommand.Build(); + var result = cmd.Parse(""); + var option = (Option)cmd.Options.Single(o => o.Name == "fail-on"); + Assert.AreEqual("error", result.GetValueForOption(option)); + } + + [TestMethod] + public void Build_FailOnOption_AcceptsValidValuesAndRejectsInvalid() + { + var cmd = CheckCommand.Build(); + var option = (Option)cmd.Options.Single(o => o.Name == "fail-on"); + + Assert.AreEqual("warning", cmd.Parse("--fail-on warning").GetValueForOption(option)); + Assert.IsTrue(cmd.Parse("--fail-on bogus").Errors.Count > 0); + } + [TestMethod] public async Task HandleAsync_NonExistentCwd_Returns2() { @@ -96,6 +117,29 @@ public async Task HandleAsync_WarningFinding_FailOnWarning_Returns1() } } + private const string InfoFindingJson = + """{"results":[{"check_id":"r","path":"f.ts","start":{"line":1,"col":1},"end":{"line":1,"col":1},"extra":{"message":"m","severity":"INFO","lines":"x"}}]}"""; + + [TestMethod] + public async Task HandleAsync_InfoFinding_FailOnInfo_Returns1() + { + if (OperatingSystem.IsWindows()) Assert.Inconclusive("Fake scanner uses a shell script; Unix-only"); + var (tmpDir, fakeBinDir) = CreateFakeOpengrepEnv(exitCode: 1, stdout: InfoFindingJson); + var savedPath = Environment.GetEnvironmentVariable("PATH"); + Environment.SetEnvironmentVariable("PATH", fakeBinDir + ":" + savedPath); + try + { + var result = await CheckCommand.HandleAsync(tmpDir, null, "text", null, "info"); + Assert.AreEqual(1, result); + } + finally + { + Environment.SetEnvironmentVariable("PATH", savedPath); + Directory.Delete(tmpDir, recursive: true); + Directory.Delete(fakeBinDir, recursive: true); + } + } + [TestMethod] public async Task HandleAsync_FakeScanner_ScannerWarning_Returns0() { From dd38da548132786dc62714093d604fd9084524f2 Mon Sep 17 00:00:00 2001 From: Tim Pohlmann Date: Fri, 26 Jun 2026 18:38:22 +0000 Subject: [PATCH 3/3] refactor: normalize fail-on threshold once; drop unused using Address review feedback: lower-case the --fail-on value a single time in HandleAsync instead of per-finding inside Fails(), and remove the unused System.CommandLine.Parsing import from the tests. Co-Authored-By: Claude Opus 4.8 --- src/Dolphin/Cli/CheckCommand.cs | 11 ++++++----- tests/Dolphin.Tests/CheckCommandTests.cs | 1 - 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Dolphin/Cli/CheckCommand.cs b/src/Dolphin/Cli/CheckCommand.cs index d349a75..04ae633 100644 --- a/src/Dolphin/Cli/CheckCommand.cs +++ b/src/Dolphin/Cli/CheckCommand.cs @@ -122,14 +122,15 @@ internal static async Task HandleAsync( Formatter.Print(result.Findings, format); - return result.Findings.Any(f => Fails(f.Severity, failOn)) ? 1 : 0; + var threshold = failOn.ToLowerInvariant(); + return result.Findings.Any(f => Fails(f.Severity, threshold)) ? 1 : 0; } // Whether a finding of the given severity should cause a non-zero exit under the - // configured threshold. Listed explicitly so behavior does not depend on the - // Severity enum's declaration order. - private static bool Fails(Severity severity, string failOn) => - failOn.ToLowerInvariant() switch + // configured threshold (which must already be lower-cased). Listed explicitly so + // behavior does not depend on the Severity enum's declaration order. + private static bool Fails(Severity severity, string threshold) => + threshold switch { "info" => true, "warning" => severity is Severity.Error or Severity.Warning, diff --git a/tests/Dolphin.Tests/CheckCommandTests.cs b/tests/Dolphin.Tests/CheckCommandTests.cs index 4007f2d..f0a6a1f 100644 --- a/tests/Dolphin.Tests/CheckCommandTests.cs +++ b/tests/Dolphin.Tests/CheckCommandTests.cs @@ -1,5 +1,4 @@ using System.CommandLine; -using System.CommandLine.Parsing; using System.Diagnostics; using System.Text.Json; using Dolphin.Cli;