-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
gh-140381: Make test_profiling tests deterministic to fix flakiness #141731
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
Conversation
| mock.patch("sys.stdout", captured_output), | ||
| ): | ||
| try: | ||
| # Sample for up to SHORT_TIMEOUT seconds, but process exits after fixed iterations |
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.
This is basically just a longer duration - if we stop sampling, then the expected function (which doesn't run in the first 5 iterations) won't show up and the test will fail.
Perhaps we can check for the first function that runs in the test? Or else sample the entire process so that we know it exits after having run the function we're checking for?
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.
Right now is sampling the entire process. The iterations are such that SHORT_TIMEOUT is effectively too long for the typical duration of that process
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.
Okay yeah, I agree (after going through a complicated thought process involving waiting for the process to actually start, which would also require a timeout, which would also be SHORT_TIMEOUT...)
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.
Yeah good point. In any case the duration starts counting after the process has started and tell us it's ready. The profiler waits for confirmation from a special wrapper before starting to profile so the time fo the process to start shouldn't count. I still think 30 seconds is more than enough in any case as this is racy anyway. I will look if I can simplify what we check to make it a bit safer
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.
I have simplified the test a lot, hopefully this does the trick
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 0cd4097 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141731%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit d9124f3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141731%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
…ness The test_sampling_basic_functionality, test_sample_target_script, and test_sample_target_module tests were failing intermittently on slow architectures (i686/s390x) because they relied on a time-based workload with an infinite loop (while True). On slow machines, the profiler's sampling window would end before enough iterations executed, causing slow_fibonacci to not appear in the profiling output.
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 9b4a823 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141731%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
The test_sampling_basic_functionality, test_sample_target_script, and
test_sample_target_module tests were failing intermittently on slow
architectures (i686/s390x) because they relied on a time-based workload
with an infinite loop (while True). On slow machines, the profiler's
sampling window would end before enough iterations executed, causing
slow_fibonacci to not appear in the profiling output.
test_profilingflaky fail on i686/s390x builds in Fedora Linux #140381