cleanup: replace dial with newclient #7967
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7967 +/- ##
==========================================
- Coverage 82.17% 82.04% -0.13%
==========================================
Files 381 381
Lines 38539 38535 -4
==========================================
- Hits 31668 31615 -53
- Misses 5564 5603 +39
- Partials 1307 1317 +10
|
| grpc.WithTransportCredentials(insecure.NewCredentials()), | ||
| grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, stateRecordingBalancerName))) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| defer client.Close() | ||
|
|
||
| client.Connect() |
There was a problem hiding this comment.
There is a call to testutils.StayConnected below, do we need to call connect here? Please consider this every time you replace Dial with NewClient across all PRs.
There was a problem hiding this comment.
getting the below error without connect() call
timed out waiting for state 0 (CONNECTING) in flow [CONNECTING READY IDLE CONNECTING]
| ctx, dialCancel := context.WithTimeout(context.Background(), defaultTestTimeout) | ||
| defer dialCancel() | ||
| cc, err := grpc.DialContext(ctx, "", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(d)) | ||
| cc, err := grpc.NewClient("localhost:0", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(d)) |
There was a problem hiding this comment.
Use passthrough instead of changing the hostname. I've mentioned the usage of custom dialers before.
| } | ||
| cc.Connect() |
There was a problem hiding this comment.
Instead of calling cc.Connect here make the callers of setupForSecurityTests call it only if needed.
| } | ||
| cc.Connect() |
There was a problem hiding this comment.
Instead of calling cc.Connect here make the callers of setupWithManagementServerAndListener call it only if needed.
There was a problem hiding this comment.
Similar comment for other setup functions.
| if cc1, err = grpc.NewClient(lis1.Addr().String(), grpc.WithBlock(), grpc.WithTransportCredentials(insecure.NewCredentials())); err != nil { | ||
| t.Fatal("Failed to create clientConn to a server in \"not-serving\" state") |
There was a problem hiding this comment.
This is not the same assertion as before. You need to make an RPC and check that it fails.
This reverts commit bf780fe.
Partially address: #7049
RELEASE NOTES: None