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 <rsc@golang.org>
pull/12296/head
Russ Cox 2 years ago
parent d4bf47766e
commit 28f5502828

@ -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

Loading…
Cancel
Save