test(message): deflake HLC timestamp serde_json::Value round-trip test - #3107
test(message): deflake HLC timestamp serde_json::Value round-trip test#3107phil-opp wants to merge 1 commit into
Conversation
The test `a_freshly_generated_hlc_id_uses_all_sixteen_bytes` asserted `HLC::default().new_timestamp().get_id().size() == 16`. Under uhlc 0.9, `HLC::default()` seeds its id via `ID::rand()` (a uniformly random 128-bit value), whose most-significant byte is zero ~1/256 of the time, so the id occupies only 15 significant little-endian bytes and the assertion fails on roughly 1 in 256 runs — an intermittent CI failure since dora-message is exercised by `cargo test --all`. The full-width (>u64) property is already pinned deterministically by `timestamp_survives_a_serde_json_value_round_trip` via `golden_timestamp` (top byte 0x10). This test's remaining value is confirming a real, randomly-generated `HLC::default()` timestamp survives the `serde_json::Value` round trip, which is lossless for an id of any width. Drop the probabilistic size assertion and keep the round-trip check. Fixes #3104 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EZXbbi2V3Dw4174Xn3AgMX
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
🤖 Automated review by Claude Code — fully automated review, not vetted by a human. No issues found. This is a clean test-quality fix. The dropped assertion ( The retained round-trip check still exercises meaningful behavior: it builds a real Generated by Claude Code |
Summary
Fixes #3104.
The test
a_freshly_generated_hlc_id_uses_all_sixteen_bytesinlibraries/message/tests/uhlc_wire_format.rswas non-deterministic and failed on roughly 1 in 256cargo testruns. Becausedora-messageis exercised bycargo test --allin both the PR gate and nightly, this introduced an intermittent, hard-to-reproduce CI failure.Root cause
Under uhlc 0.9,
HLC::default()seeds its id viaID::rand()— a uniformly random 128-bit value.ID::size()returns the number of significant little-endian bytes, sosize() == 16only holds when the most-significant byte is non-zero. That byte is zero with probability1/256, so the assertionassert_eq!(timestamp.get_id().size(), 16)failed ~0.39% of the time. Under uhlc 0.5 the id was seeded from a UUID whose top bits were fixed version/variant nibbles, so it was effectively always 16 bytes — the assertion was implicitly safe then and became flaky after the 0.5 → 0.9 bump (#3016).No production path depends on
size() == 16; the JSON round-trip works for any non-zero id. This is a test-quality defect, not a runtime bug.Fix
The full-width (
> u64) property is already pinned deterministically bytimestamp_survives_a_serde_json_value_round_trip, which usesgolden_timestamp(top byte0x10). This test's remaining value is confirming that a real, randomly-generatedHLC::default()timestamp survives theserde_json::Valueround trip — which is lossless for an id of any width.So the probabilistic
size()assertion is dropped and the real-HLC round-trip check is kept (and the test renamed accordingly). This removes the flakiness while preserving the real-HLC coverage the original test added, on top of the deterministic full-width coverage that already exists.Testing
cargo test -p dora-message --test uhlc_wire_format— all 7 tests pass.cargo fmt -p dora-message -- --check— clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01EZXbbi2V3Dw4174Xn3AgMX
Generated by Claude Code