Skip to content
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

Allow disabling time auto-advance behavior in tests #5200

Closed
wants to merge 1 commit into from

Conversation

Dentosal
Copy link

Closes #4522. See that issue for a more extensive approach.

Motivation

Some programs have background tests that use sleeping for timings while working with IO. Testing such programs with auto-advancing time is currently quite complicated, as it's not possilbe to time the IO correctly..

Solution

This PR adds a way to disable the auto-advance behavior, so that tests can directly manage time using time::advance. It's simply a boolean flag, which is checked when time would be auto-advanced otherwise.

@github-actions github-actions bot added the R-loom Run loom tests on this PR label Nov 16, 2022
@Dentosal Dentosal force-pushed the disable-auto-advance branch from fd36235 to 2877174 Compare November 16, 2022 16:51
@Darksonn
Copy link
Contributor

There's already a very similar PR open in #5115.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-time Module: tokio/time labels Nov 16, 2022
@jorendorff
Copy link
Contributor

As this comment suggests, it's possible to think of the two PRs as independent. This PR seems valuable regardless of whether #5115 is accepted.

It won't help my use case though, because my goal is to use auto-advance, not turn it off!

For our code, I think manually advancing time will prove too tricky. In general, a blocking task, sleep, or timeout could be an implementation detail of any async method, right? So the top-level test code would have to see through all abstractions in order to "know" when to advance time and by how much.

I can't be sure #5115 by itself is up to the task either, but I think letting individual tasks momentarily disable auto-advance is the way to make auto-advance useful in nontrivial cases.

@Darksonn
Copy link
Contributor

Sure, the PRs can be thought of as independently, but the implementations are quite similar.

@madadam
Copy link
Contributor

madadam commented Jan 12, 2023

It seems #5115 already implements all the necessary functionality, it just doesn't expose it to the users of tokio. So all that this PR needs to do is to expose it.

In the meantime, a workaround can be to spawn a dummy blocking task for the time during which one desires to inhibit auto-advance:

let (tx, rx) = mpsc::oneshot();
task::spawn_blocking(move || { rx.blocking_recv() });
// auto-advance is inhibited here
tx.send(());
// auto-advance is enabled again

@Darksonn
Copy link
Contributor

Ah, yeah, I paused looking at this PR due to that other one, but its merged now.

@Darksonn
Copy link
Contributor

This PR has gotten quite out of date. I'm sorry about that. I'll close it for now, but if you want to continue working on this, then feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-time Module: tokio/time R-loom Run loom tests on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to (temporarily) disable time auto-advance when paused
4 participants