Skip to content
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

Set timeout of a wider range of the test [MOD-6441] #216

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

MeirShpilraien
Copy link
Collaborator

@MeirShpilraien MeirShpilraien commented Jan 11, 2024

The PR changed the test timeout feature to be applied on a wider range of the test lifetime, including initialization and shutdown.
In order to achieve this, the PR changes the way the timeout works. instead of sleeping for the given amount of timeout, the timeout mechanism now calculate the time on which the timeout will reach and periodically check for the timeout. This allows the test to reset the timeout if needed (for example when running on a class of tests and not on a single function) while the timeout still apply on the entire test lifetime from start to end.

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (553f633) 32.52% compared to head (2a201e2) 32.45%.

Files Patch % Lines
RLTest/__main__.py 0.00% 32 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #216      +/-   ##
==========================================
- Coverage   32.52%   32.45%   -0.07%     
==========================================
  Files          17       17              
  Lines        2586     2591       +5     
==========================================
  Hits          841      841              
- Misses       1745     1750       +5     
Flag Coverage Δ
unittests 32.45% <0.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -385,12 +387,16 @@ def watcher_thread(self):
print(Colors.Bred("Failed on timeout function, %s" % str(e)))
os._exit(1)

def reset(self):
self.timeout_time = time.time() + self.timeout
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it acquire the condition while making this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python has a GIL anyway so it will not cause the other thread not to see the update. The worst that can happened is that there will be a context switch before the timeout_time was set, in this case we can consider it as not been set yet.

@MeirShpilraien MeirShpilraien merged commit bc0597d into master Jan 11, 2024
23 checks passed
@MeirShpilraien MeirShpilraien deleted the apply_wider_timeout branch January 11, 2024 13:03
@alonre24 alonre24 changed the title Set timeout of a wider range of the test. Set timeout of a wider range of the test [MOD-6441] Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants