From: Bryan C. Mills <bcmills@google.com>
Date: Mon, 11 Dec 2023 21:45:04 +0000 (-0500)
Subject: net: unskip and attempt to deflake TestAcceptTimeout
X-Git-Tag: go1.23rc1~1404
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=1a07550962a9f0123284a0efbe11c486fb1a452e;p=gostls13.git

net: unskip and attempt to deflake TestAcceptTimeout

This test has been unconditionally skipped for over five years.
It may be that whatever was causing it to flake has been fixed.
And if it hasn't been fixed, it isn't providing any value.
Let's unskip it for the Go 1.23 development cycle and see what happens.

Let's also use a separate listener for each test case, so that a
leaked Dial goroutine from one case won't interfere with the other.

Fixes #17948 (maybe).

Change-Id: I239f22ca5d5a44388b9aa0ed4d81e451c6342617
Reviewed-on: https://go-review.googlesource.com/c/go/+/548940
Commit-Queue: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---

diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go
index ca86f31ef2..3a903f8f73 100644
--- a/src/net/timeout_test.go
+++ b/src/net/timeout_test.go
@@ -7,7 +7,6 @@ package net
 import (
 	"errors"
 	"fmt"
-	"internal/testenv"
 	"io"
 	"os"
 	"runtime"
@@ -166,19 +165,7 @@ func TestDialTimeoutMaxDuration(t *testing.T) {
 	}
 }
 
-var acceptTimeoutTests = []struct {
-	timeout time.Duration
-	xerrs   [2]error // expected errors in transition
-}{
-	// Tests that accept deadlines in the past work, even if
-	// there's incoming connections available.
-	{-5 * time.Second, [2]error{os.ErrDeadlineExceeded, os.ErrDeadlineExceeded}},
-
-	{50 * time.Millisecond, [2]error{nil, os.ErrDeadlineExceeded}},
-}
-
 func TestAcceptTimeout(t *testing.T) {
-	testenv.SkipFlaky(t, 17948)
 	t.Parallel()
 
 	switch runtime.GOOS {
@@ -186,49 +173,62 @@ func TestAcceptTimeout(t *testing.T) {
 		t.Skipf("not supported on %s", runtime.GOOS)
 	}
 
-	ln := newLocalListener(t, "tcp")
-	defer ln.Close()
+	timeouts := []time.Duration{
+		-5 * time.Second,
+		10 * time.Millisecond,
+	}
 
-	var wg sync.WaitGroup
-	for i, tt := range acceptTimeoutTests {
-		if tt.timeout < 0 {
-			wg.Add(1)
-			go func() {
-				defer wg.Done()
-				d := Dialer{Timeout: 100 * time.Millisecond}
-				c, err := d.Dial(ln.Addr().Network(), ln.Addr().String())
-				if err != nil {
-					t.Error(err)
-					return
-				}
-				c.Close()
-			}()
-		}
+	for _, timeout := range timeouts {
+		timeout := timeout
+		t.Run(fmt.Sprintf("%v", timeout), func(t *testing.T) {
+			t.Parallel()
 
-		if err := ln.(*TCPListener).SetDeadline(time.Now().Add(tt.timeout)); err != nil {
-			t.Fatalf("$%d: %v", i, err)
-		}
-		for j, xerr := range tt.xerrs {
-			for {
-				c, err := ln.Accept()
-				if xerr != nil {
-					if perr := parseAcceptError(err); perr != nil {
-						t.Errorf("#%d/%d: %v", i, j, perr)
-					}
-					if !isDeadlineExceeded(err) {
-						t.Fatalf("#%d/%d: %v", i, j, err)
+			ln := newLocalListener(t, "tcp")
+			defer ln.Close()
+
+			if timeout >= 0 {
+				// Don't dial the listener at all, so that Accept will hang.
+			} else {
+				// A deadline in the past should cause Accept to fail even if there are
+				// incoming connections available. Try to make one available before the
+				// call to Accept happens. (It's ok if the timing doesn't always work
+				// out that way, though: the test should pass regardless.)
+				dialDone := make(chan struct{})
+				t.Cleanup(func() { <-dialDone })
+
+				go func() {
+					defer close(dialDone)
+					d := Dialer{}
+					c, err := d.Dial(ln.Addr().Network(), ln.Addr().String())
+					if err != nil {
+						t.Error(err)
+						return
 					}
-				}
-				if err == nil {
 					c.Close()
-					time.Sleep(10 * time.Millisecond)
-					continue
-				}
-				break
+				}()
+
+				time.Sleep(10 * time.Millisecond)
 			}
-		}
+
+			if err := ln.(*TCPListener).SetDeadline(time.Now().Add(timeout)); err != nil {
+				t.Fatal(err)
+			}
+			t.Logf("ln.SetDeadline(time.Now().Add(%v))", timeout)
+
+			c, err := ln.Accept()
+			if err == nil {
+				c.Close()
+			}
+			t.Logf("ln.Accept: %v", err)
+
+			if perr := parseAcceptError(err); perr != nil {
+				t.Error(perr)
+			}
+			if !isDeadlineExceeded(err) {
+				t.Error("wanted deadline exceeded")
+			}
+		})
 	}
-	wg.Wait()
 }
 
 func TestAcceptTimeoutMustReturn(t *testing.T) {