feat: move Clock::timed method to tick by schgoo · Pull Request #336 · microsoft/oxidizer

schgoo · 2026-03-24T15:33:06Z

For cachet this helper function was added to a Clock extension to time async futures and return the result and duration (primarily for telemetry). I'm proposing moving this to the tick crate.

codecov · 2026-03-24T15:37:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (a5fd76d) to head (05a53ab).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main     #336    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         187      207    +20     
  Lines       14618    15364   +746     
========================================
+ Hits        14618    15364   +746

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Moves an async timing helper from cachet’s telemetry extension into the shared tick crate, making it available as an inherent Clock::timed API for measuring future execution duration (useful for telemetry and tests with controlled time).

Changes:

Added tick::Timed / tick::TimedResult and implemented Clock::timed.
Added tests and an example demonstrating the timing API.
Updated cachet to use Clock::timed and removed the old telemetry extension module.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/tick/src/timed.rs	New `Timed` future + `TimedResult` output type for measuring elapsed time.
crates/tick/src/lib.rs	Wires the new module and re-exports `Timed` / `TimedResult`.
crates/tick/src/clock.rs	Adds `Clock::timed` and associated tests.
crates/tick/examples/timed_result.rs	Adds a runnable example using `Clock::timed`.
crates/tick/Cargo.toml	Registers the new example (Tokio-gated).
crates/cachet/src/wrapper.rs	Switches from `timed_async` extension to `Clock::timed`.
crates/cachet/src/telemetry/mod.rs	Removes telemetry `ext` module from the module tree.
crates/cachet/src/telemetry/ext.rs	Deletes the old `ClockExt`/`Timed` implementation previously local to `cachet`.
crates/cachet/src/refresh.rs	Switches to `Clock::timed`.
crates/cachet/src/fallback.rs	Switches to `Clock::timed` (including promotion insert timing).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T15:38:46Z

crates/tick/src/clock.rs

+    pub fn timed<F>(&self, f: F) -> Timed<F>
+    where
+        F: Future,
+    {


Clock::timed uses F: Future, but Future isn’t in scope in this module (no use std::future::Future; and it’s not in the prelude). This will not compile as-is; import std::future::Future or fully qualify the bound.

crates/tick/src/timed.rs

Copilot · 2026-03-24T15:38:47Z

crates/tick/src/timed.rs

+/// use std::time::Duration;
+///


The doctest imports std::time::Duration but doesn’t use it. This produces an unused_imports warning when running doctests; consider removing the import to keep the example warning-free.

Suggested change

/// use std::time::Duration;

///

Copilot · 2026-03-24T15:38:47Z

crates/tick/src/clock.rs

+    /// use std::time::Duration;
+    ///


This doc example imports std::time::Duration but doesn’t use it, which causes an unused_imports warning in doctest compilation. Consider removing the unused import so the example stays warning-free.

Suggested change

/// use std::time::Duration;

///

Copilot · 2026-03-24T15:38:48Z

crates/tick/src/timed.rs

+impl<F: Future> Future for Timed<F> {
+    type Output = TimedResult<F::Output>;


Timed's Future implementation uses the Future trait (impl<F: Future> Future for Timed<F>) but Future isn’t imported or fully qualified, which will fail to compile. Add use std::future::Future; (or fully qualify std::future::Future in both places).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T10:40:44Z

crates/tick/src/clock.rs

+    #[tokio::test]
+    async fn timed_handles_pending() {
+        use std::pin::Pin;
+        use std::sync::Arc;


This new #[tokio::test] is missing #[cfg_attr(miri, ignore)]. CI runs Miri for the workspace, and Tokio-based async tests are typically ignored under Miri elsewhere in this file.

Copilot · 2026-03-25T10:40:45Z

crates/cachet/src/fallback.rs

-                .clock
-                .timed_async(self.inner.primary.insert(key.clone(), v.clone()))
-                .await;
+            let timed_insert = self.inner.clock.timed(self.inner.primary.insert(key.clone(), v.clone())).await;


This statement is formatted as a single very long line. Please run rustfmt (or break the call across lines) to match the project's formatting expectations and avoid rustfmt CI failures.

Suggested change

let timed_insert = self.inner.clock.timed(self.inner.primary.insert(key.clone(), v.clone())).await;

let timed_insert = self

.inner

.clock

.timed(self.inner.primary.insert(key.clone(), v.clone()))

.await;

Copilot · 2026-03-25T10:40:45Z

crates/tick/src/clock.rs

+    ///
+    /// # async fn timed_example(clock: &Clock) {
+    /// let TimedResult { result, duration } = clock.timed(async { 42 }).await;
+    /// assert_eq!(result, 42);


The doctest example introduces unused bindings/imports (Duration import and the duration binding from TimedResult). This can produce warnings in doctest builds; consider removing the unused import and/or using _duration (or asserting on duration) to keep the example warning-free.

Suggested change

/// assert_eq!(result, 42);

/// assert_eq!(result, 42);

/// assert!(duration >= Duration::from_millis(0));

Copilot · 2026-03-25T10:40:45Z

crates/tick/src/timed.rs

+/// use std::time::Duration;
+///
+/// use tick::{Clock, TimedResult};
+///
+/// # async fn example(clock: &Clock) {
+/// let TimedResult { result, duration } = clock.timed(async { 42 }).await;


This doctest example has unused items (the Duration import and the duration binding). Consider removing the unused import and/or renaming duration to _duration (or asserting on it) so the example stays warning-free when compiled as a doctest.

Suggested change

/// use std::time::Duration;

///

/// use tick::{Clock, TimedResult};

///

/// # async fn example(clock: &Clock) {

/// let TimedResult { result, duration } = clock.timed(async { 42 }).await;

/// use tick::{Clock, TimedResult};

///

/// # async fn example(clock: &Clock) {

/// let TimedResult { result, duration: _duration } = clock.timed(async { 42 }).await;

Copilot · 2026-03-25T10:40:45Z

crates/tick/src/clock.rs

+    #[tokio::test]
+    async fn timed_measures_duration() {
+        let control = ClockControl::new();
+        let clock = control.to_clock();


This new #[tokio::test] is not annotated with #[cfg_attr(miri, ignore)], unlike other Tokio-based tests in this module. Since CI runs cargo miri test --all-features, this test should be ignored under Miri (or rewritten to avoid Tokio).

psandana · 2026-03-25T15:22:23Z

crates/tick/src/timed.rs

 ///
 /// # async fn example(clock: &Clock) {
 /// let TimedResult { result, duration } = clock.timed(async { 42 }).await;
+/// println!("Result: {}, Duration: {:?}", result, duration);


schgoo added 2 commits March 24, 2026 11:15

Move timed_async to tick

6eb159e

Add documentation

2ff96e0

Copilot AI review requested due to automatic review settings March 24, 2026 15:33

Copilot started reviewing on behalf of schgoo March 24, 2026 15:34 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

psandana approved these changes Mar 24, 2026

View reviewed changes

Update crates/tick/src/timed.rs

a315a97

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 25, 2026 10:35

Copilot started reviewing on behalf of schgoo March 25, 2026 10:35 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Fix doctests, resolve clippy and comments

05a53ab

psandana reviewed Mar 25, 2026

View reviewed changes

psandana approved these changes Mar 25, 2026

View reviewed changes

		impl<F: Future> Future for Timed<F> {
		type Output = TimedResult<F::Output>;

	/// assert_eq!(result, 42);
	/// assert_eq!(result, 42);
	/// assert!(duration >= Duration::from_millis(0));

Conversation

schgoo commented Mar 24, 2026

Uh oh!

codecov bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

psandana Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 24, 2026 •

edited

Loading