From 5a43a452327b0571320b4a895b4f87833c6b94e3 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 28 Jan 2022 18:27:31 +0100 Subject: [PATCH] deflake TestRoundTripSocks5AndNewConnection unit test The test was asserting the error from the proxy server, to avoid false positives it was using a channel to communicate that the test has been ended and the error was legit. However, the channel was executed using a defer statement, and the connection on the server could be closed accidentally by the connection request, that was done later and used another defer statement to close it. Since the connection defer was done later, it was executed before the channel close. --- .../pkg/util/httpstream/spdy/roundtripper_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go index 3a326a9b247..779d03b9679 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go @@ -438,7 +438,6 @@ func (i *Interceptor) Rewrite(ctx context.Context, req *socks5.Request) (context // be sure to unset environment variable https_proxy (if exported) before testing, otherwise the testing will fail unexpectedly. func TestRoundTripSocks5AndNewConnection(t *testing.T) { - t.Skip("Flake https://issues.k8s.io/107708") localhostPool := localhostCertPool(t) for _, redirect := range []bool{false, true} { @@ -568,10 +567,7 @@ func TestRoundTripSocks5AndNewConnection(t *testing.T) { if err != nil { t.Fatalf("socks5Server: proxy_test: Listen: %v", err) } - defer func() { - close(closed) - l.Close() - }() + defer l.Close() go func(shoulderror bool) { conn, err := l.Accept() @@ -584,6 +580,10 @@ func TestRoundTripSocks5AndNewConnection(t *testing.T) { } if err := proxyHandler.ServeConn(conn); err != nil && !shoulderror { + // If the connection request is closed before the channel is closed + // the test will fail with a ServeConn error. Since the test only return + // early if expects shouldError=true, the channel is closed at the end of + // the test, just before all the deferred connections Close() are executed. if isClosed() { return } @@ -684,6 +684,9 @@ func TestRoundTripSocks5AndNewConnection(t *testing.T) { t.Fatalf("unexpected proxy user: %v", authUser) } } + + // The channel must be closed before any of the connections are closed + close(closed) }) } })