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

Add infrastructure for trimming and NativeAOT test apps. #48024

Merged
merged 9 commits into from
May 4, 2023

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented May 2, 2023

This infrastructure was taken from https://github.com/dotnet/runtime/tree/c62f69be1405a8e41b56ffc05f22d791bf4c7d2d/eng/testing/linker and modified to work in dotnet/aspnetcore.

Added the first test:

  • Generic host + value type container builder (verify that error is thrown)

Contributes to #45860

It is probably easiest to compare the files in eng/testing/linker to the files in dotnet/runtime to see what I changed. I can prepare that in a repo if people want. Or just use Beyond Compare (or similar) to diff between the two repos.

Working on hooking this into CI now.

This infrastructure was taken from https://github.com/dotnet/runtime/tree/c62f69be1405a8e41b56ffc05f22d791bf4c7d2d/eng/testing/linker and modified to work in dotnet/aspnetcore.

Added the first test:
- Generic host + value type container builder (verify that error is thrown)

Contributes to dotnet#45860
@eerhardt eerhardt requested review from JamesNK and joperezr May 2, 2023 16:31
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 2, 2023
@ghost
Copy link

ghost commented May 2, 2023

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Left some minor comments and questions, but other than that (and assuming the builds work 😃) this LGTM.

eerhardt added 7 commits May 2, 2023 16:56
Use `Sdk="Microsoft.NET.Sdk"` and hook into CoreBuild for the published app test projects in order to skip the build correctly.
Ensure the IsPublishedAppTestProject projects aren't built until the shared fx is built.
Skip "Build" and skip pubilshing for Helix.
@eerhardt eerhardt marked this pull request as ready for review May 3, 2023 23:58
@eerhardt eerhardt requested review from Tratcher, halter73, a team and wtgodbe as code owners May 3, 2023 23:58
@mitchdenny
Copy link
Member

So if I am reading this correctly, we basically have a set of console apps/ASP.NET apps in a single source file that we generate a project for, then publish (with AOT) and run them. And the return code from the program when it exits indicates success or failure?

@eerhardt
Copy link
Member Author

eerhardt commented May 4, 2023

So if I am reading this correctly, we basically have a set of console apps/ASP.NET apps in a single source file that we generate a project for, then publish (with AOT) and run them. And the return code from the program when it exits indicates success or failure?

💯 percent correct.

Directory.Build.props Outdated Show resolved Hide resolved
eng/testing/linker/trimmingTests.targets Show resolved Hide resolved
Reference the Microsoft.AspNetCore.App shared framework assemblies with ProjectReference.
This allows for easy dev workflow. Just need to update the product code, and re-run the tests.
-->
<ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Did app.ref not work?

Copy link
Member Author

Choose a reason for hiding this comment

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

It works, but it is a bit heavier than this approach.

For example, the "incremental" run of .\.dotnet\dotnet.exe test .\src\DefaultBuilder\test\Microsoft.AspNetCore.NativeAotTests\ takes ~15s using app.ref and takes ~10s with this approach.

We can always switch this as we go forward if we need to. But for now I'd rather do the minimal.

Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

Seems reasonable enough to me

@eerhardt eerhardt merged commit 2860263 into dotnet:main May 4, 2023
@eerhardt eerhardt deleted the AddTrimAotTestInfrastructure branch May 4, 2023 15:51
@ghost ghost added this to the 8.0-preview5 milestone May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants