Bump Go version to v1.22

.github/workflows/ci.yaml

@@ -15,14 +15,14 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: [1.19.x, 1.20.x, 1.21.x]
+        go-version: [1.20.x, 1.21.x, 1.22.x]
     steps:
       - name: Checkout Code
         uses: actions/checkout@v4
         with:
           fetch-depth: 1
       - name: Install Go
-        uses: actions/setup-go@v4
+        uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go-version }}
       - name: Test
@@ -32,5 +32,5 @@ jobs:
         # conflicting guidance, run only on the most recent supported version.
         # For the same reason, only check generated code on the most recent
         # supported version.
-        if: matrix.go-version == '1.21.x'
+        if: matrix.go-version == '1.22.x'
         run: make checkgenerate && make lint

.golangci.yml

@@ -39,6 +39,7 @@ linters:
     - golint            # deprecated by Go team
     - gomnd             # some unnamed constants are okay
     - ifshort           # deprecated by author
+    - inamedparam       # convention is not followed
     - interfacer        # deprecated by author
     - ireturn           # "accept interfaces, return structs" isn't ironclad
     - lll               # don't want hard limits for line length

Makefile

@@ -67,11 +67,11 @@ checkgenerate:
 
 $(BIN)/license-header: Makefile
 	@mkdir -p $(@D)
-	go install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.26.1
+	go install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.29.0
 
 $(BIN)/golangci-lint: Makefile
 	@mkdir -p $(@D)
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.1
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56.2
 
 $(BIN)/checklocks: Makefile
 	@mkdir -p $(@D)

attribute/attribute_test.go

@@ -55,5 +55,6 @@ func TestAttributeKeysUniquePointers(t *testing.T) {
 	// were inadvertently defined as an empty struct, then
 	// NewKey would always return the same pointer. This
 	// guards against such a mistake.)
+	//nolint:testifylint // avoid false positive complaint about useless-assert
 	assert.NotSame(t, NewKey[string](), NewKey[string]())
 }

balancer_test.go

@@ -123,7 +123,7 @@ func TestConnManager(t *testing.T) {
 	require.Equal(t, newAddrs, latestUpdate.newAddrs)
 	require.Equal(t, []conn.Conn{conn4, conn5, conn6}, latestUpdate.removeConns)
 	conns = pool.SnapshotConns()
-	require.Equal(t, 10, len(conns))
+	require.Len(t, conns, 10)
 	conn1i7 := balancertesting.FindConn(conns, resolver.Address{HostPort: "1.2.3.1"}, 7)
 	conn1i8 := balancertesting.FindConn(conns, resolver.Address{HostPort: "1.2.3.1"}, 8)
 	conn1i9 := balancertesting.FindConn(conns, resolver.Address{HostPort: "1.2.3.1"}, 9)
@@ -154,7 +154,7 @@ func TestConnManager(t *testing.T) {
 	require.Empty(t, latestUpdate.newAddrs)
 	require.Equal(t, []conn.Conn{conn1i8, conn1i9, conn2i11, conn3i13}, latestUpdate.removeConns)
 	conns = pool.SnapshotConns()
-	require.Equal(t, 6, len(conns))
+	require.Len(t, conns, 6)
 	// make sure attributes on existing connections were updated to latest
 	// values from resolver
 	require.Equal(t, attrs1a, conn1.Address().Attributes)

client.go

@@ -337,7 +337,7 @@ type RedirectFunc func(req *http.Request, via []*http.Request) error
 // up to the given number of redirects. If a request sequence results in more
 // redirects than the given limit, the request will fail.
 func FollowRedirects(limit int) RedirectFunc {
-	return func(req *http.Request, via []*http.Request) error {
+	return func(_ *http.Request, via []*http.Request) error {
 		if len(via) > limit {
 			return fmt.Errorf("too many redirects (> %d)", limit)
 		}
@@ -382,7 +382,7 @@ func (opts *clientOptions) applyDefaults() {
 		opts.schemes = map[string]Transport{}
 	}
 	if opts.redirectFunc == nil {
-		opts.redirectFunc = func(req *http.Request, via []*http.Request) error {
+		opts.redirectFunc = func(_ *http.Request, _ []*http.Request) error {
 			return http.ErrUseLastResponse
 		}
 	}

client_test.go

@@ -44,7 +44,7 @@ func TestNewClient_Basic(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("got it"))
 	}))
 	clientHTTP := makeClient(t, ctx,
@@ -66,13 +66,13 @@ func TestNewClient_MultipleTargets(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr1 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr1 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("tweedle dee"))
 	}))
-	addr2 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr2 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("tweedle dum"))
 	}))
-	addr3 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr3 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("twinkle twinkle little bat")) //nolint:dupword //intentional!
 	}))
 	client := makeClient(t, ctx)
@@ -126,15 +126,15 @@ func TestNewClient_LoadBalancing(t *testing.T) {
 
 	ctx := context.Background()
 	var counters [3]atomic.Int32
-	addr1 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr1 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		counters[0].Add(1)
 		_, _ = w.Write(([]byte)("got it!"))
 	}))
-	addr2 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr2 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		counters[1].Add(1)
 		_, _ = w.Write(([]byte)("got it!"))
 	}))
-	addr3 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr3 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		counters[2].Add(1)
 		_, _ = w.Write(([]byte)("got it!"))
 	}))
@@ -180,9 +180,9 @@ func TestNewClient_TransportConfig(t *testing.T) {
 		}
 		_, _ = w.Write(([]byte)("got it"))
 	})
-	addr1 := startServer(t, ctx, handler)
-	addr2 := startServer(t, ctx, handler)
-	addr3 := startServer(t, ctx, handler)
+	addr1 := startServer(t, handler)
+	addr2 := startServer(t, handler)
+	addr3 := startServer(t, handler)
 	var dialCount atomic.Int32
 	tlsConf := &tls.Config{ServerName: "example.com"} //nolint:gosec
 	transportOption := WithTransport("http", transportFunc(func(scheme, target string, options TransportConfig) RoundTripperResult {
@@ -294,9 +294,9 @@ func TestNewClient_CustomTransport(t *testing.T) {
 
 	ctx := context.Background()
 	client := makeClient(t, ctx,
-		WithTransport("foo", transportFunc(func(scheme, target string, options TransportConfig) RoundTripperResult {
+		WithTransport("foo", transportFunc(func(_, _ string, _ TransportConfig) RoundTripperResult {
 			return RoundTripperResult{
-				RoundTripper: roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+				RoundTripper: roundTripperFunc(func(_ *http.Request) (*http.Response, error) {
 					recorder := httptest.NewRecorder()
 					_, _ = recorder.WriteString("foo bar")
 					return recorder.Result(), nil
@@ -312,10 +312,10 @@ func TestNewClient_CloseIdleTransports(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr1 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr1 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("got it"))
 	}))
-	addr2 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr2 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("got it"))
 	}))
 	client := makeClient(t, ctx,
@@ -370,7 +370,7 @@ func TestNewClient_Timeouts(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// optional request body indicates number of milliseconds to delay before returning
 		body, err := io.ReadAll(r.Body)
 		if err != nil {
@@ -434,10 +434,10 @@ func TestNewClient_Proxy(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("got it"))
 	}))
-	proxyAddr, proxyCounter := startProxy(t, ctx)
+	proxyAddr, proxyCounter := startProxy(t)
 
 	res := fakeResolver{map[string][]string{
 		"foo.com": {addr},
@@ -481,7 +481,7 @@ func TestNewClient_TLS(t *testing.T) {
 	require.NoError(t, err, "loading localhost cert failed")
 
 	ctx := context.Background()
-	server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("success"))
 	}))
 	server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} //nolint:gosec
@@ -516,10 +516,10 @@ func TestNewClient_DisallowUnconfiguredTarget(t *testing.T) {
 	ensureGoroutinesCleanedUp(t)
 
 	ctx := context.Background()
-	addr1 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr1 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("got it"))
 	}))
-	addr2 := startServer(t, ctx, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	addr2 := startServer(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write(([]byte)("don't got it"))
 	}))
 	client := makeClient(t, ctx, WithAllowBackendTarget("http", addr1))
@@ -533,7 +533,9 @@ func TestNewClient_DisallowUnconfiguredTarget(t *testing.T) {
 func sendGetRequest(t *testing.T, ctx context.Context, client *Client, url string, expectations func(*testing.T, *http.Response, error)) {
 	t.Helper()
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
-	require.NoError(t, err)
+	if !assert.NoError(t, err) { //nolint:testifylint
+		return
+	}
 	resp, err := client.Do(req)
 	expectations(t, resp, err)
 }
@@ -547,16 +549,23 @@ func sendPostRequest(t *testing.T, ctx context.Context, client *Client, url stri
 	expectations(t, resp, err)
 }
 
+//nolint:testifylint // must use assert for concurrent calls
 func expectSuccess(contents string) func(*testing.T, *http.Response, error) {
 	return func(t *testing.T, resp *http.Response, err error) {
 		t.Helper()
-		require.NoError(t, err)
+		if !assert.NoError(t, err) {
+			return
+		}
 		body, err := io.ReadAll(resp.Body)
-		require.NoError(t, err)
+		if !assert.NoError(t, err) {
+			return
+		}
 		err = resp.Body.Close()
-		require.NoError(t, err)
-		require.Equal(t, http.StatusOK, resp.StatusCode)
-		require.Equal(t, contents, string(body))
+		if !assert.NoError(t, err) {
+			return
+		}
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+		assert.Equal(t, contents, string(body))
 	}
 }
 
@@ -568,9 +577,11 @@ func expectError(message string) func(*testing.T, *http.Response, error) {
 		}
 		// if no error, we must drain and close body
 		_, err = io.ReadAll(resp.Body)
-		require.NoError(t, err)
+		if !assert.NoError(t, err) { //nolint:testifylint
+			return
+		}
 		err = resp.Body.Close()
-		require.NoError(t, err)
+		assert.NoError(t, err)
 	}
 }
 
@@ -588,33 +599,22 @@ func expectRedirect(location string) func(*testing.T, *http.Response, error) {
 	}
 }
 
-//nolint:revive // linter wants ctx first, but t first is okay
-func startServer(t *testing.T, ctx context.Context, handler http.Handler) string {
+func startServer(t *testing.T, handler http.Handler) string {
 	t.Helper()
 
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	require.NoError(t, err)
-	svr := http.Server{
+	svr := httptest.NewUnstartedServer(nil)
+	svr.Config = &http.Server{
 		Handler:           h2c.NewHandler(handler, &http2.Server{}),
 		ReadHeaderTimeout: 5 * time.Second,
 	}
-	go func() {
-		err := svr.Serve(listener)
-		require.Equal(t, http.ErrServerClosed, err)
-	}()
-	t.Cleanup(func() {
-		shutdownCtx, shutdownCancel := context.WithTimeout(ctx, 5*time.Second)
-		defer shutdownCancel()
-		err := svr.Shutdown(shutdownCtx)
-		require.NoError(t, err)
-	})
-	t.Logf("Number of goroutines after server %s started: %d", listener.Addr().String(), runtime.NumGoroutine())
+	svr.Start()
+	t.Cleanup(svr.Close)
+	t.Logf("Number of goroutines after server %s started: %d", svr.Listener.Addr().String(), runtime.NumGoroutine())
 
-	return listener.Addr().String()
+	return svr.Listener.Addr().String()
 }
 
-//nolint:revive // linter wants ctx first, but t first is okay
-func startProxy(t *testing.T, ctx context.Context) (string, *atomic.Int32) {
+func startProxy(t *testing.T) (string, *atomic.Int32) {
 	t.Helper()
 
 	var count atomic.Int32
@@ -666,7 +666,7 @@ func startProxy(t *testing.T, ctx context.Context) (string, *atomic.Int32) {
 		w.WriteHeader(resp.StatusCode)
 		_, _ = io.Copy(w, resp.Body)
 	})
-	return startServer(t, ctx, handler), &count
+	return startServer(t, handler), &count
 }
 
 func ensureGoroutinesCleanedUp(t *testing.T) {

go.mod

@@ -1,6 +1,6 @@
 module github.com/bufbuild/httplb
 
-go 1.19
+go 1.20
 
 require (
 	github.com/jonboulle/clockwork v0.4.0

health/polling_test.go

@@ -87,7 +87,7 @@ func TestPollingCheckerThresholds(t *testing.T) {
 		select {
 		case connection <- response:
 			err := testClock.BlockUntilContext(ctx, 1)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			testClock.Advance(interval)
 		case <-tracker:
 			t.Fatal("unexpected health state update")

picker/leastloaded_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/bufbuild/httplb/conn"
 	"github.com/bufbuild/httplb/picker"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLeastLoadedRoundRobinPicker(t *testing.T) {
@@ -32,14 +33,14 @@ func TestLeastLoadedRoundRobinPicker(t *testing.T) {
 	dummyConn := conn.Conn(nil)
 	pick := picker.NewLeastLoadedRoundRobin(nil, dummyConns{[]conn.Conn{dummyConn}})
 	connection, _, err := pick.Pick(&http.Request{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, dummyConn, connection)
 
 	// TODO: test (whitebox?) that state is retained between pickers
 
 	pick = picker.NewLeastLoadedRoundRobin(pick, dummyConns{[]conn.Conn{dummyConn}})
 	connection, _, err = pick.Pick(&http.Request{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, dummyConn, connection)
 }
 
@@ -51,13 +52,13 @@ func TestLeastLoadedRandomPicker(t *testing.T) {
 	dummyConn := conn.Conn(nil)
 	pick := picker.NewLeastLoadedRandom(nil, dummyConns{[]conn.Conn{dummyConn}})
 	connection, _, err := pick.Pick(&http.Request{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, dummyConn, connection)
 
 	// TODO: test (whitebox?) that state is retained between pickers
 
 	pick = picker.NewLeastLoadedRandom(pick, dummyConns{[]conn.Conn{dummyConn}})
 	connection, _, err = pick.Pick(&http.Request{})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, dummyConn, connection)
 }