feat(nuget): Make nuget restore errors non fatal

[dupdob]

Slight change: make nuget failures non fatal (log error but continue), as it makes no sense for the general case, especially since selecting the proper msbuild and nuget combination is tricky on complex environment.

Having a proper handling of nuget version would be better, but not sure it is worth the needed effort

Fixes #3067.

[rouke-broersma]

Nuget restore is not optional iirc because buildalyzer performs a clean which deletes the packages, I'm not sure this would be a valid scenario?

[dupdob]

Well, in practice, Stryker only make a package restore attempt if the analysis is failed. Judging by the log the author of #3067 provided me, Nuget restore did not even happen in V4.0.6 for his/her project. It happens only with V4.1+ (because 1 project analysis fails for some reason). In any case, this results in aborting the whole mutation session, which is a shame.

Digging into Nuget issues, I discovered that 'unable to find MsBuild version xxx' was a recurrent issue with no clear solution nor workaround, so I am not sure we can fix this properly.

As an alternative, we could also retry a failed nuget session WITHOUT specifying the msbuild version.
But should we abort is this also fails?

[dupdob]

remark: I did not notice there was a test explicitly testing for failure; my test runner (NCrunch) does not support yet the platform conditional attributes 😄 , so this test is not visible on my ide.
My bad

[rouke-broersma]

Well, in practice, Stryker only make a package restore attempt if the analysis is failed. Judging by the log the author of #3067 provided me, Nuget restore did not even happen in V4.0.6 for his/her project. It happens only with V4.1+ (because 1 project analysis fails for some reason). In any case, this results in aborting the whole mutation session, which is a shame.

Digging into Nuget issues, I discovered that 'unable to find MsBuild version xxx' was a recurrent issue with no clear solution nor workaround, so I am not sure we can fix this properly.

As an alternative, we could also retry a failed nuget session WITHOUT specifying the msbuild version. But should we abort is this also fails?

Yes that makes sense. We added the nuget version because of other conflicts between nuget version and msbuild version, but if it fails regardless we might retry without a specific version.

If I remember correctly we used to even do restore -> analyze -> restore because analyze would always fail without restore, but the analzse would clear the nuget packages and then compilation would not work. Not sure if this is still what we do nor if it's still relevant, but iirc this is how we first made dotnet framework support functional.

But now that I think about it, the restore might only be mandatory for projects that use nuget.config + .packages folder instead of PackageReference and global package cache, so for modern (dotnet framework) sdk style projects it might no longer be required for analyze.

[dupdob]

If I remember correctly we used to even do restore -> analyze -> restore because analyze would always fail without restore, but the analzse would clear the nuget packages and then compilation would not work. Not sure if this is still what we do nor if it's still relevant, but iirc this is how we first made dotnet framework support functional.

I remember it too.

But now that I think about it, the restore might only be mandatory for projects that use nuget.config + .packages folder instead of PackageReference and global package cache, so for modern (dotnet framework) sdk style projects it might no longer be required for analyze.

I made some attempts to discover when nuget was needed when I rewrote the project analysis phase (hoping to get rid of it altogether): failed to understand when and why it was needed. But it is still sometimes needed. 🤷‍♂️

I added a retry logic without msbuildversion.

[dupdob]

lol, it looks like we were both updating the branch simultaneously 😄

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud