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

Bring up full library test CI runs on iOS / tvOS devices #59503

Merged
merged 80 commits into from
Nov 2, 2021

Conversation

steveisok
Copy link
Member

This change enables device runs on CI by building each test app on the helix instance it was deployed to. In past attempts, we looked at enhancing what takes place on the build machine via build tricks, compressing / cleaning up files early, etc and we could not overcome the need for excessively long timeouts and far more disk space.

The change also adopts the patterns established in the wasm test build, giving us the opportunity to support different scenario runs as well as workloads testing in the future.

Once the test runs turn green, we'll update the yml to only run the configurations on the rolling build.

@ghost
Copy link

ghost commented Sep 22, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

This change enables device runs on CI by building each test app on the helix instance it was deployed to. In past attempts, we looked at enhancing what takes place on the build machine via build tricks, compressing / cleaning up files early, etc and we could not overcome the need for excessively long timeouts and far more disk space.

The change also adopts the patterns established in the wasm test build, giving us the opportunity to support different scenario runs as well as workloads testing in the future.

Once the test runs turn green, we'll update the yml to only run the configurations on the rolling build.

Author: steveisok
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@steveisok
Copy link
Member Author

What the change also supports is being able to build and run fullaot on any of the other apple configurations. MacCatalyst being the other one we want to run, but up until this point haven't been able to.

This change enables device runs on CI by building each test app on the helix instance it was deployed to.  In past attempts, we looked at enhancing the build step only via build tricks, compressing / cleaning up files early, etc and we could not overcome the need for excessively long timeouts and far more disk space.

The change also adopts the patterns established in the wasm test build, giving us the opportunity to support different scenario runs as well as workloads testing in the future.
eng/testing/tests.ioslike.targets Show resolved Hide resolved
eng/testing/tests.ioslike.targets Outdated Show resolved Hide resolved
eng/testing/tests.targets Outdated Show resolved Hide resolved
src/libraries/sendtohelix.proj Outdated Show resolved Hide resolved
src/libraries/sendtohelixhelp.proj Outdated Show resolved Hide resolved
@@ -87,6 +91,12 @@
<HelixPostCommand Include="taskkill.exe /f /im corerun.exe"/>
</ItemGroup>

<ItemGroup Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'iOSSimulator'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it for iOS only for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, is good for both. Should include tvOS and tvOSSimulator as well

Copy link
Member

Choose a reason for hiding this comment

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

Should it be added in this PR?

src/libraries/sendtohelixhelp.proj Outdated Show resolved Hide resolved
src/libraries/tests.proj Show resolved Hide resolved
@premun
Copy link
Member

premun commented Sep 23, 2021

@steveisok if it's the same to you, let's start with tvOS rather than iOS. We just have way more devices there.

Or if iOS is more important, I'd prefer to do both to be honest. Let's start getting data and see how the tests go and we can start working on infrastructural issues

@steveisok steveisok marked this pull request as ready for review October 21, 2021 12:51
steveisok pushed a commit to steveisok/runtime that referenced this pull request Oct 21, 2021
In an effort to better utilize CI resources, this change will only run the System.Runtime library test per PR for Android and Wasm configurations.  The full libraries and runtime tests will move to only run on the post-PR validation pipeline.

Note: iOS/tvOS/MacCatalyst are excluded until dotnet#59503 lands
@premun
Copy link
Member

premun commented Oct 26, 2021

Nice! The job was 32 hours of work but done within an hour, this is feasible for a rolling build!

eng/testing/tests.targets Outdated Show resolved Hide resolved
eng/testing/tests.targets Outdated Show resolved Hide resolved
eng/testing/tests.targets Show resolved Hide resolved
eng/testing/tests.ioslike.targets Outdated Show resolved Hide resolved
src/libraries/sendtohelixhelp.proj Show resolved Hide resolved
@@ -87,6 +91,12 @@
<HelixPostCommand Include="taskkill.exe /f /im corerun.exe"/>
</ItemGroup>

<ItemGroup Condition="'$(TargetOS)' == 'iOS' or '$(TargetOS)' == 'iOSSimulator'">
Copy link
Member

Choose a reason for hiding this comment

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

Should it be added in this PR?

src/libraries/sendtohelixhelp.proj Outdated Show resolved Hide resolved
src/libraries/sendtohelixhelp.proj Outdated Show resolved Hide resolved
<UsingTask TaskName="MonoAOTCompiler" AssemblyFile="$(MonoAOTCompilerTasksAssemblyPath)" />
<UsingTask TaskName="RuntimeConfigParserTask" AssemblyFile="$(MonoTargetsTasksAssemblyPath)" />

<!-- TODO: this breaks runtime tests on Helix due to the file not being there for some reason. Once this is fixed we can remove the UpdateRuntimePack target here -->
Copy link
Member

Choose a reason for hiding this comment

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

Should we get an issue to track this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. There may be already. Will look

Copy link
Member

Choose a reason for hiding this comment

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

we should file an issue for this if it doesn't exist.

@steveisok
Copy link
Member Author

@akoeplinger Please give this a review when you have a moment.

@steveisok steveisok mentioned this pull request Oct 29, 2021
6 tasks
steveisok added a commit that referenced this pull request Oct 29, 2021
…60727)

In an effort to better utilize CI resources, this change will only run the System.Runtime library test per PR for Android and Wasm configurations. The full libraries and runtime tests will move to only run on the post-PR validation pipeline.

The summary of changes are:

- System.Runtime tests will only trigger per PR for Android x86/x64 and Wasm AOT / EAT
- All mono runtime builds will only run on the post PR validation build (rolling build)
- WasmBuildTests will only run on the post PR validation build (rolling build)
- Wasm on Windows will only build

Note: iOS/tvOS/MacCatalyst are excluded until #59503 lands
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

Left a few comments but looks great overall :)

eng/pipelines/runtime-staging.yml Outdated Show resolved Hide resolved
<ResolvedFileToPublish TrimMode="$(TrimMode)" />

<!-- Don't trim the main assembly.
TrimMode="" is needed so the root assemblies are correctly identified -->
<ResolvedFileToPublish TrimMode="" Condition="'%(FileName)' == '$(AssemblyName)'" />

<!-- TODO: find out why these assemblies aren't copied by copyused even though they're referenced -->
<ResolvedFileToPublish TrimMode="Copy" Condition="'$(EnableSoftTrimming)' == 'true' and '%(FileName)' == 'System.ComponentModel.EventBasedAsync'" />
Copy link
Member

Choose a reason for hiding this comment

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

looks like this is still unaddressed, we should have a tracking issue for it

<UsingTask TaskName="MonoAOTCompiler" AssemblyFile="$(MonoAOTCompilerTasksAssemblyPath)" />
<UsingTask TaskName="RuntimeConfigParserTask" AssemblyFile="$(MonoTargetsTasksAssemblyPath)" />

<!-- TODO: this breaks runtime tests on Helix due to the file not being there for some reason. Once this is fixed we can remove the UpdateRuntimePack target here -->
Copy link
Member

Choose a reason for hiding this comment

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

we should file an issue for this if it doesn't exist.

@steveisok
Copy link
Member Author

I'll add the issues and reference them in a separate PR.

@steveisok steveisok merged commit 8c6aeea into dotnet:main Nov 2, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 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.

6 participants