PlatformPlatform icon indicating copy to clipboard operation
PlatformPlatform copied to clipboard

Refactor to use TimeProvider via injected service

Open egil opened this issue 1 month ago • 2 comments

Summary & Motivation

While reading the documentation, I noticed "Use TimeProvider.System.GetUtcNow() and not DateTime.UtcNow().".

This does not make much sense, as using TimeProvider.System.GetUtcNow() is the same as using DateTime.UtcNow(). To leverage TimeProvider, you have to use the one available in the DI container. That, when done, allows you to use a fake TimeProvider during testing, which allows you to control time as well as timers.

In my experience, what you want to do is have a keyed singleton service for a TimeProvider that all your application logic uses. That way, when you need to control time during testing, you are only affecting time for code you own, and know how behave related to time. This leads to more stable tests.

Thus, I have refactored the use of TimeProvider.System.GetUtcNow() to using a shared TimeProvider.

There are also a few places where TimeProvider.System.GetUtcNow() has been replaced by DateTimeOffset.UtcNow(), because time cannot be controlled for them anyway - they work with 3rd party that uses DateTime.UtcNow() and thus cannot work with TimeProvider. That pattern of explicitly using DateTimeOffset.UtcNow() in places where a TimeProvider should not be used makes it more clear that a future dev should not switch these to TimeProvider.

Optional, but a nice addition I think, is a custom attribute that inherits from "FromKeyedServices" attribute, FromPlatformServices, that makes working and getting the keyed TimeProvider more convenient.

Checklist

  • [x] I have ~~added~~ updated tests, or done manual regression tests
  • [x] I have updated the documentation, if necessary

egil avatar Oct 18 '25 22:10 egil

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 18 '25 22:10 CLAassistant

Hi @egil,

Thanks for the pull-request.

I introduced TimeProvider just when it was introduced in .NET 8, but never got around to truly making it possible to inject it. I will take a look at your Pull Request asap.

You should run pp check (dotnet run check from the developer-cli folder), and fix all static code analysis warnings.

As you noticed, there are conventions for commit messages, which made the Pull Request Conventions fail. One line in imperative form (no description, as nobody sees them). Also, please squash your commits into two idiomatic commits (that is, remove "Fix spelling mistake", "Remove unused parameter", "Revert line breaks") ;)

tjementum avatar Oct 19 '25 11:10 tjementum