Conversation
Object records carry only a type name, so the checker downstream decides
ownership by matching the namespace prefix against the repository under test.
Namespaces are shared and no rule over the name separates the owners at any
precision. The dataset's `_asm` field records the assembly that declared the
type at capture; this surfaces it so the checker can read it.
The checker sees this TestResult and never the dataset file, so an event on
the failure path is the only channel. The wording is a contract with
CI_Toolkit's VersioningRunner, which parses it in ParseObjectEventAssembly:
Object <FullTypeName> declared in "<TypeName>, <AssemblyName>"
Read from the raw json rather than the deserialised object, because the
serialiser skips every field whose name begins with an underscore and because
this runs where there is often no object left to ask. The reader is a
depth-aware scan rather than a regex, so an identically named field on a
nested fragment cannot be mistaken for the record's own, and rather than a
JSON library, which would mean a new dependency for one field read.
Emitted only on the failure path, only when the field is present, and never
for method records: their declaring assembly already reaches the checker
through the Method event, `_asm` is prohibited on Methods.json, and a method
record's top-level type is System.Reflection.MethodBase, so emitting one would
state something untrue.
DescriptionFromJson is not touched. Its positional splits are unchanged.
|
Note on the It is not caused by this change.
That condition is BHoM/internal-tickets#34, which is where the decision about it sits. The same failure, with the same message, occurred three times on #348 (one It also does not gate: the required contexts on All eight other checks pass, including |
1 participant
NOTE: Depends on
BHoM/CI_Toolkit#22
Merge after that one. It teaches the checker to read the field; this one emits it. Landing this first would emit something nothing reads.
It also depends on the dataset backfill, which adds the field to the 9.2 object records. That is built but not yet raised, and without it this pull request has nothing to emit.
Timing. This is under a release freeze and is not expected to merge yet. Review is welcome in the meantime; it will sit until the freeze lifts.
Issues addressed by this PR
Refs BHoM/internal-tickets#35
The versioning check has to decide which repository a finding belongs to. An object record carries a type name and nothing else, so ownership is inferred from the namespace, and namespaces are shared across repositories. It therefore reports other repositories' failures against your pull request:
BHoM/BHoM_Adapterrun31844962538reported 1,056 failures, none of them that repository's.BHoM/BHoMrun33849699768reported 145 findings, all attributed by namespace guess and none by evidence.A separate change records the declaring assembly on each object record in the 9.2 dataset. It is built and not yet raised. This pull request surfaces that field into the test result, which is the only thing the checker reads. Until both land this changes nothing observable, because the field is absent from the dataset today and there is nothing for the emitter to emit.
The effect is that findings which were never this repository's stop being reported against it. Applied to those same 145 findings: 98 are dropped as correctly another repository's, 46 remain unverified for unrelated reasons, and 1 becomes a real failure.
Test files
Run end to end against the real dataset and the real checker, by inducing a failure on three real records:
BH.oM.Structure.Elements.BarStructure_oMBH.Revit.oM.ModelQA…ColumnContinuityConditionRevit_ModelQA_oM_2022BH.oM.Structure.Results.ResultEnvelope\1[[…]]`StructuralEngineering_oMThe third row is the point of the change: its namespace says this repository, its declaring assembly says otherwise, and the declaring assembly is right.
Changelog
Versioning object-record failures now carry the assembly that declares the type, so the check attributes them by evidence instead of by namespace.
Additional comments
Expect one new red, and it is correct.
BH.oM.Structure.Loads.LoadCombinationbecomes a real failure onBHoM/BHoM. That record is declared byStructure_oM, whichBHoM/BHoMbuilds, so the finding genuinely is that repository's. Until now it was excused because ownership could not be established, not because it was someone else's.ci-versioningis not a required check anywhere, so this is visible rather than blocking.Confidence, stated honestly. The 98 / 46 / 1 figures are derived, not observed. They come from the artefact of a real CI run (
34100313934) with its 145 diagnostics replayed through the real checker code, and were confirmed by two independent routes that agree. They have not been observed in a live run of this change, because that needs this, BHoM/CI_Toolkit#22, and the dataset backfill merged together. The three-record table above is observed, end to end, on this branch.The contract, which is worth scrutinising because it cannot be changed quietly later. The checker parses exactly this shape:
Rewording it does not fail loudly. It silently turns the feature off and returns every finding to the namespace guess. The quoted part holds the assembly alone because closed generic type names contain commas, and a comma-delimited form cannot represent one.
Scope. The event is emitted only on the failure path, only when the field is present, and never for method records, whose declaring assembly already reaches the checker and whose top-level type is
System.Reflection.MethodBase.Helpers/DescriptionFromJson.csis untouched; it reads fixed positional indices and nothing here goes near them.The field is read from the raw json rather than the deserialised object, because the deserialiser skips fields beginning with an underscore. The reader is depth-aware so a nested fragment carrying the same field name cannot be mistaken for the record's own.