From 309fe1f0c66334d5754e81d352377e00b21e4f5a Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Thu, 5 Dec 2024 16:26:46 +0100 Subject: [PATCH] predicates/traffic: deflake TestTrafficSegmentSplit Use fixed random sequence to deflake TestTrafficSegmentSplit. Fixes #2665 Signed-off-by: Alexander Yastrebov --- predicates/traffic/export_test.go | 11 +++++++++++ predicates/traffic/rand_test.go | 18 ++++++++++++++++++ predicates/traffic/segment.go | 17 +++++++++++------ predicates/traffic/segment_test.go | 4 +++- 4 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 predicates/traffic/rand_test.go diff --git a/predicates/traffic/export_test.go b/predicates/traffic/export_test.go index d1d32b2be2..b09d373e0e 100644 --- a/predicates/traffic/export_test.go +++ b/predicates/traffic/export_test.go @@ -1,3 +1,14 @@ package traffic +import "github.com/zalando/skipper/routing" + var ExportRandomValue = randomValue + +func WithRandFloat64(ps routing.PredicateSpec, randFloat64 func() float64) routing.PredicateSpec { + if s, ok := ps.(*segmentSpec); ok { + s.randFloat64 = randFloat64 + } else { + panic("invalid predicate spec, expected *segmentSpec") + } + return ps +} diff --git a/predicates/traffic/rand_test.go b/predicates/traffic/rand_test.go new file mode 100644 index 0000000000..a84e6ce89c --- /dev/null +++ b/predicates/traffic/rand_test.go @@ -0,0 +1,18 @@ +package traffic_test + +import ( + "math/rand/v2" + "sync" +) + +var ( + testRandMu sync.Mutex + testRand = rand.New(rand.NewPCG(0x5EED_1, 0x5EED_2)) +) + +// testRandFloat64 is a helper function to generate fixed sequence of random float64 values for testing. +func testRandFloat64() float64 { + testRandMu.Lock() + defer testRandMu.Unlock() + return testRand.Float64() +} diff --git a/predicates/traffic/segment.go b/predicates/traffic/segment.go index f9dc323860..799fd10c0a 100644 --- a/predicates/traffic/segment.go +++ b/predicates/traffic/segment.go @@ -9,8 +9,13 @@ import ( ) type ( - segmentSpec struct{} - segmentPredicate struct{ min, max float64 } + segmentSpec struct { + randFloat64 func() float64 + } + segmentPredicate struct { + randFloat64 func() float64 + min, max float64 + } ) type contextKey struct{} @@ -19,7 +24,7 @@ var randomValue contextKey // NewSegment creates a new traffic segment predicate specification func NewSegment() routing.WeightedPredicateSpec { - return &segmentSpec{} + return &segmentSpec{rand.Float64} } func (*segmentSpec) Name() string { @@ -42,12 +47,12 @@ func (*segmentSpec) Name() string { // r50: Path("/test") && TrafficSegment(0.0, 0.5) -> ; // r30: Path("/test") && TrafficSegment(0.5, 0.8) -> ; // r20: Path("/test") && TrafficSegment(0.8, 1.0) -> ; -func (*segmentSpec) Create(args []any) (routing.Predicate, error) { +func (s *segmentSpec) Create(args []any) (routing.Predicate, error) { if len(args) != 2 { return nil, predicates.ErrInvalidPredicateParameters } - p, ok := &segmentPredicate{}, false + p, ok := &segmentPredicate{randFloat64: s.randFloat64}, false if p.min, ok = args[0].(float64); !ok || p.min < 0 || p.min > 1 { return nil, predicates.ErrInvalidPredicateParameters @@ -72,7 +77,7 @@ func (*segmentSpec) Weight() int { } func (p *segmentPredicate) Match(req *http.Request) bool { - r := routing.FromContext(req.Context(), randomValue, rand.Float64) + r := routing.FromContext(req.Context(), randomValue, p.randFloat64) // min == max defines a never-matching interval and always yields false return p.min <= r && r < p.max } diff --git a/predicates/traffic/segment_test.go b/predicates/traffic/segment_test.go index 87e93397a8..aff10b1f61 100644 --- a/predicates/traffic/segment_test.go +++ b/predicates/traffic/segment_test.go @@ -148,7 +148,9 @@ func TestTrafficSegmentSplit(t *testing.T) { RoutingOptions: routing.Options{ FilterRegistry: builtin.MakeRegistry(), Predicates: []routing.PredicateSpec{ - traffic.NewSegment(), + // Use fixed random sequence to deflake the test, + // see https://github.com/zalando/skipper/issues/2665 + traffic.WithRandFloat64(traffic.NewSegment(), testRandFloat64), }, }, Routes: eskip.MustParse(`