diff --git a/.github/actions/ci-versioning/action.yml b/.github/actions/ci-versioning/action.yml index 83ddfab..f1db736 100644 --- a/.github/actions/ci-versioning/action.yml +++ b/.github/actions/ci-versioning/action.yml @@ -594,7 +594,11 @@ runs: dotnet restore "${{ steps.ver_sln.outputs.path }}" dotnet build "${{ steps.ver_sln.outputs.path }}" --no-restore -c ${{ steps.ver_config.outputs.config }} --nologo + # The path is emitted rather than repeated. The runner reads the same tree to resolve an + # object record's declaring assembly, and two literals of one path in one file is how they + # drift apart. - name: Validate versioning datasets + id: datasets if: steps.changed.outputs.count != '0' shell: pwsh run: | @@ -609,6 +613,7 @@ runs: exit 1 } Write-Host "::notice title=Versioning::Found versioning datasets for: $($versions.Name -join ', ')" + "root=$path" | Out-File $env:GITHUB_OUTPUT -Append # Precondition, not a policy choice. Attribution narrows to the namespaces this # repository's own assemblies declare; with no assemblies there is nothing to narrow to, @@ -746,8 +751,14 @@ runs: # line, so no annotation is created. Measured on a sandbox validation run: # Classification reported real/SignatureResolved=2 while zero finding # annotations were produced, against two on a comparable production run. + # --datasets is the tree the step above validated. An object record carries its + # declaring assembly in the record's own `_asm` field, and this is the only place the + # runner can read it: the TestResult tree the test returns does not carry it. + # Attribution falls back to the namespace prefix for any record the dataset does not + # answer for, so this is inert until the backfill lands. & "${{ steps.runner.outputs.runner_exe }}" ` --assemblies 'C:\ProgramData\BHoM\Assemblies' ` + --datasets '${{ steps.datasets.outputs.root }}' ` --subject-assembly-list 'subject-assemblies.txt' ` --configuration 'Release' ` $(if ('${{ steps.vercond.outputs.file }}') { '--version-conditional', '${{ steps.vercond.outputs.file }}' }) ` @@ -883,6 +894,16 @@ runs: $md += "| Dataset versions | $(if ($coverage.DatasetVersions -eq 0) { 'all staged' } else { 'previous only' }) |" $md += "| Assemblies loaded | $($coverage.LoadedAssemblies) |" $md += "| FromJsonDatasets entry points invoked | $($coverage.VerifyEntryPoints) |" + # Zero means every object record was attributed by namespace prefix, which is the + # state until the dataset backfill lands. Printed either way, for the same reason the + # two rows above it are. + $md += "| Dataset types carrying a declaring assembly | $($coverage.TypesWithDeclaringAssembly) |" + # Findings discarded at attribution as another repository's. Printed either way, + # because without it a run that filtered everything out and a run that found + # nothing wrong produce the same table. It counts leaves dropped at attribution + # and is not a diff against a previous run. + $dropped = $coverage.DroppedByDeclaringAssembly + $coverage.DroppedByNamespaceFallback + $md += "| Dropped as another repository's | $dropped ($($coverage.DroppedByDeclaringAssembly) by declaring assembly, $($coverage.DroppedByNamespaceFallback) by namespace prefix) |" } $md += "| Build configuration | ``$(if ($configuration) { $configuration } else { 'not recorded' })`` |" $md += "" diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs index 30f6d8d..3b647a3 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs @@ -6,7 +6,7 @@ namespace VersioningRunner.Tests // Load order is not cosmetic. ProbeDeclaringType takes its verdict from the first // loaded assembly that yields the declaring type, and 42 type names in the fleet are // defined by more than one assembly, so the enumeration order decides the - // classification for those. See CI_Toolkit#161. + // classification for those. public class AssemblyLoadOrderTests { private static string[] Names(IEnumerable paths) diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs new file mode 100644 index 0000000..540bbc8 --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/DatasetProvenanceTests.cs @@ -0,0 +1,355 @@ +using System.Text.Json; +using VersioningRunner.Commands; +using Xunit; + +namespace VersioningRunner.Tests +{ + // The map from a dataset record's type name to the assembly that declared it. + // + // Every fixture here names either a public BHoM assembly or a placeholder that resolves + // to nothing. Real records name the oM assemblies of private Revit tools, and a test + // fixture carries such a name into a public repository as readily as a comment does. + public class DatasetProvenanceTests + { + // A closed generic. Its name carries commas and brackets, and it is the shape that the + // earlier message-based route silently lost 23 of in the 9.2 dataset. + private const string ClosedGeneric = + "BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, StructuralEngineering_oM, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null]]"; + + // ------------------------------------------------------------------ + // The union across versions + // ------------------------------------------------------------------ + + [Fact] + public void AgreedAcrossVersions_MapsTheType() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + Assert.Empty(map.Disputed); + Assert.Equal(2, map.VersionsRead); + } + + // The reason this reads every version rather than the newest. Two of the 205 + // object-record findings across the 15 saved run artefacts resolve only against 9.2, + // and both are NoMethodEvent findings carrying no declaring assembly, which is exactly + // the population the field exists to serve. A newest-only read loses them and falls + // back to the namespace guess without saying so. + [Fact] + public void PresentInOneVersionOnly_IsStillMapped() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Environment.Elements.Panel", "Environment_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("Environment_oM", map.DeclaringAssemblyFor("BH.oM.Environment.Elements.Panel")); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + [Fact] + public void DifferentFamiliesAcrossVersions_AreDisputedAndNotMapped() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Structure.Elements.Panel", "Structure_oM")) + .WithObjects("9.3", Record("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Null(map.DeclaringAssemblyFor("BH.oM.Structure.Elements.Panel")); + Assert.False(map.ByTypeName.ContainsKey("BH.oM.Structure.Elements.Panel")); + Assert.Equal(0, map.TypesMapped); + } + + [Fact] + public void ADisputedTypeCarriesEveryAssemblyThatClaimedIt() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Structure.Elements.Panel", "Structure_oM")) + .WithObjects("9.3", Record("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal( + ["StructuralEngineering_oM", "Structure_oM"], + map.Disputed["BH.oM.Structure.Elements.Panel"]); + } + + // Ordinal-sorted rather than enumeration-ordered, so the warning text and the artefact + // row do not change between two runs over the same datasets. + [Fact] + public void DisputedCandidatesAreOrderedIndependentlyOfTheVersionTheyCameFrom() + { + using var ab = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Alpha_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Zulu_oM")); + using var ba = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Acoustic.Panel", "Zulu_oM")) + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Alpha_oM")); + + Assert.Equal( + DatasetProvenance.Build(ab.Root).Disputed["BH.oM.Acoustic.Panel"], + DatasetProvenance.Build(ba.Root).Disputed["BH.oM.Acoustic.Panel"]); + } + + // Two years of one repository are not two answers. Attribution strips the year, so + // calling this a disagreement would drop the finding to the namespace guess for no + // gain, which is the failure "compare year-insensitively" exists to prevent. + [Fact] + public void YearVariantsAcrossVersions_AreNotADisagreement() + { + using var datasets = new TempDatasets() + .WithObjects("9.2", Record("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_X_oM_2022")) + .WithObjects("9.3", Record("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_X_oM_2023")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.Disputed); + Assert.Equal("Revit_X_oM_2022", map.DeclaringAssemblyFor("BH.oM.Adapters.Revit.Elements.ModelInstance")); + } + + [Fact] + public void AClosedGenericKeyRoundTripsWhole() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record(ClosedGeneric, "StructuralEngineering_oM")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal("StructuralEngineering_oM", map.DeclaringAssemblyFor(ClosedGeneric)); + } + + // ------------------------------------------------------------------ + // Inertness. The state of the tracked dataset before the backfill lands. + // ------------------------------------------------------------------ + + [Fact] + public void RecordsCarryingNoField_LeaveTheMapEmptyWithoutFailing() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", + Record("BH.oM.Acoustic.Panel", assembly: null), + Record("BH.oM.Structure.Elements.Panel", assembly: null)); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.ByTypeName); + Assert.Empty(map.Disputed); + Assert.Null(map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // The denominator. Without it an empty map reads the same whether the dataset carries + // no field or the runner read nothing at all, and those need different action. + [Fact] + public void RecordsCarryingNoField_AreStillCountedAsRead() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", + Record("BH.oM.Acoustic.Panel", assembly: null), + Record("BH.oM.Structure.Elements.Panel", assembly: null)); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(2, map.RecordsRead); + Assert.Equal(0, map.TypesMapped); + } + + [Fact] + public void BlankLinesAreNotRecords() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM"), "", " "); + + Assert.Equal(1, DatasetProvenance.Build(datasets.Root).RecordsRead); + } + + // ------------------------------------------------------------------ + // A field that is present but unusable reads as absent + // ------------------------------------------------------------------ + + [Theory] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : 7, \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : null, \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : \"\", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\", \"_asm\" : \" \", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_asm\" : \"Acoustic_oM\", \"_bhomVersion\" : \"9.3\" }")] + [InlineData("{ \"_t\" : \"\", \"_asm\" : \"Acoustic_oM\", \"_bhomVersion\" : \"9.3\" }")] + public void AnUnusableRecordMapsNothingAndIsNotAnError(string line) + { + using var datasets = new TempDatasets().WithObjects("9.3", line); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Empty(map.ByTypeName); + Assert.Equal(1, map.RecordsRead); + Assert.Equal(0, map.LinesUnparseable); + } + + // Counted and reported rather than thrown. Losing the whole map over one bad line + // would convert one dataset defect into a fleet-wide fallback to the namespace guess. + [Theory] + [InlineData("{ \"_t\" : \"BH.oM.Acoustic.Panel\" ")] + [InlineData("not json at all")] + [InlineData("[ \"an array, not a record\" ]")] + public void AMalformedLineIsCountedAndSkipped(string line) + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM"), line); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(1, map.LinesUnparseable); + Assert.Equal(2, map.RecordsRead); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // ------------------------------------------------------------------ + // What is and is not read + // ------------------------------------------------------------------ + + // Both files carry `_t: System.Reflection.MethodBase` on every record, so the key + // identifies nothing, and their leaves already reach the runner with a declaring + // assembly from the Method event. + [Fact] + public void MethodsAndAdaptersAreNotRead() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithFile("9.3", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")) + .WithFile("9.3", "Adapters.json", Record("System.Reflection.MethodBase", "Acoustic_Adapter")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Null(map.DeclaringAssemblyFor("System.Reflection.MethodBase")); + Assert.Equal(1, map.RecordsRead); + } + + [Fact] + public void AVersionWithoutAnObjectsJsonIsSkippedRatherThanFatal() + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")) + .WithFile("9.2", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")); + + var map = DatasetProvenance.Build(datasets.Root); + + Assert.Equal(1, map.VersionsRead); + Assert.Equal("Acoustic_oM", map.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + // ------------------------------------------------------------------ + // The two states that are a broken precondition rather than an empty result + // ------------------------------------------------------------------ + + [Fact] + public void AnAbsentRootThrows() + { + string missing = Path.Combine(Path.GetTempPath(), "versioning-runner-no-such-datasets-" + Guid.NewGuid()); + + var thrown = Assert.Throws(() => DatasetProvenance.Build(missing)); + Assert.Contains(missing, thrown.Message, StringComparison.Ordinal); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void AnUnsuppliedRootThrows(string? root) + { + Assert.Throws(() => DatasetProvenance.Build(root)); + } + + [Fact] + public void ARootWithNoObjectsJsonAnywhereThrows() + { + using var datasets = new TempDatasets() + .WithFile("9.2", "Methods.json", Record("System.Reflection.MethodBase", "Acoustic_Engine")) + .WithFile("9.3", "Adapters.json", Record("System.Reflection.MethodBase", "Acoustic_Adapter")); + + Assert.Throws(() => DatasetProvenance.Build(datasets.Root)); + } + + [Fact] + public void ARootWithNoVersionDirectoriesThrows() + { + using var datasets = new TempDatasets(); + + Assert.Throws(() => DatasetProvenance.Build(datasets.Root)); + } + + // ------------------------------------------------------------------ + // Lookup + // ------------------------------------------------------------------ + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("No.Such.Type")] + public void LookupOfSomethingNotMappedYieldsNull(string? typeName) + { + using var datasets = new TempDatasets() + .WithObjects("9.3", Record("BH.oM.Acoustic.Panel", "Acoustic_oM")); + + Assert.Null(DatasetProvenance.Build(datasets.Root).DeclaringAssemblyFor(typeName)); + } + + [Fact] + public void TheEmptyMapAnswersNullRatherThanThrowing() + { + Assert.Null(DeclaringAssemblyMap.Empty.DeclaringAssemblyFor("BH.oM.Acoustic.Panel")); + } + + /*************************************/ + /**** Fixtures ****/ + /*************************************/ + + // Field order matches a real record: `_t` first, `_asm` immediately before the last + // `_bhomVersion`, which is where the backfill and the capture both put it. + private static string Record(string type, string? assembly, string version = "9.3") + { + string field = assembly is null + ? string.Empty + : $"\"_asm\" : {JsonSerializer.Serialize(assembly)}, "; + + return $"{{ \"_t\" : {JsonSerializer.Serialize(type)}, \"Name\" : \"fixture\", " + + $"{field}\"_bhomVersion\" : \"{version}\" }}"; + } + + private sealed class TempDatasets : IDisposable + { + public string Root { get; } + + public TempDatasets() + { + Root = Path.Combine(Path.GetTempPath(), "versioning-runner-datasets-" + Guid.NewGuid()); + Directory.CreateDirectory(Root); + } + + public TempDatasets WithObjects(string version, params string[] lines) + => WithFile(version, "Objects.json", lines); + + public TempDatasets WithFile(string version, string fileName, params string[] lines) + { + string directory = Path.Combine(Root, version); + Directory.CreateDirectory(directory); + File.WriteAllLines(Path.Combine(directory, fileName), lines); + return this; + } + + public void Dispose() + { + // A locked or already-removed temp tree must not fail the test that produced it; + // the leftover is a few kilobytes under TEMP. Anything else is a real fault and + // is left to propagate. + try { Directory.Delete(Root, recursive: true); } + catch (IOException) { } + catch (UnauthorizedAccessException) { } + } + } + } +} diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs new file mode 100644 index 0000000..a3ca163 --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/ObjectRecordProvenanceTests.cs @@ -0,0 +1,620 @@ +using System.Reflection; +using VersioningRunner.Commands; +using VersioningRunner.Models; +using VersioningRunner.Tests.Fixtures; +using Xunit; + +namespace VersioningRunner.Tests +{ + // Object records carry a type name and nothing else, so attribution has always guessed from + // the namespace. The dataset's `_asm` field closes that, and these pin the runner half. + // + // Where the map comes from is DatasetProvenanceTests' subject. These drive it in as a value, + // because what they pin is how a leaf consumes it: the join key, the Method event's + // precedence over it, and what a disagreement does. The join key is the leaf description, + // which for an object record is the `_t` value verbatim. + // + // The limit of this coverage: it proves the runner reads a map correctly, not that a live + // FromJsonDatasets tree produces leaves that hit one. Only a real run shows that. + public class ObjectRecordProvenanceTests + { + // What the staged Objects.json resolved, as the collector receives it. + private static DeclaringAssemblyMap Map(params (string Type, string Assembly)[] entries) + => new( + entries.ToDictionary(e => e.Type, e => e.Assembly, StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal), + VersionsRead: 1, RecordsRead: entries.Length, TypesMapped: entries.Length, LinesUnparseable: 0); + + // A type two dataset versions named different assembly families for. + private static DeclaringAssemblyMap Disputed(string type, params string[] assemblies) + => new( + new Dictionary(StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal) { [type] = assemblies }, + VersionsRead: 2, RecordsRead: assemblies.Length, TypesMapped: 0, LinesUnparseable: 0); + + // A closed generic. Its name contains commas, which is why the quoted part holds the + // assembly alone: a comma-delimited form silently lost all 23 of these in the 9.2 + // dataset. This one is the case that mattered most, because its declaring assembly is + // another repository's while its namespace is the subject's, so losing the field left + // the original misattribution in place. + private const string ClosedGeneric = + "BH.oM.Structure.Results.ResultEnvelope`1[[BH.oM.Structure.Results.ConnectionForce, StructuralEngineering_oM, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null]]"; + + private const string MethodEvent = + "Method TryGetValueFromSource from { \"_t\" : \"System.Type\", \"Name\" : \"BH.Revit.Engine.Core.Compute, Revit_Core_Engine_2022, Version=9.0.0.0, Culture=neutral, PublicKeyToken=null\", \"_bhomVersion\" : \"9.2\" } failed to deserialise."; + + private static FakeTestResult Tree(string description, params string[] events) + { + var leaf = new FakeTestInfo + { + Status = "Error", + Description = description, + Message = "Error: Returned null from json.", + Information = events.Select(m => (object)new FakeEventMessage { Message = m }).ToList() + }; + return new FakeTestResult + { + Status = "Error", + Information = [new FakeTestResult { Status = "Error", Information = [leaf] }] + }; + } + + // Subject names arrive as file names with extension, exactly as ReadSubjectAssemblyListFrom + // produces them, so the extension handling is exercised rather than assumed away. + private static ClosureContext ClosureForSubject(params string[] subjectFileNames) + { + var loaded = new HashSet(StringComparer.Ordinal); + return new ClosureContext( + loaded, + new HashSet(loaded.Select(RunCommand.StripConfigSuffix), StringComparer.Ordinal), + new HashSet( + RunCommand.ReadSubjectAssemblyListFrom(subjectFileNames) + .Select(f => RunCommand.StripConfigSuffix(Path.GetFileNameWithoutExtension(f))), + StringComparer.Ordinal)); + } + + // ------------------------------------------------------------------ + // Reading the field + // ------------------------------------------------------------------ + + // The join. The leaf's description is the record's `_t` value, so the map is keyed on + // exactly what arrives here. Measured on the 15 saved run artefacts: 205 of 205 + // object-record findings match a dataset record on this key, with no fuzzy matching. + [Fact] + public void TheLeafDescriptionIsTheKeyIntoTheMap() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); + + Assert.Equal("Acoustic_oM", Assert.Single(diagnostics).DeclaringAssembly); + } + + // A map that answers for other types but not this one leaves the leaf where it was. + [Fact] + public void ATypeTheDatasetDoesNotNameIsNotGivenAnAssembly() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM"))); + + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); + } + + // The whole point of the field: this record used to be attributable only by prefix. + [Fact] + public void ObjectRecordNamingTheSubjectsAssembly_IsAttributedByDeclaringAssembly() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + var (attributable, basis) = RunCommand.AttributeToSubject( + "BH.oM.Acoustic.Panel", "Acoustic_oM", new HashSet(StringComparer.Ordinal), closure); + + Assert.True(attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, basis); + } + + [Fact] + public void ObjectRecordNamingAnotherRepositorysAssembly_IsNotAttributed() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + var (attributable, basis) = RunCommand.AttributeToSubject( + "BH.oM.Structure.Elements.Panel", "Structure_oM", new HashSet(StringComparer.Ordinal), closure); + + Assert.False(attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, basis); + } + + // ------------------------------------------------------------------ + // Inertness. These are the tests that say this PR does nothing on its own. + // ------------------------------------------------------------------ + + // No map supplied at all, which is what every caller that does not pass one gets and + // what the runner does without --datasets. Identical to the behaviour before this change. + [Fact] + public void NoMap_LeavesTheDeclaringAssemblyNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", "Failed to convert the string into a type: BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics); + + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); + } + + // The dataset read before the backfill lands: records present, none carrying the field. + [Fact] + public void AnEmptyMap_LeavesTheDeclaringAssemblyNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: DeclaringAssemblyMap.Empty); + + Assert.Null(Assert.Single(diagnostics).DeclaringAssembly); + } + + // A record cannot be both, and the method path must not change. If the map happens to + // answer for a method leaf's description, the Method event still wins, because it is + // read first and the dataset is only consulted when it yielded nothing. + [Fact] + public void MethodEventStillWins_WhenTheMapAlsoAnswers() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.Revit.Engine.Core.Compute. }", MethodEvent), + (_, _) => (true, AttributionBasis.NotRecorded), null, + (_, _, _) => (null, ClassificationPath.DeclaringTypeNotLoaded, Array.Empty()), diagnostics, + provenance: Map(("BH.Revit.Engine.Core.Compute. }", "Acoustic_oM"))); + + Assert.Equal("Revit_Core_Engine_2022", Assert.Single(diagnostics).DeclaringAssembly); + } + + // Whole closure discards the declaring assembly for attribution: Execute wires + // `(d, _) =>` and supplies no ClosureContext. So on a run with no subject list this + // change cannot alter which findings are reported, only what the artefact records. + // The backfill's own validation runs were whole-closure, so they could not have + // exercised any of the attribution behaviour above. + [Fact] + public void WholeClosure_IgnoresTheDeclaringAssemblyForAttribution() + { + var nsPrefixes = new HashSet(["BH.oM.Acoustic"], StringComparer.Ordinal); + Func wholeClosure = + (d, _) => (RunCommand.IsFromLoadedNamespace(d, nsPrefixes), AttributionBasis.NotRecorded); + + var withAsm = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + wholeClosure, null, null, withAsm, + provenance: Map(("BH.oM.Acoustic.Panel", "Somebody_Elses_oM"))); + + Assert.Equal(AttributionBasis.NotRecorded, Assert.Single(withAsm).AttributedBy); + Assert.True(withAsm[0].CountedAsReal); + } + + // ------------------------------------------------------------------ + // The Revit year. The backfill wrote an arbitrary lowest year on 158 records, and this + // is what makes that harmless rather than a silent misattribution. + // ------------------------------------------------------------------ + + [Theory] + [InlineData("Revit_X_oM_2022")] + [InlineData("Revit_X_oM_2023")] + [InlineData("Revit_X_oM_2024")] + [InlineData("Revit_X_oM_2025")] + [InlineData("Revit_X_oM_2026")] + public void AnyYearAttributesToASubjectBuildingAnyOtherYear(string recordedAssembly) + { + // The subject built Release2024, so infer-verification-config staged exactly one year. + var closure = ClosureForSubject("Revit_X_oM_2024.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly(recordedAssembly, closure), + $"{recordedAssembly} must attribute to a Release2024 build: all five year variants are " + + "one repository, so the year carries no information about ownership"); + } + + // "More precise" would be wrong here, and this is the shape of the regression it would + // cause: an exact-year match fails in four configurations out of five and the finding is + // dropped as another repository's. Recorded as a test so nobody reintroduces it. + [Fact] + public void TheYearIsNotComparedExactly() + { + var closure = ClosureForSubject("Revit_X_oM_2024.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Revit_X_oM_2022", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("Revit_Y_oM_2022", closure)); + } + + // StripConfigSuffix is anchored `_20\d{2}$`, so an `_asm` carrying a file extension does + // not match the anchor, passes through unchanged, and attributes to nobody. This is why + // the backfill wrote bare simple names, matching how method records already express a + // declaring assembly. Pinned so the dataset convention cannot drift without a red test. + [Fact] + public void AnAssemblyNameCarryingAnExtensionDoesNotAttribute() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Acoustic_oM", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("Acoustic_oM.dll", closure)); + } + + // SubjectBaseNames is built with StringComparer.Ordinal while the file-name set upstream + // is OrdinalIgnoreCase, so the base-name comparison is case-sensitive where the name + // comparison is not. Not a defect today, because both sides come from the same build + // output, but it is a real edge and it should fail visibly if it ever starts mattering. + [Fact] + public void BaseNameComparisonIsCaseSensitive() + { + var closure = ClosureForSubject("Acoustic_oM.dll"); + + Assert.True(RunCommand.IsFromSubjectAssembly("Acoustic_oM", closure)); + Assert.False(RunCommand.IsFromSubjectAssembly("acoustic_om", closure)); + } + + // ------------------------------------------------------------------ + // Ambiguity on object records + // ------------------------------------------------------------------ + + [Fact] + public void MoreThanOneAssemblyDeclaringTheType_IsRecordedAsCandidates() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Revit_X_oM_2022", "Revit_X_oM_2023"], + provenance: Map(("BH.oM.Acoustic.Panel", "Revit_X_oM_2022"))); + + var only = Assert.Single(diagnostics); + Assert.NotNull(only.DeclaringTypeCandidates); + Assert.Equal(2, only.DeclaringTypeCandidates!.Count); + } + + // One candidate is the ordinary case; carrying it would add a noise row to every finding. + [Fact] + public void ASingleDeclaringAssembly_LeavesCandidatesNull() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Acoustic_oM"], + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); + + Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); + } + + // Nothing resolved, so there is no type the dataset vouched for and scanning the closure + // would answer a question nobody asked. This is the other half of inertness. + [Fact] + public void NoMapEntry_DoesNotScanForCandidates() + { + bool called = false; + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel", "Failed to convert the string into a type: BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => { called = true; return []; }, + provenance: DeclaringAssemblyMap.Empty); + + Assert.False(called, "a record the dataset does not answer for must not be probed"); + Assert.Null(Assert.Single(diagnostics).DeclaringTypeCandidates); + } + + // Versions disagreeing is not resolved by picking one. The finding stays unattributed + // and carries what the dataset claimed, so the ambiguity is counted rather than + // normalised away. Measured 0 across the 1711 type names the two backfilled versions + // share, so this path is expected to stay empty and is pinned so it stays honest if not. + [Fact] + public void ADisputedTypeIsLeftUnattributedAndCarriesTheClaims() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); + + var only = Assert.Single(diagnostics); + Assert.Null(only.DeclaringAssembly); + Assert.Equal(["StructuralEngineering_oM", "Structure_oM"], only.DeclaringTypeCandidates!); + } + + // The dataset's disagreement is the answer, not the closure's. Probing would replace a + // statement about which assembly declared the type with a statement about which + // assemblies could have, and they are different questions. + [Fact] + public void ADisputedTypeDoesNotFallBackToTheClosureScan() + { + bool called = false; + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => { called = true; return ["Something_Else_oM"]; }, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); + + Assert.False(called); + Assert.Equal(["StructuralEngineering_oM", "Structure_oM"], Assert.Single(diagnostics).DeclaringTypeCandidates!); + } + + [Fact] + public void ProbeTypeCandidates_ReturnsEveryAssemblyDeclaringTheType() + { + var loaded = new List { typeof(RunCommand).Assembly, typeof(object).Assembly }; + + Assert.Equal(["VersioningRunner"], + RunCommand.ProbeTypeCandidates(loaded, typeof(RunCommand).FullName!)); + Assert.Empty(RunCommand.ProbeTypeCandidates(loaded, "No.Such.Type")); + } + + // ------------------------------------------------------------------ + // Attribution and reclassification driven together, through the real + // AttributeToSubject rather than a stub. + // + // No test supplied both dataset provenance and a closure until this point, which is + // why the case below went unnoticed: with a stubbed isAttributable the coupling + // between the two is never exercised. + // ------------------------------------------------------------------ + + // A closure as a real run has one: the subject built one Revit year, and that year is + // loaded. ClosureForSubject leaves both loaded sets empty, which cannot reach the + // reclassification block at all. + private static ClosureContext ClosureBuilding(string loadedAssembly) + { + var loaded = new HashSet([loadedAssembly], StringComparer.Ordinal); + var bases = new HashSet(loaded.Select(RunCommand.StripConfigSuffix), StringComparer.Ordinal); + return new ClosureContext(loaded, bases, bases); + } + + private static Func RealAttribution( + ClosureContext closure, params string[] subjectNamespaces) + { + var ns = new HashSet(subjectNamespaces, StringComparer.Ordinal); + return (d, asm) => RunCommand.AttributeToSubject(d, asm, ns, closure); + } + + // The dataset records the lowest Revit year present at capture, which is 2022 on every + // one of the 162 year-suffixed records in 9.3. A repository that has moved on builds a + // later year. That is a difference in how the field was written, not a statement that a + // configuration was skipped, and the finding is real: the record was deserialised + // against the assemblies that are loaded, and it failed against them. There is no + // per-assembly probe here to have been unable to run. + [Fact] + public void AnObjectRecordNamingAnUnbuiltYear_IsStillARealFailure() + { + var closure = ClosureBuilding("Revit_Y_oM_2024"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Adapters.Revit.Elements.ModelInstance"), + RealAttribution(closure, "BH.oM.Adapters.Revit.Elements"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Revit_Y_oM_2024"], + provenance: Map(("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_Y_oM_2022"))); + + var only = Assert.Single(diagnostics); + Assert.Equal("Revit_Y_oM_2022", only.DeclaringAssembly); + Assert.Equal(AttributionBasis.DeclaringAssembly, only.AttributedBy); + Assert.Null(only.Cause); + Assert.Equal(ClassificationPath.NoMethodEvent, only.Path); + Assert.True(only.CountedAsReal, + "an object record is deserialised against what is loaded, so a failure is real: " + + "the recorded Revit year is how the field was written, not a configuration that was skipped"); + } + + // The same record where the repository does still build the recorded year. This one + // never reached the reclassification block, because the exact name is loaded, and it is + // here so the fix is not credited with behaviour that already worked. + [Fact] + public void AnObjectRecordNamingABuiltYear_IsAlsoARealFailure() + { + var closure = ClosureBuilding("Revit_Y_oM_2022"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Adapters.Revit.Elements.ModelInstance"), + RealAttribution(closure, "BH.oM.Adapters.Revit.Elements"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Revit_Y_oM_2022"], + provenance: Map(("BH.oM.Adapters.Revit.Elements.ModelInstance", "Revit_Y_oM_2022"))); + + Assert.True(Assert.Single(diagnostics).CountedAsReal); + } + + // Attribution, not reclassification, is what drops another repository's record, and it + // drops it before the block is reached. Pinned because it is the outcome the block's + // foreign branch looks like it provides and does not. + [Fact] + public void AnObjectRecordDeclaredByAnotherRepository_IsDroppedAtAttribution() + { + var closure = ClosureBuilding("Revit_Y_oM_2024"); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + RealAttribution(closure, "BH.oM.Structure.Elements"), null, null, diagnostics, + closure: closure, + probeTypeCandidates: _ => ["Structure_oM"], + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM"))); + + Assert.Empty(diagnostics); + } + + // ------------------------------------------------------------------ + // Where a candidate list came from. + // + // The three sources mean different things and only one is about enumeration order, + // so the run-level warning splits on this. Recorded rather than derived from Path + // plus a null check, because that derivation breaks silently the first time either + // of those moves for an unrelated reason. + // ------------------------------------------------------------------ + + [Fact] + public void CandidatesFromAClosureScan_AreNotRecordedAsOrderDependent() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + probeTypeCandidates: _ => ["Revit_X_oM_2022", "Revit_X_oM_2023"], + provenance: Map(("BH.oM.Acoustic.Panel", "Revit_X_oM_2022"))); + + Assert.Equal(CandidateSource.ClosureScan, Assert.Single(diagnostics).CandidatesFrom); + } + + [Fact] + public void CandidatesFromADatasetDispute_AreRecordedAsSuch() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Disputed("BH.oM.Structure.Elements.Panel", "StructuralEngineering_oM", "Structure_oM")); + + Assert.Equal(CandidateSource.DatasetDispute, Assert.Single(diagnostics).CandidatesFrom); + } + + // The pre-existing source, and the only one where enumeration order decides anything. + [Fact] + public void CandidatesFromTheSignatureProbe_StayRecordedAsOrderDependent() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.Revit.Engine.Core.Compute. }", MethodEvent), + (_, _) => (true, AttributionBasis.NotRecorded), null, + (_, _, _) => (null, ClassificationPath.SignatureResolved, ["One_Engine", "Two_Engine"]), + diagnostics); + + Assert.Equal(CandidateSource.SignatureProbe, Assert.Single(diagnostics).CandidatesFrom); + } + + [Fact] + public void NoCandidates_LeaveTheSourceUnrecorded() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM"))); + + Assert.Equal(CandidateSource.NotRecorded, Assert.Single(diagnostics).CandidatesFrom); + } + + // ------------------------------------------------------------------ + // Findings dropped at attribution. + // + // A dropped leaf produces no diagnostic row, so before this it was counted nowhere + // and a run that discarded everything read exactly like a run that found nothing. + // ------------------------------------------------------------------ + + [Fact] + public void ADroppedFinding_IsCountedAgainstTheEvidenceThatDroppedIt() + { + var drops = new AttributionDrops(); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.DeclaringAssembly), null, null, diagnostics, + provenance: Map(("BH.oM.Structure.Elements.Panel", "Structure_oM")), + drops: drops); + + Assert.Empty(diagnostics); + Assert.Equal(1, drops.Total); + Assert.Equal(1, drops.ByDeclaringAssembly); + Assert.Equal(0, drops.ByNamespaceFallback); + } + + [Fact] + public void ADropOnTheNamespaceGuess_IsCountedSeparately() + { + var drops = new AttributionDrops(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.NamespaceFallback), null, null, null, + drops: drops); + + Assert.Equal(1, drops.ByNamespaceFallback); + Assert.Equal(0, drops.ByDeclaringAssembly); + } + + // The denominator's other half: a kept finding must not be counted as dropped, or the + // two numbers stop summing to the population and neither can be trusted. + [Fact] + public void AKeptFinding_IsNotCountedAsDropped() + { + var drops = new AttributionDrops(); + var diagnostics = new List(); + + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Acoustic.Panel"), + (_, _) => (true, AttributionBasis.DeclaringAssembly), null, null, diagnostics, + provenance: Map(("BH.oM.Acoustic.Panel", "Acoustic_oM")), + drops: drops); + + Assert.Single(diagnostics); + Assert.Equal(0, drops.Total); + } + + // Every existing caller passes nothing, and must keep working. + [Fact] + public void NoCounterSupplied_IsNotAnError() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree("BH.oM.Structure.Elements.Panel"), + (_, _) => (false, AttributionBasis.DeclaringAssembly), null, null, diagnostics); + + Assert.Empty(diagnostics); + } + + // ------------------------------------------------------------------ + // Closed generics. The class the earlier message-based route silently lost. + // ------------------------------------------------------------------ + + // 23 of the 9.2 dataset's 1,713 records are closed generics, whose names carry commas + // and brackets. Reaching them through a formatted message lost all 23 to the delimiter, + // with no diagnostic, and one of them was attributed to the wrong repository as a + // result. A dictionary key has no delimiter to lose them to, and the description arrives + // as the record wrote it. DatasetProvenanceTests pins the same property at map level. + [Fact] + public void AClosedGenericIsAWholeKey() + { + var diagnostics = new List(); + RunCommand.ExtractFilteredResult( + Tree(ClosedGeneric), + (_, _) => (true, AttributionBasis.NotRecorded), null, null, diagnostics, + provenance: Map((ClosedGeneric, "StructuralEngineering_oM"))); + + Assert.Equal("StructuralEngineering_oM", Assert.Single(diagnostics).DeclaringAssembly); + } + + // The case that mattered: the type's namespace is the subject's, so the namespace guess + // claims it, while the declaring assembly says it is another repository's. Under the old + // format this record fell back and stayed misattributed. + [Fact] + public void AClosedGenericDeclaredElsewhereIsNotAttributedToTheSubject() + { + var closure = ClosureForSubject("Structure_oM.dll"); + var subjectNs = new HashSet(["BH.oM.Structure.Results"], StringComparer.Ordinal); + + var withField = RunCommand.AttributeToSubject( + ClosedGeneric, "StructuralEngineering_oM", subjectNs, closure); + Assert.False(withField.Attributable); + Assert.Equal(AttributionBasis.DeclaringAssembly, withField.Basis); + + // What the fallback does when the dataset does not answer for the record, which is + // every object record until the backfill lands. + var withoutField = RunCommand.AttributeToSubject(ClosedGeneric, null, subjectNs, closure); + Assert.True(withoutField.Attributable); + Assert.Equal(AttributionBasis.NamespaceFallback, withoutField.Basis); + } + } +} diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs index 1f38105..5f5de61 100644 --- a/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/RunCommandTests.cs @@ -1020,7 +1020,7 @@ public class ProbeDeclaringTypeTests public void TypeInTwoLoadedAssemblies_RecordsBothCandidates() { // The same assembly listed twice stands in for two repos declaring into one - // namespace, which is the real shape of CI_Toolkit#161. The probe result must be + // namespace, which is the real shape of the collision. The probe result must be // unchanged and the ambiguity must be visible. var twice = new List { typeof(string).Assembly, typeof(string).Assembly }; diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs new file mode 100644 index 0000000..2522d58 --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/DatasetProvenance.cs @@ -0,0 +1,179 @@ +using System.Text.Json; + +namespace VersioningRunner.Commands; + +// The dataset root cannot supply a map at all. +// +// Not thrown for an empty map. A root that exists and whose records carry no `_asm` is the +// state before the backfill lands, and it has to read as inert rather than broken. +public sealed class DatasetProvenanceException : Exception +{ + public DatasetProvenanceException(string message) : base(message) { } +} + +// One object record's declaring assembly, keyed by the type name the runner sees on a leaf. +// +// `TypesMapped` counts agreed entries only. `RecordsRead` counts every non-blank line, +// including those carrying no `_asm`, because it is the denominator that lets a run say +// "0 of 40375 records carry the field" rather than reporting an empty map with no context. +public sealed record DeclaringAssemblyMap( + IReadOnlyDictionary ByTypeName, + IReadOnlyDictionary> Disputed, + int VersionsRead, + int RecordsRead, + int TypesMapped, + int LinesUnparseable) +{ + public static DeclaringAssemblyMap Empty { get; } = new( + new Dictionary(StringComparer.Ordinal), + new Dictionary>(StringComparer.Ordinal), + 0, 0, 0, 0); + + // Null when the dataset names no assembly for the type, and null when versions disagree + // about which. A disagreement is not resolved by picking one: the caller falls back to the + // namespace guess, which is at least recorded as a guess. + public string? DeclaringAssemblyFor(string? typeFullName) + => !string.IsNullOrEmpty(typeFullName) && ByTypeName.TryGetValue(typeFullName, out string? assembly) + ? assembly + : null; +} + +// Where a failing object record's declaring assembly comes from. +// +// The record carries it, in the `_asm` field, and ci-versioning stages the dataset to a known +// path on the same machine in the same job: the action asserts that directory exists and +// enumerates its version folders in a step before the runner runs. So this reads the same file +// the versioning test read, rather than depending on the test to restate the field in a message +// the runner then parses back out. +// +// Only Objects.json is read. Methods.json and Adapters.json carry +// `_t: System.Reflection.MethodBase` on every record, so `_t` identifies nothing there, and +// their leaves already reach the runner with a declaring assembly from the Method event. +public static class DatasetProvenance +{ + /*************************************/ + /**** Public Methods ****/ + /*************************************/ + + // Every version directory present, not the newest. + // + // A pull request reads one version, but which one is a property of Versioning_Toolkit's + // source rather than of anything the runner can see, and deriving it would mean regexing a + // version list out of FromJson.cs. Reading the union costs nothing and is what makes the + // answer independent of that. Measured on the two versions carrying the field: 1711 shared + // type names, 0 disagreements. Measured on the 15 saved run artefacts: 203 of 205 + // object-record findings resolve against 9.3 and the other 2 only against 9.2, so a + // newest-only read would have missed both, and both are NoMethodEvent findings with no + // declaring assembly, which is precisely the population this field exists to serve. + public static DeclaringAssemblyMap Build(string? datasetsRoot) + { + if (string.IsNullOrWhiteSpace(datasetsRoot)) + throw new DatasetProvenanceException("No versioning dataset path was supplied."); + + if (!Directory.Exists(datasetsRoot)) + throw new DatasetProvenanceException( + $"Versioning dataset directory not found at {datasetsRoot}. The build of " + + "Verification.sln stages it through Versioning_Test.csproj's PostBuild step, so its " + + "absence means that build did not run or did not stage."); + + List objectFiles = Directory.EnumerateDirectories(datasetsRoot) + .OrderBy(d => d, StringComparer.Ordinal) + .Select(d => Path.Combine(d, "Objects.json")) + .Where(File.Exists) + .ToList(); + + if (objectFiles.Count == 0) + throw new DatasetProvenanceException( + $"No version directory under {datasetsRoot} contains an Objects.json. The directory " + + "exists but holds no object records, so no finding could be attributed by declaring " + + "assembly and every one would fall back to the namespace guess."); + + // Ordinal-sorted so a disputed entry's candidate list, and the artefact it lands in, do + // not depend on directory enumeration order. + var seen = new Dictionary>(StringComparer.Ordinal); + int records = 0; + int unparseable = 0; + + foreach (string file in objectFiles) + { + foreach (string line in File.ReadLines(file)) + { + if (string.IsNullOrWhiteSpace(line)) + continue; + + records++; + + string? type; + string? assembly; + try + { + using JsonDocument document = JsonDocument.Parse(line); + if (document.RootElement.ValueKind != JsonValueKind.Object) + { + unparseable++; + continue; + } + + type = StringProperty(document.RootElement, "_t"); + assembly = StringProperty(document.RootElement, "_asm"); + } + catch (JsonException) + { + // Counted and reported by the caller rather than thrown. A record the + // dataset cannot express is the dataset's problem, and the versioning test + // reading the same line produces its own finding for it; losing the whole + // map over one line would convert that into a fleet-wide fallback. + unparseable++; + continue; + } + + if (type is null || assembly is null) + continue; + + if (!seen.TryGetValue(type, out SortedSet? assemblies)) + seen[type] = assemblies = new SortedSet(StringComparer.Ordinal); + + assemblies.Add(assembly); + } + } + + var agreed = new Dictionary(StringComparer.Ordinal); + var disputed = new Dictionary>(StringComparer.Ordinal); + + foreach ((string type, SortedSet assemblies) in seen) + { + if (IsOneAssemblyFamily(assemblies)) + agreed[type] = assemblies.First(); + else + disputed[type] = assemblies.ToArray(); + } + + return new DeclaringAssemblyMap(agreed, disputed, objectFiles.Count, records, agreed.Count, unparseable); + } + + /*************************************/ + /**** Private Methods ****/ + /*************************************/ + + // Two versions naming Revit_X_oM_2022 and Revit_X_oM_2023 are not disagreeing. + // They are the same repository under two build configurations, attribution strips the year + // anyway, and treating the pair as disputed would drop the finding to the namespace guess + // for no gain. That is the failure "compare year-insensitively" exists to prevent, applied + // one layer earlier. Different families are a real disagreement and stay disputed. + private static bool IsOneAssemblyFamily(SortedSet assemblies) + => assemblies.Count == 1 + || assemblies.Select(RunCommand.StripConfigSuffix).Distinct(StringComparer.Ordinal).Count() == 1; + + // A field present but not a string, or present and blank, reads the same as absent. Both + // leave the caller on its existing path rather than mapping the type to nothing. + private static string? StringProperty(JsonElement record, string name) + { + if (!record.TryGetProperty(name, out JsonElement value) || value.ValueKind != JsonValueKind.String) + return null; + + string? text = value.GetString(); + return string.IsNullOrWhiteSpace(text) ? null : text; + } + + /*************************************/ +} diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index 0a1a324..807160d 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -16,7 +16,8 @@ public static int Execute( bool testAll, string? subjectAssemblyList = null, string? configuration = null, - IReadOnlyCollection? versionConditionalMethods = null) + IReadOnlyCollection? versionConditionalMethods = null, + string? datasetsPath = null) { s_configuration = configuration; s_versionConditional = versionConditionalMethods is null @@ -30,6 +31,26 @@ public static int Execute( else Console.WriteLine($"Version-conditional method list: scan performed, {s_versionConditional.Count} method(s) found."); + // Built before the assemblies load, so a broken precondition costs nothing. The two + // states it throws on are a missing dataset root and a root holding no Objects.json, + // and both mean no object record could be attributed by declaring assembly. Neither is + // reachable through ci-versioning, whose "Validate versioning datasets" step already + // exits 1 on the same conditions before this runs. + DeclaringAssemblyMap provenance; + try + { + provenance = datasetsPath is null + ? DeclaringAssemblyMap.Empty + : DatasetProvenance.Build(datasetsPath); + } + catch (DatasetProvenanceException ex) + { + Console.Error.WriteLine($"::error title=Versioning::{ex.Message}"); + return 1; + } + + ReportProvenance(provenance, datasetsPath); + var loaded = LoadAssemblies(assembliesPath); var methods = FindAllFromJsonDatasetsMethods(loaded); @@ -134,6 +155,8 @@ public static int Execute( Func Candidates)> probeSignature = (typeFullName, methodName, declaringAssembly) => ProbeDeclaringType(loaded, typeFullName, methodName, declaringAssembly); + Func> probeTypeCandidates = t => ProbeTypeCandidates(loaded, t); + var drops = new AttributionDrops(); foreach (var method in methods) { object? rawResult; @@ -160,7 +183,7 @@ public static int Execute( return 1; } - var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex); + var partial = ExtractFilteredResult(rawResult, isAttributable, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, drops); allFailures.AddRange(partial.Failures); } @@ -279,7 +302,10 @@ public static int Execute( SubjectAssemblies: s_subjectAssemblyCount, SubjectTypes: s_subjectTypeCount, DatasetVersions: testAll ? 0 : 1, - RecordsUnverified: unresolvableSkips.Count); + RecordsUnverified: unresolvableSkips.Count, + TypesWithDeclaringAssembly: provenance.TypesMapped, + DroppedByDeclaringAssembly: drops.ByDeclaringAssembly, + DroppedByNamespaceFallback: drops.ByNamespaceFallback); var result = new VersioningResult { @@ -307,6 +333,29 @@ public static int Execute( $"{coverage.LoadedAssemblies} assembl(ies) loaded; " + $"{coverage.VerifyEntryPoints} FromJsonDatasets entry point(s) invoked; " + $"{coverage.RecordsUnverified} record(s) attributed but not verified"); + + // Printed in every state, zero included. This is the only account of findings that + // left the run without producing a row, so a run that filtered everything out and a + // run that found nothing wrong are otherwise the same output. + // + // Counts leaves dropped at attribution. It is NOT a diff against a previous run and + // must not be read as one: it does not say how many findings some earlier + // configuration would have reported, even where the two happen to coincide. + Console.WriteLine( + $"Attribution: {result.FailureCount + unresolvableSkips.Count} finding(s) kept, " + + $"{drops.Total} dropped as another repository's " + + $"({drops.ByDeclaringAssembly} by declaring assembly, " + + $"{drops.ByNamespaceFallback} by namespace prefix)."); + + // A green that rests entirely on having discarded everything is a different statement + // from a green that examined findings and found them sound, and the exit code cannot + // tell them apart. + if (drops.Total > 0 && result.FailureCount == 0 && unresolvableSkips.Count == 0) + Console.Error.WriteLine( + $"::warning title=Versioning::Every one of the {drops.Total} finding(s) in this run was " + + "attributed to another repository, so this result rests on attribution being right rather " + + "than on nothing having failed."); + if (configuration is not null) Console.WriteLine($"Configuration: {configuration}"); @@ -314,11 +363,36 @@ public static int Execute( // result.Failures. Counting every diagnostic made the total disagree with the detail as // soon as a finding could be reclassified to unverified: the warning claimed N ambiguous // findings while fewer than N were listed. - int ambiguous = diagnostics.Count(d => d.CountedAsReal && d.DeclaringTypeCandidates is { Count: > 1 }); - if (ambiguous > 0) + // + // Split by source. A single message over all three was wrong for two of them: only the + // signature probe lets enumeration order decide the answer. The closure scan reports how + // many assemblies also declare a type the dataset had already settled, and a dataset + // dispute means nothing was attributed at all. Telling someone load order explained a + // finding it had no part in sends them to the wrong place. + var ambiguousReal = diagnostics + .Where(d => d.CountedAsReal && d.DeclaringTypeCandidates is { Count: > 1 }) + .ToList(); + + int orderDependent = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.SignatureProbe); + if (orderDependent > 0) + Console.Error.WriteLine( + $"::warning title=Versioning::{orderDependent} finding(s) were attributed by probing the loaded " + + "assemblies for the declaring type, and more than one matched, so which assembly was recorded " + + "depended on the order the closure was enumerated in."); + + int alsoDeclared = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.ClosureScan); + if (alsoDeclared > 0) + Console.Error.WriteLine( + $"::warning title=Versioning::{alsoDeclared} finding(s) name a type that more than one loaded " + + "assembly declares. The dataset named the declaring assembly and settled each one, so this is " + + "reported for visibility rather than because the answer was in doubt."); + + int disputed = ambiguousReal.Count(d => d.CandidatesFrom == CandidateSource.DatasetDispute); + if (disputed > 0) Console.Error.WriteLine( - $"::warning title=Versioning::{ambiguous} finding(s) have a declaring type present in more than one loaded assembly, " + - "so their classification depended on assembly enumeration order. See CI_Toolkit#161."); + $"::warning title=Versioning::{disputed} finding(s) name a type whose declaring assembly the " + + "dataset versions disagree about, so none was attributed and each fell back to the namespace " + + "prefix."); const int maxLogged = 50; foreach (var failure in result.Failures.Take(maxLogged)) @@ -332,7 +406,19 @@ public static int Execute( if (d?.VersionConditional == VersionConditionalState.Yes) context.Add($"Signature is version-conditional. Built as {d.Configuration ?? "an unrecorded configuration"}."); if (d?.DeclaringTypeCandidates is { Count: > 1 } cands) - context.Add($"Declaring type present in {cands.Count} loaded assemblies ({string.Join(", ", cands.Take(3))}), so classification depended on enumeration order."); + { + string joined = string.Join(", ", cands.Take(3)); + context.Add(d.CandidatesFrom switch + { + CandidateSource.SignatureProbe => + $"Declaring type present in {cands.Count} loaded assemblies ({joined}), so classification depended on enumeration order.", + CandidateSource.ClosureScan => + $"Declaring assembly came from the dataset ({d.DeclaringAssembly}). {cands.Count} loaded assemblies also declare this type ({joined}); the dataset settled it, not enumeration order.", + CandidateSource.DatasetDispute => + $"Dataset versions disagree about the declaring assembly ({joined}), so this finding was not attributed and fell back to the namespace prefix.", + _ => $"Declaring type present in {cands.Count} loaded assemblies ({joined}).", + }); + } string suffix = context.Count > 0 ? " " + string.Join(" ", context) : string.Empty; Console.Error.WriteLine($"::error title=Versioning::{failure.Description}: {failure.Message}{suffix}"); @@ -411,9 +497,9 @@ internal static int ExitCodeFor(VersioningStatus status) // ProbeDeclaringType walks the loaded list and takes its verdict from the FIRST // assembly that yields the declaring type. Where two repos declare the same type, // whichever is enumerated first decides whether the finding reads as a genuine - // regression or as an infrastructure problem. That is CI_Toolkit#161's mechanism: - // BH.Revit.Engine.Core.Compute is defined by both Revit_Core_Engine and - // Revit_ModelQA_Engine, and 42 such type-level collisions exist across the fleet. + // regression or as an infrastructure problem. BH.Revit.Engine.Core.Compute is declared + // by Revit_Core_Engine and by one other assembly, and 42 such type-level collisions + // exist across the fleet. // // Directory.GetFiles documents no ordering. Measured on windows-2025-vs2026 across // four cold-rebuild runs on separate runners, NTFS returned exactly @@ -490,7 +576,8 @@ public static VersioningResult ExtractFilteredResult(object? rawResult, List (IsFromLoadedNamespace(d, nsPrefixes), AttributionBasis.NotRecorded), - typeIndex: BuildLoadedTypeIndex(loaded)); + typeIndex: BuildLoadedTypeIndex(loaded), + probeTypeCandidates: t => ProbeTypeCandidates(loaded, t)); } public static VersioningResult ExtractFilteredResult( @@ -502,7 +589,16 @@ public static VersioningResult ExtractFilteredResult( ClosureContext? closure = null, // Defaults to Empty so a caller that is not exercising resolution keeps the // pre-existing behaviour rather than having findings reclassified underneath it. - LoadedTypeIndex? typeIndex = null) + LoadedTypeIndex? typeIndex = null, + // Null means object-record ambiguity is not scanned, which is the pre-existing + // behaviour and what every caller that does not supply a closure should get. + Func>? probeTypeCandidates = null, + // The dataset's answer for an object record's declaring assembly. Empty by default, so + // a caller that supplies nothing keeps the namespace fallback it had before. + DeclaringAssemblyMap? provenance = null, + // Counts findings discarded at attribution. Optional so every existing caller is + // unchanged; when absent the drops are simply not counted, as before. + AttributionDrops? drops = null) { if (rawResult is null) return new VersioningResult @@ -513,7 +609,7 @@ public static VersioningResult ExtractFilteredResult( }; var failures = new List(); - CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, depth: 0); + CollectLeafFailures(rawResult, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex ?? LoadedTypeIndex.Empty, probeTypeCandidates, provenance ?? DeclaringAssemblyMap.Empty, drops, depth: 0); var status = failures.Count > 0 ? VersioningStatus.Error : VersioningStatus.Pass; return new VersioningResult @@ -531,7 +627,8 @@ private static void CollectLeafFailures( List? unresolvableSkips, Func Candidates)>? probeSignature, List? diagnostics, ClosureContext? closure, - LoadedTypeIndex typeIndex, int depth) + LoadedTypeIndex typeIndex, Func>? probeTypeCandidates, + DeclaringAssemblyMap provenance, AttributionDrops? drops, int depth) { // BHoM's TestResult tree has at most 3 levels under the root (outer → per-version // summary → individual type result). Depth 5 gives headroom for unexpected nesting @@ -583,9 +680,47 @@ private static void CollectLeafFailures( .Select(ParseMethodEventAssembly) .FirstOrDefault(a => a is not null); + // Object records carry no Method event, so the assembly comes from the dataset's + // `_asm` field instead, read from the staged Objects.json rather than from anything + // in this tree. Consulted only when the Method event yielded nothing, so the method + // path is untouched: a record cannot be both. + // + // The description is the join key because for an object record it is the `_t` value + // verbatim. That is structural rather than lucky: FromJsonItem reaches IToText only + // when the result is non-null and not a CustomObject, and every path with that + // combination returns a PassResult, so an Error leaf always carries + // DescriptionFromJson's output, which is the record's first quoted field. + // + // Null throughout until the dataset carries `_asm`, which is what makes this change + // inert on its own. + string? provenanceType = null; + IReadOnlyList? disputedAssemblies = null; + if (declaringAssembly is null) + { + string? fromDataset = provenance.DeclaringAssemblyFor(desc); + if (fromDataset is not null) + { + declaringAssembly = fromDataset; + provenanceType = desc; + } + else if (provenance.Disputed.TryGetValue(desc, out IReadOnlyList? claims)) + { + // Versions named different assembly families for this type. Left unattributed + // on purpose: picking one would be a guess dressed as a fact, and the + // namespace fallback at least records itself as a guess. + disputedAssemblies = claims; + } + } + var (attributable, attributedBy) = isAttributable(desc, declaringAssembly); if (!attributable) + { + // Counted before returning. This is the only place a finding leaves the run + // without producing a row, so without the count a filtered-out run and a + // clean run are indistinguishable in every output. + drops?.Count(attributedBy); return; + } // DescriptionFromJson mangles many method entries to ". }", // losing the method name. The Method event still carries both, so prefer it. @@ -603,15 +738,48 @@ private static void CollectLeafFailures( ? ClassificationPath.UnresolvableTypeAbsent : ClassificationPath.UnresolvableFromEvents; IReadOnlyList candidates = Array.Empty(); + var candidatesFrom = CandidateSource.NotRecorded; // No type-level cause recorded means the blocker may be in the signature // rather than the payload, which only reflection over the method can tell. if (cause is null) { if (eventType is null || eventMethod is null) + { path = ClassificationPath.NoMethodEvent; + + // An object record has no method, so the signature probe cannot run and the + // path stays NoMethodEvent. The ambiguity question is still live and is + // answerable from the type alone: if more than one loaded assembly declares + // it, `_asm` resolved to one of several and the run should say so rather + // than normalise it away silently. Without this the metric below can never + // count an object record, because candidates only ever came from the + // method probe. + // + // The field carries two kinds of ambiguity and they are not the same + // question. Dataset versions disagreeing about which assembly declared the + // type takes precedence, because in that case nothing resolved and scanning + // the closure would answer a question nobody asked. Otherwise it is the + // original one: how many loaded assemblies could have declared it. The + // dataset case is reported separately at run level, so the two remain + // tellable apart; measured 0 across the 1711 type names the two backfilled + // versions share. + if (disputedAssemblies is not null) + { + candidates = disputedAssemblies; + candidatesFrom = CandidateSource.DatasetDispute; + } + else if (probeTypeCandidates is not null && provenanceType is not null) + { + candidates = probeTypeCandidates(provenanceType); + candidatesFrom = CandidateSource.ClosureScan; + } + } else if (probeSignature is not null) + { (cause, path, candidates) = probeSignature(eventType, eventMethod, declaringAssembly); + candidatesFrom = CandidateSource.SignatureProbe; + } else path = ClassificationPath.ProbeNotSupplied; } @@ -624,7 +792,24 @@ private static void CollectLeafFailures( // describes code we were never asked about. When nothing answered, the path is // DeclaringTypeNotLoaded, which is the signal that the type is genuinely gone; // reclassifying that would convert a real removal into a silent pass. + // Not for object records, and provenanceType is how they are told apart: it is set + // only when the dataset answered. + // + // Both outcomes below need a per-assembly probe to have been prevented from running, + // and an object record has none. It is deserialised against whatever is loaded, so a + // failure is a failure of the code that was built, and "that configuration was not + // compiled" is not an escape available to it. The declaring assembly here is also + // not evidence about which configuration ran: the backfill records the lowest Revit + // year present at capture, 2022 on all 162 year-suffixed records in 9.3, so matching + // it exactly against LoadedNames asks a question about how the field was written. + // That is the exact-year comparison the locked decisions rule out. + // + // Without this, a repository that has moved past 2022 sees a real object-record + // regression demoted to unverified with a cause that reads as a build gap: Error + // becomes Warning and the job exits 0. Nine assembly families are exposed and all + // nine still build 2022 today, so it is latent until one of them drops it. if (cause is null && closure is not null && declaringAssembly is not null + && provenanceType is null && candidates.Count > 0 && !closure.LoadedNames.Contains(declaringAssembly)) { @@ -689,7 +874,8 @@ private static void CollectLeafFailures( DeclaringTypeCandidates: candidates.Count > 1 ? candidates : null, Configuration: s_configuration, VersionConditional: ClassifyVersionConditional(eventType, eventMethod), - AttributedBy: attributedBy)); + AttributedBy: attributedBy, + CandidatesFrom: candidatesFrom)); } else { @@ -697,7 +883,7 @@ private static void CollectLeafFailures( { string childStatus = child.GetType().GetProperty("Status")?.GetValue(child)?.ToString() ?? "Pass"; if (childStatus is "Error" or "Warning") - CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, depth + 1); + CollectLeafFailures(child, isAttributable, failures, unresolvableSkips, probeSignature, diagnostics, closure, typeIndex, probeTypeCandidates, provenance, drops, depth + 1); } } } @@ -989,6 +1175,86 @@ public static (string? DeclaringType, string? MethodName) ParseMethodEvent(strin return assembly.Length > 0 ? assembly : null; } + // What the dataset read produced, on stdout beside Attribution and Classification because + // it is coverage evidence rather than an annotation. + // + // Printed in every state, including the two that report nothing. An empty map reads + // identically whether the dataset carries no `_asm` yet, or the path was never supplied, or + // the runner read a tree with nothing in it, and those need different action. A number that + // appears only when it is non-zero cannot be told from one nobody measured. + private static void ReportProvenance(DeclaringAssemblyMap provenance, string? datasetsPath) + { + if (datasetsPath is null) + { + Console.WriteLine( + "Provenance: no dataset path supplied, so no object record can be attributed by " + + "declaring assembly and every one falls back to the namespace guess."); + return; + } + + Console.WriteLine( + $"Provenance: {provenance.VersionsRead} dataset version(s), {provenance.RecordsRead} record(s), " + + $"{provenance.TypesMapped} type(s) mapped to a declaring assembly, " + + $"{provenance.Disputed.Count} disputed."); + + if (provenance.RecordsRead > 0 && provenance.TypesMapped == 0 && provenance.Disputed.Count == 0) + { + Console.WriteLine( + $"Provenance: 0 of {provenance.RecordsRead} record(s) carry the declaring-assembly field, " + + "so object records are attributed by namespace as before. Expected until the dataset " + + "backfill lands."); + } + + if (provenance.LinesUnparseable > 0) + { + Console.Error.WriteLine( + $"::warning title=Versioning::{provenance.LinesUnparseable} dataset record(s) could not be " + + "parsed and were skipped, so any finding on one of them falls back to the namespace guess."); + } + + // Measured 0 across the 1711 type names the two backfilled versions share, so this is + // expected to stay silent. It is a warning rather than a counter because the first time + // it is not silent is the first time a type name stops identifying one repository, and + // that is worth someone reading rather than a number in an artefact. + if (provenance.Disputed.Count > 0) + { + var named = provenance.Disputed + .OrderBy(d => d.Key, StringComparer.Ordinal) + .Take(10) + .Select(d => $"{d.Key} ({string.Join(", ", d.Value)})"); + + Console.Error.WriteLine( + $"::warning title=Versioning::{provenance.Disputed.Count} type(s) are named with different " + + "declaring assemblies by different dataset versions, so they are left to the namespace " + + $"guess rather than resolved to one: {string.Join("; ", named)}"); + } + } + + // Every loaded assembly that yields the named type. The type-only half of + // ProbeDeclaringType's candidate collection, for records that have no method to probe. + // + // Not shared with ProbeDeclaringType, deliberately. That loop interleaves the signature + // probe with the candidate walk and takes its verdict from the first assembly that + // answers; splitting it would give the method path a second pass over the closure and + // change code this PR has no reason to touch. + internal static IReadOnlyList ProbeTypeCandidates(List loaded, string typeFullName) + { + var candidates = new List(); + foreach (var asm in loaded) + { + Type? type; + try { type = asm.GetType(typeFullName, throwOnError: false); } + catch { continue; } + + if (type is null) + continue; + + try { candidates.Add(asm.GetName().Name ?? "(unnamed)"); } + catch { candidates.Add("(unnamed)"); } + } + return candidates; + } + // A failure attributed to the subject that could not actually be verified, and the // non-BHoM type identity that made it unverifiable. public readonly record struct UnverifiedFailure(string Description, string Cause); @@ -1046,7 +1312,7 @@ internal static VersionConditionalState ClassifyVersionConditional(string? typeF // // The loop used to return on the first match, which made the classification depend on // assembly enumeration order wherever two repos declare into the same namespace - // (CI_Toolkit#161, measured on BH.Revit.Engine.Core). The probe result still comes from + // (measured on BH.Revit.Engine.Core). The probe result still comes from // the first match, so behaviour is unchanged; the candidate list is recorded so a // classification that could have gone either way is visible rather than silent. internal static (string? Cause, ClassificationPath Path, IReadOnlyList Candidates) ProbeDeclaringType( diff --git a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs index 27e1e94..efa3920 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Models/VersioningResult.cs @@ -103,6 +103,53 @@ public enum AttributionBasis NamespaceFallback, } +// Where a finding's candidate list came from. Recorded explicitly because the three sources +// mean different things and only one of them is about enumeration order, so a single message +// over all of them is wrong for two. Derivable today from Path plus a null check on +// DeclaringAssembly, which is the coupling this exists to avoid: that derivation breaks +// silently the first time either of those moves for an unrelated reason. +public enum CandidateSource +{ + // No candidate list, or one recorded before this was tracked. + NotRecorded, + // Method record. More than one loaded assembly matched the signature probe, so which one + // was recorded genuinely depended on the order the closure was enumerated in. + SignatureProbe, + // Object record. The dataset named the declaring assembly and settled it; the closure was + // then scanned to report how many other loaded assemblies also declare the type. The scan + // did not decide anything, so enumeration order did not either. + ClosureScan, + // Object record. Dataset versions named different assembly families for the type, so + // nothing was attributed and the finding fell back to the namespace prefix. + DatasetDispute, +} + +// Findings dropped at attribution, which produce no diagnostic row and were previously +// counted nowhere. Without this a run reporting zero findings because nothing failed and one +// reporting zero because attribution discarded everything are the same output. +// +// Counts leaves dropped at attribution. NOT a diff against a previous run: it does not say +// how many findings an earlier configuration would have reported, and must not be read that +// way even when the two coincide. +public sealed class AttributionDrops +{ + public int ByDeclaringAssembly { get; private set; } + public int ByNamespaceFallback { get; private set; } + public int ByUnrecordedBasis { get; private set; } + + public int Total => ByDeclaringAssembly + ByNamespaceFallback + ByUnrecordedBasis; + + public void Count(AttributionBasis basis) + { + switch (basis) + { + case AttributionBasis.DeclaringAssembly: ByDeclaringAssembly++; break; + case AttributionBasis.NamespaceFallback: ByNamespaceFallback++; break; + default: ByUnrecordedBasis++; break; + } + } +} + // What this run actually built, needed to tell "the recorded declaring // assembly is missing because it is someone else's" from "because we did not compile that // configuration" from "because it was genuinely removed". Passed explicitly rather than @@ -138,8 +185,8 @@ public record FailureDiagnostic( string? DeclaringType, string? DeclaringAssembly, int EventCount, - // Every assembly in the loaded set that yielded the declaring type. More than one - // means the classification depended on enumeration order (see CI_Toolkit#161). + // Every assembly that claimed the declaring type. What more than one means depends on + // where the list came from, which is why CandidatesFrom is recorded beside it. IReadOnlyList? DeclaringTypeCandidates = null, // Build configuration this run compiled, carried per row so a finding read on its // own is interpretable. Human-legible; it does not by itself explain a divergence. @@ -147,7 +194,10 @@ public record FailureDiagnostic( VersionConditionalState VersionConditional = VersionConditionalState.Unknown, // Which evidence attributed this failure to the subject. Per row rather than only as a // total, so a reader can tell whether any individual finding rests on the ambiguous path. - AttributionBasis AttributedBy = AttributionBasis.NotRecorded); + AttributionBasis AttributedBy = AttributionBasis.NotRecorded, + // Where DeclaringTypeCandidates came from. Only SignatureProbe means the answer depended + // on enumeration order. + CandidateSource CandidatesFrom = CandidateSource.NotRecorded); // Coverage denominator. A verdict without one cannot be interpreted: a pass over zero // methods reads identically to a pass over seven thousand. BHoMBot reported object @@ -165,7 +215,18 @@ public record CoverageCounts( // Present so a green cannot be read as a clean sweep of SubjectTypes: a run reporting // 1191 types checked and 145 records unverified has not examined what the first number // implies. Counts records, not types, because that is the unit the dataset iterates. - int RecordsUnverified = 0); + int RecordsUnverified = 0, + // Type names the staged dataset resolved to a declaring assembly. Zero means every object + // record was attributed by namespace prefix, which is the pre-backfill state and reads + // identically in the artefact to a run that never found the dataset at all. The log says + // which, for 90 days; this is the part that outlives it. + int TypesWithDeclaringAssembly = 0, + // Findings discarded at attribution as another repository's, split by the evidence that + // discarded them. Present for the same reason as RecordsUnverified: without it, a run + // that filtered everything out and a run that found nothing wrong are the same output. + // These count leaves dropped at attribution and are not a diff against a previous run. + int DroppedByDeclaringAssembly = 0, + int DroppedByNamespaceFallback = 0); public class VersioningResult { diff --git a/tools/VersioningRunner/src/VersioningRunner/Program.cs b/tools/VersioningRunner/src/VersioningRunner/Program.cs index 244f28f..7270632 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Program.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Program.cs @@ -29,10 +29,14 @@ if (args.Length == 0 || args.Contains("--help") || args.Contains("-h")) { - Console.WriteLine("Usage: VersioningRunner [--assemblies ] [--output ] [--test-all] [--subject-assembly-list ] [--configuration ] [--version-conditional ]"); + Console.WriteLine("Usage: VersioningRunner [--assemblies ] [--datasets ] [--output ] [--test-all] [--subject-assembly-list ] [--configuration ] [--version-conditional ]"); Console.WriteLine(); Console.WriteLine("Options:"); Console.WriteLine(" --assemblies BHoM assemblies folder (default: C:\\ProgramData\\BHoM\\Assemblies)"); + Console.WriteLine(" --datasets Versioning dataset root, holding one directory per version."); + Console.WriteLine(" Every Objects.json under it is read for the `_asm` field, which"); + Console.WriteLine(" gives an object record its declaring assembly. Omit the flag to"); + Console.WriteLine(" attribute object records by namespace prefix, as before."); Console.WriteLine(" --output Write JSON results to this file"); Console.WriteLine(" --test-all Test all historical dataset versions (default: previous version only)"); Console.WriteLine(" --configuration Build configuration this run compiled, recorded per finding."); @@ -47,6 +51,13 @@ } string assemblies = GetArg(args, "--assemblies") ?? @"C:\ProgramData\BHoM\Assemblies"; + +// No default, unlike --assemblies. A default would make the runner fail hard wherever that +// tree is absent, and versioning-full-history.yml drives this exe without the dataset guard +// ci-versioning runs first. Absent means "nobody supplied a dataset", which is the +// pre-existing behaviour, and the run says so on stdout rather than degrading quietly. +string? datasets = GetArg(args, "--datasets"); + string? output = GetArg(args, "--output"); bool testAll = args.Contains("--test-all"); string? subject = GetArg(args, "--subject-assembly-list"); @@ -70,7 +81,7 @@ .ToArray(); } -return RunCommand.Execute(assemblies, output, testAll, subject, configuration, versionConditional); +return RunCommand.Execute(assemblies, output, testAll, subject, configuration, versionConditional, datasets); static string? GetArg(string[] args, string flag) {