From 6758a192daf6df359e13898ccd2f209b66197a8b Mon Sep 17 00:00:00 2001 From: "F." Date: Mon, 4 May 2026 10:31:02 +0200 Subject: [PATCH] test: replace t.Skip with t.Fatalf for replication-factor assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precondition failures in distributed cache tests (read-repair, stale quorum, and phase-1 basic quorum) were silently swallowed via t.Skip, masking genuine test-environment regressions as benign skips. - \`TestDistMemoryReadRepair\`: Skip "replication factor <2" → Fatalf with the actual owner count; RF=2 with 2 registered nodes must always yield ≥2 owners. - \`TestDistMemoryStaleQuorum\`: Skip "replication factor !=3" → Fatalf; RF=3 with 3 registered nodes must yield exactly 3 owners. - \`TestDistPhase1BasicQuorum\`: Remove all skip-on-miss / skip-on-mismatch branches (placeholders from before hint-replay was wired). Now strictly asserts that nodeC receives and holds the correct replicated value within the 3 s deadline, since hinted handoff replay is fully operational (see TestHintedHandoffReplay, TestDistFailureRecovery). Co-Authored-By: Oz --- ...cache_distmemory_remove_readrepair_test.go | 5 +++- ...hypercache_distmemory_stale_quorum_test.go | 6 +++- tests/integration/dist_phase1_test.go | 30 +++++++++++-------- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/tests/hypercache_distmemory_remove_readrepair_test.go b/tests/hypercache_distmemory_remove_readrepair_test.go index e9eabb2..f975928 100644 --- a/tests/hypercache_distmemory_remove_readrepair_test.go +++ b/tests/hypercache_distmemory_remove_readrepair_test.go @@ -64,9 +64,12 @@ func TestDistMemoryReadRepair(t *testing.T) { const key = "rr-key" + // Setup uses RF=2 with 2 nodes registered — Lookup must return both. + // A <2 result indicates a test-environment regression rather than a + // benign condition. owners := dc.Ring.Lookup(key) if len(owners) < 2 { - t.Skip("replication factor <2") + t.Fatalf("expected >=2 owners with RF=2 setup, got %d", len(owners)) } item := &cache.Item{Key: key, Value: "val"} diff --git a/tests/hypercache_distmemory_stale_quorum_test.go b/tests/hypercache_distmemory_stale_quorum_test.go index 6cb9bd9..2398b6a 100644 --- a/tests/hypercache_distmemory_stale_quorum_test.go +++ b/tests/hypercache_distmemory_stale_quorum_test.go @@ -32,9 +32,13 @@ func TestDistMemoryStaleQuorum(t *testing.T) { const key = "sq-key" + // Setup uses RF=3 with 3 nodes registered — Lookup must return all + // three. A non-3 result indicates a test-environment regression + // (e.g., a node failed to join membership), not a benign condition + // to skip past. owners := dc.Ring.Lookup(key) if len(owners) != 3 { - t.Skip("replication factor !=3") + t.Fatalf("expected 3 owners with RF=3 setup, got %d", len(owners)) } item := &cache.Item{Key: key, Value: "v1"} diff --git a/tests/integration/dist_phase1_test.go b/tests/integration/dist_phase1_test.go index a93eb17..7e298de 100644 --- a/tests/integration/dist_phase1_test.go +++ b/tests/integration/dist_phase1_test.go @@ -75,7 +75,14 @@ func awaitNodeReplication(ctx context.Context, node *backend.DistMemory, key str return false } -// TestDistPhase1BasicQuorum is a scaffolding test verifying three-node quorum Set/Get over HTTP transport. +// TestDistPhase1BasicQuorum verifies three-node quorum Set/Get over the +// HTTP transport. Originally a scaffolding test that t.Skipf'd when +// replication hadn't propagated to the third node ("hint replay not yet +// observable" — true at the time the test was written). Hint replay is +// now fully wired (TestHintedHandoffReplay, TestDistFailureRecovery), +// so the test now asserts strictly: every owner must hold the value +// within the 3 s deadline. A failure here means the HTTP-transport +// replication path regressed. func TestDistPhase1BasicQuorum(t *testing.T) { t.Parallel() @@ -112,19 +119,16 @@ func TestDistPhase1BasicQuorum(t *testing.T) { assertValue(t, got.Value) - if awaitNodeReplication(ctx, nodeC, "k1", 3*time.Second) { - return - } - - it, ok := nodeC.Get(ctx, "k1") - if !ok { - t.Skipf("hint replay not yet observable; will be validated after full wiring (missing item)") - - return - } + // Replication must reach C within deadline; the previous skip-on-miss + // branches were placeholders for pre-hint-replay behavior. Locally + // this completes in ~500 ms across 20 runs. + if !awaitNodeReplication(ctx, nodeC, "k1", 3*time.Second) { + it, present := nodeC.Get(ctx, "k1") + if !present { + t.Fatalf("nodeC never received replicated value within 3s") + } - if !valueOK(it.Value) { - t.Skipf("value mismatch after wait") + t.Fatalf("nodeC has wrong value after wait: %T %v", it.Value, it.Value) } }