From dbb456d04ab4d999fccb72ee576edb3a7641a39a Mon Sep 17 00:00:00 2001
From: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Date: Mon, 21 Aug 2023 11:48:16 +0200
Subject: [PATCH] routesrv: fix flaky TestESkipBytesHandlerWithOldLastModified
 (#2531)

* fix flaky test by mocking clock instead of using sleep.
* add tracing tag on route update

Follows up on #2526

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
---
 routesrv/eskipbytes.go    | 3 ++-
 routesrv/export_test.go   | 7 +++++++
 routesrv/polling.go       | 1 +
 routesrv/routesrv.go      | 2 +-
 routesrv/routesrv_test.go | 7 +++++--
 5 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 routesrv/export_test.go

diff --git a/routesrv/eskipbytes.go b/routesrv/eskipbytes.go
index 7be7869265..b7e17a6c2a 100644
--- a/routesrv/eskipbytes.go
+++ b/routesrv/eskipbytes.go
@@ -27,6 +27,7 @@ type eskipBytes struct {
 	mu           sync.RWMutex
 
 	tracer ot.Tracer
+	now    func() time.Time
 }
 
 // formatAndSet takes a slice of routes and stores them eskip-formatted
@@ -42,7 +43,7 @@ func (e *eskipBytes) formatAndSet(routes []*eskip.Route) (_ int, initialized boo
 
 	updated = !bytes.Equal(e.data, data)
 	if updated {
-		e.lastModified = time.Now()
+		e.lastModified = e.now()
 		e.data = data
 		e.etag = fmt.Sprintf(`"%x"`, sha256.Sum256(e.data))
 		e.count = len(routes)
diff --git a/routesrv/export_test.go b/routesrv/export_test.go
new file mode 100644
index 0000000000..5c967f6f4c
--- /dev/null
+++ b/routesrv/export_test.go
@@ -0,0 +1,7 @@
+package routesrv
+
+import "time"
+
+func SetNow(rs *RouteServer, now func() time.Time) {
+	rs.poller.b.now = now
+}
diff --git a/routesrv/polling.go b/routesrv/polling.go
index 2e4978468c..e06765560f 100644
--- a/routesrv/polling.go
+++ b/routesrv/polling.go
@@ -104,6 +104,7 @@ func (p *poller) poll(wg *sync.WaitGroup) {
 			}
 			if updated {
 				logger.Info(LogRoutesUpdated)
+				span.SetTag("routes.updated", true)
 				routesUpdated.SetToCurrentTime()
 			}
 			span.SetTag("routes.count", routesCount)
diff --git a/routesrv/routesrv.go b/routesrv/routesrv.go
index b980281feb..d333411199 100644
--- a/routesrv/routesrv.go
+++ b/routesrv/routesrv.go
@@ -42,7 +42,7 @@ func New(opts skipper.Options) (*RouteServer, error) {
 		return nil, err
 	}
 
-	b := &eskipBytes{tracer: tracer}
+	b := &eskipBytes{tracer: tracer, now: time.Now}
 	bs := &eskipBytesStatus{b: b}
 	handler := http.NewServeMux()
 	handler.Handle("/health", bs)
diff --git a/routesrv/routesrv_test.go b/routesrv/routesrv_test.go
index a680125ca9..0908bb5091 100644
--- a/routesrv/routesrv_test.go
+++ b/routesrv/routesrv_test.go
@@ -163,6 +163,8 @@ func TestMain(m *testing.M) {
 func TestNotInitializedRoutesAreNotServed(t *testing.T) {
 	defer tl.Reset()
 	ks, _ := newKubeServer(t)
+	defer ks.Close()
+
 	rs := newRouteServer(t, ks)
 
 	w := getRoutes(rs)
@@ -513,8 +515,9 @@ func TestESkipBytesHandlerWithOldLastModified(t *testing.T) {
 	lastModified := w1.Header().Get("Last-Modified")
 	header := map[string]string{"If-Modified-Since": lastModified}
 
-	// Last-Modified has a second precision so we need to wait a bit for it to change
-	time.Sleep(2 * time.Second)
+	// Last-Modified has a second precision so we need to wait a bit for it to change.
+	// Move clock forward instead of using time.Sleep that makes test flaky.
+	routesrv.SetNow(rs, func() time.Time { return time.Now().Add(2 * time.Second) })
 
 	// update the routes, which also updated the e.lastModified
 	handler.set(newKubeAPI(t, loadKubeYAML(t, "testdata/lb-target-single.yaml")))