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

Build less of libs for NativeAOT testing #89153

Closed
wants to merge 2 commits into from

Conversation

MichalStrehovsky
Copy link
Member

We don't care about any of the System.Speech, Microsoft.Extensions.*,... stuff.

Made possible with #89005.

Cc @dotnet/ilc-contrib

We don't care about any of the System.Speech, Microsoft.Extensions.*,... stuff.
@ghost
Copy link

ghost commented Jul 19, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

We don't care about any of the System.Speech, Microsoft.Extensions.*,... stuff.

Made possible with #89005.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

MichalStrehovsky added a commit that referenced this pull request Jul 19, 2023
Similar to #89153. I don't think we need System.Speech & co for tools tests.
@@ -208,7 +208,7 @@ extends:
jobParameters:
timeoutInMinutes: 120
nameSuffix: NativeAOT
buildArgs: -s clr.aot+host.native+libs -rc $(_BuildConfig) -lc Release -hc Release
buildArgs: -s clr.aot+host.native+libs.sfx+libs.native -rc $(_BuildConfig) -lc Release -hc Release
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: build libs.native before libs.sfx. That's the expected order...

@MichalStrehovsky
Copy link
Member Author

Looking at the test failures, looks like EventPipe testing relies on some Microsoft.Extensions crap and since we test EventPipe as part of smoke tests now, we need the whole product. This would have been enough 6 months ago when we didn't have EventPipe, but not anymore. Oh well :'(.

@MichalStrehovsky MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch July 19, 2023 06:05
@MichalStrehovsky
Copy link
Member Author

I'm still going to use this in my personal workflows though because I don't run event pipe testing, so @ViktorHofer your #89005 is still super useful.

@ViktorHofer
Copy link
Member

As discussed offline with Michal, this would work just fine if the runtime tests in question could use ProjectReferences and bring in the required out-of-band libraries that are needed by the specific tests. Given that the current infra around runtime tests is fragile around PackageReferences/ProjectReferences we will not pursue this further at this point. cc @jkoritzinsky

@jkoritzinsky
Copy link
Member

Once we do part two of test consolidation, we can revisit this.

hoyosjs pushed a commit that referenced this pull request Jul 19, 2023
Similar to #89153. I don't think we need System.Speech & co for tools tests.
@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2023
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.

None yet

3 participants