[internal/otelarrow] Resolve test flakes; skip one still-flaky test #34794
Conversation
This file contains 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
mwear
reviewed
Aug 21, 2024
jmacd
changed the title
jmacd
changed the title
|
This test is failing for other reasons in my local environment. The local failure was an expectation that more than one stream was used, and I find the test wasn't controlling for that variable. I introduced retries for thoroughness and a tiny sleep call to ensure the expected >1 streams takes place. It can be no worse for the Windows problem, and I see it has passed at least once on Windows (from the history above, though see other test flakes on Windows). |
pjanotti
approved these changes
Aug 21, 2024
jpkrohling
requested changes
Aug 22, 2024
jmacd
changed the title
jmacd
changed the title
|
I've submitted #34823 for the |
|
There is still a flaky test, the problem appears to be a real problem combined with a slower test start than on the non-windows host. I have added a |
jmacd
changed the title
|
The windows/internal test group passes with my changes. The other tests are still preventing a ✅. Hoping to unblock the release and CI process, I left one |
|
I recommend we merge this as-is, because (a) it will take some time to properly fix the one skipped test, (b) I have four PRs open in this repository and it becomes difficult to move anything in this situation. |
jpkrohling
removed request for
atoulme,
bogdandrutu,
dashpole,
tigrannajaryan,
djaglowski,
mx-psi,
dmitryax,
songy23,
evan-bradley,
TylerHelmuth,
MovieStoreGuy and
crobert-1
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
jpkrohling
changed the title
codeboten
added a commit
that referenced
this pull request
Sep 6, 2024
**Description:** Restore a skipped test, after understanding the nature of the problem. The problem was mostly addressed in #34794, which left the test disabled. The test had been flaky because while testing for an out-of-memory condition, the test could fail for timeout or other reason. To make the test more reliable, this now waits until at least one ArrowTraces span has been received by both components. After one span is available, it checks that the expected log messages are present on both sides. **Link to tracking Issue:** Fixes #34719. **Testing:** ✅ --------- Co-authored-by: Curtis Robert <crobert@splunk.com> Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
f7o
pushed a commit
to f7o/opentelemetry-collector-contrib
that referenced
this pull request
Sep 12, 2024
…pen-telemetry#34794) **Description:** Fixes the causes of flakiness in most cases by using a callback to terminate the test without resorting to sleep statements. There is still one flaky test that for reasons not understood, does not pass. Fortunately, it fails in a repeatable way, and I will debug as part of open-telemetry#34719. **Link to tracking Issue:** open-telemetry#34719 --------- Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de> Co-authored-by: Juraci Paixão Kröhling <juraci@kroehling.de>
f7o
pushed a commit
to f7o/opentelemetry-collector-contrib
that referenced
this pull request
Sep 12, 2024
**Description:** Restore a skipped test, after understanding the nature of the problem. The problem was mostly addressed in open-telemetry#34794, which left the test disabled. The test had been flaky because while testing for an out-of-memory condition, the test could fail for timeout or other reason. To make the test more reliable, this now waits until at least one ArrowTraces span has been received by both components. After one span is available, it checks that the expected log messages are present on both sides. **Link to tracking Issue:** Fixes open-telemetry#34719. **Testing:** ✅ --------- Co-authored-by: Curtis Robert <crobert@splunk.com> Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
jriguera
pushed a commit
to springernature/opentelemetry-collector-contrib
that referenced
this pull request
Oct 4, 2024
**Description:** Restore a skipped test, after understanding the nature of the problem. The problem was mostly addressed in open-telemetry#34794, which left the test disabled. The test had been flaky because while testing for an out-of-memory condition, the test could fail for timeout or other reason. To make the test more reliable, this now waits until at least one ArrowTraces span has been received by both components. After one span is available, it checks that the expected log messages are present on both sides. **Link to tracking Issue:** Fixes open-telemetry#34719. **Testing:** ✅ --------- Co-authored-by: Curtis Robert <crobert@splunk.com> Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
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.
Description: Fixes the causes of flakiness in most cases by using a callback to terminate the test without resorting to sleep statements. There is still one flaky test that for reasons not understood, does not pass. Fortunately, it fails in a repeatable way, and I will debug as part of #34719.
Link to tracking Issue: #34719