Skip to content

RFC: quickcheck in core vs. third-party test dependency #115

Description

@bobzhang

Decision to make

Should quickcheck become part of core (bobzhang/toml exposes generators / Arbitrary instances for TomlValue), or stay a third-party, test-only dependency as it is today?

Goal constraint: keep the public API minimal.


Where we are today

  • quickcheck is a test-only dependency of one internal package: internal/qc_model (... for "test" in moon.pkg).
  • The property test round-trips a proxy type — SimpleDocument / SimpleValue — not TomlValue directly: generate → to_tomlto_string@toml.parsefrom_toml → assert equal.
  • TomlValue itself has no Arbitrary and no Shrink impl. The only Arbitrary in play is the library's built-in one for String / Int64 / Bool, used via @gen.Gen::spawn().
  • Generators/shrinkers are plain functions passed explicitly to @qc.forall_shrink(...) — no trait wiring.

Minimal surface actually used (~20 symbols)

Area Symbols
Driver/report forall_shrink, quick_check_silence, classify, counterexample, Property, Shrink::shrink
Generators Gen[T], pure, int_range, char_range, Gen::spawn, one_of, frequency, sized, .scale, .fmap, .bind, .array_with_size, liftA2, liftA3

Everything else the library ships (the quick_check_fn* family, collect/filter/label, .ap/.join/such_that, tuple/triple/quad, specialized shrink_* helpers, …) is unused.


What we verified while investigating

  1. Trait-style on the proxy hits the orphan rule. impl @quickcheck.Arbitrary for SimpleDocument from the black-box test package fails to compile — error 4061: "Cannot implement foreign trait for foreign type". Two ways around it, both confirmed green:
    • convert the tests black-box → white-box (_test.mbt_wbtest.mbt) so the type is local — but you lose black-box testing; or
    • wrap in a newtype local to the test package: struct RoundTrippable(SimpleDocument) + impl Arbitrary/Shrink on the wrapper — keeps black-box, quickcheck stays test-only. (This is exactly how the library's own modifiers package is built: struct Positive[T](T), struct NonEmptyArray[T](Array[T]), …)
  2. For TomlValue in core there is no orphan problem — the type is local to core, so impl Arbitrary/Shrink for TomlValue is coherent and needs no newtype or white-box gymnastics.
  3. Round-trip caveat. Full TomlValue includes floats and mixed-type arrays, which don't survive exact render→parse equality. A canonical Arbitrary for TomlValue is therefore a parser-fuzzing generator, not a round-trip law. The round-trip-safe subset is precisely what SimpleValue encodes today.
  4. Dependency cost of "in core". Making quickcheck a non-test import of core pulls moonbitlang/quickcheck + core/quickcheck + splitmix + bigint + list into core's runtime dependency closure — i.e. every consumer of the parser compiles them, including parse-only users who never test.

Options

A — Keep third-party / test-only (status quo)

  • Pros: zero runtime deps for library consumers; black-box tests preserved; flexible free-function generators (several generators per type, e.g. bare / complex / neg-exponent keys — impossible under one canonical trait impl).
  • Cons: no reusable generator for downstream; anyone property-testing against TomlValue re-invents generation.

B — Integrate into core (impl Arbitrary/Shrink for TomlValue in the core package)

  • Pros: canonical, discoverable, reusable generator; downstream gets @qc.quick_check_fn(fn(v: TomlValue) { ... }) for free; tiny public surface (two impls).
  • Cons: quickcheck enters core's runtime dep closure (all parse-only users pay for it); single canonical instance (no alternative strategies without newtypes); round-trip generator still needs a RoundTrippable(TomlValue) newtype to express the safe subset.

C — Companion package bobzhang/toml/quickcheck (recommended)

A thin package that depends on core and exposes:

  • impl Arbitrary/Shrink for TomlValue — full parser fuzzing, and
  • struct RoundTrippable(TomlValue) with its own Arbitrary — the round-trip law generator (folds today's SimpleValue proxy into a newtype over the real type).

Consumers who want fuzzing add bobzhang/toml/quickcheck; parse-only users pull nothing extra. Reusable and no core dependency creep. Because MoonBit resolves deps per-package, TomlValue is local to core (no orphan), yet the quickcheck weight stays opt-in.


Trade-off at a glance

A: third-party B: in core C: companion pkg
Reusable TomlValue generator for downstream
Core stays parse-only (no quickcheck runtime dep)
Black-box tests preserved n/a
Public API stays minimal ⚠️ (adds impls to core) ✅ (isolated)
Effort none low low–medium

Recommendation

Option C. It satisfies "keep the API minimal" in the sense that matters most — core's runtime surface and dependency closure stay untouched — while still giving downstream a canonical, reusable generator. If cross-package friction ever proves not worth it, collapsing C into B later is trivial; going the other way (extracting quickcheck back out of core) is not.

Interim, regardless of choice: the internal proxy test is fine as-is. Trait-style there is a net negative (more boilerplate, and it costs either black-box testing or a newtype wrapper for no real gain).

Decision

[to fill in]


Drafted via Claude Code during a design investigation; all compiler-verified claims above were reproduced locally.

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