Runtime: Delete tests specific to .NET Framework?

[danmosemsft]

It never was and is never going to be a goal to use CoreFX tests to find bugs in .NET Framework. Really their only value is as documentation, where we intentionally changed behavior, we created two similar tests next to each other. This is an expensive kind of documentation, that runs every day, inevitably fails from time to time (given how many there are, and the fact that .NET Core is more reliable now). It would be enough documentation to have a comment, or the mere fact that a test is excluded from .NET Framework runs.

There is still some value in having .NET Framework runs, and making new tests default to running on both. But there is a cost too, and in future more and more often, new tests will not apply to .NET Framework because new tests will mostly be for new API and new behaviors. Several times a week, someone writes a test that fails in CI only on .NET Framework, because they didn't think to exclude it. It would be nice to change the default, so new tests don't run on .NET Framework, but I expect that would be too fiddly (involve marking all our existing tests somehow). At some point we will probably want to stop the .NET Framework runs entirely, relying on the tests we have built up (and taking care when we change their assertions) but perhaps that's going a little too far for now (?)

Suggestion: make a pass and delete all tests and test branches that are .NET Framework only. Thoughts?

@jkotas @safern @stephentoub @ViktorHofer @ericstj @marek-safar @hughbe

[danmosemsft]

@davidsh

[stephentoub]

make a pass and delete all tests and test branches that are .NET Framework only

This sounds reasonable.

[marek-safar]

No objections from me.

[MarcoRossignoli]

Several times a week, someone writes a test that fails in CI only on .NET Framework, because they didn't think to exclude it.

Confirm often I don't do double testing(also time consuming), also 99% of time I fail on UAP, I don't know if the story is the same.

[danmosemsft]

@MarcoRossignoli note that this first step won't help much with that because tests will still run on both frameworks by default (although we might change that later, as mentioned above). For now this is proposing just getting rid of the tests that only apply to .NET Framework.

@jkotas ?

@davidsh @geoffkizer thoughts for networking?

[danmosemsft]

Oh, I see @jkotas already did thumbs up 👍

[jkotas]

This makes sense for components that ship in .NET Core Runtime where .NET Framework is not shipping config and we run the tests on .NET Framework just to check for parity.

We should continue to maintain .NET Framework specific paths coverage for components where .NET Framework is a shipping config. Examples of such components are System.IO.Pipelines or Microsoft.Diagnostics.Tracing.EventSource.Redist.

[ViktorHofer]

where .NET Framework is a shipping config

What about OOB packages with a .NET Standard <= 2.0 profile (consumable by .NET Framework)?

[jkotas]

What about OOB packages with a .NET Standard <= 2.0 profile (consumable by .NET Framework)?

.NET Framework is a shipping config for these. We should maintain the test coverage.

[davidsh]

@davidsh @geoffkizer thoughts for networking?

We should continue to maintain .NET Framework specific paths coverage for components where .NET Framework is a shipping config. Examples of such components are System.IO.Pipelines or Microsoft.Diagnostics.Tracing.EventSource.Redist.

This is the case for System.Net.Http.WinHttpHandler which is a separate NuGet package that ships on .NET Core and .NET Framework. While SocketsHttpHandler is the new default for the underlying HTTP stack of HttpClientHandler, WinHttpHandler still ships separately to address .NET Framework customers that need HTTP/2 and other features specific to WinHttpHandler.

As part of supporting that, we also use the broad set of Http tests in System.Net.Http library to run against .NET Core and .NET Framework. And we have ways of invoking WinHttpHandler instead of SocketsHttpHandler to run against for those tests.

I don't we are ready to see this test capability go away just yet.

[davidsh]

@karelz

[danmosemsft]

Up for grabs. If anyone in the community is interested in having a go at this, please let me know so that I can make sure you don't waste time on the libraries we don't want to change (the ones that ship downlevel)

[kislovs]

@danmosemsft I'm the guy from the community who is interested to help with that. Please, tell what libraries you want to clean up.
And, (_if it's possible_) give a couple of examples to help understand what exactly you want to clean up (I think it's be helpful for any future helpers).

[kmhigashioka]

Me too 🙋‍♂️

[danmosemsft]

@kislovs @kmhigashioka welcome.

You are looking for

1. Unit tests that _only_ apply to .NET Framework. These should simply be deleted

Tests are marked to only run on .NET Framework using attributes like this [SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)] or possibly [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFullFramework))] or similar. Such tests can simply be deleted.

1. Paths within other tests that only apply to .NET Framework. These paths should be deleted, retaining the test but marking it so it does not run on .NET Framework any more.

Tests designate a path that only applies to .NET Framework using conditions based on PlatformDetection.IsFullFramework or PlatformDetection.IsNetfx.....

To mark a test to explicitly not run on .NET Framework (remember, we will still by default run our tests on .NET Framework, we are just removing .NET Framework-specific tests and test codepaths) use [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] -- if there's already a similar attribute, or-it eg [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework | TargetFrameworkMonikers.Uap)]

1. Tests that are temporarily disabled for the .NET Framework can now be permanently disabled for the .NET Framework. These may be found because they have attributes similar to [ActiveIssue(..., TargetFrameworkMonikers.NetFramework ... )]. You can disable them as in 2 above.

Feel free to preserve any useful comments for 2 and 3 above.

For all this effort we want to exclude certain libraries that are still relevant to .NET Framework. Let me define those below.

[danmosemsft]

@ericstj what is the simplest way to designate the libraries that are still shipped for .NET Framework?
Eg., they have a pkgproj and within that is a ProjectReference to their src whose SupportedFramework list includes net.... Is that right? Is there a better way?

[ViktorHofer]

I think just looking at Configuration.props in the pkg folder should be enough. Check if netfx|net\d{2,3} is present.

[ViktorHofer]

Or do you also want to find packages that harvest netfx resources? I would not recommend including those to the exclusion list as they will stay in the package infinitely.

You probably already thought about that but please don't forget to include packages with a <= netstandard2.0 configuration to the exclusion list.

[ericstj]

looking at Configuration.props in the pkg folder

Should be src folder, and yes it is any one that has a net* or netstandard2.0 or less value.

Another way that's more programmatic is to build the -allconfigurations leg, then examine the package reports (eg: f:dotnet\corefx2\artifacts\packages\Debug\reports) for all .netframework targets which have a locally built binary (eg: under artifacts). You can do this with JSON, or reference the packaging task and use our object model.

[safern]

Should be src folder, and yes it is any one that has a net* or netstandard2.0 or less value.

To test it, we should actually build it in the netfx vertical, so only values that would apply, would be netstandard/netfx right? If not we can't run its tests against netfx, right?

[ericstj]

Yeah since you're only looking for a true/false that'd work. You can just do a netfx vertical build then examine the runtime folder (F:dotnet\corefx2\artifacts\bin\runtime\netfx-Windows_NT-Debug-x64), omitting stuff that was copied instead of built.

[danmosemsft]

I'm trying to figure what that means in concrete terms. I built build -f netfx and now I have lots of files in artifacts\bin\netfx\netfx-Debug but they all have the same timestamp and they definitely mostly aren't in this category. I think folks here need explicit instructions.

[safern]

I don't think you need to look under artifacts\bin\netfx -- they should look under artifacts\bin\runtime\netfx-Windows-*\ which is where we binplace what we build. However that folder has more than just what we build, we also bring some stuff from packages and some tooling. So I just wrote a small msbuild target, and plumb it into Directory.Build.targets that gets whatever is been binplaced with a matching configuration in a netfx vertical:

<Target Name="GetBuiltItemsNetfx"
          Condition="'@(BinPlaceDir)' != '' AND '$(IsSourceProject)' == 'true'"
          AfterTargets="BinPlaceFiles">

      <MakeDir Directories="E:\repos\corefxCopy\corefx\artifacts\tempNetfx" />

      <Copy SourceFiles="@(_BinPlaceItems)"
          OverwriteReadOnlyFiles="true"
          DestinationFolder="E:\repos\corefxCopy\corefx\artifacts\tempNetfx" />
  </Target>

Then just ran build -f netfx.

The output, which I believe are the projects we care about since are the ones that have a netfx or netstandard configuration and are picked in our vertical, are:

Microsoft.Diagnostics.Tracing.EventSource
Microsoft.IO.Redist
Microsoft.Win32.Registry.AccessControl
Microsoft.Win32.Registry
Microsoft.Win32.SystemEvents
System.CodeDom
System.Collections.Immutable
System.ComponentModel.Annotations
System.Composition.AttributedModel
System.Composition.Convention
System.Composition.Hosting
System.Composition.Runtime
System.Composition.TypedParts
System.Configuration.ConfigurationManager
System.Data.Odbc
System.Data.SqlClient
System.Diagnostics.DiagnosticSource
System.Diagnostics.EventLog
System.Diagnostics.PerformanceCounter
System.Drawing.Common
System.IO.FileSystem.AccessControl
System.IO.Packaging
System.IO.Pipelines
System.IO.Ports
System.Json
System.Net.Http.WinHttpHandler
System.Net.WebSockets.WebSocketProtocol
System.Numerics.Tensors
System.Numerics.Vectors
System.Reflection.DispatchProxy
System.Reflection.Emit
System.Reflection.Emit.ILGeneration
System.Reflection.Emit.Lightweight
System.Reflection.Metadata
System.Reflection.MetadataLoadContext
System.Reflection.TypeExtensions
System.Runtime.CompilerServices.Unsafe
System.Runtime.WindowsRuntime
System.Runtime.WindowsRuntime.UI.Xaml
System.Security.AccessControl
System.Security.Cryptography.Cng
System.Security.Cryptography.OpenSsl
System.Security.Cryptography.Pkcs
System.Security.Cryptography.ProtectedData
System.Security.Cryptography.Xml
System.Security.Permissions
System.Security.Principal.Windows
System.ServiceModel.Syndication
System.ServiceProcess.ServiceController
System.Text.Encoding.CodePages
System.Text.Encodings.Web
System.Text.Json
System.Threading.AccessControl
System.Threading.Channels
System.Threading.Tasks.Dataflow

also
 System.Runtime.Serialization.Formatters

[danmosemsft]

Thanks @safern. OK @kmhigashioka @kislovs are you good to go? If you're still interested, please go through libraries in this repo that are NOT in the list above, and follow the guidance I put above.

Thanks! That will simplify our tests.

[ViktorHofer]

Also exclude System.Runtime.Serialization.Formatters as that is important for cross-serialization testing.

[kislovs]

@danmosemsft yeah, sure. I will take a look when I will have free time :+1:

[kmhigashioka]

:+1:

[danmosemsft]

Thanks @kislovs @kmhigashioka