[8.x](backport #43317) Fix flaky envoyproxy + apache tests #43392
Conversation
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
Fix flaky timeout tests. These tests confirmed that their timeouts worked by scanning the error text returned from the http request. The error text that they expected was: `error in http fetch: error making http request: Get "http://127.0.0.1:57060/stats": context deadline exceeded (Client.Timeout exceeded while awaiting headers)` However there are two other semi-common forms: `error in http fetch: error making http request: Get "http://127.0.0.1:57052/stats": context deadline exceeded` `error in http fetch: error making http request: Get "http://127.0.0.1:32839/stats": net/http: request canceled (Client.Timeout exceeded while awaiting headers)` This result is still a successful check of the timeout feature, even though the propagated error string varies slightly. I've switched to checking substrings that are stable between all error strings. I also reverted some of the changes in #43283 that were unrelated to the test flakiness, and that made the test potentially less stable. These tests previously had near-identical code -- the original version used a channel to terminate the server response deterministically when the test was over, but #43283 changed this to a fixed Sleep call that requires 5ms precision on timer triggers (which is _usually_ true but not always guaranteed on CI machines. Tests shouldn't use magic timeout values when a deterministic alternative is available, and since the channel code turned out to be unrelated to the flakiness I think we should keep it.) (cherry picked from commit cb097b4)
mergify
bot
requested review from
faec and
mauri870
and removed request for
a team
botelastic
bot
added
the
needs_team
github-actions
bot
added
flaky-test
Contributor
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
botelastic
bot
removed
the
needs_team
mauri870
approved these changes
Mar 21, 2025
Contributor
Author
|
This pull request has not been merged yet. Could you please review and merge it @faec? 🙏 |
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.
Fix flaky timeout tests.
These tests confirmed that their timeouts worked by scanning the error text returned from the http request. The error text that they expected was:
error in http fetch: error making http request: Get "http://127.0.0.1:57060/stats": context deadline exceeded (Client.Timeout exceeded while awaiting headers)However there are two other semi-common forms:
error in http fetch: error making http request: Get "http://127.0.0.1:57052/stats": context deadline exceedederror in http fetch: error making http request: Get "http://127.0.0.1:32839/stats": net/http: request canceled (Client.Timeout exceeded while awaiting headers)This result is still a successful check of the timeout feature, even though the propagated error string varies slightly. I've switched to checking substrings that are stable between all error strings.
I also reverted some of the changes in #43283 that were unrelated to the test flakiness, and that made the test potentially less stable. These tests previously had near-identical code -- the original version used a channel to terminate the server response deterministically when the test was over, but #43283 changed this to a fixed Sleep call that requires 5ms precision on timer triggers (which is usually true but not always guaranteed on CI machines. Tests shouldn't use magic timeout values when a deterministic alternative is available, and since the channel code turned out to be unrelated to the flakiness I think we should keep it.)
This is an automatic backport of pull request #43317 done by Mergify.