-
Notifications
You must be signed in to change notification settings - Fork 310
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
Bump timeout in integration tests to 10 minutes #3146
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Eduardo Apolinario <[email protected]>
Code Review Agent Run #a4599cActionable Suggestions - 1
Review Details
|
Changelist by BitoThis pull request implements the following key changes.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3146 +/- ##
===========================================
- Coverage 92.24% 51.96% -40.29%
===========================================
Files 118 206 +88
Lines 4991 21851 +16860
Branches 0 2837 +2837
===========================================
+ Hits 4604 11355 +6751
- Misses 387 9854 +9467
- Partials 0 642 +642 ☔ View full report in Codecov by Sentry. |
This is weird. All task executions succeed, but the workflow execution still fails sometimes. It's weird that this only happens to the last 3 executions, kicked off by either test_attr_access_sd, test_sd_attr, and test_signal_approve_reject. I'm suspecting that it has to do with load, but still looking for the smoking gun. |
Tracking issue
NA
Why are the changes needed?
We're seeing some of the tests in the integration test suite fail semi-frequently. After investigating with https://github.com/tmate-io/tmate here I confirmed that the tasks end executing successfully, but the flyteremote call only waits for 5 minutes.
What changes were proposed in this pull request?
Set a unified timeout of 10 min per execution in flyteremote integration tests.
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
PR extends integration test timeout from 5 to 10 minutes by introducing TIMEOUT_IN_MIN constant and updating all remote.wait() calls. This change addresses semi-frequent test failures and ensures consistent timeout behavior across the test suite.Unit tests added: False
Estimated effort to review (1-5, lower is better): 1