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

Don't build ReferenceSystemPrivateCoreLib tests with Mono #58232

Merged
merged 5 commits into from
Aug 30, 2021

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Aug 27, 2021

New tests that extend the System.Private.CoreLib API should add to the ref assemblies in src/libraries and the tests themselves should be library tests.

Updated the workflow doc: the Clr.Native subset is all that is need if we're interested in testing Mono.

Contributes to #58266

When building tests that reference System.Private.CoreLib, use the one
from the specified RuntimeFlavor (or fall back to coreclr if it's
unset - although src/tests/build.sh sets it always)

This means we only need to build the Clr.Native subset if we're
interested in testing Mono.
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Aug 27, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

When building tests that reference System.Private.CoreLib, use the one from the specified RuntimeFlavor (or fall back to coreclr if it's unset - although src/tests/build.sh sets it always)

This means we only need to build the Clr.Native subset if we're interested in testing Mono.

Author: lambdageek
Assignees: -
Labels:

area-Infrastructure

Milestone: -

@lambdageek
Copy link
Member Author

Even better would be if there was a Clr.Run subset that just build corerun, but let's start with baby steps.

@lambdageek lambdageek marked this pull request as draft August 27, 2021 03:20
@lambdageek lambdageek force-pushed the cleanup-mono-testing-no-full-clr branch 3 times, most recently from 781bfe7 to a46acb0 Compare August 27, 2021 04:31
@lambdageek lambdageek force-pushed the cleanup-mono-testing-no-full-clr branch from a46acb0 to 8b82790 Compare August 27, 2021 04:33
Some tests depend on the intrenals of CoreCLR's
System.Private.CoreLib (specifically
Internal.Runtime.InteropServices).  Don't build those tests when
compiling against Mono's System.Private.CoreLib.
@lambdageek lambdageek changed the title [tests] Use RuntimeFlavor to build ReferenceSystemPrivateCoreLib tests Build runtime tests using RuntimeFlavor System.Private.CoreLib Aug 27, 2021
@lambdageek lambdageek changed the title Build runtime tests using RuntimeFlavor System.Private.CoreLib Build runtime tests using RuntimeFlavor-specific System.Private.CoreLib Aug 27, 2021
@jkotas
Copy link
Member

jkotas commented Aug 27, 2021

for example while working on a new corelib API

The typical workflow for working on a new corelib API is to add it to reference assembly under libraries and then you do not need to worry about referencing corelib from tests at all, and you can also write the tests under libraries that is the preferred place for API tests.

I think we should be working on getting rid of the tests that reference CoreLib directly, no need to grow the hack.

@lambdageek lambdageek marked this pull request as ready for review August 27, 2021 15:11
@lambdageek lambdageek force-pushed the cleanup-mono-testing-no-full-clr branch from 90a0ee1 to 1e354e0 Compare August 27, 2021 15:32
@lambdageek lambdageek changed the title Build runtime tests using RuntimeFlavor-specific System.Private.CoreLib Don't build ReferenceSystemPrivateCoreLib tests with Mono Aug 27, 2021
@lambdageek
Copy link
Member Author

I think we should be working on getting rid of the tests that reference CoreLib directly, no need to grow the hack.

👍

We should avoid adding new ReferenceSystemPrivateCoreLib tests
@lambdageek lambdageek merged commit d033e1a into dotnet:main Aug 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants