Add support for .NET 8

[martincostello]

• Add net8.0 TFM.
• Bump packages to .NET 8 versions where relevant.
• Use the new System.TimeProvider abstraction to resolve Use the new TimeProvider API from .NET 8 #1070.

Relates to #1141, depends on #1738, resolves #1786.

[codecov-commenter]

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (65b0bdc) 84.56% compared to head (d3e0fec) 84.59%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
src/Polly/AsyncPolicy.ExecuteOverloads.cs	50.00%	6 Missing ⚠️
src/Polly/AsyncPolicy.TResult.ExecuteOverloads.cs	50.00%	3 Missing ⚠️
src/Polly/Policy.ExecuteOverloads.cs	75.00%	2 Missing ⚠️
src/Polly/Policy.TResult.ExecuteOverloads.cs	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1144      +/-   ##
==========================================
+ Coverage   84.56%   84.59%   +0.02%     
==========================================
  Files         308      308              
  Lines        6791     6802      +11     
  Branches     1044     1049       +5     
==========================================
+ Hits         5743     5754      +11     
  Misses        839      839              
  Partials      209      209              

Flag	Coverage Δ
linux	84.59% <83.09%> (+0.02%)	⬆️
macos	84.59% <83.09%> (+0.02%)	⬆️
windows	84.59% <83.09%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[martincostello]

@martintmk Do you want to push your changes to use System.TimeProvider into this branch?

[martintmk]

@martintmk Do you want to push your changes to use System.TimeProvider into this branch?

Yup, but first let me create another PR that's prerequisite for this. Then I'll push changes directly to yours.

[martincostello]

I'll rebase this once #1192 is merged, then you should be able to put the TimeProvider changes on top of that.

[martincostello]

@martintmk Could you rebase this onto main so we can update to preview 5?

[martintmk]

@martintmk Could you rebase this onto main so we can update to preview 5?

Rebased. Few tests are failing, we can address that later.

[martincostello]

I'll look at bumping this to preview 5 and fixing the tests today.

[martincostello]

I tweaked a few assertions with what I thought was the correct change, but I wasn't 100% what the intended behaviour change was for the other failures.

[martincostello]

I'll try and update this again once #1355 is merged.

[martincostello]

Blocked on adopting Microsoft.Extensions.TimeProvider.Testing until the fix for dotnet/runtime#88000 is in a publicly available preview of .NET 8, which is probably not going to be until preview 7.

[martincostello]

@martintmk Would be good to get some feedback on the changes in 7be0f93. I've removed the Moq-based implementation so we're not introspecting on the implementation details of the clock too much, but I'm not 100% sure the test refactoring is correct, even though they pass locally.

I also simplified the TimeProviderExtensions code as comparing it to the code in the NuGet version, we seem to duplicate a fair number of the conditions already in that code. If we just call it then we can remove the "is it a mock or not?" branching - assuming I understood it correctly.

[martintmk]

@martintmk Would be good to get some feedback on the changes in 7be0f93. I've removed the Moq-based implementation so we're not introspecting on the implementation details of the clock too much, but I'm not 100% sure the test refactoring is correct, even though they pass locally.

I also simplified the TimeProviderExtensions code as comparing it to the code in the NuGet version, we seem to duplicate a fair number of the conditions already in that code. If we just call it then we can remove the "is it a mock or not?" branching - assuming I understood it correctly.

@martincostello, the extra code in TimeProviderExtensions for the synchronous path was done to avoid the sync-over-async pattern. But honestly, I am pretty fine with this change. It makes out code simpler and it only happens when resilience event occurs, so not on the hot path.

Your test refactorings are fine, otherwise some mutants would have survived. But I suggest to make those changes in the main branch too.

[martincostello]

Cool, thanks. Yep, today I'll tidy up this branch a bit and then cherry-pick some of the changes to go into main now rather than wait on 8.0 RTM.

[martincostello]

Looks like there's a flaky test when testing for .NET Framework related to the metrics stuff.

[burger100]

Does it means Polly doesn't support .net version prior to .net 8.0 anymore ?
After an update to version 8.2.0 I got build error saying that TimeProvider is ambigous between our internal TimeProvider and the new System.TimeProvider added in .net8.0. Our project is still targetting .net7.0.

[martincostello]

No it doesn't - it just means that Polly now includes a net8.0 target and exposes a property to allow you to configure a custom TimeProvider.

Previously we also had our own internal copy of TimeProvider, which we were able to delete by using the NuGet package instead.

If you have your own copy of it you'll either need to remove it, rename it or alias it to resolve the conflict.

This isn't an issue unique to your use of Polly, you would have the same problem if you used any other NuGet package that also added a dependency on the new TimeProvider APIs.