Try to fix flaky profiler spec #3185
Merged
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
**What does this PR do?** This test attempts a fix to the flaky profiler tests that were added in #3162 (and that bit us in #3179). I'm not able to reproduce the issue locally, but I suspect the issue is that the spec as written was racy: 1. We start a background thread and synchronize with it via a queue 2. The thing we assert on happens AFTER the synchronization (e.g. some method called after the queue is used to signal) Thus, I suspect the issue is that once in a while, Ruby switches threads at just the wrong time: between 1 and 2, and thus our assertion fails. Instead, I've changed the specs to have a `wait_for` step that waits for the stack trace of the thread to be what we previously only assumed it to be, and only then do we run the other assertions. I call this fix "an attempt" since I wasn't able to reproduce the issue, BUT overall it's harmless: either I got it right and this fixes it for got, or I got it wrong, and we'll keep seeing the flaky specs, and I'll figure out something else to try. **Motivation:** Profiler always aims to have no flaky tests. **Additional Notes:** N/A **How to test the change?** Validate that CI is still green, and stays that way ;) Fixes DataDog/ruby-guild#53
TonyCTHsu
approved these changes
Oct 5, 2023
…laky-profiler-spec
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.
What does this PR do?
This test attempts a fix to the flaky profiler tests that were added in #3162 (and that bit us in #3179).
I'm not able to reproduce the issue locally, but I suspect the issue is that the spec as written was racy:
Thus, I suspect the issue is that once in a while, Ruby switches threads at just the wrong time: between 1 and 2, and thus our assertion fails.
Instead, I've changed the specs to have a
wait_forstep that waits for the stack trace of the thread to be what we previously only assumed it to be, and only then do we run the other assertions.I call this fix "an attempt" since I wasn't able to reproduce the issue, BUT overall it's harmless: either I got it right and this fixes it for now, or I got it wrong, and we'll keep seeing the flaky specs, and I'll figure out something else to try.
Motivation:
Profiler always aims to have no flaky tests.
Additional Notes:
N/A
How to test the change?
Validate that CI is still green, and stays that way ;)
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Fixes datadog/ruby-guild#53