Add timeout exception for .NET 5 or greater targets.

[benjaminsampica]

Closes #49 .

[benjaminsampica]

I noticed there is a slight mismatch between the MockHttp.Tests project multi-targeting and the MockHttp project multi-targeting so the #if preprocessor condition fails specifically on the .net5.0 target (and thus the build). Not sure what you'd like to do here as this feature on HttpClient came in as part of .NET 5 but MockHttp doesn't target .NET 5.

[skwasjer]

Thanks!

Hmm, yea, the issue is indeed that .NET 5 and .NET Core 3.1 test runs resolve the NET Standard 2.x assemblies.

I removed .NET 5 from the runtime code as it was EOL as I'm sure you are aware, but IIRC I still had to use .NET 5 in the test project as a bit of a hack to run tests against the .NET Standard 2.1 build, which has some minor nuances (https://github.com/skwasjer/MockHttp/search?q=netstandard2_1).

To be honest, I dont think there's not a good way to fix it except for doing a runtime check (if .NET Standard 2.x). Because fundamentally HttpClient lives in the .NET runtime, not .NET Standard, which is the problem here. You could build and run .NET Core 2.0, 2.1, 3.0 and 3.1, 5.0 all against the .NET Standard 2.0 version of MockHttp, but the HttpClient has nuances amongst all those versions.

So, with that said, here's some example macros to test a specific .NET (Core) version, that could be used in context of #if NETSTANDARD2_0_OR_GREATER:

using System.Runtime.InteropServices;

private static bool IsNetCore => RuntimeInformation.FrameworkDescription.StartsWith(".NET Core", StringComparison.OrdinalIgnoreCase);
private static bool IsNetCore31 => RuntimeInformation.FrameworkDescription.StartsWith(".NET Core 3.1", StringComparison.OrdinalIgnoreCase);
private static bool IsNet5 => RuntimeInformation.FrameworkDescription.StartsWith(".NET 5", StringComparison.OrdinalIgnoreCase);

You can still skip this for .NET 6 and up ofc and use the preprocessor directive for it instead.

wdyt or other ideas?

[benjaminsampica]

Thanks for the detailed response! The only other option that comes to mind besides something runtime would be adding back assembly building for .Net 5.0 since it has nuances specific to (starting at) that runtime. I understand it’s EOL but I also have plenty of legacy code that I’m walking forward, too, and such things aren’t always so easy to drop. 😅

LMK how you’d like to proceed!

[skwasjer]

Bringing back .NET 5 introduces another issue. Test coverage is currently automatically picking the appropriate target framework version as follows:

Test runtime	MockHttp target runtime
.NET 7	.NET 7
.NET 6	.NET 6
.NET 5	.NET Standard 2.1
.NET Core 3.1	.NET Standard 2.1 (this is actually a mistake, I intended to have it run against 2.0 but forgot to configure it)
.NET 4.8	.NET 4.8
.NET 4.7.2	.NET 4.7.2
.NET 4.6.2	.NET 4.6.2

So bringing .NET 5 back would now favor the .NET 5 version over .NET Standard 2.1 in the tests. To fill that hole, it also means adding .NET Core 2.1 back (ugh!!) to run tests against .NET Standard 2.0 so that .NET Core 3.1 can keep testing against .NET Standard 2.1. It is a little more involved to maintain all these targets 😄

I am not completely against it, but the next question becomes, for how long do I have to maintain it? As such, to make life easier, that's why I typically follow the EOL roadmap.

A runtime check is simpler, but also won't win beauty contest though. But I would accept it if you need a quick fix. If you really want/need a .NET 5 target, I would prefer that to be done in a separate change/PR.

[edit] what I can do is accept this PR if you change it to work from .NET 6 onwards. Then can have a look again at bringing .NET 5 back and readdress this issue for it specifically.

[benjaminsampica]

Following the EOL schedule makes sense to me - I don’t expect you to back-cap if it’s a mess!

I’m down to either runtime check which, like you said, isn’t pretty, or change the pre processor to .net 6. If I had to pick, I’d probably choose the runtime check for the short term as the other doesn’t feel “complete” to me. If you decide later to republish the .NET 5 assembly, it would then be functionally the same and you could just cleanup the runtime check.

What do you think?

[skwasjer]

If I had to pick, I’d probably choose the runtime check for the short term as the other doesn’t feel “complete” to me.

Agreed 👍

[codecov]

Codecov Report

Merging #50 (538804e) into main (df1bbfa) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
src/MockHttp/Responses/TimeoutBehavior.cs	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[skwasjer]

Thanks a lot, don't mind the failed build, there's a flaky test of something unrelated, which I will address separately.