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

FakeTimeProvider with auto advance enabled - should calling ToString() cause time to advance? #4268

Closed
egil opened this issue Aug 10, 2023 · 3 comments · Fixed by #4269
Closed
Assignees
Labels
area-fundamentals good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue

Comments

@egil
Copy link
Contributor

egil commented Aug 10, 2023

I was looking over the code and noticed that ToString() calls GetUtcNow(). If AutoAdvanceAmount is greater than zero, then this would advance time whenever ToString is invoked.

In normal operations that is likely not going to cause problems, but if I understand how (visual) debuggers work correctly e.g. the ones in Visual Studio, if I am debugging my app and I am inspecting a FakeTimeProvider instance, VS will call ToString() causing the clock to advance. That is going to make debugging time-related issues pretty damn hard.

I think the fix is pretty simple:

-public override string ToString() => GetUtcNow().ToString("yyyy-MM-ddTHH:mm:ss.fff", CultureInfo.InvariantCulture);
+public override string ToString() => _now.ToString("yyyy-MM-ddTHH:mm:ss.fff", CultureInfo.InvariantCulture);
@sebastienros
Copy link
Member

Would you mind submitting a PR with your suggestion, and a unit test that checks that?

@RussKie RussKie added help wanted Up for grabs. We would accept a PR to help resolve this issue good first issue Good for newcomers. area-testing labels Aug 10, 2023
@egil
Copy link
Contributor Author

egil commented Aug 11, 2023

Sure, ill see what I can do. Was not able to get the build/tests running locally, but ill see what github CI can do for me. Off to bed. Ill check back in tomorrow.

@RussKie
Copy link
Member

RussKie commented Aug 11, 2023

Was not able to get the build/tests running locally,

Have a look at our build guide - https://github.com/dotnet/extensions/blob/main/docs/building.md. Holler, if you have any specific issues, and we'll try to help.

@ghost ghost removed the work in progress 🚧 label Aug 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-fundamentals good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants