fix(quickcheck): omit empty categories from ObservationSummary debug rendering - #3992
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves snapshot stability/readability for QuickCheck reports by changing ObservationSummary’s Debug rendering to omit empty observation categories (labels/classes), matching the existing failure_report output behavior and reducing noise in snapshot-first workflows.
Changes:
- Replace
derive(@debug.Debug)onObservationSummarywith a custom@debug.Debug::to_reprthat conditionally includeslabels/classesonly when non-empty. - Update an existing snapshot expectation to drop the now-omitted empty
classes: {}field. - Add a new snapshot test intended to assert classes-only rendering omits the empty
labelsfield.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| quickcheck/observation.mbt | Adds a custom Debug impl for ObservationSummary that omits empty categories for snapshot-friendly output. |
| quickcheck/driver_test.mbt | Updates one snapshot and adds a new snapshot test for the new classes-only rendering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test "classes-only summary omits the empty labels field" { | ||
| let report = @quickcheck.report( | ||
| (_ : Int) => true, | ||
| observe=input => [@quickcheck.classify(input >= 0, "non-negative")], |
There was a problem hiding this comment.
Not flaky as-is (the driver's default seed=37 makes the run deterministic), but the robustness point stands — the count would drift if Int generation or the default seed ever changed, and this test only exercises the rendering shape. Fixed in 1f9fa57: the classification is now unconditional (classify(true, "observed")), so the count always equals the test count.
Coverage Report for CI Build 5815Coverage increased (+0.003%) to 90.323%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
| if !self.classes.is_empty() { | ||
| fields["classes"] = Repr(self.classes) | ||
| } | ||
| @debug.Repr::record(fields) |
There was a problem hiding this comment.
@debug.Repr(Map[
.. if !self.labels.is_empty() { [ ("labels", Repr(self.labels)]},
.. if !self.class.is_empty() { ...}
])
There was a problem hiding this comment.
Done — the record is now built with conditional spreads in the literal. One deliberate deviation from the snippet: kept Repr::record(...) instead of Repr(Map(...)), since rendering the summary as a Map would quote the keys ({ "labels": ... }) and churn every observation snapshot; record keeps the unquoted field style.
…rendering
With snapshot tests as the primary way to consume reports, the Debug
rendering is effectively API surface. A classes-only summary previously
rendered a noisy labels: {} field (and vice versa); empty categories are
now omitted, so classify-based coverage snapshots read as
observations={ classes: {...} }.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: classify unconditionally so the class count always equals the test count and cannot drift if Int generation or the default seed changes; the test only exercises the rendering shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback: construct the field map declaratively in the literal instead of mutating it. Repr::record is kept (rather than Repr on a Map) so keys keep the unquoted record rendering that snapshots rely on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d7f21fb to
26a8f24
Compare
In the snapshot-first testing workflow, a report's
Debugrendering is effectively the API surface — and every classes-only coverage snapshot (the common case when usingclassify) currently carries a noisylabels: {}field, e.g. in a downstream project (moonbit-community/toml-parser#122):ObservationSummarynow renders via a customDebugimpl that omits empty categories, so the above readsobservations={ classes: {...} }(symmetrically, labels-only summaries dropclasses: {}). The textfailure_reportpath already skipped empty categories; this aligns the Debug path.Test plan: new classes-only rendering test; one existing labels-only snapshot updated (drops
, classes: {});moon test quickcheck68/68;moon check --deny-warn,moon fmt,moon infoclean (no interface change — the type is private).🤖 Generated with Claude Code