From 1ef796630633c70bbc62f2c302a6dfe1dda91697 Mon Sep 17 00:00:00 2001 From: albertteoh Date: Mon, 14 Sep 2020 15:37:59 +1000 Subject: [PATCH 1/5] Fix failing ServerInUseHostPort test Signed-off-by: albertteoh --- cmd/query/app/server.go | 2 +- cmd/query/app/server_test.go | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 1fa96f3c2c5..d44cfdaf367 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -74,7 +74,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *Que tracer: tracer, grpcServer: grpcServer, httpServer: createHTTPServer(querySvc, options, tracer, logger), - separatePorts: (grpcPort != httpPort), + separatePorts: grpcPort != httpPort, unavailableChannel: make(chan healthcheck.Status), }, nil } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index ca2473acdc0..5c20fdd2eb5 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -17,6 +17,7 @@ package app import ( "context" "net" + "os" "sync" "testing" "time" @@ -26,6 +27,7 @@ import ( "github.com/stretchr/testify/mock" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" + "golang.org/x/sys/unix" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -74,7 +76,7 @@ func TestServerBadHostPort(t *testing.T) { func TestServerInUseHostPort(t *testing.T) { - for _, hostPort := range [2]string{":8080", ":8081"} { + for _, hostPort := range [2]string{"127.0.0.1:8080", "127.0.0.1:8081"} { conn, err := net.Listen("tcp", hostPort) assert.NoError(t, err) @@ -84,7 +86,10 @@ func TestServerInUseHostPort(t *testing.T) { assert.NoError(t, err) err = server.Start() - assert.NotNil(t, err) + syscallErr, ok := err.(*net.OpError).Err.(*os.SyscallError) + assert.True(t, ok) + assert.Equal(t, syscallErr.Err, unix.EADDRINUSE) + conn.Close() if server.grpcConn != nil { server.grpcConn.Close() From 501a80d8a6c35982e26ac5085e9c4733f22c0ea4 Mon Sep 17 00:00:00 2001 From: albertteoh Date: Tue, 15 Sep 2020 15:38:35 +1000 Subject: [PATCH 2/5] Dynamically allocate port. Remove strict error assertion. Signed-off-by: albertteoh --- cmd/query/app/server_test.go | 61 +++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 5c20fdd2eb5..e85916ffd45 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -17,7 +17,6 @@ package app import ( "context" "net" - "os" "sync" "testing" "time" @@ -27,7 +26,6 @@ import ( "github.com/stretchr/testify/mock" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" - "golang.org/x/sys/unix" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -75,31 +73,44 @@ func TestServerBadHostPort(t *testing.T) { } func TestServerInUseHostPort(t *testing.T) { + const availableHostPort = "127.0.0.1:0" + conn, err := net.Listen("tcp", availableHostPort) + defer func() { conn.Close() }() + assert.NoError(t, err) - for _, hostPort := range [2]string{"127.0.0.1:8080", "127.0.0.1:8081"} { - conn, err := net.Listen("tcp", hostPort) - assert.NoError(t, err) - - server, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{HTTPHostPort: "127.0.0.1:8080", GRPCHostPort: "127.0.0.1:8081", BearerTokenPropagation: true}, - opentracing.NoopTracer{}) - assert.NoError(t, err) - - err = server.Start() - syscallErr, ok := err.(*net.OpError).Err.(*os.SyscallError) - assert.True(t, ok) - assert.Equal(t, syscallErr.Err, unix.EADDRINUSE) - - conn.Close() - if server.grpcConn != nil { - server.grpcConn.Close() - } - if server.httpConn != nil { - server.httpConn.Close() - } - + testCases := []struct { + name string + httpHostPort string + grpcHostPort string + }{ + {"HTTP host port clash on " + conn.Addr().String(), conn.Addr().String(), availableHostPort}, + {"GRPC host port clash on " + conn.Addr().String(), availableHostPort, conn.Addr().String()}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + server, err := NewServer( + zap.NewNop(), + &querysvc.QueryService{}, + &QueryOptions{ + HTTPHostPort: tc.httpHostPort, + GRPCHostPort: tc.grpcHostPort, + BearerTokenPropagation: true, + }, + opentracing.NoopTracer{}, + ) + assert.NoError(t, err) + + err = server.Start() + assert.Error(t, err) + + if server.grpcConn != nil { + server.grpcConn.Close() + } + if server.httpConn != nil { + server.httpConn.Close() + } + }) } - } func TestServer(t *testing.T) { From 32e5ab66fe4482e11d52e5ba3efc81abf65aec9d Mon Sep 17 00:00:00 2001 From: albertteoh Date: Fri, 18 Sep 2020 14:59:49 +1000 Subject: [PATCH 3/5] Remove unstable addresses from test names Signed-off-by: albertteoh --- cmd/query/app/server_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index e85916ffd45..49aa83db0c4 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -83,18 +83,18 @@ func TestServerInUseHostPort(t *testing.T) { httpHostPort string grpcHostPort string }{ - {"HTTP host port clash on " + conn.Addr().String(), conn.Addr().String(), availableHostPort}, - {"GRPC host port clash on " + conn.Addr().String(), availableHostPort, conn.Addr().String()}, + {"HTTP host port clash", conn.Addr().String(), availableHostPort}, + {"GRPC host port clash", availableHostPort, conn.Addr().String()}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { server, err := NewServer( - zap.NewNop(), - &querysvc.QueryService{}, + zap.NewNop(), + &querysvc.QueryService{}, &QueryOptions{ - HTTPHostPort: tc.httpHostPort, - GRPCHostPort: tc.grpcHostPort, - BearerTokenPropagation: true, + HTTPHostPort: tc.httpHostPort, + GRPCHostPort: tc.grpcHostPort, + BearerTokenPropagation: true, }, opentracing.NoopTracer{}, ) From 62a4971cf77bd4c3dbf8e3c051bb191fd9e400bb Mon Sep 17 00:00:00 2001 From: Albert <26584478+albertteoh@users.noreply.github.com> Date: Fri, 18 Sep 2020 15:24:22 +1000 Subject: [PATCH 4/5] Update cmd/query/app/server_test.go Co-authored-by: Yuri Shkuro Signed-off-by: albertteoh --- cmd/query/app/server_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 49aa83db0c4..bef661ccf56 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -75,8 +75,8 @@ func TestServerBadHostPort(t *testing.T) { func TestServerInUseHostPort(t *testing.T) { const availableHostPort = "127.0.0.1:0" conn, err := net.Listen("tcp", availableHostPort) - defer func() { conn.Close() }() - assert.NoError(t, err) + require.NoError(t, err) + defer func() { require.NoError(t, conn.Close()) }() testCases := []struct { name string From 51eb5dfc1ce5d4a7890792a98e4fc37539b16523 Mon Sep 17 00:00:00 2001 From: albertteoh Date: Fri, 18 Sep 2020 15:37:21 +1000 Subject: [PATCH 5/5] Add require dependency Signed-off-by: albertteoh --- cmd/query/app/server_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index bef661ccf56..cfd5f34669a 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -24,6 +24,7 @@ import ( opentracing "github.com/opentracing/opentracing-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer"