From 4b4b171bb3039b4fabd2a25b54a0a4a0f0e619e0 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Thu, 15 Oct 2026 14:08:32 +0100 Subject: [PATCH 1/3] fix(ci-compliance): survive a file the engine throws on A rule that threw escaped Main and killed the process before any output was written. The check went red, which is right, but it named no file, emitted no coverage line, and discarded the findings already gathered for every other file in the run. Catch per file: count it, emit an annotation naming the file, and carry on. The run still fails, now on the failure count rather than by dying. --- .../src/ComplianceRunner/ComplianceRunner.cs | 228 +++++++++++------- .../Compliance.Shared/FileAccounting.cs | 25 +- .../Integration/ComplianceRunnerE2ETests.cs | 60 +++++ .../Unit/FileAccountingTests.cs | 75 +++++- 4 files changed, 283 insertions(+), 105 deletions(-) diff --git a/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs b/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs index 9c3f75e..b974f9e 100644 --- a/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs +++ b/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs @@ -54,108 +54,127 @@ static int Main(string[] args) continue; } - TestResult resultForThisFile; - - if (checkType == "project") + // One file must not take the run down with it. A rule that throws used to escape + // Main, which killed the process before OutputEmitter ran: the check went red with + // no annotation, no coverage line, and no name for the file that caused it, and the + // findings already gathered for every other file were lost. The run still fails + // below on accounting.Failed, so this converts a silent abort into a stated one + // rather than making the check more permissive. + try { - if (file.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)) - resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.CheckProjectFile(file, orgUrl); - else - resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.CheckAssemblyInfo(file, orgUrl); - - // Remap absolute location paths back to the relative file path so - // annotations point to the correct diff line — mirrors BHoMBot ProjectCompliance.cs. - // Note: .OfType() silently drops any ITestInformation that is not exactly - // BH.oM.Test.CodeCompliance.Error. Currently safe — the project engine only emits - // Error-typed findings — but revisit if new finding sub-types are introduced. - if (resultForThisFile?.Information != null) + TestResult resultForThisFile; + + if (checkType == "project") { - resultForThisFile.Information = resultForThisFile.Information - .OfType() - .Select(e => (ITestInformation)new BH.oM.Test.CodeCompliance.Error - { - Status = e.Status, - Message = e.Message, - Name = e.Name, - BHoM_Guid = e.BHoM_Guid, - UTCTime = e.UTCTime, - DocumentationLink = e.DocumentationLink, - Location = new BH.oM.Test.CodeCompliance.Location + if (file.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)) + resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.CheckProjectFile(file, orgUrl); + else + resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.CheckAssemblyInfo(file, orgUrl); + + // Remap absolute location paths back to the relative file path so + // annotations point to the correct diff line — mirrors BHoMBot ProjectCompliance.cs. + // Note: .OfType() silently drops any ITestInformation that is not exactly + // BH.oM.Test.CodeCompliance.Error. Currently safe — the project engine only emits + // Error-typed findings — but revisit if new finding sub-types are introduced. + if (resultForThisFile?.Information != null) + { + resultForThisFile.Information = resultForThisFile.Information + .OfType() + .Select(e => (ITestInformation)new BH.oM.Test.CodeCompliance.Error { - FilePath = file, - Line = e.Location?.Line - } - }) - .ToList(); + Status = e.Status, + Message = e.Message, + Name = e.Name, + BHoM_Guid = e.BHoM_Guid, + UTCTime = e.UTCTime, + DocumentationLink = e.DocumentationLink, + Location = new BH.oM.Test.CodeCompliance.Location + { + FilePath = file, + Line = e.Location?.Line + } + }) + .ToList(); + } } - } - else - { - // Test_Toolkit's [Path(@"..._Engine\\..")] rules match a literal backslash; - // flip git's forward-slash paths or every rule silently no-ops. - resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.RunChecks(file.Replace('/', '\\'), checkType); - - // Group findings one-per-line and remap the path to the diff-relative file, - // mirroring BHoMBot's GroupInformation -> GroupErrors pipeline (code/copyright/ - // documentation only; project has its own remap above). GroupErrors concatenates - // each finding's FullMessage() into the grouped Error.Message, so downstream these - // annotations are built from that message directly (see the preformatted branch in - // AnnotationConvert) rather than via IFullMessage, which would append the - // documentation suffix a second time. - if (resultForThisFile?.Information != null) + else { - var errors = resultForThisFile.Information - .OfType() - .ToList(); - resultForThisFile.Information = BH.Engine.Test.CodeCompliance.Modify - .GroupErrors(errors, file) - .Select(e => (ITestInformation)e) - .ToList(); + // Test_Toolkit's [Path(@"..._Engine\\..")] rules match a literal backslash; + // flip git's forward-slash paths or every rule silently no-ops. + resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.RunChecks(file.Replace('/', '\\'), checkType); + + // Group findings one-per-line and remap the path to the diff-relative file, + // mirroring BHoMBot's GroupInformation -> GroupErrors pipeline (code/copyright/ + // documentation only; project has its own remap above). GroupErrors concatenates + // each finding's FullMessage() into the grouped Error.Message, so downstream these + // annotations are built from that message directly (see the preformatted branch in + // AnnotationConvert) rather than via IFullMessage, which would append the + // documentation suffix a second time. + if (resultForThisFile?.Information != null) + { + var errors = resultForThisFile.Information + .OfType() + .ToList(); + resultForThisFile.Information = BH.Engine.Test.CodeCompliance.Modify + .GroupErrors(errors, file) + .Select(e => (ITestInformation)e) + .ToList(); + } } - } - if (resultForThisFile == null) - { - accounting.CountNoResult(); - Console.WriteLine($" [SKIP] No result returned for: {file}"); - continue; - } + if (resultForThisFile == null) + { + accounting.CountNoResult(); + Console.WriteLine($" [SKIP] No result returned for: {file}"); + continue; + } - if (verbose) Console.WriteLine($" Result Status: {resultForThisFile.Status}"); - - accounting.CountExamined(); - mergedResult = mergedResult.Merge(resultForThisFile); - - // Code/copyright/documentation findings have been through GroupErrors, whose - // Error.Message is already the concatenated FullMessage(); take it verbatim. - // - // Project findings use IFullMessage, which appends the " - For more information - // see " documentation suffix. This is a deliberate, documented divergence from - // BHoMBot: BHoMBot's ProjectCompliance emitted the bare Error.Message with no suffix, - // even though it carried the DocumentationLink. Every other compliance check surfaces - // that link (via GroupErrors -> FullMessage), and the project engine attaches a - // DocumentationLink to every finding specifically so it can be shown, so BHoMBot's - // omission was an inconsistency. Keeping the suffix makes all five checks uniform and - // points authors at the remediation guidance. Nothing downstream parses this text - // (pass/fail is status-driven; SARIF carries the link separately). - bool preformatted = checkType != "project"; - var information = resultForThisFile.Information ?? Enumerable.Empty(); - var perFileAnnotations = information.Select(i => i.ToAnnotationEquivalent(preformatted)).ToList(); - var infoList = information.ToList(); - - for (int i = 0; i < perFileAnnotations.Count; i++) - { - var a = perFileAnnotations[i]; - var displayPath = string.IsNullOrEmpty(a.FilePath) ? file : a.FilePath; - if (verbose) + if (verbose) Console.WriteLine($" Result Status: {resultForThisFile.Status}"); + + accounting.CountExamined(); + mergedResult = mergedResult.Merge(resultForThisFile); + + // Code/copyright/documentation findings have been through GroupErrors, whose + // Error.Message is already the concatenated FullMessage(); take it verbatim. + // + // Project findings use IFullMessage, which appends the " - For more information + // see " documentation suffix. This is a deliberate, documented divergence from + // BHoMBot: BHoMBot's ProjectCompliance emitted the bare Error.Message with no suffix, + // even though it carried the DocumentationLink. Every other compliance check surfaces + // that link (via GroupErrors -> FullMessage), and the project engine attaches a + // DocumentationLink to every finding specifically so it can be shown, so BHoMBot's + // omission was an inconsistency. Keeping the suffix makes all five checks uniform and + // points authors at the remediation guidance. Nothing downstream parses this text + // (pass/fail is status-driven; SARIF carries the link separately). + bool preformatted = checkType != "project"; + var information = resultForThisFile.Information ?? Enumerable.Empty(); + var perFileAnnotations = information.Select(i => i.ToAnnotationEquivalent(preformatted)).ToList(); + var infoList = information.ToList(); + + for (int i = 0; i < perFileAnnotations.Count; i++) { - Console.WriteLine($" - [{a.Level}] {displayPath}:{a.LineStart}:{a.ColumnStart}" + - $"-{a.LineEnd}:{a.ColumnEnd} [{a.RuleName}]"); - Console.WriteLine($" {a.Message}"); - if (i < infoList.Count) - AnnotationConvert.LogDetailedFinding(infoList[i]); + var a = perFileAnnotations[i]; + var displayPath = string.IsNullOrEmpty(a.FilePath) ? file : a.FilePath; + if (verbose) + { + Console.WriteLine($" - [{a.Level}] {displayPath}:{a.LineStart}:{a.ColumnStart}" + + $"-{a.LineEnd}:{a.ColumnEnd} [{a.RuleName}]"); + Console.WriteLine($" {a.Message}"); + if (i < infoList.Count) + AnnotationConvert.LogDetailedFinding(infoList[i]); + } + allAnnotations.Add(a); } - allAnnotations.Add(a); + } + catch (Exception ex) + { + accounting.CountFailed(); + allAnnotations.Add(FailureAnnotation(file, ex)); + + // Stack trace to stderr, not into the annotation. GitHub truncates a long + // annotation message, and the one thing the author needs is the file name. + Console.Error.WriteLine($" [FAIL] {file}: {ex.GetType().Name}: {ex.Message}"); + Console.Error.WriteLine(ex.ToString()); } } @@ -164,7 +183,30 @@ static int Main(string[] args) accounting); // Exit code mirrors BHoMBot: failure only on Error; Warning and Pass are both success. - return mergedResult.Status == TestStatus.Error ? 1 : 0; + // A file the engine threw on also fails the run. It is not a finding about the code, so + // it does not reach mergedResult, and without this a crash would exit 0 and report a + // green check having examined less than it was asked to. + return mergedResult.Status == TestStatus.Error || accounting.Failed > 0 ? 1 : 0; } + /// + /// The annotation for a file the engine threw on. Carries the exception type and message so + /// the author can tell a malformed file from an environment problem, and says plainly that + /// this file was not checked, because a failure annotation that looks like a finding would + /// send them looking for a defect in their own code. + /// + static Annotation FailureAnnotation(string file, Exception ex) => new Annotation + { + FilePath = file, + LineStart = 1, + LineEnd = 1, + ColumnStart = 1, + ColumnEnd = 1, + Level = "failure", + RuleName = "FileCouldNotBeAnalysed", + Message = $"This file could not be analysed and was not checked. " + + $"{ex.GetType().Name}: {ex.Message} " + + $"Every other file in this run was checked normally.", + UTCTime = DateTime.UtcNow, + }; } diff --git a/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileAccounting.cs b/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileAccounting.cs index d167c42..08e8eb0 100644 --- a/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileAccounting.cs +++ b/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileAccounting.cs @@ -4,10 +4,11 @@ /// Counts what a compliance runner did with the files it was handed, and renders that as a /// coverage denominator. /// -/// Both runners walk the same four-exit loop: a file is dropped by the filter, dropped because -/// it is not on disk, dropped because the engine returned no result, or examined. Without a -/// count of those, a run that examined every file and a run that examined none produce the same -/// output, so a green check carries no evidence that anything was checked. +/// Both runners walk the same five-exit loop: a file is dropped by the filter, dropped because +/// it is not on disk, dropped because the engine returned no result, examined, or failed because +/// the engine threw on it. Without a count of those, a run that examined every file and a run +/// that examined none produce the same output, so a green check carries no evidence that +/// anything was checked. /// /// Deliberately free of BHoM types and of any dependency on either runner, so it can be /// compiled directly into the hermetic test project and its arithmetic and wording tested @@ -37,19 +38,31 @@ public sealed class FileAccounting /// Files the runner actually examined and merged into its verdict. public int Examined { get; private set; } + /// + /// Files the engine threw on. Counted apart from the other drops because it is the only one + /// that means the check could not answer, rather than that it had nothing to answer about. + /// + public int Failed { get; private set; } + public FileAccounting(int handedIn) => HandedIn = handedIn; public void CountNotRelevant() => NotRelevant++; public void CountNotOnDisk() => NotOnDisk++; public void CountNoResult() => NoResult++; public void CountExamined() => Examined++; + public void CountFailed() => Failed++; /// /// True when files were supplied and none of them was examined. The calling workflow exits /// before invoking a runner when it has no files at all, so this is the anomalous case /// rather than the empty one: something selected these files and the runner used none. + /// + /// A run with failures is excluded deliberately. This flag exists to catch a check that + /// reports success having inspected nothing, and its warning says exactly that. A run that + /// failed on every file is already red and already carries an annotation per file, so + /// raising it here would contradict the exit code. /// - public bool ExaminedNothing => HandedIn > 0 && Examined == 0; + public bool ExaminedNothing => HandedIn > 0 && Examined == 0 && Failed == 0; /// /// The coverage line. Leads with what was examined rather than with what was handed in: @@ -75,6 +88,7 @@ private string DropSummary() if (NotRelevant > 0) parts.Add($"{NotRelevant} not relevant to this check"); if (NotOnDisk > 0) parts.Add($"{NotOnDisk} not found on disk"); if (NoResult > 0) parts.Add($"{NoResult} returned no result"); + if (Failed > 0) parts.Add($"{Failed} could not be analysed"); return parts.Count == 0 ? "none dropped" : string.Join(", ", parts); } @@ -86,5 +100,6 @@ private string DropSummary() ["notRelevant"] = NotRelevant, ["notOnDisk"] = NotOnDisk, ["noResult"] = NoResult, + ["failed"] = Failed, }; } diff --git a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs index 4641311..ccde9ba 100644 --- a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs +++ b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs @@ -115,4 +115,64 @@ public void GitHubOutput_ForPassingRun_ProducesNoFindingAnnotations() "the one annotation a passing run may produce is the report that it examined nothing"); }); } + + // ── A file the engine throws on ─────────────────────────────────────────── + + /// + /// Writes a .cs file the compliance engine cannot analyse, under a directory name the code + /// rules' path gate accepts, and returns its full path along with the directory to clean up. + /// + /// The content is malformed C#. Parsed, it leaves a parameter with no identifier, and the + /// first rule to index into a parameter name throws. Note it carries no preprocessor + /// directive: a directive is one way to reach this state and not the only one, so the test + /// does not depend on how the file got here. + /// + private static (string File, string Dir) WriteUnanalysableFile() + { + string dir = Path.Combine(Path.GetTempPath(), "bhom-ci-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(Path.Combine(dir, "Probe_Adapter")); + string file = Path.Combine(dir, "Probe_Adapter", "ProbeAdapter.cs"); + File.WriteAllText(file, + "namespace BH.Adapter.Probe\n" + + " m_value = new FileInfo(filePath).Directory.FullName;\n"); + return (file, dir); + } + + [Test] + [Category("RequiresBHoM")] + [Description("A file the engine throws on fails the run, names itself, and does not stop the other files being reported.")] + public void UnanalysableFile_FailsTheRunAndIsNamed_WithoutLosingOtherFindings() + { + var (bad, dir) = WriteUnanalysableFile(); + + // A second file with a real violation, so the test can tell "the run continued" from + // "the run died quietly". Capital parameter name breaks InputParameterStartsLower. + string good = Path.Combine(dir, "Probe_Adapter", "ProbeHelper.cs"); + File.WriteAllText(good, + "namespace BH.Adapter.Probe\n{\n public static class ProbeHelper\n {\n" + + " public static int ProbeHelperMethod(int BadParam) { return BadParam; }\n" + + " }\n}\n"); + + try + { + var (exitCode, stdout) = RunnerFixture.Run("ComplianceRunner", + "code", "--output", "github", bad, good); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(1), "a file the engine could not analyse must fail the run"); + Assert.That(stdout, Does.Contain("ProbeAdapter.cs"), + "the failing file must be named, or the author cannot act on the red"); + Assert.That(stdout, Does.Contain("could not be analysed")); + Assert.That(stdout, Does.Contain("1 of 2 file(s) examined"), + "the coverage line must survive, which it cannot if the process died"); + Assert.That(stdout, Does.Contain("InputParameterStartsLower"), + "the other file's findings must still be reported; losing them was the defect"); + }); + } + finally + { + try { Directory.Delete(dir, recursive: true); } catch { /* temp dir, best effort */ } + } + } } diff --git a/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileAccountingTests.cs b/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileAccountingTests.cs index 10cb7cb..d37b220 100644 --- a/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileAccountingTests.cs +++ b/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileAccountingTests.cs @@ -33,14 +33,15 @@ public void EveryFileExamined_ReportsFullCoverageAndNoDrops() [Test] public void EachDropReasonIsCountedAndNamedSeparately() { - var a = new FileAccounting(4); + var a = new FileAccounting(5); a.CountNotRelevant(); a.CountNotOnDisk(); a.CountNoResult(); + a.CountFailed(); a.CountExamined(); Assert.That(a.CoverageLine(), Is.EqualTo( - "Coverage: 1 of 4 file(s) examined; 1 not relevant to this check, 1 not found on disk, 1 returned no result.")); + "Coverage: 1 of 5 file(s) examined; 1 not relevant to this check, 1 not found on disk, 1 returned no result, 1 could not be analysed.")); } [Test] @@ -55,9 +56,67 @@ public void ADropReasonThatDidNotOccurIsNotMentioned() Assert.That(a.CoverageLine(), Does.Contain("2 not relevant to this check")); Assert.That(a.CoverageLine(), Does.Not.Contain("not found on disk")); Assert.That(a.CoverageLine(), Does.Not.Contain("returned no result")); + Assert.That(a.CoverageLine(), Does.Not.Contain("could not be analysed")); }); } + // ── Files the engine threw on ───────────────────────────────────────────────── + + [Test] + public void AFailedFileIsCountedApartFromTheOtherDrops() + { + // "could not be analysed" and "not relevant" mean opposite things: one is the check + // declining a file, the other is the check being unable to answer about it. Folding + // the second into the first would hide exactly the case this counter exists for. + var a = new FileAccounting(2); + a.CountExamined(); + a.CountFailed(); + + Assert.Multiple(() => + { + Assert.That(a.Failed, Is.EqualTo(1)); + Assert.That(a.NotRelevant, Is.EqualTo(0)); + Assert.That(a.NoResult, Is.EqualTo(0)); + Assert.That(a.CoverageLine(), Is.EqualTo( + "Coverage: 1 of 2 file(s) examined; 1 could not be analysed.")); + }); + } + + [Test] + public void AFailedFileDoesNotCountAsExamined() + { + // The whole point of the per-file catch is that the run continues. If a failure also + // counted as examined, the coverage line would claim the file was checked. + var a = new FileAccounting(1); + a.CountFailed(); + + Assert.That(a.Examined, Is.EqualTo(0)); + } + + [Test] + public void AllFilesFailed_IsNotReportedAsAGreenThatExaminedNothing() + { + // Caught by running the patched runner on a single unanalysable file. The warning + // text says the check "reports success without having inspected anything", which is + // false here: a failure exits non-zero. The flag means vacuous green specifically, + // so a red run must not raise it. + var a = new FileAccounting(1); + a.CountFailed(); + + Assert.That(a.ExaminedNothing, Is.False, + "a run that failed on every file is red, not a green that examined nothing"); + } + + [Test] + public void ExaminedNothingStillFiresWhenTheDropsAreBenign() + { + // The guard above must not silence the case this flag exists for. + var a = new FileAccounting(2); + a.CountNotRelevant(); a.CountNotOnDisk(); + + Assert.That(a.ExaminedNothing, Is.True); + } + // ── The case this exists for ────────────────────────────────────────────────── [Test] @@ -116,18 +175,19 @@ public void SomeExamined_IsNotFlagged() [Test] public void ThePayloadCarriesEveryCounter() { - var a = new FileAccounting(4); - a.CountNotRelevant(); a.CountNotOnDisk(); a.CountNoResult(); a.CountExamined(); + var a = new FileAccounting(5); + a.CountNotRelevant(); a.CountNotOnDisk(); a.CountNoResult(); a.CountFailed(); a.CountExamined(); var p = a.ToPayload(); Assert.Multiple(() => { - Assert.That(p["handedIn"], Is.EqualTo(4)); + Assert.That(p["handedIn"], Is.EqualTo(5)); Assert.That(p["examined"], Is.EqualTo(1)); Assert.That(p["notRelevant"], Is.EqualTo(1)); Assert.That(p["notOnDisk"], Is.EqualTo(1)); Assert.That(p["noResult"], Is.EqualTo(1)); + Assert.That(p["failed"], Is.EqualTo(1)); }); } @@ -140,9 +200,10 @@ public void TheCountsReconcile() for (int i = 0; i < 2; i++) a.CountNotRelevant(); for (int i = 0; i < 3; i++) a.CountNotOnDisk(); a.CountNoResult(); - for (int i = 0; i < 4; i++) a.CountExamined(); + a.CountFailed(); + for (int i = 0; i < 3; i++) a.CountExamined(); - Assert.That(a.NotRelevant + a.NotOnDisk + a.NoResult + a.Examined, + Assert.That(a.NotRelevant + a.NotOnDisk + a.NoResult + a.Failed + a.Examined, Is.EqualTo(a.HandedIn)); } } From bc8411658c00e97f876a4d30ffc8ad934c1156ac Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Thu, 15 Oct 2026 14:21:49 +0100 Subject: [PATCH 2/3] test(ci-compliance): pin the exit code to the failure count alone The first crash test runs a second file carrying a real violation, so the run exits 1 on the merged Error regardless. A mutation removing the failure term from the exit code survived it. This runs the unanalysable file on its own, where nothing else can make the run red. --- .../Integration/ComplianceRunnerE2ETests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs index ccde9ba..2a05720 100644 --- a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs +++ b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/ComplianceRunnerE2ETests.cs @@ -175,4 +175,35 @@ public void UnanalysableFile_FailsTheRunAndIsNamed_WithoutLosingOtherFindings() try { Directory.Delete(dir, recursive: true); } catch { /* temp dir, best effort */ } } } + + [Test] + [Category("RequiresBHoM")] + [Description("A run whose only file could not be analysed still fails, on the failure alone.")] + public void UnanalysableFileAlone_StillFailsTheRun() + { + // Separate from the test above because that one cannot see this. Its second file carries + // a real violation, so the run exits 1 on the merged Error whether or not failures count + // towards the exit code. A mutation removing the failure term from the exit code + // survived it. With nothing else in the run there is no other reason to be red, so the + // exit code here rests on the failure count and nothing else. + var (bad, dir) = WriteUnanalysableFile(); + try + { + var (exitCode, stdout) = RunnerFixture.Run("ComplianceRunner", + "code", "--output", "github", bad); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(1), + "a run that could analyse nothing must not report success"); + Assert.That(stdout, Does.Contain("0 of 1 file(s) examined")); + Assert.That(stdout, Does.Not.Contain("reports success without having inspected anything"), + "that warning is for a green that examined nothing, and this run is red"); + }); + } + finally + { + try { Directory.Delete(dir, recursive: true); } catch { /* temp dir, best effort */ } + } + } } From a1662946f5c14ed5b907f507d6679f037203d4e8 Mon Sep 17 00:00:00 2001 From: Seun Akanni Date: Thu, 15 Oct 2026 14:29:49 +0100 Subject: [PATCH 3/3] style(ci-compliance): drop em dashes from the comments touched by the re-indent --- .../src/ComplianceRunner/ComplianceRunner.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs b/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs index b974f9e..3ec8840 100644 --- a/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs +++ b/tools/ComplianceRunner/src/ComplianceRunner/ComplianceRunner.cs @@ -72,10 +72,10 @@ static int Main(string[] args) resultForThisFile = BH.Engine.Test.CodeCompliance.Compute.CheckAssemblyInfo(file, orgUrl); // Remap absolute location paths back to the relative file path so - // annotations point to the correct diff line — mirrors BHoMBot ProjectCompliance.cs. + // annotations point to the correct diff line, mirroring BHoMBot ProjectCompliance.cs. // Note: .OfType() silently drops any ITestInformation that is not exactly - // BH.oM.Test.CodeCompliance.Error. Currently safe — the project engine only emits - // Error-typed findings — but revisit if new finding sub-types are introduced. + // BH.oM.Test.CodeCompliance.Error. Currently safe, because the project engine only emits + // Error-typed findings, but revisit if new finding sub-types are introduced. if (resultForThisFile?.Information != null) { resultForThisFile.Information = resultForThisFile.Information