Fix test_timing flakiness under Windows
#508
Conversation
Observed: ``` > assert delay_ms <= elapsed_time_ms < delay_ms + generous_tolerance E assert 500.0 <= 490.03173828125 test_event.py:41: AssertionError ``` The generous_tolerance also needs to be applied when checking the lower bound.
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
|
Since the CI was green and the last commit is comment-only, go ahead and admin-merge it. |
|
| @@ -37,8 +38,13 @@ def test_timing(init_cuda, enable_timing): | |||
| # We only want to exercise the __sub__ method, this test is not meant | |||
| # to stress-test the CUDA driver or time.sleep(). | |||
| delay_ms = delay_seconds * 1000 | |||
| generous_tolerance = 20 | |||
| assert delay_ms <= elapsed_time_ms < delay_ms + generous_tolerance | |||
| if os.name == "nt": # noqa: SIM108 | |||
There was a problem hiding this comment.
what do we gain by giving different tolerances to each OS? could we set both to 100?
There was a problem hiding this comment.
Yes, but this is a little stricter; I figured Leo believes that's better.
In the long run, assuming that everything is stable (cuda.core released, and this usually works), a tighter tolerance could uncover new bugs elsewhere. So we'd contribute to the greater good.
There was a problem hiding this comment.
I didn't even notice that it was already merged ha. Looks good to me, I was mostly just curious.
Observed:
The
generous_tolerancealso needs to be applied when checking the lower bound.