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

[API Proposal]: FakeTimeProvider, option to allow going backwards #5072

Open
jjxtra opened this issue Mar 29, 2024 · 8 comments
Open

[API Proposal]: FakeTimeProvider, option to allow going backwards #5072

jjxtra opened this issue Mar 29, 2024 · 8 comments
Assignees
Labels
api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews api-suggestion Early API idea and discussion, it is NOT ready for implementation area-fundamentals

Comments

@jjxtra
Copy link

jjxtra commented Mar 29, 2024

Background and motivation

I'm using reflection to get around FakeTimeProvider inability to go backwards. Would be nice if a bool could be added to allow setting time earlier.

Thanks for considering.

API Proposal

public sealed class FakeTimeProvider : TimeProvider
{
    public bool AllowBackwardsTimeProgression { get; set; }
}

API Usage

// Fancy the value
FakeTimeProvider t = ...
t.AllowBackwardsTimeProgression = true;
t.SetUtcNow(new DateTimeOffset(1990, 1, 1, 1, 1, 1));
t.SetUtcNow(new DateTimeOffset(1984, 1, 1, 1, 1, 1));

Alternative Designs

No response

Risks

None I can think of. This new property will default to false.

@jjxtra jjxtra added api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged labels Mar 29, 2024
@geeknoid
Copy link
Member

geeknoid commented Apr 2, 2024

@stephentoub I presume it's possible for the normal time provider's idea of current time to move backwards? And this presumably doesn't affect any outstanding timers (since they are created with relative time), and presumably wouldn't affect the value returned by GetTimestamp?

@jjxtra
Copy link
Author

jjxtra commented Apr 2, 2024

The internal property storing the time for FakeTimeProvider is called _now and is simply a DateTimeOffset value.

@KalleOlaviNiemitalo
Copy link

If this is for testing how your application would react to a server administrator abruptly setting the clock -- then I think the method should also support setting a future wall-clock time without affecting elapsed time.

var fake = new FakeTimeProvider(new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero));
long startStamp = fake.GetTimestamp();

// simulate 1 second passing
fake.Advance(TimeSpan.FromSeconds(1));
Debug.Assert(fake.GetElapsedTime(startStamp) == TimeSpan.FromSeconds(1));

// simulate 59 more seconds passing
fake.SetUtcNow(new DateTimeOffset(1970, 1, 1, 0, 1, 0, TimeSpan.Zero));
Debug.Assert(fake.GetElapsedTime(startStamp) == TimeSpan.FromMinutes(1));

// NEW: simulate an administrator setting the clock forward
// no effect on elapsed time
fake.SetUtcNow(new DateTimeOffset(1970, 1, 1, 0, 2, 0, TimeSpan.Zero), abrupt: true);
Debug.Assert(fake.GetElapsedTime(startStamp) == TimeSpan.FromMinutes(1));

// simulate 60 more seconds passing
fake.SetUtcNow(new DateTimeOffset(1970, 1, 1, 0, 3, 0, TimeSpan.Zero));
Debug.Assert(fake.GetElapsedTime(startStamp) == TimeSpan.FromMinutes(2));

// NEW: simulate an administrator setting the clock backward
// no effect on elapsed time
fake.SetUtcNow(new DateTimeOffset(1970, 1, 1, 0, 2, 50, TimeSpan.Zero), abrupt: true); 
Debug.Assert(fake.GetElapsedTime(startStamp) == TimeSpan.FromMinutes(2));

@stephentoub
Copy link
Member

@stephentoub I presume it's possible for the normal time provider's idea of current time to move backwards? And this presumably doesn't affect any outstanding timers (since they are created with relative time), and presumably wouldn't affect the value returned by GetTimestamp?

Should be fine. If we find some place it's not, I'd consider that a bug.

geeknoid pushed a commit that referenced this issue May 31, 2024
This addresses #5072 by allowing time to be
adjusted forwards and backwards in order to simulate
the system's clock being adjusted. Messing around
with the time in this way doesn't affect outstanding
timers.
geeknoid pushed a commit that referenced this issue May 31, 2024
This addresses #5072 by allowing time to be
adjusted forwards and backwards in order to simulate
the system's clock being adjusted. Messing around
with the time in this way doesn't affect outstanding
timers.
geeknoid added a commit that referenced this issue Jun 1, 2024
This addresses #5072 by allowing time to be
adjusted forwards and backwards in order to simulate
the system's clock being adjusted. Messing around
with the time in this way doesn't affect outstanding
timers.

Co-authored-by: Martin Taillefer <mataille@microsoft.com>
@evgenyfedorov2
Copy link
Contributor

@geeknoid -can this issue be closed as fixed by #5192 ?

@KalleOlaviNiemitalo
Copy link

The AdjustTime method still has an ExperimentalAttribute. Are issues in this repository usually closed before APIs are reviewed and declared stable?

@geeknoid
Copy link
Member

@evgenyfedorov2 We need an API review to approve the new API, so we can remove the experimental attribute, and then we can close this issue.

@amadeuszl amadeuszl added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Feb 17, 2025
@amadeuszl amadeuszl self-assigned this Feb 17, 2025
@amadeuszl
Copy link
Contributor

I've sent a request to API Review Board for a review, so we can go out of experimental stage for that method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews api-suggestion Early API idea and discussion, it is NOT ready for implementation area-fundamentals
Projects
None yet
Development

No branches or pull requests

6 participants