Skip to content

Fix node duration formatting when rounding crosses a unit boundary - #15145

Open
RonnyPfannschmidt wants to merge 1 commit into
mainfrom
claude/project-thread-0ll2wg
Open

RonnyPfannschmidt wants to merge 1 commit into
mainfrom
claude/project-thread-0ll2wg

Conversation

@RonnyPfannschmidt

@RonnyPfannschmidt RonnyPfannschmidt commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Requested by Ronny · project thread

🤖 Written by Claude Opus 5.5 via Claude Code for the pytest maintainers; I prompted it, it did the work, I read it.

Merge strategy: squash.

Before: with console_output_style = times, a duration just under a unit boundary was printed with too many digits, e.g. 0.99996ms as 1000.0us and 59.9996s as 0m 60s. TestProgressOutputStyle::test_times failed on main because of this (windows-py311 job, output test_foo.py ..... 1000.0us).

After: the same durations print as 1.000ms and 1m 0s.

How: format_node_duration now formats the value first and only keeps a unit if the rounded number stays below that unit's limit, otherwise it moves to the next one. Minutes and hours are computed from the rounded whole seconds. New parametrized cases cover each boundary; all of them fail on main.

No library was used: humanize and humanfriendly produce long-form text ("999 milliseconds and 960 microseconds"), not the compact at-most-7-character column this output needs, and humanfriendly.format_timespan(59.9996) has the same rounding issue ("60 seconds").

  • Tests added
  • AI agent credited in Co-authored-by trailer
  • Changelog entry

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Oct 6, 2026
@RonnyPfannschmidt RonnyPfannschmidt self-assigned this Oct 6, 2026

Copy link
Copy Markdown
Member Author

🤖 Written by Claude Opus 5.5 via Claude Code for the pytest maintainers; I prompted it, it did the work, I read it.

Why not use a library for this formatting? I checked the two common ones against the boundary values from the new tests:

seconds humanize.precisedelta humanize.naturaldelta humanfriendly.format_timespan this PR
0.00099996 1 millisecond 1 millisecond 0 seconds 1.000ms
0.0123 12 milliseconds and 300 microseconds 12 milliseconds 0.01 seconds 12.30ms
0.99996 999 milliseconds and 960 microseconds 999 milliseconds 1 second 1.000s
59.9996 59 seconds, 999 milliseconds and 600 microseconds 59 seconds 60 seconds 1m 0s
119.6 1 minute, 59 seconds and 600 milliseconds 2 minutes 1 minute and 59.6 seconds 2m 0s
  • Both produce long-form text. The times progress column needs compact output of at most 7 characters.
  • humanfriendly.format_timespan(59.9996) gives "60 seconds", which is the same rounding-across-a-boundary bug this PR fixes.
  • Either one would add a runtime dependency to pytest to replace about 15 lines of code.

So the PR keeps the hand-written format_node_duration.


Generated by Claude Code

``format_node_duration`` picked the unit before rounding, so a duration
such as 0.99996ms was shown as ``1000.0us`` instead of ``1.000ms``
(and 59.9996s as ``0m 60s``). This made
``TestProgressOutputStyle::test_times`` fail intermittently on fast CI
runners.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VzXq7WcB4fSibrBDvLFXAQ
@RonnyPfannschmidt
RonnyPfannschmidt force-pushed the claude/project-thread-0ll2wg branch from a698a77 to 3212d47 Compare October 6, 2026 14:06
@RonnyPfannschmidt
RonnyPfannschmidt marked this pull request as ready for review October 6, 2026 14:06

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants