diff --git a/tools/ComplianceRunner/src/DatasetComplianceRunner/DatasetComplianceRunner.cs b/tools/ComplianceRunner/src/DatasetComplianceRunner/DatasetComplianceRunner.cs index d781e6f..79c15b1 100644 --- a/tools/ComplianceRunner/src/DatasetComplianceRunner/DatasetComplianceRunner.cs +++ b/tools/ComplianceRunner/src/DatasetComplianceRunner/DatasetComplianceRunner.cs @@ -38,6 +38,7 @@ static int Main(string[] args) var mergedResult = new TestResult() { Status = TestStatus.Pass, Information = new List() }; var allAnnotations = new List(); var accounting = new FileAccounting(files.Count); + var demotedTotal = 0; foreach (var file in files) { @@ -75,6 +76,18 @@ static int Main(string[] args) continue; } + // The Confidence rule errors on a dataset that leaves the field Undefined, which + // is right for a dataset the repository ships and wrong for a fixture driving + // its own tests. Demoted here rather than in the engine, and before the merge + // rather than at the annotation, because Modify.Merge combines only the two + // top-level statuses and the exit code reads the merged one. + if (FileFilter.IsUnitTestFixture(file)) + { + resultForThisFile = DatasetSeverity.DemoteToWarning( + resultForThisFile, DatasetSeverity.ConfidenceRule, out int demotedHere); + demotedTotal += demotedHere; + } + if (verbose) Console.WriteLine($" Result Status: {resultForThisFile.Status}"); accounting.CountExamined(); @@ -121,6 +134,16 @@ static int Main(string[] args) } } + // Guarded on the format for the same reason OutputEmitter guards the coverage line: + // json and sarif put a payload on stdout and nothing may precede it. + if (demotedTotal > 0 && (outputFormat == "console" || outputFormat == "github")) + { + string notice = DatasetSeverity.DemotionNotice(demotedTotal); + Console.WriteLine(outputFormat == "github" + ? $"::notice title=Dataset confidence::{notice}" + : $"NOTICE: {notice}"); + } + const string checkType = "dataset"; OutputEmitter.Write(outputFormat, checkType, mergedResult.Status, allAnnotations, sarifFilePath, verbose, BH.Engine.Base.Query.DocumentationURL("DevOps/Code%20Compliance%20and%20CI/Compliance%20Checks/"), diff --git a/tools/ComplianceRunner/src/Shared/Compliance.Shared/DatasetSeverity.cs b/tools/ComplianceRunner/src/Shared/Compliance.Shared/DatasetSeverity.cs new file mode 100644 index 0000000..2a3f582 --- /dev/null +++ b/tools/ComplianceRunner/src/Shared/Compliance.Shared/DatasetSeverity.cs @@ -0,0 +1,132 @@ +using System; +using System.Collections.Generic; +using BH.Engine.Base; // ShallowClone +using BH.oM.Test; // TestStatus +using BH.oM.Test.Results; // TestResult, ITestInformation +using Error = BH.oM.Test.CodeCompliance.Error; + +/// +/// Reports one dataset rule at a lower severity for one kind of file, without changing what the +/// rule looks for or whether it runs. +/// +/// The Confidence rule errors on any dataset whose source information leaves Confidence +/// Undefined. That is the right answer for a dataset a repository distributes and the wrong one +/// for a fixture that exists to drive the repository's own tests, where the field says nothing +/// useful and blocking a merge on it costs more than it buys. The field is still expected on +/// both, which is why this demotes the finding rather than suppressing it or dropping the file. +/// +/// It lives here and not in the compliance engine deliberately. The engine judges a dataset; it +/// has no concept of a unit-test fixture and should not acquire one for this. The convention +/// that a .ci directory holds test data belongs to CI, so the severity decision belongs to CI too. +/// +public static class DatasetSeverity +{ + /// + /// The engine's name for the Confidence finding, as it appears on the Error and therefore on + /// Annotation.RuleName. + /// + /// + /// This is a string owned by another repository, which is the one fragile part of this. The + /// name is still the most stable handle available: matching on message text breaks on the + /// first reword, and matching on position in the information list breaks on the first + /// reorder. The guard is a test that runs the real engine over a dataset with Confidence + /// unset and asserts this exact value comes back, so a rename upstream fails a build here + /// instead of silently restoring the error for every fixture. + /// + public const string ConfidenceRule = "Dataset confidence error"; + + /// + /// Returns a result in which every Error carrying is reported as + /// a Warning, with the top-level status recomputed from what is left. Returns the input + /// unchanged, by reference, when nothing matches. + /// + /// How many findings were demoted, for the caller to report. + /// + /// The top-level status has to be recomputed, not just the finding's. Modify.Merge combines + /// the two top-level statuses and never looks at Information, and the runner's exit code + /// reads the merged top-level status. Demoting only the finding would leave the check red + /// with no error annotation to account for it, which is worse than leaving it alone. + /// + /// Recomputed as the most severe status remaining rather than set to Warning, so a fixture + /// that also fails to deserialise stays an Error. Those two cannot co-occur today, because + /// IsValidDataset returns early on a deserialisation failure, but that is the engine's + /// current shape rather than a guarantee, and assuming it here would be assuming it twice. + /// + /// The input is cloned rather than mutated. Nothing in the runner reads the original + /// afterwards, so this buys nothing today; it costs one allocation per fixture and means a + /// caller that does read it later is not surprised. + /// + public static TestResult DemoteToWarning(TestResult result, string ruleName, out int demoted) + { + demoted = 0; + + // Not null-tolerant on the result itself. The only caller returns early on a null + // result and counts it, so accepting one here would hide that path rather than guard + // it. A null Information is different: the runner defends against it two lines later + // and so does this. + if (result.Information == null) + return result; + + var rewritten = new List(result.Information.Count); + + foreach (var information in result.Information) + { + if (information is Error error + && error.Status == TestStatus.Error + && string.Equals(error.Name, ruleName, StringComparison.Ordinal)) + { + var demotedError = error.ShallowClone(); + demotedError.Status = TestStatus.Warning; + rewritten.Add(demotedError); + demoted++; + } + else + { + rewritten.Add(information); + } + } + + if (demoted == 0) + return result; + + var updated = result.ShallowClone(); + updated.Information = rewritten; + updated.Status = MostSevere(rewritten); + return updated; + } + + /// The most severe status across a set of findings, Pass when there are none. + private static TestStatus MostSevere(IEnumerable information) + { + var worst = TestStatus.Pass; + + foreach (var entry in information) + { + if (entry.Status == TestStatus.Error) + return TestStatus.Error; + + if (entry.Status == TestStatus.Warning) + worst = TestStatus.Warning; + } + + return worst; + } + + /// + /// The line reported when findings were demoted, so a green check states that it downgraded + /// something rather than implying there was nothing to downgrade. + /// + /// + /// Same argument as the self-skip summary line: a check that goes green for a reason the + /// reader cannot see is indistinguishable from one that found nothing, and this change makes + /// two repositories go green at once. + /// + /// The scope named here must match FileFilter.IsUnitTestFixture. They were allowed to + /// drift apart once, when the predicate broadened from the pair .ci/Datasets to any .ci + /// descendant and this string did not, which left the log describing a narrower rule than the + /// one that ran. A test asserts the wording for that reason. + /// + public static string DemotionNotice(int demoted) => + $"{demoted} Confidence finding(s) in unit-test fixtures under a .ci directory reported as " + + "warnings rather than errors. The field is still expected on those datasets."; +} diff --git a/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileFilter.cs b/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileFilter.cs index 4b7ddab..a1dff8a 100644 --- a/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileFilter.cs +++ b/tools/ComplianceRunner/src/Shared/Compliance.Shared/FileFilter.cs @@ -89,6 +89,47 @@ private static bool IsUnderDatasetsDirectory(string file) return false; } + /// + /// True when a file sits anywhere under a ".ci" directory, case-insensitive, which is where + /// a repository keeps the inputs to its own tests rather than anything it ships. + /// + /// + /// Separates a dataset a repository ships from one that exists only to drive its own tests. + /// The engine has no concept of that difference and should not: it is a convention about + /// where CI puts things, so it is decided here rather than in the compliance rule. + /// + /// Anchored on a whole segment, not on a substring. A substring test matches a directory + /// named My.ci or .cix, which is the same fault IsUnderDatasetsDirectory above was + /// changed to avoid. + /// + /// Not root-anchored, so a nested SomeProject/.ci counts. That directory is a .ci directory + /// wherever it sits, and the answer must not change because a caller passed an absolute path + /// rather than a repository-relative one. + /// + /// Any .ci descendant rather than .ci/Datasets specifically, which is the broader of the two + /// readings and matches what IsRelevantFile already does for the project check. The narrower + /// one left a gap: the repository convention puts test projects under .ci/unit-tests/, and a + /// fixture under one of those would have been examined and not demoted. Both spellings give + /// the same answer on the fleet as it stands, so this closes a class rather than changing an + /// outcome. + /// + public static bool IsUnitTestFixture(string file) + { + var normalized = file.Replace("\\", "/"); + + int lastSlash = normalized.LastIndexOf('/'); + if (lastSlash < 0) + return false; + + foreach (var segment in normalized.Substring(0, lastSlash).Split('/')) + { + if (segment.Equals(".ci", StringComparison.OrdinalIgnoreCase)) + return true; + } + + return false; + } + /// /// True for a versioning upgrade map, named Versioning_<digits>.json. /// diff --git a/tools/ComplianceRunner/tests/Compliance.Tests/Compliance.Tests.csproj b/tools/ComplianceRunner/tests/Compliance.Tests/Compliance.Tests.csproj index a5bd949..ce28087 100644 --- a/tools/ComplianceRunner/tests/Compliance.Tests/Compliance.Tests.csproj +++ b/tools/ComplianceRunner/tests/Compliance.Tests/Compliance.Tests.csproj @@ -32,6 +32,11 @@ Unit/OutputEmitterTests.cs OutputEmitter.Write takes a BH.oM.Test.TestStatus. Unit/AnnotationConvertTests.cs Constructs BH.oM.Test.CodeCompliance.Error and exercises the ITestInformation conversion path. + Unit/DatasetSeverityTests.cs DatasetSeverity rewrites a BH.oM.Test.Results.TestResult + and reads BH.oM.Test.CodeCompliance.Error, so neither it + nor its tests can be linked into the hermetic project. + Its path predicate is BHoM-free and does live there, in + FileFilter, which is the reason the two are split apart. Integration/* Invoke the built runner executables against real engines. Note the boundary is NOT unit-versus-integration: three of the six original unit test diff --git a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/DatasetComplianceRunnerE2ETests.cs b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/DatasetComplianceRunnerE2ETests.cs index a445f2f..0835783 100644 --- a/tools/ComplianceRunner/tests/Compliance.Tests/Integration/DatasetComplianceRunnerE2ETests.cs +++ b/tools/ComplianceRunner/tests/Compliance.Tests/Integration/DatasetComplianceRunnerE2ETests.cs @@ -246,4 +246,271 @@ public void UnanalysableFileAlone_StillFailsTheRun() try { Directory.Delete(dir, recursive: true); } catch { /* temp dir, best effort */ } } } + + // ── Confidence severity by dataset kind ────────────────────────────────── + + /// + /// Writes one dataset under and returns it with the + /// root to clean up. The content is the same every time, so the only variable across these + /// tests is where the file sits. + /// + private static (string File, string Root) WriteDataset(string relativeDirectory, string name, string json) + { + string root = Path.Combine(Path.GetTempPath(), "bhom-ci-" + Guid.NewGuid().ToString("N")); + string directory = Path.Combine(root, relativeDirectory.Replace('/', Path.DirectorySeparatorChar)); + Directory.CreateDirectory(directory); + + string file = Path.Combine(directory, name); + File.WriteAllText(file, json); + return (file, root); + } + + /// A dataset that deserialises and leaves Confidence unset, which is the rule under test. + private const string ConfidenceUnsetDataset = + "{ \"_t\" : \"BH.oM.Data.Library.Dataset\", \"Name\" : \"Probe\", " + + "\"SourceInformation\" : { \"_t\" : \"BH.oM.Data.Library.Source\", " + + "\"Title\" : \"Probe title\", \"Author\" : \"Probe author\", \"Confidence\" : \"Undefined\" }, " + + "\"Data\" : [] }"; + + [Test] + [Category("RequiresBHoM")] + [Description("The same dataset blocks under Datasets/ and only warns under .ci/Datasets/. One behaviour, both halves.")] + [TestCase("Datasets", 1, "failure", TestName = "ConfidenceUnset_InAProductionDataset_FailsTheRun")] + [TestCase(".ci/Datasets", 0, "warning", TestName = "ConfidenceUnset_InAUnitTestFixture_OnlyWarns")] + public void ConfidenceSeverity_DependsOnWhereTheDatasetSits( + string directory, int expectedExitCode, string expectedLevel) + { + var (file, root) = WriteDataset(directory, "Conf.json", ConfidenceUnsetDataset); + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "json", file); + var payload = JsonDocument.Parse(stdout).RootElement; + + var confidence = payload.GetProperty("annotations").EnumerateArray() + .Single(a => a.GetProperty("ruleName").GetString() == "Dataset confidence error"); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(expectedExitCode)); + Assert.That(confidence.GetProperty("level").GetString(), Is.EqualTo(expectedLevel)); + Assert.That(payload.GetProperty("coverage").GetProperty("examined").GetInt32(), Is.EqualTo(1), + "the demotion must not change what is selected or examined"); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("A fixture that does not deserialise still fails. Only the Confidence rule is demoted, and only its severity.")] + public void AFixtureThatDoesNotDeserialise_StillFailsTheRun() + { + var (file, root) = WriteDataset(".ci/Datasets", "Bad.json", "not a dataset"); + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "json", file); + var payload = JsonDocument.Parse(stdout).RootElement; + + var deserialisation = payload.GetProperty("annotations").EnumerateArray() + .Single(a => a.GetProperty("ruleName").GetString() == "Dataset deserialisation error"); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(1)); + Assert.That(payload.GetProperty("status").GetString(), Is.EqualTo("Error")); + Assert.That(deserialisation.GetProperty("level").GetString(), Is.EqualTo("failure")); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("A fixture's missing title is still a warning. The other source rules are untouched by the split.")] + public void AFixturesOtherSourceWarnings_AreUnchanged() + { + const string noTitle = + "{ \"_t\" : \"BH.oM.Data.Library.Dataset\", \"Name\" : \"Probe\", " + + "\"SourceInformation\" : { \"_t\" : \"BH.oM.Data.Library.Source\", " + + "\"Author\" : \"a\", \"Confidence\" : \"High\" }, \"Data\" : [] }"; + + var (file, root) = WriteDataset(".ci/Datasets", "NoTitle.json", noTitle); + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "json", file); + var payload = JsonDocument.Parse(stdout).RootElement; + + var title = payload.GetProperty("annotations").EnumerateArray() + .Single(a => a.GetProperty("ruleName").GetString() == "Dataset source title error"); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(0)); + Assert.That(title.GetProperty("level").GetString(), Is.EqualTo("warning")); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("A demoted finding and a real one in the same run: the run is red and the demotion did not swallow either.")] + public void ADemotedFixtureAlongsideAFailingOne_StillFailsTheRun() + { + string root = Path.Combine(Path.GetTempPath(), "bhom-ci-" + Guid.NewGuid().ToString("N")); + string fixtures = Path.Combine(root, ".ci", "Datasets"); + Directory.CreateDirectory(fixtures); + + string demoted = Path.Combine(fixtures, "Conf.json"); + File.WriteAllText(demoted, ConfidenceUnsetDataset); + string broken = Path.Combine(fixtures, "Bad.json"); + File.WriteAllText(broken, "not a dataset"); + + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "json", demoted, broken); + var payload = JsonDocument.Parse(stdout).RootElement; + + var levels = payload.GetProperty("annotations").EnumerateArray() + .Where(a => a.GetProperty("ruleName").GetString()!.Length > 0) + .ToDictionary(a => a.GetProperty("ruleName").GetString()!, + a => a.GetProperty("level").GetString()); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(1)); + Assert.That(levels["Dataset confidence error"], Is.EqualTo("warning")); + Assert.That(levels["Dataset deserialisation error"], Is.EqualTo("failure")); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("A green that demoted something says so, rather than looking like a run that found nothing.")] + public void ADemotedRun_ReportsWhatItDowngraded() + { + var (file, root) = WriteDataset(".ci/Datasets", "Conf.json", ConfidenceUnsetDataset); + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "github", file); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(0)); + Assert.That(stdout, Does.Contain("::notice title=Dataset confidence::1 Confidence finding")); + Assert.That(stdout, Does.Contain("The field is still expected on those datasets.")); + Assert.That(stdout, Does.Not.Contain("under .ci/Datasets"), + "the notice must describe the scope the predicate applies, which is any .ci descendant"); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("json and sarif carry a payload on stdout, so the demotion notice must not precede it.")] + [TestCase("json")] + [TestCase("sarif")] + public void TheDemotionNotice_DoesNotContaminateAMachineReadablePayload(string format) + { + var (file, root) = WriteDataset(".ci/Datasets", "Conf.json", ConfidenceUnsetDataset); + try + { + var (_, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", format, file); + + Assert.Multiple(() => + { + Assert.That(stdout, Does.Not.Contain("Confidence finding(s) in unit-test fixtures")); + Assert.That(() => JsonDocument.Parse(stdout), Throws.Nothing, + "stdout must be the payload and nothing else, which is the property at risk here"); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + [Test] + [Category("RequiresBHoM")] + [Description("SARIF carries the demoted finding at warning too. The level is mapped in a second place.")] + public void ADemotedFinding_IsAWarningInSarifAsWell() + { + var (file, root) = WriteDataset(".ci/Datasets", "Conf.json", ConfidenceUnsetDataset); + try + { + var (exitCode, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "sarif", file); + var results = JsonDocument.Parse(stdout).RootElement + .GetProperty("runs")[0].GetProperty("results"); + + var confidence = results.EnumerateArray() + .Single(r => r.GetProperty("ruleId").GetString()!.Contains("Dataset confidence error")); + + Assert.Multiple(() => + { + Assert.That(exitCode, Is.EqualTo(0)); + Assert.That(confidence.GetProperty("level").GetString(), Is.EqualTo("warning"), + "SarifBuilder maps the level separately from the GitHub emitter, so it needs its own assertion"); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + + /// + /// Pins the rule name the demotion keys on against the engine that actually produces it. + /// + /// DatasetSeverity matches a string owned by Test_Toolkit. If that name is reworded upstream + /// the demotion silently stops matching, every fixture goes back to blocking, and nothing + /// points at the cause. This fails the build here instead. It deliberately uses the literal + /// rather than the constant on one side, so a change to the constant cannot make it pass. + /// + [Test] + [Category("RequiresBHoM")] + [Description("The engine still calls the Confidence finding what DatasetSeverity expects.")] + public void TheEngineStillNamesTheConfidenceRuleWhatTheDemotionExpects() + { + var (file, root) = WriteDataset("Datasets", "Conf.json", ConfidenceUnsetDataset); + try + { + var (_, stdout) = RunnerFixture.Run("DatasetComplianceRunner", "--output", "json", file); + var payload = JsonDocument.Parse(stdout).RootElement; + + var ruleNames = payload.GetProperty("annotations").EnumerateArray() + .Select(a => a.GetProperty("ruleName").GetString()) + .ToList(); + + Assert.Multiple(() => + { + Assert.That(ruleNames, Does.Contain("Dataset confidence error"), + "the engine's name for this finding has changed; DatasetSeverity.ConfidenceRule no longer matches it"); + Assert.That(DatasetSeverity.ConfidenceRule, Is.EqualTo("Dataset confidence error"), + "the constant has been edited away from the name the engine emits"); + }); + } + finally + { + try { Directory.Delete(root, recursive: true); } catch { /* temp dir, best effort */ } + } + } + } diff --git a/tools/ComplianceRunner/tests/Compliance.Tests/Unit/DatasetSeverityTests.cs b/tools/ComplianceRunner/tests/Compliance.Tests/Unit/DatasetSeverityTests.cs new file mode 100644 index 0000000..a6ea375 --- /dev/null +++ b/tools/ComplianceRunner/tests/Compliance.Tests/Unit/DatasetSeverityTests.cs @@ -0,0 +1,204 @@ +using NUnit.Framework; +using System.Collections.Generic; +using BH.oM.Test; +using BH.oM.Test.Results; +using Error = BH.oM.Test.CodeCompliance.Error; +using DatasetSource = BH.oM.Test.CodeCompliance.DatasetSource; + +/// +/// The demotion itself, in isolation from the engine and the runner. +/// +/// These construct the TestResult shape IsValidDataset returns rather than calling it, so a +/// failure here points at the demotion and not at the rule. The engine-backed half, including +/// the pin on the rule name, is in DatasetComplianceRunnerE2ETests. +/// +[TestFixture] +public class DatasetSeverityTests +{ + private static Error Finding(string name, TestStatus status) => + new Error { Name = name, Status = status, Message = name }; + + private static TestResult Result(TestStatus status, params ITestInformation[] information) => + new TestResult { Status = status, Information = new List(information) }; + + [Test] + [Description("The rule being demoted drops to Warning, and so does the top-level status the exit code reads.")] + public void ConfidenceError_BecomesAWarning_AndTheResultStopsBeingAnError() + { + var result = Result(TestStatus.Error, Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error)); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demoted, Is.EqualTo(1)); + Assert.That(demotedResult.Status, Is.EqualTo(TestStatus.Warning), + "Modify.Merge reads only the top-level status, so leaving it Error would keep the check red with no error annotation"); + Assert.That(demotedResult.Information[0].Status, Is.EqualTo(TestStatus.Warning)); + }); + } + + [Test] + [Description("A deserialisation failure is untouched. This is the case the split must not weaken.")] + public void ADifferentRule_IsLeftAlone() + { + var result = Result(TestStatus.Error, Finding("Dataset deserialisation error", TestStatus.Error)); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demoted, Is.EqualTo(0)); + Assert.That(demotedResult.Status, Is.EqualTo(TestStatus.Error)); + Assert.That(demotedResult, Is.SameAs(result), "nothing matched, so nothing should have been rebuilt"); + }); + } + + [Test] + [Description("A fixture carrying both keeps its Error. The status is recomputed from what is left, not set to Warning.")] + public void ConfidenceAlongsideAnotherError_StaysAnError() + { + var result = Result(TestStatus.Error, + Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error), + Finding("Dataset deserialisation error", TestStatus.Error)); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demoted, Is.EqualTo(1)); + Assert.That(demotedResult.Status, Is.EqualTo(TestStatus.Error), + "the surviving error must still fail the run"); + Assert.That(demotedResult.Information[0].Status, Is.EqualTo(TestStatus.Warning)); + Assert.That(demotedResult.Information[1].Status, Is.EqualTo(TestStatus.Error)); + }); + } + + [Test] + [Description("The Pass-level DatasetSource notice is carried through, not promoted or dropped.")] + public void TheProvenanceNotice_SurvivesUntouched() + { + var notice = new DatasetSource { Status = TestStatus.Pass, Message = "Confidence: Undefined" }; + var result = Result(TestStatus.Error, notice, Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error)); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out _); + + Assert.Multiple(() => + { + Assert.That(demotedResult.Information, Has.Count.EqualTo(2)); + Assert.That(demotedResult.Information[0], Is.SameAs(notice)); + Assert.That(demotedResult.Information[0].Status, Is.EqualTo(TestStatus.Pass)); + Assert.That(demotedResult.Status, Is.EqualTo(TestStatus.Warning)); + }); + } + + [Test] + [Description("A finding already at Warning is not counted as demoted, so the reported figure stays honest.")] + public void AConfidenceFindingAlreadyAtWarning_IsNotCountedAgain() + { + var result = Result(TestStatus.Warning, Finding(DatasetSeverity.ConfidenceRule, TestStatus.Warning)); + + DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.That(demoted, Is.EqualTo(0)); + } + + [Test] + [Description("Matching is exact. A near-miss on the rule name must not silently demote something else.")] + [TestCase("dataset confidence error")] + [TestCase("Dataset Confidence Error")] + [TestCase("Dataset confidence")] + [TestCase("Dataset confidence error ")] + public void TheRuleNameIsMatchedExactly(string name) + { + var result = Result(TestStatus.Error, Finding(name, TestStatus.Error)); + + DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.That(demoted, Is.EqualTo(0)); + } + + [Test] + [Description("The caller's result is not mutated, so a later reader sees what the engine returned.")] + public void TheInputIsNotMutated() + { + var original = Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error); + var result = Result(TestStatus.Error, original); + + DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out _); + + Assert.Multiple(() => + { + Assert.That(result.Status, Is.EqualTo(TestStatus.Error)); + Assert.That(original.Status, Is.EqualTo(TestStatus.Error)); + }); + } + + [Test] + [Description("Every finding demoted leaves a Warning, not a Pass, because the findings are still there.")] + public void TwoConfidenceErrors_BothDemoted_LeaveAWarning() + { + var result = Result(TestStatus.Error, + Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error), + Finding(DatasetSeverity.ConfidenceRule, TestStatus.Error)); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demoted, Is.EqualTo(2)); + Assert.That(demotedResult.Status, Is.EqualTo(TestStatus.Warning)); + }); + } + + [Test] + [Description("A null Information does not throw. The runner defends against it, and so does this.")] + public void ANullInformation_IsReturnedUnchanged() + { + var result = new TestResult { Status = TestStatus.Pass, Information = null }; + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demotedResult, Is.SameAs(result)); + Assert.That(demoted, Is.EqualTo(0)); + }); + } + + [Test] + [Description("An empty Information does not throw and reports nothing demoted.")] + public void AnEmptyInformation_IsReturnedUnchanged() + { + var result = Result(TestStatus.Pass); + + var demotedResult = DatasetSeverity.DemoteToWarning(result, DatasetSeverity.ConfidenceRule, out int demoted); + + Assert.Multiple(() => + { + Assert.That(demotedResult, Is.SameAs(result)); + Assert.That(demoted, Is.EqualTo(0)); + }); + } + + [Test] + [Description("The notice names the count, so a green check states what it downgraded.")] + public void TheDemotionNotice_CarriesTheCount() + { + Assert.That(DatasetSeverity.DemotionNotice(55), Does.StartWith("55 Confidence finding")); + } + + [Test] + [Description("The notice describes the scope the predicate actually applies. These drifted apart once.")] + public void TheDemotionNotice_NamesTheSameScopeAsThePredicate() + { + string notice = DatasetSeverity.DemotionNotice(1); + + Assert.Multiple(() => + { + Assert.That(notice, Does.Contain(".ci")); + Assert.That(notice, Does.Not.Contain(".ci/Datasets"), + "the predicate matches any .ci descendant, so the notice must not claim the narrower pair"); + }); + } +} diff --git a/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileFilterTests.cs b/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileFilterTests.cs index 1b154d0..781ff4f 100644 --- a/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileFilterTests.cs +++ b/tools/ComplianceRunner/tests/Compliance.Unit.Tests/Unit/FileFilterTests.cs @@ -109,4 +109,49 @@ public bool IsDatasetFile(string file) public bool IsVersioningUpgradeMap(string file) => FileFilter.IsVersioningUpgradeMap(file); } + + [TestFixture] + public class IsUnitTestFixtureTests + { + // The shape that exists in practice. + [TestCase(".ci/Datasets/Compute/Split.json", ExpectedResult = true)] + [TestCase(".ci/Datasets/foo.json", ExpectedResult = true)] + [TestCase(@".ci\Datasets\foo.json", ExpectedResult = true)] // backslash separators + [TestCase(".CI/Datasets/foo.json", ExpectedResult = true)] // segment case-insensitive + [TestCase(".Ci/DataSets/foo.json", ExpectedResult = true)] + + // Not root-anchored. A nested .ci directory is still a .ci directory, and the answer + // must not change because a caller passed an absolute path. + [TestCase("SomeProject/.ci/Datasets/foo.json", ExpectedResult = true)] + [TestCase("a/b/c/.ci/Datasets/deep/nested/x.json", ExpectedResult = true)] + [TestCase("C:/work/repo/.ci/Datasets/foo.json", ExpectedResult = true)] + + // Any .ci descendant, not only .ci/Datasets. The repository convention puts test + // projects under .ci/unit-tests/, and a fixture dropped beside one is test data by the + // same argument. The narrower pair test left exactly this gap. + [TestCase(".ci/unit-tests/Datasets/foo.json", ExpectedResult = true)] + [TestCase(".ci/unit-tests/Foo_Tests/FooTests/Datasets/x.json", ExpectedResult = true)] + [TestCase(".ci/DatasetsOld/foo.json", ExpectedResult = true)] + [TestCase(".ci/foo.json", ExpectedResult = true)] + [TestCase("Datasets/.ci/foo.json", ExpectedResult = true)] + + // A production dataset, which is the case the whole split exists to keep blocking. + [TestCase("Datasets/Structure/EPD.json", ExpectedResult = false)] + [TestCase("DataSets/Structure/DesignData/x.json", ExpectedResult = false)] + [TestCase("Revit_Engine/Datasets/UK/Regional.json", ExpectedResult = false)] + + // The anchoring, which is the reason this is not a substring test. Each of these + // contains ".ci" in the path and must not match, because no whole segment is ".ci". + [TestCase("My.ci/Datasets/foo.json", ExpectedResult = false)] + [TestCase("x.ci/Datasets/foo.json", ExpectedResult = false)] + [TestCase(".cix/Datasets/foo.json", ExpectedResult = false)] + [TestCase("ci/Datasets/foo.json", ExpectedResult = false)] // no leading dot + + // Only the directory portion is read, so a file named .ci does not take itself out. + [TestCase("a/b/.ci", ExpectedResult = false)] + [TestCase("Datasets.json", ExpectedResult = false)] + [TestCase("foo.json", ExpectedResult = false)] // no directory at all + public bool IsUnitTestFixture(string file) + => FileFilter.IsUnitTestFixture(file); + } }