Skip to content

Make sleep() name its own parameter when given NaN - #3511

Open
dylanpulver wants to merge 2 commits into
python-trio:mainfrom
dylanpulver:fix/sleep-nan-parameter-name
Open

Make sleep() name its own parameter when given NaN#3511
dylanpulver wants to merge 2 commits into
python-trio:mainfrom
dylanpulver:fix/sleep-nan-parameter-name

Conversation

@dylanpulver

Copy link
Copy Markdown

trio.sleep(float("nan")) raises ValueError: deadline must not be NaN, naming a value the caller never passed. sleep takes seconds.

sleep validates seconds < 0 itself but lets NaN through to the deadline check in _core, which is where the wording comes from. Its docstring already promises the error:

Raises:
    ValueError: if *seconds* is negative or NaN.

move_on_after duplicates the same two checks deliberately, with the comment # duplicate validation logic to have the correct parameter name. sleep never got the same treatment. This adds the missing NaN check there, so all nine paths now name the parameter they actually take: seconds for sleep, fail_after and move_on_after, deadline for sleep_until, fail_at and move_on_at.

The existing test_timeouts_raise_value_error could not have caught this. Its pattern was ^(deadline|seconds) must (not )*be (non-negative|NaN)$, an alternation accepting either spelling for every function, so both the old and new messages pass it. Each case now pins the exact message for its own parameter; reverting _timeouts.py fails the test with - deadline must not be NaN / + seconds must not be NaN.

Context: #2493 reported that sleep(nan) slept forever and was fixed at the deadline level, which is why the value is rejected today but under the wrong name.

159 pass across test_timeouts.py and _core/_tests/test_run.py.

🤖 Generated with Claude Code

https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo

dylanpulver and others added 2 commits September 10, 2026 14:17
sleep() validated seconds < 0 itself but let NaN reach the deadline
check in _core, so it raised "deadline must not be NaN" for a value the
caller passed as seconds. Its docstring already documents ValueError for
NaN. move_on_after duplicates this validation on purpose, with a comment
saying it is to get the parameter name right; sleep never did.

The existing test could not catch it: its pattern was an alternation over
both spellings, so either message passed. Each case now pins the message
for the parameter that function actually takes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (a3d7cdd) to head (6cf73b1).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3511   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             128          128           
  Lines           19454        19457    +3     
  Branches         1321         1322    +1     
===============================================
+ Hits            19454        19457    +3     
Files with missing lines Coverage Δ
src/trio/_tests/test_timeouts.py 100.00000% <100.00000%> (ø)
src/trio/_timeouts.py 100.00000% <100.00000%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jakkdl

jakkdl commented Sep 14, 2026

Copy link
Copy Markdown
Member

this looks like a good fix, but CI is failing

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.

2 participants