Deterministic build

[iBrotNano]

The build action was broken by #870. I had to remove the net5.0 target framework to make it work. It is already out of support so it should be removed anyway.

This pull request targets #795.

The idea is to make the configuration in build.props conditional. The standard way to use Shouldly is by referencing it in a test project. It should be no big deal to alter the build configuration of those. But if Shouldly is used in a non-test project those settings can break their builds. By setting a property <UseShouldlyInNonTestProject>true</UseShouldlyInNonTestProject> in the CSPROJ the settings in build.props should be ignored.

This would address #795 (comment) without the need for multiple nugets.

It's an alternative to #795 (comment). Maybe more descriptive.

[sungam3r]

The build action was broken by #870

In what way?

It is already out of support so it should be removed anyway.

Wait a bit. Why? Just turning off this TFM you throw out the whole added code under #if NET5_0_OR_GREATER condition.

[iBrotNano]

The build action was broken by #870

In what way?

It is already out of support so it should be removed anyway.

Wait a bit. Why? Just turning off this TFM you throw out the whole added code under #if NET5_0_OR_GREATER condition.

This was the output of the build:

Warning: C:\Program Files\dotnet\sdk\7.0.200\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.EolTargetFrameworks.targets(28,5): warning NETSDK1138: The target framework 'net5' is out of support and will not receive security updates in the future. Please refer to https://aka.ms/dotnet-core-support for more information about the support policy. [D:\a\shouldly\shouldly\src\Shouldly\Shouldly.csproj::TargetFramework=net5]
D:\a\shouldly\shouldly\src\Shouldly\Shouldly.csproj : error NU1505: Warning As Error: Duplicate 'PackageDownload' items found. Remove the duplicate items or use the Update functionality to ensure a consistent restore behavior. The duplicate 'PackageDownload' items are: Microsoft.NETCore.App.Ref [5.0.0], Microsoft.NETCore.App.Ref [5.0.0]. [TargetFramework=net5]

Build FAILED.

It seems that adding the TFM net5.0 adds Microsoft.NETCore.App.Ref implicitly. That causes Warning As Error: Duplicate 'PackageDownload' items found.. But you are right. It would throw out the code for the type Half. I guess this was the purpose to add net5 at all.

I could observe that builds i made locally only show the first warning. Not the error NU1505: Warning As Error. This is only shown in the GitHub Action. Something must be different in the build container. I found dotnet/sdk#24747

It might depend on the NuGet version of the build sdk.

I think it would be best to add <WarningsNotAsErrors>NU1505</WarningsNotAsErrors> to show the warning but enforce that this specific warning is not treated as an error and leave net5 as TFM for now. You could upgrade later to net6.0 and/or net7.0

[iBrotNano]

Now the build in appveyor failed after I habe added documentation about the flag in README.md. The failing test is Shouldly.Tests.ShouldBe.ShouldBeUnique.ComparerScenario.ComparerNotEqualsShouldFail. I can't see how this could be related to a change in README.md.

[iBrotNano]

This seems not to work like expected. I am closing the PR until I found a solution.

[iBrotNano]

Ok. I figured out that MSBuild evaluates build properties in a way that it is not as easy as i thought to make it configureable. I described the configuration options and fallbacks in READEME.md.

[slang25]

I've had a look, and this looks good, however I'm at the conclusion the the build.props should never have been included.

The build.props settings are too intrusive, and now more than ever given there is less .NET Framework about, and deterministic builds are becoming more popular. I think we just remove it, and get the docs updated to address the side-effects if the assertion message stack walking fails.

[iBrotNano]

Hi,

what i could read in #795 was that there are two scenarios. The first is what most of the users will do. Use Shoudly in a test project. #795 (comment) explains that the settings in the build.props are needed to generate messages. I could see, that it is not possible to navigate from the message in VS Test Explorer to the source code if build.props are missing.

The second scenario is that people want to use Shouldly in a project as a dependency. In this case build.props might overwrite their build configuration. I think this is an edge case and #795 (comment) points out the problem.

I stumbled over this because of exactly this. For me this solution would be good enough. I can use all features locally in Visual Studio. On the build server I circumvent the settings with a flag.

[slang25]

@iBrotNano @sungam3r could you give me your opinion on #883

I'm leaning towards this because what's proposed in this PR is fine, if you already know what's gone wrong. I've had many reports of people spending hours of investigation to then find Shoudly being culprit, that's something I think we should be avoiding as much as possible.

[slang25]

Resolved in version 4.2.1