internal/singleflight: deflake TestDoDupSuppress

Currently TestDoDupSuppress can fail if the goroutines created by its
loop run sequentially. This is rare, but it has caused failures on the
dashboard and in stress testing.

While I think there's no way to eliminate all possible thread
schedules that could make this test fail because it depends on waiting
until a Group.Do blocks, it is possible to make it much more robust.

This commit deflakes this test by forcing at least one invocation of
fn to start and all goroutines to reach the line just before the Do
call before allowing fn to proceed. fn then waits 10 milliseconds
before returning to allow the goroutines to pass through the Do.

With this change, in 50,000 runs of the stress testing configuration,
the number of calls to fn never even exceeded 1.

Fixes #11784.

Change-Id: Ie5adf5764545050ec407619769a656251c4cff04
Reviewed-on: https://go-review.googlesource.com/12681
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/internal/singleflight/singleflight_test.go b/src/internal/singleflight/singleflight_test.go
index 890ab62e0336a11e5c19a2d4b06334e46398cf69..c0ec0240c729146ba57f133117ecbd662bd178d0 100644 (file)
--- a/src/internal/singleflight/singleflight_test.go
+++ b/src/internal/singleflight/singleflight_test.go
@@ -42,21 +42,30 @@ func TestDoErr(t *testing.T) {
 
 func TestDoDupSuppress(t *testing.T) {
        var g Group
+       var wg1, wg2 sync.WaitGroup
        c := make(chan string, 1)
        var calls int32
        fn := func() (interface{}, error) {
-               atomic.AddInt32(&calls, 1)
+               if atomic.AddInt32(&calls, 1) == 1 {
+                       // First invocation.
+                       wg1.Done()
+               }
                v := <-c
                c <- v // pump; make available for any future calls
+
+               time.Sleep(10 * time.Millisecond) // let more goroutines enter Do
+
                return v, nil
        }
 
        const n = 10
-       var wg sync.WaitGroup
+       wg1.Add(1)
        for i := 0; i < n; i++ {
-               wg.Add(1)
+               wg1.Add(1)
+               wg2.Add(1)
                go func() {
-                       defer wg.Done()
+                       defer wg2.Done()
+                       wg1.Done()
                        v, err, _ := g.Do("key", fn)
                        if err != nil {
                                t.Errorf("Do error: %v", err)
@@ -67,10 +76,12 @@ func TestDoDupSuppress(t *testing.T) {
                        }
                }()
        }
-       time.Sleep(10 * time.Millisecond) // let some goroutines above block in Do
+       wg1.Wait()
+       // At least one goroutine is in fn now and all of them have at
+       // least reached the line before the Do.
        c <- "bar"
-       wg.Wait()
+       wg2.Wait()
        if got := atomic.LoadInt32(&calls); got <= 0 || got >= n {
-               t.Errorf("number of calls = %d; want over 0 and less than n", got)
+               t.Errorf("number of calls = %d; want over 0 and less than %d", got, n)
        }
 }