[WIP] build-test - fix several thousands C# and NuGet warnings #19109
Conversation
|
cc @jashook @AaronRobinsonMSFT pls cc anyone else who should be involved |
tests/scripts/scripts.csproj
Outdated
| @@ -29,6 +29,7 @@ | |||
| <PrereleaseResolveNuGetPackages>false</PrereleaseResolveNuGetPackages> | |||
| <RuntimeIdentifiers>win7-x86;win7-x64</RuntimeIdentifiers> | |||
| <IsTestProject>false</IsTestProject> | |||
| <NoWarn>NU1603</NoWarn> | |||
There was a problem hiding this comment.
This could be resolved, instead, by updating to the actual version resolved.
There was a problem hiding this comment.
NU1603 is an upgrade warning
| @@ -25,6 +25,7 @@ | |||
| <RuntimeIdentifier Condition="'$(OSGroup)' == 'Linux'">linux-x64</RuntimeIdentifier> | |||
| <RuntimeIdentifier Condition="'$(OSGroup)' == 'OSX'">osx-x64</RuntimeIdentifier> | |||
| <NugetRuntimeIdentifier>$(RuntimeIdentifier)</NugetRuntimeIdentifier> | |||
| <NoWarn>NU1603,NU1605</NoWarn> | |||
There was a problem hiding this comment.
NU1605 could also be resolved by updating to the actual version resolved (this is a downgrade warning).
| @@ -55,7 +55,7 @@ | |||
| <AddAdditionalExplicitAssemblyReferences>false</AddAdditionalExplicitAssemblyReferences> | |||
| <GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute> | |||
| <!-- Disable some C# warnings for the tests. --> | |||
| <NoWarn>78,162,164,168,169,219,251,252,414,429,642,649,652,675,1691,1717,1718,3001,3002,3003,3005,3008</NoWarn> | |||
| <NoWarn>78,162,164,168,169,219,251,252,414,429,618,642,649,652,675,1685,1691,1717,1718,3001,3002,3003,3005,3008</NoWarn> | |||
There was a problem hiding this comment.
We shouldn't hide CS0618, it is listing code being used that was marked Obsolete
There was a problem hiding this comment.
I agree with this for production code, but I think it is fine to disable it for tests.
We need to have tests for obsolete APIs. It is pretty annoying to disable the warning individually around every test that is testing obsolete API.
There was a problem hiding this comment.
Agreed. Testing code has different requirements with respect to code marked Obsolete. Regardless if it is marked that way or not users can use the API and as such it must be tested.
| @@ -55,7 +55,7 @@ | |||
| <AddAdditionalExplicitAssemblyReferences>false</AddAdditionalExplicitAssemblyReferences> | |||
| <GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute> | |||
| <!-- Disable some C# warnings for the tests. --> | |||
| <NoWarn>78,162,164,168,169,219,251,252,414,429,642,649,652,675,1691,1717,1718,3001,3002,3003,3005,3008</NoWarn> | |||
| <NoWarn>78,162,164,168,169,219,251,252,414,429,618,642,649,652,675,1685,1691,1717,1718,3001,3002,3003,3005,3008</NoWarn> | |||
There was a problem hiding this comment.
CS1685 sounds like RefAssemblies are misconfigured somewhere.
There was a problem hiding this comment.
CS1685 was left due to the https://github.com/dotnet/corefx/issues/31395. As long as benchmark.csproj and performance.csproj are built against incompatible frameworks (netstandard1.4 and netstandard1.6 with netcoreapp3.0 explicitly referenced assemblies) CS1685 will persist. Best solution would be to move this projects to netstandard2.0 which is according to tools compatible with netcoreapp3.0
tests/runtest.proj
Outdated
| @@ -255,7 +255,7 @@ namespace $([System.String]::Copy($(Category)).Replace(".","_").Replace("\",""). | |||
| { | |||
| sErrorText = System.IO.File.ReadAllText(errorFile)%3B | |||
| } | |||
| catch(Exception ex) | |||
There was a problem hiding this comment.
Please include this in the message. This is a test and as such we need the exception details. It is easy to address:
sErrorText = $"Unable to read error file: {errorFIle}\n{ex}"%3B
Please also update the logic in the other location for the exception.
| @@ -18,7 +18,7 @@ | |||
| <Version>$(XunitPackageVersion)</Version> | |||
| </PackageReference> | |||
| <PackageReference Include="Microsoft.DotNet.BuildTools.TestSuite"> | |||
| <Version>1.0.0-prerelease-00629-04</Version> | |||
| <Version>1.0.0-prerelease-00704-04</Version> | |||
There was a problem hiding this comment.
Package v1.0.0-prerelease-00629-04 is not available, updated to version resolved by NuGet
| <SystemCompositionVersions>1.3.0-preview3-26501-04</SystemCompositionVersions> | ||
| <XUnitNetcoreExtensionsVersion>2.2.0-preview1-02902-01</XUnitNetcoreExtensionsVersion> | ||
| <SystemCompositionVersions>1.3.0-$(MicrosoftPrivateCoreFxNETCoreAppPackageVersion.Substring(6))</SystemCompositionVersions> | ||
| <XUnitNetcoreExtensionsVersion>2.2.0-preview1-03025-01</XUnitNetcoreExtensionsVersion> |
There was a problem hiding this comment.
Composition packages version has been updated to always point to latest v1.3.0 available. Version 1.3.0-preview3-26501-04 has been always chosen over any 1.3.0-preview1-xxxxx-xx see https://github.com/dotnet/corefx/issues/31395
| <PackageReference Include="System.Memory"> | ||
| <Version>$(MicrosoftPrivateCoreFxNETCoreAppPackageVersion)</Version> | ||
| <NoWarn>NU1603</NoWarn> | ||
| <!-- Warnings NU1603 and NU1605 are raised due to the presence of v4.6.0-preview3-2650x-xx assemblies on myget--> | ||
| <!-- They prevent usage of newer 4.6.0-preview1 assemblies since NuGet treats preview3 as version always higher than preview1 --> | ||
| </PackageReference> |
There was a problem hiding this comment.
Warnings suppression is due to: https://github.com/dotnet/corefx/issues/31395
4creators
changed the title
|
Closing as changes are handled by multiple PRs |
Recent changes to test harness introduced several thousands new warnings from both C# code and NuGet references. This PR fixes most of them.