Skip to content

Commit

Permalink
Fix goroutine leak in server test.
Browse files Browse the repository at this point in the history
In `TestResponseHandler`, the test runs the server in a goroutine,
but doesn't guarantee that the goroutine completes before the end of
the test. In addition to simply leaking the goroutine, this can panic
if there is an error after the test completes. In that case, it is
unnecessarily hard to debug.

Signed-off-by: James Peach <jpeach@apache.org>
  • Loading branch information
jpeach committed Dec 2, 2021
1 parent f62def5 commit 4574c39
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions pkg/server/v3/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/envoyproxy/go-control-plane/pkg/server/stream/v3"
"github.com/envoyproxy/go-control-plane/pkg/server/v3"
"github.com/envoyproxy/go-control-plane/pkg/test/resource/v3"
"github.com/stretchr/testify/assert"
)

type mockConfigWatcher struct {
Expand Down Expand Up @@ -303,13 +304,17 @@ func TestServerShutdown(t *testing.T) {
func TestResponseHandlers(t *testing.T) {
for _, typ := range testTypes {
t.Run(typ, func(t *testing.T) {
done := make(chan struct{})
ctx, cancel := context.WithCancel(context.Background())

config := makeMockConfigWatcher()
config.responses = makeResponses()
s := server.NewServer(context.Background(), config, server.CallbackFuncs{})
s := server.NewServer(ctx, config, server.CallbackFuncs{})

// make a request
resp := makeMockStream(t)
resp.recv <- &discovery.DiscoveryRequest{Node: node, TypeUrl: typ}

go func(rType string) {
var err error
switch rType {
Expand All @@ -332,9 +337,8 @@ func TestResponseHandlers(t *testing.T) {
case opaqueType:
err = s.StreamAggregatedResources(resp)
}
if err != nil {
t.Errorf("Stream() => got %v, want no error", err)
}
assert.NoError(t, err)
close(done)
}(typ)

// check a response
Expand All @@ -347,6 +351,9 @@ func TestResponseHandlers(t *testing.T) {
case <-time.After(1 * time.Second):
t.Fatalf("got no response")
}

cancel()
<-done
})
}
}
Expand Down

0 comments on commit 4574c39

Please sign in to comment.