From 28f5502828e88ac81a4cc5f1fd00bd4b2091189f Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 26 Apr 2023 10:26:58 -0400 Subject: [PATCH] scrape: fix two loop variable scoping bugs in test Consider code like: for i := 0; i < numTargets; i++ { stopFuncs = append(stopFuncs, func() { time.Sleep(i*20*time.Millisecond) }) } Because the loop variable i is shared by all closures, all the stopFuncs sleep for numTargets*20 ms. If the i were made per-iteration, as we are considering for a future Go release, the stopFuncs would have sleep durations ranging from 0 to (numTargets-1)*20 ms. Two tests had code like this and were checking that the aggregate sleep was at least numTargets*20 ms ("at least as long as the last target slept"). This is only true today because i == numTarget during all the sleeps. To keep the code working even if the semantics of this loop change, this PR computes d := time.Duration((i+1)*20) * time.Millisecond outside the closure (but inside the loop body), and then each closure has its own d. Now the sleeps range from 20 ms to numTargets*20 ms, keeping the test passing (and probably behaving closer to the intent of the test author). The failure being fixed can be reproduced by using the current Go development branch with GOEXPERIMENT=loopvar go test Signed-off-by: Russ Cox --- scrape/scrape_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 2a2ca0948..60e335b5b 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -190,8 +190,9 @@ func TestScrapePoolStop(t *testing.T) { labels: labels.FromStrings(model.AddressLabel, fmt.Sprintf("example.com:%d", i)), } l := &testLoop{} + d := time.Duration((i+1)*20) * time.Millisecond l.stopFunc = func() { - time.Sleep(time.Duration(i*20) * time.Millisecond) + time.Sleep(d) mtx.Lock() stopped[t.hash()] = true @@ -273,8 +274,9 @@ func TestScrapePoolReload(t *testing.T) { discoveredLabels: labels, } l := &testLoop{} + d := time.Duration((i+1)*20) * time.Millisecond l.stopFunc = func() { - time.Sleep(time.Duration(i*20) * time.Millisecond) + time.Sleep(d) mtx.Lock() stopped[t.hash()] = true