-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[mono][infra] Disable failing apple mobile tests #92108
Merged
kotlarmilos
merged 12 commits into
dotnet:main
from
kotlarmilos:improvement/apple-mobile-jobs
Sep 25, 2023
+11
−3
Merged
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
1222bd7
Disable failing tests
kotlarmilos 3302d9a
Increase apple mobile Helix timeout
kotlarmilos 660b0fa
Increase apple mobile Helix timeout
kotlarmilos c60d371
Increase Helix timeout for apple mobile simulators
kotlarmilos 4a2ca56
Increase timeout
kotlarmilos acce514
Merge branch 'dotnet:main' into improvement/apple-mobile-jobs
kotlarmilos f2a9fbd
Add tracking issues and increase timeout for apple mobile simulators
kotlarmilos ee9dfef
Increase timeout
kotlarmilos b788171
Increase timeout
kotlarmilos 7d1bae2
Increase timeout
kotlarmilos ac2bdc0
Test the simulators without --reset-simulator
kotlarmilos 5cbab29
Update the timeout for simulators
kotlarmilos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3699,6 +3699,12 @@ | |
<ExcludeList Include = "$(XunitTestBinBase)/baseservices/exceptions/unhandled/**"> | ||
<Issue>System.Diagnostics.Process is not supported</Issue> | ||
</ExcludeList> | ||
<ExcludeList Include = "$(XunitTestBinBase)/JIT/SIMD/Vector3Interop_r/**"> | ||
<Issue>https://github.com/dotnet/runtime/issues/92129</Issue> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once this PR lands we should remove blocking CI labels. |
||
</ExcludeList> | ||
<ExcludeList Include = "$(XunitTestBinBase)/JIT/SIMD/Vector3Interop_ro/**"> | ||
<Issue>https://github.com/dotnet/runtime/issues/92129</Issue> | ||
</ExcludeList> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(TargetArchitecture)' == 'wasm' or '$(TargetsMobile)' == 'true'"> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
<AllowUnsafeBlocks>true</AllowUnsafeBlocks> | ||
<InvariantGlobalization>true</InvariantGlobalization> | ||
<CLRTestTargetUnsupported Condition="'$(IlcMultiModule)' == 'true'">true</CLRTestTargetUnsupported> | ||
<!-- Tracking issue: https://github.com/dotnet/runtime/issues/90460 --> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
<CLRTestTargetUnsupported Condition="'$(TargetOS)' == 'tvos' and '$(TargetArch)' == 'arm64'">true</CLRTestTargetUnsupported> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="StackTraceMetadata.cs" /> | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how I feel about this change. This has the potential to back things up considerably should the work item go on and on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that simulators are too slow. Since there is a coverage on devices, we may reduce the testing scope on the simulators, disabling the
JIT
andSIMD
subsets for example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 hours seems like too much to me. Is it truly that slow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does it spend the most amount of time: building vs execution ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tvossimulator-x64 Release AllSubsets_Mono_RuntimeTests:
During the cancelled
Send to Helix
job -JIT_Intrinsics
alone take up:~24mins
fromhttps://helix.dot.net/api/jobs/5ff7ae64-ef29-4e8c-88dd-3c3a7481ed6c/workitems/JIT_Intrinsics?api-version=2019-06-17
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually just a few minutes, and it's not changed recently. Then again, we don't have that many tests either.
I find it weird that the simulator is so much slower than device though for you (assuming you run the same set of tests for both), in our experience simulator has always been the faster of the two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test execution on simulators is not identical to the devices. For example, for
tracing/eventpipe/buffersize
the test duration is almost the same. However, there is a log on simulators after each test that kills the simulator, which may stale the execution:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the xharness it is expected:
https://github.com/dotnet/xharness/blob/a3a749a7056623c665bba226fe843152f413f044/src/Microsoft.DotNet.XHarness.Apple/Orchestration/BaseOrchestrator.cs#L476-L496
I measured the interval between two
tracing/eventpipe
tests. Assuming both test executions are approximately the same, it takes about 22s on a device and about 64s on a simulator, which is almost 3x slower.At the start of the test execution, xharness tries to shutdown the simulator:
After that, it restarts the simulator:
There is a command which looks like creating a new simulator at the start of the test execution:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simulator runs get unstable after running for a while. The shutdown / restart is meant to protect against that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the
xcrun simctl shutdown
andxcrun simctl boot
are time-consuming. Without the--reset-simulator
parameter, the tests take the same amount of time as they do for devices.I suggest disabling it for the runtime tests and monitoring the CI.