Skip to content

Fix two stale SEDML export test expectations - #1835

Open
jcschaff wants to merge 1 commit into
masterfrom
fix/sedml-test-bookkeeping
Open

Fix two stale SEDML export test expectations#1835
jcschaff wants to merge 1 commit into
masterfrom
fix/sedml-test-bookkeeping

Conversation

@jcschaff

@jcschaff jcschaff commented Aug 5, 2026

Copy link
Copy Markdown
Member

Two of the five failures in the nightly SEDML_SBML_IT regression are test
bookkeeping, not product defects. Both models are in the slow set, so they run
only on the nightly and never in the merge gate.

biomodel_188880263.vcml — undeclared unsupported applications

The test declared no unsupported applications; the exporter reports eight. Every
application in that model is rule-based, and SBML export does not support
reaction rules, so the exporter is right and the declaration was simply never
filled in. The assertion has been printing the exact lines to add:

declared: []
found:    [Stochastic mRNA Number Sweep, Stochastic Make mRNA Sweep Const Prot,
           Stochastic mRNA Number Sweep Const Nucl Chaparones, Stochastic Make mRNA Sweep,
           Stochastic, Continuous_Low L1DNA, Network Free, Continuous_Med L1DNA]
           …each: "has reaction rules, SBML Export is not supported"

Added all eight, in the file-ordered position the list already uses.

biomodel_34826524.vcml — a known fault that no longer happens

It had SEDML_FAULT.MATH_OVERRIDE_NOT_EQUIVALENT registered, so it failed for
passing:

file SBML.biomodel_34826524.vcml passed SEDML Round trip, but knownSEDMLFault was set
  ==> expected: <null>

Removed the entry.

Verification

Run individually — these need -Dtest.include.slow=true, since both are
slow-gated and are otherwise skipped:

model before after
biomodel_188880263.vcml AssertionFailedError pass, 51s
biomodel_34826524.vcml AssertionFailedError pass, 346s

Note this repo's PR CI cannot exercise either: SEDML_SBML_IT runs only in
regression.yml, and these two are additionally behind test.include.slow. The
checks on this PR say nothing about the change; the runs above are the evidence.

Incidentally biomodel_34826524 took 346s locally against the 129s recorded next
to it in slowModels, so that annotation is optimistic.

Not included

The other three nightly failures are genuine export defects — exported SED-ML
referencing species and parameters by XPath that are absent from the exported
SBML, plus one duplicate SED-ML id. Those deserve a real fix rather than a
knownSEDMLFault entry, which would make a live bug permanently invisible. See
also #1827, which makes those failures legible in CI output (they currently
report as a bare » OmexValidation).

🤖 Generated with Claude Code

https://claude.ai/code/session_01SvqmME7MkRUNEYje5HpiLt

Two of the five failures in the nightly SEDML_SBML_IT regression are test
bookkeeping rather than product defects. Both are in the slow set, so they only
run on the nightly and never in the merge gate.

biomodel_188880263.vcml declared no unsupported applications, but the exporter
correctly reports eight: every application in that model is rule-based, and SBML
export does not support reaction rules. The assertion message has been printing
the exact lines to add. Added them.

biomodel_34826524.vcml had SEDML_FAULT.MATH_OVERRIDE_NOT_EQUIVALENT registered
for a fault that no longer occurs, so it failed for *passing*: "file
SBML.biomodel_34826524.vcml passed SEDML Round trip, but knownSEDMLFault was set".
Removed the entry.

Verified by running each model, which needs -Dtest.include.slow=true since both
are slow-gated. Both now pass: 188880263 in 51s, 34826524 in 346s.

This does not touch the remaining three failures, which are genuine SED-ML export
defects (exported SED-ML referencing entities by XPath that are absent from the
exported SBML, plus a duplicate SED-ML id) and want a real fix rather than a
knownSEDMLFault entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SvqmME7MkRUNEYje5HpiLt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant