Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uses of require in goroutines #16953

Merged
merged 4 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions api/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,17 @@ func startMockServer(t *testing.T) *mockServer {
// startMockServerWithListener starts a new mock server with the provided listener
func startMockServerWithListener(t *testing.T, l net.Listener) *mockServer {
srv := newMockServer(l.Addr().String())
t.Cleanup(srv.grpc.Stop)

errCh := make(chan error, 1)
go func() {
require.NoError(t, srv.grpc.Serve(l))
errCh <- srv.grpc.Serve(l)
}()

t.Cleanup(func() {
srv.grpc.Stop()
require.NoError(t, <-errCh)
})

return srv
}

Expand Down
33 changes: 23 additions & 10 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,10 @@ func testAuditOn(t *testing.T, suite *integrationTestSuite) {
Port: helpers.Port(t, nodeConf.SSH.Addr.Addr),
ForwardAgent: tt.inForwardAgent,
})
require.NoError(t, err)
if err != nil {
endC <- err
return
}
cl.Stdout = myTerm
cl.Stdin = myTerm

Expand Down Expand Up @@ -437,7 +440,8 @@ func testAuditOn(t *testing.T, suite *integrationTestSuite) {

// wait for session to end:
select {
case <-endC:
case err := <-endC:
require.NoError(t, err)
case <-time.After(10 * time.Second):
t.Fatalf("%s: Timeout waiting for session to finish.", tt.comment)
}
Expand Down Expand Up @@ -3678,7 +3682,10 @@ func testAuditOff(t *testing.T, suite *integrationTestSuite) {
Host: Host,
Port: helpers.Port(t, teleport.SSH),
})
require.NoError(t, err)
if err != nil {
endCh <- err
return
}
cl.Stdout = myTerm
cl.Stdin = myTerm
err = cl.SSH(ctx, []string{}, false)
Expand Down Expand Up @@ -3708,7 +3715,8 @@ func testAuditOff(t *testing.T, suite *integrationTestSuite) {
select {
case <-time.After(1 * time.Minute):
t.Fatalf("Timed out waiting for session to end.")
case <-endCh:
case err := <-endCh:
require.NoError(t, err)
}

// audit log should have the fact that the session occurred recorded in it
Expand Down Expand Up @@ -3842,35 +3850,40 @@ func testPAM(t *testing.T, suite *integrationTestSuite) {

termSession := NewTerminal(250)

errCh := make(chan error)

// Create an interactive session and write something to the terminal.
ctx, cancel := context.WithCancel(context.Background())
go func() {
cl, err := teleport.NewClient(helpers.ClientConfig{
Login: suite.Me.Username,
Cluster: helpers.Site,
Host: Host,
Port: helpers.Port(t, teleport.SSH),
})
require.NoError(t, err)
if err != nil {
errCh <- err
return
}

cl.Stdout = termSession
cl.Stdin = termSession

termSession.Type("\aecho hi\n\r\aexit\n\r\a")
err = cl.SSH(context.TODO(), []string{}, false)
if !isSSHError(err) {
require.NoError(t, err)
errCh <- err
return
}

cancel()
errCh <- nil
}()

// Wait for the session to end or timeout after 10 seconds.
select {
case <-time.After(10 * time.Second):
dumpGoroutineProfile()
t.Fatalf("Timeout exceeded waiting for session to complete.")
case <-ctx.Done():
case err := <-errCh:
require.NoError(t, err)
}

// If any output is expected, check to make sure it was output.
Expand Down
9 changes: 7 additions & 2 deletions lib/backend/firestore/firestorebk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,13 @@ func TestDeleteDocuments(t *testing.T) {

lis, err := net.Listen("tcp", "localhost:0")
require.NoError(t, err)
go func() { require.NoError(t, srv.Serve(lis)) }()
t.Cleanup(srv.Stop)

errCh := make(chan error, 1)
go func() { errCh <- srv.Serve(lis) }()
t.Cleanup(func() {
srv.Stop()
require.NoError(t, <-errCh)
})

ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
Expand Down
3 changes: 2 additions & 1 deletion lib/kube/proxy/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/client-go/kubernetes"
authztypes "k8s.io/client-go/kubernetes/typed/authorization/v1"
Expand Down Expand Up @@ -213,7 +214,7 @@ func (c *testContext) startKubeService(t *testing.T) {
if errors.Is(err, http.ErrServerClosed) {
return
}
require.NoError(t, err)
assert.NoError(t, err)
}()
}

Expand Down
3 changes: 1 addition & 2 deletions lib/reversetunnel/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ func TestCachingResolver(t *testing.T) {
// We start a goroutine that mutates the underlying NetAddr, but without invalidating the cache.
// The caching resolver must return a pointer to a copy of the NetAddr to avoid a data race.
go func() {
addr, _, err := resolver(context.Background())
require.NoError(t, err)
addr, _, _ := resolver(context.Background())
// data race check: write to *addr
addr.Addr = ""
}()
Expand Down
3 changes: 2 additions & 1 deletion lib/srv/alpnproxy/forward_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/lib/defaults"
Expand Down Expand Up @@ -150,7 +151,7 @@ func createForwardProxyWithConfig(t *testing.T, config ForwardProxyConfig) *Forw
})

go func() {
require.NoError(t, forwardProxy.Start())
assert.NoError(t, forwardProxy.Start())
}()
return forwardProxy
}
3 changes: 2 additions & 1 deletion lib/srv/alpnproxy/local_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/aws/aws-sdk-go/aws/session"
v4 "github.com/aws/aws-sdk-go/aws/signer/v4"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/lib/srv/alpnproxy/common"
Expand Down Expand Up @@ -153,7 +154,7 @@ func createAWSAccessProxySuite(t *testing.T, cred *credentials.Credentials) *Loc
})
go func() {
err := lp.StartAWSAccessProxy(context.Background())
require.NoError(t, err)
assert.NoError(t, err)
}()
return lp
}
5 changes: 3 additions & 2 deletions lib/utils/concurrentqueue/queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -48,8 +49,8 @@ func TestOrdering(t *testing.T) {
for i := 0; i < testItems; i++ {
itm := <-q.Pop()
val, ok := itm.(int)
require.True(t, ok)
require.Equal(t, i, val)
assert.True(t, ok)
assert.Equal(t, i, val)
}
}()

Expand Down
3 changes: 2 additions & 1 deletion operator/controllers/resources/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (

"github.com/google/uuid"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
Expand Down Expand Up @@ -154,7 +155,7 @@ func (s *testSetup) startKubernetesOperator(t *testing.T) {

go func() {
err := s.operator.Start(ctx)
require.NoError(t, err)
assert.NoError(t, err)
}()
}

Expand Down
11 changes: 9 additions & 2 deletions tool/tsh/tsh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,13 +345,17 @@ func TestOIDCLogin(t *testing.T) {

var didAutoRequest atomic.Bool

errCh := make(chan error)
go func() {
watcher, err := authServer.NewWatcher(ctx, types.Watch{
Kinds: []types.WatchKind{
{Kind: types.KindAccessRequest},
},
})
require.NoError(t, err)
if err != nil {
errCh <- err
return
}
for {
select {
case event := <-watcher.Events():
Expand All @@ -362,12 +366,14 @@ func TestOIDCLogin(t *testing.T) {
RequestID: event.Resource.(types.AccessRequest).GetName(),
State: types.RequestState_APPROVED,
})
require.NoError(t, err)
didAutoRequest.Store(true)
errCh <- err
return
case <-watcher.Done():
errCh <- nil
return
case <-ctx.Done():
errCh <- nil
return
}
}
Expand All @@ -390,6 +396,7 @@ func TestOIDCLogin(t *testing.T) {
})

require.NoError(t, err)
require.NoError(t, <-errCh)

// verify that auto-request happened
require.True(t, didAutoRequest.Load())
Expand Down