Skip to content

Nothing catches a declaration-only drift in the golden fixtures #16

Description

@cayossarian

What happened

tests/conformance/fixtures/golden_tree.json and golden_wire.json declared a connection/count property on every circuit and on both lugs devices for two releases after the emitter stopped publishing it.

Upstream removed the declaration in ebus-panel-sim 0.6.0 (f71138d, "stop declaring connection/count, which no configuration could publish") because the snapshot fields behind it were never assigned by any code path, so a consumer reading $description waited for a value that could not arrive. This repository was pinned to 0.6.1 well before that was noticed here, and the fixtures went on declaring the property anyway. It surfaced only when #11's pin bump to 0.8.0 forced a regeneration: 32 properties moved from match to omission in the golden report, alongside the three relay changes that bump was actually about.

Nothing was broken on the wire — the simulator had been publishing correctly the whole time. What was wrong was our record of what it publishes, which is the artifact consumer authors are pointed at.

Why both guards miss it

There are two tests over the golden fixtures, and the gap is the space between them.

tests/conformance/test_golden.py::test_golden_report_is_unchanged is documented to fail "on ANY change to the published surface". It compares golden_report.json against a conformance report computed from golden_tree.json — one committed fixture against another. No emitter runs. A fixture that has drifted from the emitter agrees with itself perfectly, so the test stays green forever and the drift is invisible by construction. Its docstring's promise holds only for a change made to the fixture, not for a change made to the emitter.

tests/conformance/test_wire_capture.py::test_the_committed_capture_still_matches_what_the_emitter_emits does run the live emitter, and is the test that should have caught this. It compares topic key sets: which devices exist, and which topics each publishes. connection/count never carried a value, so it never had a topic, so removing its declaration changed no key in that comparison. The test only sees a property that is published, and this one only ever existed as a declaration.

So a declaration-only change — a property added to or removed from $description without a value — passes both. That is not a narrow corner: req: MAY properties that a given config never populates are exactly the ones most likely to be added or removed upstream, and they are precisely the ones with no topic.

Proposed guard

A test that captures a live tree and compares its $description documents against the committed golden_tree.json, structurally.

scripts/capture-wire.py and panelbench.emitter_adapter.wire_capture.capture() already produce everything needed with no broker — the MQTT client is substituted — and golden_tree.json is derivable from a wire capture by taking each device's $description. So the new test would capture fresh, derive the tree shape, and assert it matches the committed one.

Compare declarations, not values: for each device, the set of node/property keys and each property's datatype, unit, format and settable. Deliberately excluded is the document's version field, which is a timestamp and differs every run, and every published value, which noise_factor and the advancing clock make irreproducible — the existing wire-capture test's docstring already makes that argument and it applies unchanged here.

That closes the loop the other two leave open: test_the_committed_capture_still_matches_what_the_emitter_emits covers what the emitter publishes, this would cover what it declares, and test_golden_report_is_unchanged continues to make the classification a reviewed artifact rather than a moving one.

Things worth deciding rather than assuming

Whether the new assertion belongs alongside the existing one in test_wire_capture.py or in test_golden.py. It guards golden_tree.json, which is test_golden.py's fixture, but it needs a live emitter run, which is test_wire_capture.py's pattern and its stated reason for living in that module.

Whether it should fail or self-heal. Failing is consistent with test_golden_report_is_unchanged's stance that a change to the published surface should be a reviewed edit and never something that slips through. The cost is that both tests then fail together on every deliberate regeneration, which is noise rather than signal; the alternative of printing the regeneration command in the failure message, as test_golden.py already does, is probably enough.

Whether golden_tree.json should keep being generated by the documented mosquitto_sub | check-conformance.py --from-stdin route at all, given that a broker-free derivation from golden_wire.json produces the same document and would let one capture feed both fixtures. #11 regenerated them that way and the pair now describe one panel at one instant, where previously they came from separate runs with disagreeing version timestamps. DEVELOPER.md's recapture instructions still describe the broker route.

Context

Found while implementing #11 (pick up ebus-panel-sim 0.8.0); the regeneration there carries the correction, so the fixtures are accurate as of that PR. This issue is only about the missing guard, which is what let it go unnoticed for two releases and would let the next one go unnoticed too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions