Skip to content

Fix test_timing flakiness under Windows #508

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 12, 2025
Merged

Fix test_timing flakiness under Windows #508

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c340ad4
Fix test_timing flakiness under Windows:
rwgk Mar 11, 2025
f30c544
Add "For Python <=3.10" to the comment.
rwgk Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions cuda_core/tests/test_event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# this software and related documentation outside the terms of the EULA
# is strictly prohibited.

import os
import time

import pytest
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

what do we gain by giving different tolerances to each OS? could we set both to 100?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't even notice that it was already merged ha. Looks good to me, I was mostly just curious.

# For Python <=3.10, the Windows timer resolution is typically limited to 15.6 ms by default.
generous_tolerance = 100
else:
# Most modern Linux kernels have a default timer resolution of 1 ms.
generous_tolerance = 20
assert delay_ms - generous_tolerance <= elapsed_time_ms < delay_ms + generous_tolerance
else:
with pytest.raises(RuntimeError) as e:
elapsed_time_ms = e2 - e1
Expand Down