-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
test: Workaround slow SRV lookups in flaking test #7957
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7957 +/- ##
==========================================
- Coverage 82.21% 82.08% -0.14%
==========================================
Files 379 379
Lines 38261 38261
==========================================
- Hits 31458 31405 -53
- Misses 5514 5551 +37
- Partials 1289 1305 +16 |
a3fb3dd
to
44aa76e
Compare
tc := testgrpc.NewTestServiceClient(cc) | ||
ctx, cancel := context.WithTimeout(context.Background(), defaultTestShortTimeout) | ||
ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a quick note that the deadline is being shortened because WaitForReady waits for context expiration before allowing the call to return?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment.
This test was migrated from
grpc.DialContext
togrpc.NewClient
in #7920. This caused the resolver to change from passthrough to dns. The test started failing on Mac and flaking on debian systems (more than 1/100 runs fail). The test uses a short timeout and expects an RPC to fail due to a bad TLS certificate. When the test fails, the RPC failure cause isDeadlineExceeded
. The SRV lookups for localhost and servicegrpclb
are causing the timeouts. SRV lookup are disabled by default.grpc-go/internal/resolver/dns/dns_resolver.go
Line 48 in e5a4eb0
SRV lookups are enabled when grpclb is imported.
grpc-go/balancer/grpclb/grpclb.go
Lines 105 to 108 in e5a4eb0
There is another file in the
test
package that imports grpclb, causing SRV lookups to be enabled for this test.grpc-go/test/channelz_test.go
Line 35 in e5a4eb0
RELEASE NOTES: None