net: use synthetic network in TestDialParallel

TestDialParallel is testing the Happy Eyeballs algorithm implementation,
which dials IPv4 and IPv6 addresses in parallel with the preferred
address family getting a head start. This test doesn't care about
the actual network operations, just the handling of the parallel
connections.

Use testHookDialTCP to replace socket creation with a function that
returns successfully, with an error, or after context cancellation
as required.

Limit tests of elapsed times to a check that the fallback deadline
has been exceeded in cases where this is expected.

This should fix persistent test flakiness.

Fixes #52173.

Change-Id: Ic93f270fccb63b24a91105a4d541479fc33a2de4
Reviewed-on: https://go-review.googlesource.com/c/go/+/410754
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/net/dial_test.go

@@ -9,6 +9,8 @@ package net
 import (
 	"bufio"
 	"context"
+	"errors"
+	"fmt"
 	"internal/testenv"
 	"io"
 	"os"
@@ -175,31 +177,9 @@ func dialClosedPort(t *testing.T) (dialLatency time.Duration) {
 }
 
 func TestDialParallel(t *testing.T) {
-	testenv.MustHaveExternalNetwork(t)
-
-	if !supportsIPv4() || !supportsIPv6() {
-		t.Skip("both IPv4 and IPv6 are required")
-	}
-
-	closedPortDelay := dialClosedPort(t)
-
 	const instant time.Duration = 0
 	const fallbackDelay = 200 * time.Millisecond
 
-	// Some cases will run quickly when "connection refused" is fast,
-	// or trigger the fallbackDelay on Windows. This value holds the
-	// lesser of the two delays.
-	var closedPortOrFallbackDelay time.Duration
-	if closedPortDelay < fallbackDelay {
-		closedPortOrFallbackDelay = closedPortDelay
-	} else {
-		closedPortOrFallbackDelay = fallbackDelay
-	}
-
-	origTestHookDialTCP := testHookDialTCP
-	defer func() { testHookDialTCP = origTestHookDialTCP }()
-	testHookDialTCP = slowDialTCP
-
 	nCopies := func(s string, n int) []string {
 		out := make([]string, n)
 		for i := 0; i < n; i++ {
@@ -223,31 +203,21 @@ func TestDialParallel(t *testing.T) {
 		// Primary is slow; fallback should kick in.
 		{[]string{slowDst4}, []string{"::1"}, "", true, fallbackDelay},
 		// Skip a "connection refused" in the primary thread.
-		{[]string{"127.0.0.1", "::1"}, []string{}, "tcp4", true, closedPortDelay},
+		{[]string{"127.0.0.1", "::1"}, []string{}, "tcp4", true, instant},
-		{[]string{"::1", "127.0.0.1"}, []string{}, "tcp6", true, closedPortDelay},
+		{[]string{"::1", "127.0.0.1"}, []string{}, "tcp6", true, instant},
 		// Skip a "connection refused" in the fallback thread.
-		{[]string{slowDst4, slowDst6}, []string{"::1", "127.0.0.1"}, "tcp6", true, fallbackDelay + closedPortDelay},
+		{[]string{slowDst4, slowDst6}, []string{"::1", "127.0.0.1"}, "tcp6", true, fallbackDelay},
 		// Primary refused, fallback without delay.
-		{[]string{"127.0.0.1"}, []string{"::1"}, "tcp4", true, closedPortOrFallbackDelay},
+		{[]string{"127.0.0.1"}, []string{"::1"}, "tcp4", true, instant},
-		{[]string{"::1"}, []string{"127.0.0.1"}, "tcp6", true, closedPortOrFallbackDelay},
+		{[]string{"::1"}, []string{"127.0.0.1"}, "tcp6", true, instant},
 		// Everything is refused.
-		{[]string{"127.0.0.1"}, []string{}, "tcp4", false, closedPortDelay},
+		{[]string{"127.0.0.1"}, []string{}, "tcp4", false, instant},
 		// Nothing to do; fail instantly.
 		{[]string{}, []string{}, "", false, instant},
 		// Connecting to tons of addresses should not trip the deadline.
 		{nCopies("::1", 1000), []string{}, "", true, instant},
 	}
 
-	handler := func(dss *dualStackServer, ln Listener) {
-		for {
-			c, err := ln.Accept()
-			if err != nil {
-				return
-			}
-			c.Close()
-		}
-	}
-
 	// Convert a list of IP strings into TCPAddrs.
 	makeAddrs := func(ips []string, port string) addrList {
 		var out addrList
@@ -262,76 +232,74 @@ func TestDialParallel(t *testing.T) {
 	}
 
 	for i, tt := range testCases {
-		dss, err := newDualStackServer()
+		i, tt := i, tt
-		if err != nil {
+		t.Run(fmt.Sprint(i), func(t *testing.T) {
-			t.Fatal(err)
+			origTestHookDialTCP := testHookDialTCP
-		}
+			defer func() { testHookDialTCP = origTestHookDialTCP }()
-		defer dss.teardown()
+			testHookDialTCP = func(ctx context.Context, network string, laddr, raddr *TCPAddr) (*TCPConn, error) {
-		if err := dss.buildup(handler); err != nil {
+				n := "tcp6"
-			t.Fatal(err)
+				if raddr.IP.To4() != nil {
-		}
+					n = "tcp4"
-		if tt.teardownNetwork != "" {
+				}
-			// Destroy one of the listening sockets, creating an unreachable port.
+				if n == tt.teardownNetwork {
-			dss.teardownNetwork(tt.teardownNetwork)
+					return nil, errors.New("unreachable")
-		}
+				}
+				if r := raddr.IP.String(); r == slowDst4 || r == slowDst6 {
+					<-ctx.Done()
+					return nil, ctx.Err()
+				}
+				return &TCPConn{}, nil
+			}
 
-		primaries := makeAddrs(tt.primaries, dss.port)
+			primaries := makeAddrs(tt.primaries, "80")
-		fallbacks := makeAddrs(tt.fallbacks, dss.port)
+			fallbacks := makeAddrs(tt.fallbacks, "80")
-		d := Dialer{
+			d := Dialer{
-			FallbackDelay: fallbackDelay,
+				FallbackDelay: fallbackDelay,
-		}
+			}
-		startTime := time.Now()
+			const forever = 60 * time.Minute
-		sd := &sysDialer{
+			if tt.expectElapsed == instant {
-			Dialer:  d,
+				d.FallbackDelay = forever
-			network: "tcp",
+			}
-			address: "?",
+			startTime := time.Now()
-		}
+			sd := &sysDialer{
-		c, err := sd.dialParallel(context.Background(), primaries, fallbacks)
+				Dialer:  d,
-		elapsed := time.Since(startTime)
+				network: "tcp",
+				address: "?",
+			}
+			c, err := sd.dialParallel(context.Background(), primaries, fallbacks)
+			elapsed := time.Since(startTime)
 
-		if c != nil {
+			if c != nil {
-			c.Close()
+				c.Close()
-		}
+			}
 
-		if tt.expectOk && err != nil {
+			if tt.expectOk && err != nil {
-			t.Errorf("#%d: got %v; want nil", i, err)
+				t.Errorf("#%d: got %v; want nil", i, err)
-		} else if !tt.expectOk && err == nil {
+			} else if !tt.expectOk && err == nil {
-			t.Errorf("#%d: got nil; want non-nil", i)
+				t.Errorf("#%d: got nil; want non-nil", i)
-		}
+			}
 
-		// We used to always use 95 milliseconds as the slop,
+			if elapsed < tt.expectElapsed || elapsed >= forever {
-		// but that was flaky on Windows.  See issue 35616.
+				t.Errorf("#%d: got %v; want >= %v, < forever", i, elapsed, tt.expectElapsed)
-		slop := 95 * time.Millisecond
+			}
-		if half := tt.expectElapsed / 2; half > slop {
-			slop = half
-		}
-		expectElapsedMin := tt.expectElapsed - slop
-		expectElapsedMax := tt.expectElapsed + slop
-		if elapsed < expectElapsedMin {
-			t.Errorf("#%d: got %v; want >= %v", i, elapsed, expectElapsedMin)
-		} else if elapsed > expectElapsedMax {
-			t.Errorf("#%d: got %v; want <= %v", i, elapsed, expectElapsedMax)
-		}
 
-		// Repeat each case, ensuring that it can be canceled quickly.
+			// Repeat each case, ensuring that it can be canceled.
-		ctx, cancel := context.WithCancel(context.Background())
+			ctx, cancel := context.WithCancel(context.Background())
-		var wg sync.WaitGroup
+			var wg sync.WaitGroup
-		wg.Add(1)
+			wg.Add(1)
-		go func() {
+			go func() {
-			time.Sleep(5 * time.Millisecond)
+				time.Sleep(5 * time.Millisecond)
-			cancel()
+				cancel()
-			wg.Done()
+				wg.Done()
-		}()
+			}()
-		startTime = time.Now()
+			// Ignore errors, since all we care about is that the
-		c, err = sd.dialParallel(ctx, primaries, fallbacks)
+			// call can be canceled.
-		if c != nil {
+			c, _ = sd.dialParallel(ctx, primaries, fallbacks)
-			c.Close()
+			if c != nil {
-		}
+				c.Close()
-		elapsed = time.Now().Sub(startTime)
+			}
-		if elapsed > 100*time.Millisecond {
+			wg.Wait()
-			t.Errorf("#%d (cancel): got %v; want <= 100ms", i, elapsed)
+		})
-		}
-		wg.Wait()
 	}
 }