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

Use the new TimeProvider API from .NET 8 #1070

Closed
Tracked by #1302
martintmk opened this issue Mar 20, 2023 · 9 comments · Fixed by #1144
Closed
Tracked by #1302

Use the new TimeProvider API from .NET 8 #1070

martintmk opened this issue Mar 20, 2023 · 9 comments · Fixed by #1144
Assignees
Labels
v8 Issues related to the new version 8 of the Polly library.
Milestone

Comments

@martintmk
Copy link
Contributor

Instead of writing our own variant of IClock that allows mocking time in tests we can now leverage the TimeProvider API that will be introduced in .NET 8.

The TimeProvider will be also available in .NET Framework by referencing the new nuget package.

Until the package is ready we can just copy the implementaiton into Polly, mark all classes as internal and once the official package is ready expose ResilienceStrategyBuilderOptions.TimeProvider property.

Wit this approach we will be using unified time abstraction and also allow easy unit-testing of strategies working with time.

@martintmk martintmk converted this from a draft issue Mar 20, 2023
@martintmk martintmk added this to the v8.0.0 milestone Mar 20, 2023
@martintmk martintmk added up-for-grabs v8 Issues related to the new version 8 of the Polly library. labels Mar 20, 2023
@martintmk martintmk moved this to Todo in Polly v8 Mar 20, 2023
@martintmk martintmk changed the title Use the new TimeProvider API from .NET Use the new TimeProvider API from .NET 8 Mar 20, 2023
@martintmk martintmk mentioned this issue Mar 22, 2023
4 tasks
@martintmk martintmk moved this from Todo to In Progress in Polly v8 Mar 22, 2023
@martintmk martintmk moved this from In Progress to Done in Polly v8 Mar 24, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

@github-actions github-actions bot added the stale Stale issues or pull requests label Jul 1, 2023
@martincostello martincostello removed the stale Stale issues or pull requests label Jul 1, 2023
@egil
Copy link

egil commented Sep 26, 2023

I would also suggest that you use https://www.nuget.org/packages/Microsoft.Bcl.TimeProvider to use TimeProvider for all the .NET versions you are supported. It has been backported to support .netstandard 2.0.

@egil
Copy link

egil commented Sep 26, 2023

Btw. I have a few issues with Microsofts FakeTimeProvider implementation, which is why I have kept my own fake around. Basically, their version does not behave consistently depending on how far you advance time. So if you are planing to use their FakeTimeProvider, check out https://github.com/egil/TimeProviderExtensions#difference-between-manualtimeprovider-and-faketimeprovider which explains the problem.

@martincostello
Copy link
Member

@egil - We flushed out a bug in preview 6, but otherwise we've not had an real issues with FakeTimeProvider which we've adopted in #1144, but thanks for the link.

The NuGet package is being used for the down-level frameworks in that PR, which will likely ship as a v8.1.0 once .NET 8 ships (as dotnet/extensions for 8.0.0 depends on us shipping our 8.0.0).

That also reminds me I need to update that PR to RC1 (I've been on holiday the last 2 weeks)...

@egil
Copy link

egil commented Sep 26, 2023

@martincostello yeah, the FakeTimeProvider works. I did contribute quite a bit to its design and code, but after a lengthy discussion, the dotnet extensions team decided on the current design, so I'm keeping my own ManualTimeProvider around.

In the latest release of ManualTimeProvider I also made it possible to auto-trigger timers one or more times. That can be useful if the timers are being created and will run in a different thread/async task. Without the auto-trigger feature, the test code doesn't deterministically know when a timer has been created and thus does not know when to advance time.

Great to hear .NET 6/7 versions of Polly will also use and support TimeProvider.

@martincostello martincostello modified the milestones: v8.0.0, v8.1.0 Sep 26, 2023
@egil
Copy link

egil commented Sep 29, 2023

@martincostello I just installed Polly v8 and I cannot see any places where I can provide a TimeProvider to Polly, which would allow me to replace it during testing.

Are you not allowing Polly users to provide a TimeProvider?

@martincostello
Copy link
Member

TimeProvider will be exposed in a future release when we can depend on the 8.0.0 package - we didn't want to ship our 8.0.0 package with a dependency on a prerelease package.

You can follow #1144 for the changes to support TimeProvider.

@egil
Copy link

egil commented Sep 29, 2023

Ahh ok, that makes sense.

@martincostello
Copy link
Member

We'll look to ship that change as soon as possible after .NET 8 RTM.

@martincostello martincostello modified the milestones: v8.1.0, v8.2.0 Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 Issues related to the new version 8 of the Polly library.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants