Skip to content

Two order-level MBO tests cannot fail for the reason they name #141

Description

@ben-dz

Two tests in the order-level Market-by-Order suite are documented, in their own doc comments, as unable to fail for the reason they name. Both are honest about it — this issue is to close them out rather than to report a surprise.

a_venue_time_skew_alone_does_not_drift_the_consumer

tests/order_level_consumer_book.rs. The trailing arm stamps source_ts_ns 5 ms older than the leader's for the same event and arrives in lockstep. The copies collapse as duplicates before any rule reads a stamp, so a mutation swapping venue time for arrival time leaves it green. Its own comment two tests below says so.

It was written as the before-picture for a design keying on venue time, and that design has since landed. a_venue_time_skew_past_the_dedup_window_refuses_the_stale_copy is the scenario that actually measures the property, and it does kill that mutation. So this test now asserts that a duplicate is collapsed, which several other tests already cover, under a name that promises something else.

Either delete it, or rename it to what it checks. The hazard is the name: a future reader trusts it as coverage of venue-time skew.

a_crawling_clock_does_not_starve_the_reseat

src/ingest/arbiter.rs. Costs ~80 ms of real sleep and can false-pass on a slow host — past the point where a round costs more than the re-seat wait, the movement trigger fires by itself and the test passes with the mechanism deleted. Its comment says not to read a pass as evidence the trigger works.

a_crawling_advance_keeps_only_the_movement_wait_fresh drives the clock directly and is the real pin; the_removed_population_tracks_the_window_not_the_lag covers the population more cheaply. Deleting this one appears to lose nothing, but that should be checked rather than assumed.

Why file this rather than fix it in the branch

Both are on bdz/mbo-full-depth-l3 (#137), which is already large. Neither affects correctness — the mechanisms they nominally cover are pinned by other tests that were mutation-checked.

Worth knowing

Six tests in this subsystem have turned out to assert nothing, across the order-level work: a lag sweep asserting a figure its generator could never produce; a cost test whose setup never entered the code it bounded; a population assertion that passed with its mechanism disabled; an anchors test that pinned its guarantee only by accident of batch size; and the two above. Every one was found by a reviewer or by mutation-checking, never by the suite going red.

The practice that caught the later ones — for each new test, break the behaviour it asserts and confirm it fails, then revert — is cheap and worth keeping for anything new in this area.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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