diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-08-11 14:21:15 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-09-23 10:02:55 +0300 |
| commit | 72b71fc5579eb9930566bcfc2df9f0dbe13e25b6 (patch) | |
| tree | 7209ca04fc8d9ca619e8bf73bf5bd8e7a6503bcf | |
| parent | 5a514bc991f1b1c727db0bffa0d7f2308d28566e (diff) | |
praefect: Fix missing error checks for factory's `Serve()` function
Many of our tests don't properly verify the error returned by the server
factory's `Serve()` function. Fix this shortcoming, which surfaces that
we mistakenly close some listeners manually that should indeed be closed
by gRPC itself. Drop the corresponding linting exception.
| -rw-r--r-- | .golangci.yml | 3 | ||||
| -rw-r--r-- | internal/praefect/server_factory_test.go | 15 |
2 files changed, 5 insertions, 13 deletions
diff --git a/.golangci.yml b/.golangci.yml index 70546143e..17cad66ec 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -52,7 +52,6 @@ linters-settings: # cases to skip the checks, but rather as a list of things we should # eventually fix. exclude-functions: - # Close - (*database/sql.DB).Close - (*database/sql.Rows).Close - (*gitlab.com/gitlab-org/gitaly/v15/client.Pool).Close @@ -68,8 +67,6 @@ linters-settings: - (io/fs.File).Close - (net.Conn).Close - (net.Listener).Close - # Serve - - (*gitlab.com/gitlab-org/gitaly/v15/internal/praefect.ServerFactory).Serve forbidigo: forbid: # Tests and code which use timing-based setups have repeatedly resulted diff --git a/internal/praefect/server_factory_test.go b/internal/praefect/server_factory_test.go index 598197311..bd3147d02 100644 --- a/internal/praefect/server_factory_test.go +++ b/internal/praefect/server_factory_test.go @@ -184,9 +184,8 @@ func TestServerFactory(t *testing.T) { listener, err := net.Listen(starter.TCP, "localhost:0") require.NoError(t, err) - defer func() { require.NoError(t, listener.Close()) }() - go praefectServerFactory.Serve(listener, false) + go func() { require.NoError(t, praefectServerFactory.Serve(listener, false)) }() praefectAddr, err := starter.ComposeEndpoint(listener.Addr().Network(), listener.Addr().String()) require.NoError(t, err) @@ -217,9 +216,8 @@ func TestServerFactory(t *testing.T) { listener, err := net.Listen(starter.TCP, "localhost:0") require.NoError(t, err) - defer func() { require.NoError(t, listener.Close()) }() - go praefectServerFactory.Serve(listener, true) + go func() { require.NoError(t, praefectServerFactory.Serve(listener, true)) }() ctx := testhelper.Context(t) certPool, err := x509.SystemCertPool() @@ -260,9 +258,8 @@ func TestServerFactory(t *testing.T) { // start with tcp address tcpListener, err := net.Listen(starter.TCP, "localhost:0") require.NoError(t, err) - defer tcpListener.Close() - go praefectServerFactory.Serve(tcpListener, false) + go func() { require.NoError(t, praefectServerFactory.Serve(tcpListener, false)) }() praefectTCPAddr, err := starter.ComposeEndpoint(tcpListener.Addr().Network(), tcpListener.Addr().String()) require.NoError(t, err) @@ -276,9 +273,8 @@ func TestServerFactory(t *testing.T) { // start with tls address tlsListener, err := net.Listen(starter.TCP, "localhost:0") require.NoError(t, err) - defer tlsListener.Close() - go praefectServerFactory.Serve(tlsListener, true) + go func() { require.NoError(t, praefectServerFactory.Serve(tlsListener, true)) }() praefectTLSAddr, err := starter.ComposeEndpoint(tcpListener.Addr().Network(), tcpListener.Addr().String()) require.NoError(t, err) @@ -294,9 +290,8 @@ func TestServerFactory(t *testing.T) { defer func() { require.NoError(t, os.RemoveAll(socketPath)) }() socketListener, err := net.Listen(starter.Unix, socketPath) require.NoError(t, err) - defer socketListener.Close() - go praefectServerFactory.Serve(socketListener, false) + go func() { require.NoError(t, praefectServerFactory.Serve(socketListener, false)) }() praefectSocketAddr, err := starter.ComposeEndpoint(socketListener.Addr().Network(), socketListener.Addr().String()) require.NoError(t, err) |