test: Fix environment-dependent flaky assertion in TestTraceInfoOnTimeoutWithSetTimeout - #1184
Merged
Conversation
…eoutWithSetTimeout
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3 #1184 +/- ##
=======================================
Coverage 99.73% 99.73%
=======================================
Files 20 20
Lines 4141 4141
=======================================
Hits 4130 4130
Misses 7 7
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
close #1166
PR Description
What does this PR do?
Fixes an environment-dependent assertion in
TestTraceInfoOnTimeoutWithSetTimeout/timeout_with_very_short_timeoutthat causes the test suite to fail when run in a network-isolated environment (e.g.,docker --network none).Why is this change necessary?
The test previously asserted
assertTrue(t, tr.DNSLookup == 0)under the assumption that a request with an extremely short timeout (1ms) will consistently abort and time out while waiting for DNS resolution. However, in an offline or heavily isolated environment, DNS resolution instantly fails via network-level errors (likeconnection refused) well before the timeout limit is reached. This records a microscopic but non-zero duration fortr.DNSLookup, causing the strict== 0assertion to fail.How does it work?
This PR updates the strict
tr.DNSLookup == 0check to a stronger, mathematically sound, and environment-agnostic invariant:This guarantees that the measured DNS phase duration logically fits within the overall bound of the request's total time—regardless of whether DNS failed instantly or if the context organically timed out.
The subsequent assertions (
ConnTime == 0,TLSHandshake == 0,TCPConnTime == 0,ServerTime == 0) remain completely unchanged. This preserves the original and primary intent of the test: ensuring that a request aborting early never successfully progresses to establishing a network connection.