Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NETStandard test suite #26644

Closed
ericstj opened this issue Jun 28, 2018 · 18 comments
Closed

NETStandard test suite #26644

ericstj opened this issue Jun 28, 2018 · 18 comments
Labels
Milestone

Comments

@ericstj
Copy link
Member

ericstj commented Jun 28, 2018

I know we've discussed this before, but I don't recall an issue ever being filed. We have a lot of tests for things that are part of .NETStandard but don't necessarily build netstandard versions of the test library that represent the netstandard surface area, nor package up a suite of netstandard tests. I can see this being useful for other folks frameworks claiming NETStandard support (eg: mono, xamarin, unity).

/cc @bartonjs @weshaggard @terrajobst

@danmoseley
Copy link
Member

@marek-safar

We talked about this in the Xamarin/Mono context, but my understanding was that they were pulling our full set of netcoreapp tests, since they pull all our sources, not just what's necessary for .NET Standard.

@marek-safar
Copy link
Contributor

This has been discussed and the main motivation is to have the tests as the way to ensure and possibly measure how compliant is a specific implementation of netstandard.

We pull CoreFX tests but they have different tweaks or we have to even disable on some platforms because we have a very different implementation, hence having a well-defined test-suite would make it quite clear what the netstandard behaviour is supposed to be. Secondly, this is not only about Xamarin usage but also about other targets, e.g Unity, WebAssembly, TVs

@danmoseley
Copy link
Member

@marek-safar how does Unity test today (if they take a subset of Mono, is there a clear subset of the tests too)

It would be interesting to have some examples of bugs in Xamarin's .NET Standard implementation that are not caught by running all tests + diffing the .NET standard API surface area.

@marek-safar
Copy link
Contributor

I think Unity is still in process of adopting NS hence they just check the API surface area.

I think the sentence "running all tests" is enough. What is "all tests" the test suite has already countless of opt-in/out options and it's not that simple to run the same test suite if you are building the test very differently that CoreFX does. Secondly, the test coverage for code which is close to runtime is not as extensive as for pure managed code similarly for things which depend on underlying implementation like timezones, culture, string collation, etc

@danmoseley
Copy link
Member

Got it. Here's a start at a breakdown of issues to solve - none seem blocking

  1. Make build adjustments to be able to build only netstandard from the root and put it in a flat folder. You can currently build platform specific configurations (netfx, uap, netcoreapp) but this may mean some netstandard test configurations are never built at all currently because there is always a more specific configuration.

  2. Make a pass through netcoreapp tests to move into netstandard configuration as possible. When we made a pass through the build some months ago, we did make test libraries build against netstandard where we could. When not possible, we split netcoreapp and netstandard configurations -- most of our test libraries (119 of 155) have a netstandard configuration, 51 of those also have a netcoreapp configuration. However this division was not done fine grained, there will be files that mix netcoreapp and netstandard tests, either from before or after this work, and files in netcoreapp that need to be in netstandard, and maybe test libraries that should have netstandard configuration but don't.

  3. Fix tests that do build against netstandard API that themselves make platform assumptions eg that it is .NET Core. Easy to discover and either fix or add conditions.

  4. Identify tests that require netcoreapp API may be themselves adding important coverage of netstandard API. Not sure how we would find that - unless there's a way to measure code coverage of the API surface itself.

@weshaggard anything to add?

Assumption: for this purpose, everyone would consume test binaries built out of CoreFX. Is that reasonable @marek-safar ?

@bartonjs
Copy link
Member

  1. Make build adjustments to be able to build only netstandard from the root and put it in a flat folder.

This is pretty much the thoughts I had shared with Eric leading up to the issue creation. My expectation is that Xamarin / Mono / Unity / etc would be able to grab a version of the test assets "for netstandard" and then use a runner for their platform to execute them. There's some "devil in the details" around ActiveIssue / PlatformDetection, but it's a good start.

  1. Make a pass through netcoreapp tests to move into netstandard configuration as possible.

I think every assembly which is purposed as the primary source of tests for a type defined "in" netstandard should build a netstandard test asset. But there's definitely API in netcoreapp that isn't in netstandard, so the test libraries would still need split builds.

  1. Identify tests that require netcoreapp API may be themselves adding important coverage of netstandard API.

Nebulous thoughts are appearing in my head like "use a tool like the linker to drop out all externally callable members and any unreachable internal members. then normalize the ccov run". Ideally we wouldn't lose any coverage of the remaining assembly between the netstandard test execution and the netcoreapp execution. (Though I certainly expect that we're not currently in that ideal state).

@marek-safar
Copy link
Contributor

@danmosemsft I think if you build your test binary against netstandard.dll then that should work for everyone, perhaps having this as nuget could work as well.

@danmoseley
Copy link
Member

@ViktorHofer can xunit build against netstandard ie could this nuget package potentially include the test harness also? That would be convenient.

@ViktorHofer
Copy link
Member

netstandard is an API, not a platform. Due to the way builds and dependency resolution work today, xUnit.net test projects must target a platform (desktop CLR, .NET Core, etc.) and run with a platform-specific runner application.

The test harness must be platform specific and shouldn't be part of any netstandard package. Just speaking about the runner here. Thinking about assembling all test assemblies in a flat folder also requires that we add our test helpers as a netstandard library. The trickiest part here probably is the RemoteExecutor which is currently built specifically for netcoreapp and uap. We could of course include a (minimal - tree shaked) self contained netcoreapp variant of it that runs everywhere.

@weshaggard
Copy link
Member

As an FYI here are some of the issues already tracking some of these issues:

https:/dotnet/corefx/issues/15667
https:/dotnet/corefx/issues/16121
https:/dotnet/corefx/issues/25083

@danmoseley
Copy link
Member

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 16, 2018

There's some "devil in the details" around ActiveIssue / PlatformDetection, but it's a good start.

Our test support libs Microsoft.DotNet.XUnitExtensions and CoreFx.Private.TestUtilities both target netstandard but the latter is no real netstandard lib as it builds against specific implementations during the v-build. The former lives in arcade and is a true netstandard lib.

With my recent xunit work the non performance related test libs stop referencing our xunit.performance libs. The remaining references are the ones mentioned above, the xunit ones (xunit.core, xunit.assert, ...) and the RemoteExecutorConsoleApp which again targets netstandard but build against specific impls during the v-build.

Mono could build CoreFx.Private.TestUtilities and RemoteExecutorConsoleApp live or we just put our netfx implementations into the flat test folder. I believe it would make more sense to discuss if we want to promote these two projects to arcade and decouple it from corefx and put them into a nuget package I.e. Microsoft.DotNet.Standard.TestSupport.

Thoughts?

@danmoseley
Copy link
Member

@joperezr is working on (1) now.

@dotnet dotnet deleted a comment from kellis1990 Sep 1, 2018
@joperezr joperezr removed their assignment Sep 4, 2018
@joperezr
Copy link
Member

joperezr commented Sep 4, 2018

Unassigning since (1) is done. I also did (2) for System.Runtime.Tests.

@danmoseley
Copy link
Member

Thanks @joperezr . Has @marek-safar 's team had a chance to try it?

@danmoseley
Copy link
Member

Also, are the changes you made (to add the new vertical) now protected by CI ?

@joperezr
Copy link
Member

joperezr commented Sep 4, 2018

Has @marek-safar 's team had a chance to try it?

Nope, I have to sync up with him in order to do this.

Also, are the changes you made (to add the new vertical) now protected by CI ?

Yes, all configurations leg is now building the netstandard vertical for tests, so that would be red in case someone breaks it.

@ericstj
Copy link
Member Author

ericstj commented May 15, 2019

I believe we've adjusted the direction here and are instead planning to share netcoreapp tests with mono / xamarin. /cc @ViktorHofer @safern

@ericstj ericstj closed this as completed May 15, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants