-
Notifications
You must be signed in to change notification settings - Fork 669
fix: extend wait timeout for test_delete_user() #1316
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
Codecov Report
@@ Coverage Diff @@
## master #1316 +/- ##
=======================================
Coverage 80.76% 80.76%
=======================================
Files 69 69
Lines 3623 3623
=======================================
Hits 2926 2926
Misses 697 697
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Thanks for looking into this one. TBH I'm not sure anymore if the sidekiq metrics are a reliable source for the slow async operations - the point was to avoid hardcoded sleeps due to the underpowered CI VMs. I haven't taken the time to investigate it, so if you can solve it that's great, but don't lose too much time over this :) |
Thanks. Doing some debugging at the moment. I'll see what I figure out. |
Not sure if this fixes the issue. Especially if the sidekick operations can not be counted on. But it should not hurt. |
Have been seeing intermittent failures of the test_delete_user() functional test. Have made the following changes to hopefully resolve the issue and if it still fails to know better why the failure occurred. * Extend the wait timeout for test_delete_user() from 30 to 60 tries of 0.5 seconds each. * Modify wait_for_sidekiq() to return True if sidekiq process terminated. Return False if the timeout expired. * Modify wait_for_sidekiq() to loop through all processes instead of assuming there is only one process. If all processes are not busy then return. * Modify wait_for_sidekiq() to sleep at least once before checking for processes being busy. * Check for True being returned in test_delete_user() call to wait_for_sidekiq()
Cool, I guess the only thing that might happen is that some rogue sidekiq processes will drag this longer, but this was probably already the case and I forget what I was trying to achieve with relying only on the first process 😁 |
No description provided.