test: add schema-driven round-trip and fixture tests - #102
Open
ehennestad wants to merge 2 commits into
Open
Conversation
Serialization and deserialization have no regression coverage beyond a handful of write-only assertions, which makes any refactor of that code unsafe. This adds two suites that lock in current behaviour. RoundTripTest checks serialization stability: serialize, load, serialize again, and require the two documents to match as an unordered set of lines. Node order within an @graph is not stable across a save and load cycle, so raw text comparison would be flaky, and comparing documents avoids deep object comparison entirely. Instances are populated by ommtest.helper.synthesizeInstance, which chooses values by property kind and reads cardinality and numeric bounds from the property validators. It never refers to a property by name, so it needs no maintenance as the model adds, moves or removes types and properties. Candidates are lazy and have fallbacks, so a validator the synthesizer does not anticipate degrades to a skipped property rather than a broken test. It populates 2349 of 2363 properties across all 292 types; the remainder are constrained by regular expressions, which is out of scope and reported as such. FixtureTest pins the document format against golden files, which the round-trip check cannot do because it only compares the library against itself. Fixture content is defined once in code and the golden files are generated from it, so refreshing after a model version bump is one call to ommtest.helper.regenerateFixtures plus a review of the diff. Round tripping all 292 types takes about three minutes, which is too slow for every commit. The suite runs a fixed-stride sample of 30 types by default and the full model when OPENMINDS_TEST_ALL_TYPES is set. The workflow sets it for push to main, where the openMINDS pipeline lands regenerated type classes, and for a new weekly schedule; pull requests run the sample. Types with known library defects are listed in ommtest.helper.knownRoundTripGap and reported as incomplete rather than failed, so the suite is usable as a gate while those defects are open. The controlled term case is expressed as a predicate rather than a list of 112 names. That file is expected to shrink as the defects are fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Test Results (R2022a)737 tests +37 720 ✅ +20 2m 32s ⏱️ +34s For more details on these failures, see this check. Results for commit dcd0b86. ± Comparison against base commit 8a8961a. ♻️ This comment has been updated with latest results. |
ehennestad
force-pushed
the
add-serialization-round-trip-tests
branch
from
August 27, 2026 22:13
2ee2dc0 to
dcd0b86
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
+ Coverage 77.07% 78.76% +1.69%
==========================================
Files 417 417
Lines 4022 4022
==========================================
+ Hits 3100 3168 +68
+ Misses 922 854 -68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ehennestad
force-pushed
the
add-serialization-round-trip-tests
branch
9 times, most recently
from
August 28, 2026 12:59
a6011f6 to
dcd0b86
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Serialization and deserialization currently have no regression coverage beyond a handful of write-only assertions in
SerializationTest. That makes any refactor of the serializer, the resolver, orloadInstancesunsafe, because there is nothing that would notice a property being silently dropped. This PR adds the coverage first, so the work that follows has a safety net.Round-trip test
RoundTripTestchecks serialization stability: serialize an instance, load it back, serialize again, and require the two documents to match. Nodes within an@graphare unordered and the collection does not guarantee a stable order across a save and load cycle, so the comparison is on the sorted set of lines rather than raw text. Using documents as the comparison surface also avoids deep object comparison, which would need its own correctness argument for handles, mixed types and reference stubs.Instances are populated by
ommtest.helper.synthesizeInstance. It chooses values by property kind — string, datetime, numeric, linked, embedded — and reads cardinality and numeric bounds out of the property validators. It never refers to a property by name, so it does not need updating when the model adds, moves or removes types and properties. Candidate values are lazy thunks with fallbacks, so a validator it does not anticipate degrades to one skipped property rather than a failed test. It populates 2349 of 2363 properties across all 292 types. The remainder are constrained by regular expressions; generating a string to satisfy an arbitrary pattern is out of scope, and those are categorised and reported rather than silently missed.The type list comes from
enumeration('openminds.enum.Types'), following the existing pattern inInstanceTest, so types added by the pipeline are covered without editing the test.Fixture test
FixtureTestpins the actual document format against golden files. The round-trip test cannot do this, because it only checks that the library agrees with itself — a change that broke the format symmetrically would still pass. The fixture graph covers a scalar string, a string list, a linked instance, an embedded instance, a controlled instance reference, a date and a number.Fixture content is defined once in
ommtest.helper.buildFixtureCollectionand the golden files are generated from it byommtest.helper.regenerateFixtures. Refreshing after a model version bump is one call plus a review of the diff. Fixture instances carry explicit IRIs, because blank node identifiers are random UUIDs and no golden file could otherwise be compared.One fixture is a legacy
openminds.ebrains.eudocument. Loading it under an active v4 model fails withOPENMINDS_MATLAB:Types:InvalidAtType; the test pins that the failure is a clear, identified error rather than silent data loss. If cross-namespace loading is implemented later, that test should change to assert the document loads.Test duration and scheduling
Round tripping all 292 types takes roughly three minutes, which is too slow to run on every commit.
ommtest.helper.roundTripTypeSelectionreturns a fixed-stride sample of 30 types by default and the full model whenOPENMINDS_TEST_ALL_TYPESis set to1. A stride rather than a curated list keeps the sample maintenance-free and still spans the model.run_tests.ymlsets the variable for push tomain, which is where the openMINDS pipeline lands regenerated type classes, and for a new weekly schedule. Pull requests run the sample.Known gaps
Types with known library defects are listed in
ommtest.helper.knownRoundTripGapand reported as incomplete rather than failed, so the suite is usable as a gate while those defects are open. The controlled term case is expressed as a predicate rather than a list of 112 type names; the remainder is a short explicit list. That file is expected to shrink, and each entry names the defect it stands for. Removing an entry when the corresponding fix lands is how the suite starts guarding the fixed behaviour.The defects the suite currently documents, all pre-existing and none addressed here:
ControlledTermBase/initializeControlledTermreceives the decoded struct, passes only the identifier todeserializeFromName, and discards the rest;deserializeFromNamethen finds no matching controlled instance and returns an empty object. This affects all 112 controlled term types.AtlasAnnotationwithlateralityset to both left and right comes back with left only.This is the first PR in a stack. The following PRs fix the defects above and rework the traversal architecture shared by the serializer, the resolver and the deserializer.
🤖 Generated with Claude Code