Skip to content

Add Instant class to conveniently track elapsed and start/end times #13397

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

Merged
merged 4 commits into from
Apr 29, 2025

Conversation

nicoddemus
Copy link
Member

Throughout the codebase we often need to track elapsed times, using a performance clock, and also start/end times that given seconds since epoch (via time.time()).

Instant encapsulates both functionalities to simplify the code and ensure we are using the correct functions from _pytest.timing, which we can mock in tests for reliability.

Built on top of #13394, will rebase once it gets merged.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Apr 26, 2025
@nicoddemus nicoddemus force-pushed the instant-utility branch 2 times, most recently from 22d2244 to 0fd0447 Compare April 26, 2025 14:34
@nicoddemus nicoddemus force-pushed the instant-utility branch 3 times, most recently from 986016f to 8bea3c8 Compare April 27, 2025 12:18
Throughout the codebase we often need to track elapsed times, using a performance clock, and also start/end times that given seconds since epoch (via `time.time()`).

`Instant` encapsulates both functionalities to simplify the code and ensure we are using the correct functions from `_pytest.timing`, which we can mock in tests for reliability.
start=start,
stop=stop,
duration=duration,
start=duration.start,
Copy link
Member

Choose a reason for hiding this comment

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

We might want to make duration itself a attribute of run result

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if I would like to expose that object as part of the public API now.

@nicoddemus nicoddemus force-pushed the instant-utility branch 2 times, most recently from d1716a6 to fd86b59 Compare April 27, 2025 12:25
@nicoddemus nicoddemus added the skip news used on prs to opt out of the changelog requirement label Apr 27, 2025
@nicoddemus nicoddemus marked this pull request as ready for review April 27, 2025 18:52
default_factory=lambda: perf_counter(), init=False
)

def duration(self) -> Duration:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def duration(self) -> Duration:
def until_current(self) -> Duration:

Copy link
Member Author

Choose a reason for hiding this comment

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

"current" seems a bit weird to me, how about until_now? as in "get me the duration until now"?

Copy link
Member

Choose a reason for hiding this comment

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

I agree

Copy link
Member Author

Choose a reason for hiding this comment

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

Decided to consult a LLM, Claude says:


Based on the code shown, I can suggest renaming the duration() method to elapsed() which would be more natural and aligns better with the concept of measuring time that has passed since the was created. This fits well with the existing property in the Duration class and is commonly used in time measurement APIs. Instant``elapsed_s
Other natural alternatives could be:

  • elapsed_time()
  • time_since()
  • measure()

However, elapsed() would be my top recommendation as it:

  1. Is concise and clear
  2. Matches common terminology in time measurement
  3. Aligns with the existing property in the codebase elapsed_s
  4. Is consistent with similar APIs in other languages and libraries

I agree, further it is the same name used by Rust's Instant (which inspired this PR):

use std::time::{Duration, Instant};
use std::thread::sleep;

fn main() {
   let now = Instant::now();

   // we sleep for 2 seconds
   sleep(Duration::new(2, 0));
   // it prints '2'
   println!("{}", now.elapsed().as_secs());
}

Agree?

Copy link
Member

Choose a reason for hiding this comment

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

absolutely, thanks for researching this 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

While at it, noticed that Duration.seconds reads much better than Duration.elapsed_s, as in instant.elapsed().seconds vs instant.elapsed().elapsed_s, so I renamed that as well. 👍

While at it, noticed that `Duration.seconds` reads much better than `Duration.elapsed_s`.
@nicoddemus
Copy link
Member Author

@RonnyPfannschmidt re-requested review. 👍

@nicoddemus nicoddemus merged commit 20c51f7 into pytest-dev:main Apr 29, 2025
28 checks passed
@nicoddemus nicoddemus deleted the instant-utility branch April 29, 2025 11:00
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 skip news used on prs to opt out of the changelog requirement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants