Merge branch 'issue-6072-part-2' into issue-7363

Author: atollena

internal/testutils/blocking_context_dialer.go

@@ -31,9 +31,11 @@ var logger = grpclog.Component("testutils")
 // BlockingDialer is a dialer that waits for Resume() to be called before
 // dialing.
 type BlockingDialer struct {
-	mu    sync.Mutex // protects holds
+	// mu protects holds.
+	mu sync.Mutex
+	// holds maps network addresses to a list of holds for that address.
 	holds map[string][]*Hold
-
+	// dialer dials connections when they are not blocked.
 	dialer *net.Dialer
 }
 
@@ -77,11 +79,16 @@ func (d *BlockingDialer) DialContext(ctx context.Context, addr string) (net.Conn
 // Hold is a handle to a single connection attempt. It can be used to block,
 // fail and succeed connection attempts.
 type Hold struct {
-	dialer  *BlockingDialer
+	// dialer is the dialer that created this hold.
+	dialer *BlockingDialer
+	// waitCh is closed when a connection attempt is received.
+	waitCh chan struct{}
+	// blockCh is closed when the connection attempt should resume.
 	blockCh chan error
-	waitCh  chan struct{}
-	err     error
-	addr    string
+	// err is the error to return when the connection attempt is failed.
+	err error
+	// addr is the address that this hold is for.
+	addr string
 }
 
 // Hold blocks the dialer when a connection attempt is made to the given addr.

internal/testutils/blocking_context_dialer_test.go

@@ -25,12 +25,15 @@ import (
 	"time"
 )
 
-const testTimeout = 5 * time.Second
+const (
+	testTimeout      = 5 * time.Second
+	testShortTimeout = 10 * time.Millisecond
+)
 
 func (s) TestBlockingDialer_NoHold(t *testing.T) {
 	lis, err := LocalTCPListener()
 	if err != nil {
-		t.Fatalf("failed to listen: %v", err)
+		t.Fatalf("Failed to listen: %v", err)
 	}
 	defer lis.Close()
 
@@ -88,8 +91,8 @@ func (s) TestBlockingDialer_HoldWaitResume(t *testing.T) {
 
 	select {
 	case <-done:
-		t.Errorf("Expected dialer to be blocked.")
-	default:
+		t.Fatalf("Expected dialer to be blocked.")
+	case <-time.After(testShortTimeout):
 	}
 
 	h.Resume() // Unblock the above goroutine.
@@ -104,7 +107,7 @@ func (s) TestBlockingDialer_HoldWaitResume(t *testing.T) {
 func (s) TestBlockingDialer_HoldWaitFail(t *testing.T) {
 	lis, err := LocalTCPListener()
 	if err != nil {
-		t.Fatalf("failed to listen: %v", err)
+		t.Fatalf("Failed to listen: %v", err)
 	}
 	defer lis.Close()
 
@@ -113,30 +116,30 @@ func (s) TestBlockingDialer_HoldWaitFail(t *testing.T) {
 
 	wantErr := errors.New("test error")
 
-	done := make(chan struct{})
+	dialError := make(chan error)
 	ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
 	defer cancel()
 	go func() {
 		_, err := d.DialContext(ctx, lis.Addr().String())
-		if !errors.Is(err, wantErr) {
-			t.Errorf("BlockingDialer.DialContext() after Fail(): got error %v, want %v", err, wantErr)
-		}
-		done <- struct{}{}
+		dialError <- err
 	}()
 
 	if !h.Wait(ctx) {
 		t.Fatalf("Timeout while waiting for a connection attempt to " + h.addr)
 	}
 	select {
-	case <-done:
-		t.Errorf("Expected dialer to still be blocked after Wait()")
-	default:
+	case err = <-dialError:
+		t.Errorf("DialContext got unblocked with err %v. Want DialContext to still be blocked after Wait()", err)
+	case <-time.After(testShortTimeout):
 	}
 
 	h.Fail(wantErr)
 
 	select {
-	case <-done:
+	case err = <-dialError:
+		if !errors.Is(err, wantErr) {
+			t.Errorf("BlockingDialer.DialContext() after Fail(): got error %v, want %v", err, wantErr)
+		}
 	case <-ctx.Done():
 		t.Errorf("Timeout waiting for connection attempt to fail.")
 	}
@@ -145,25 +148,22 @@ func (s) TestBlockingDialer_HoldWaitFail(t *testing.T) {
 func (s) TestBlockingDialer_ContextCanceled(t *testing.T) {
 	lis, err := LocalTCPListener()
 	if err != nil {
-		t.Fatalf("failed to listen: %v", err)
+		t.Fatalf("Failed to listen: %v", err)
 	}
 	defer lis.Close()
 
 	d := NewBlockingDialer()
 	h := d.Hold(lis.Addr().String())
 
-	done := make(chan struct{})
+	dialErr := make(chan error)
 	testCtx, cancel := context.WithTimeout(context.Background(), testTimeout)
 	defer cancel()
 
 	ctx, cancel := context.WithCancel(testCtx)
 	defer cancel()
 	go func() {
 		_, err := d.DialContext(ctx, lis.Addr().String())
-		if !errors.Is(err, context.Canceled) {
-			t.Errorf("BlockingDialer.DialContext() after context cancel: got error %v, want %v", err, context.Canceled)
-		}
-		done <- struct{}{}
+		dialErr <- err
 	}()
 	if !h.Wait(testCtx) {
 		t.Errorf("Timeout while waiting for a connection attempt to %q", h.addr)
@@ -172,7 +172,10 @@ func (s) TestBlockingDialer_ContextCanceled(t *testing.T) {
 	cancel()
 
 	select {
-	case <-done:
+	case err = <-dialErr:
+		if !errors.Is(err, context.Canceled) {
+			t.Errorf("BlockingDialer.DialContext() after context cancel: got error %v, want %v", err, context.Canceled)
+		}
 	case <-testCtx.Done():
 		t.Errorf("Timeout while waiting for Wait to return.")
 	}
@@ -183,7 +186,7 @@ func (s) TestBlockingDialer_ContextCanceled(t *testing.T) {
 func (s) TestBlockingDialer_CancelWait(t *testing.T) {
 	lis, err := LocalTCPListener()
 	if err != nil {
-		t.Fatalf("failed to listen: %v", err)
+		t.Fatalf("Failed to listen: %v", err)
 	}
 	defer lis.Close()