Composite Store unit tests#3576
Conversation
PR SummaryLow Risk Overview The harness drives the full store API (commit, read, iterate, proofs, rollback, export/import, historical Eight scenario tests ( Reviewed by Cursor Bugbot for commit 0aeffcb. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3576 +/- ##
==========================================
- Coverage 59.18% 58.38% -0.81%
==========================================
Files 2210 2139 -71
Lines 183250 175208 -8042
==========================================
- Hits 108460 102292 -6168
+ Misses 64998 63780 -1218
+ Partials 9792 9136 -656
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| // rolled-back keyspace. | ||
| func liveKeySetFromOracle(o *storeOracle) *liveKeySet { | ||
| s := newLiveKeySet() | ||
| for store, m := range o.stores { |
There was a problem hiding this comment.
Go map iteration order is randomized per run, so the resulting keys slice order is nondeterministic. Every later keysInUse.Sample(...) indexes into that slice, meaning which keys get read/updated/deleted/sampled depends on map order, not the RNG seed. This contradicts the harness's explicit contract ("a given seed yields a byte-identical apply/commit sequence", "fully reproducible from r's seed", framework header lines 158–160 and 439–440). The guarantee holds for Phase 1 only; it's violated everywhere liveKeySetFromOracle is used — steady-state Phases 2+ (random_test.go:82,124,157) and every migration resume (:368,392,410).
Practical impact: when CI fails and prints Random seed: N, re-running with seed N will not reproduce the failing sequence past the first rollback — the printed seed is misleading exactly when you need it. Fix is small: collect the pairs into a slice, sort by (store, key), then Add in sorted order.
| // flatkv (migrated). Use immediately before a mid-migration restart to confirm | ||
| // the test actually exercises the in-flight resume path. Ported from the | ||
| // migration framework's AssertMigrationInFlight. | ||
| func assertMigrationInFlight(t *testing.T, cs *CompositeCommitStore, oracle *storeOracle, migratingStores ...string) { |
There was a problem hiding this comment.
flakiness: assertMigrationInFlight is seed-dependent and can spuriously fail
Describe your changes and provide context
New suite of unit tests for
composite.Store