-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[ios][tests] Run functional tests with Mono and Native AOT on Helix #87773
Changes from 105 commits
74481af
ca86967
98522fd
6ade906
0e7a920
1aca721
774ed6b
034fff1
a12779b
abfe7c1
09c6097
063995d
3693155
5344255
7c426a5
30ae9eb
fff4849
df51223
0e7325e
ce43877
6304c83
6eb91b7
228ba32
f42f06a
a667de7
e0ec3aa
a681087
eb23036
a811966
9374010
44f40ca
8d47870
2eecc7e
ae8ea04
f168689
72053e0
a5c8f8b
5ac774f
9b5641c
048a5b9
13fa800
8a46827
c8515cd
d715082
99cc75b
d0911d3
7a9cace
fd39b23
a826dbe
4438806
9e06c1f
29911b9
6a420d3
d2d83b0
917ce00
dc52f2f
9bb44e6
cd8a7f1
e7e4d1a
77f7cf4
b7b285e
486c1a3
0d7d88b
8470e8f
454f40f
e260892
e50a647
f3d9b25
f14e668
297c69f
751a911
3f28fd1
1ff7d9d
bbcc827
d8e7ebd
25a057c
cedc3f0
a2d57da
c06f211
161e45f
d992084
6d13f76
3517646
a35bbb7
05170f9
295d640
5d7af31
989cbca
3b20520
d092326
d4a3a56
3b62fc8
36368e2
496acec
aa66bb1
e47d986
cf56983
26955e0
7637762
e152e73
4af46f8
8a59bca
7dd5b27
8663185
2238ba2
cc5313a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,17 +16,22 @@ | |
<!-- running aot-helix tests locally, so we can test with the same project file as CI --> | ||
<_AOTBuildCommand Condition="'$(ContinuousIntegrationBuild)' != 'true'">$(_AOTBuildCommand) /p:RuntimeSrcDir=$(RepoRoot) /p:RuntimeConfig=$(Configuration)</_AOTBuildCommand> | ||
|
||
<_AOTBuildCommand>$(_AOTBuildCommand) /p:XHARNESS_EXECUTION_DIR="$XHARNESS_EXECUTION_DIR" /p:RunAOTCompilation=$(RunAOTCompilation) /p:TargetOS=$(TargetOS) /p:TargetArchitecture=$(TargetArchitecture) /p:MonoForceInterpreter=$(MonoForceInterpreter) /p:MonoEnableLLVM=true /p:DevTeamProvisioning=$(DevTeamProvisioning) /p:UsePortableRuntimePack=true /p:Configuration=$(Configuration)</_AOTBuildCommand> | ||
<_AOTBuildCommand>$(_AOTBuildCommand) /p:XHARNESS_EXECUTION_DIR="$XHARNESS_EXECUTION_DIR" /p:RunAOTCompilation=$(RunAOTCompilation) /p:UseNativeAOTRuntime=$(UseNativeAOTRuntime) /p:TargetOS=$(TargetOS) /p:TargetArchitecture=$(TargetArchitecture) /p:MonoForceInterpreter=$(MonoForceInterpreter) /p:DevTeamProvisioning=$(DevTeamProvisioning) /p:UsePortableRuntimePack=true /p:Configuration=$(Configuration)</_AOTBuildCommand> | ||
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. Just to clarify: You removed 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.
Correct, it should be set in the project file in the Helix payload. 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. In that case, I assume 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. When AOT compiling on Helix, there is a set of parameters passed when the script is invoked, such as RunAOTCompilation, UseNativeAOTRuntime, TargetOS, TargetArchitecture. These parameters are usually set through the build command. Additionally, there are parameters that configure AOT compilation like EnableLLVM, IncludesTestRunner, UseRuntimeComponents, etc. The MonoForceInterpreter parameter is passed through the build command, so it may be reasonable to keep it within the build command on the Helix as well. 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. Thanks. We should verify what is happening during the build of functional tests, as in functional tests project files the property is treated with: So since 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. Looking at the other functional tests project files, most of them have To conclude, I feel like this needs to be refactored (maybe not in this PR), with either removing any notion of @steveisok what do you think? 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. Here is a related change #52606. My understanding is that it was added for local development and testing. 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. All the relevant tests have passed. @steveisok What do you suggest doing here? 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. @ivanpovazan Reverted the changes to be on the safe side. Please take a look again at this PR and let's try to resolve comments before the RC1 snap. 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.
I would suggest keeping it and slowly unwinding in a follow up. |
||
<_AOTBuildCommand>$(_AOTBuildCommand) </_AOTBuildCommand> | ||
|
||
<_ResetSimulatorSwitch Condition="'$(TargetOS)' == 'iossimulator' or '$(TargetOS)' == 'tvossimulator'">--reset-simulator</_ResetSimulatorSwitch> | ||
<_SignalAppEndSwitch>--signal-app-end</_SignalAppEndSwitch> | ||
<_AppleSignCommand Condition="'$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvos'">sign "$app"</_AppleSignCommand> | ||
|
||
<IncludesTestRunner Condition="'$(IncludesTestRunner)' == ''">true</IncludesTestRunner> | ||
|
||
<_AppleBuildCommand Condition="'$(IncludesTestRunner)' == 'true'">apple test</_AppleBuildCommand> | ||
<_AppleBuildCommand Condition="'$(IncludesTestRunner)' != 'true'">apple run</_AppleBuildCommand> | ||
<_AppleExpectedExitCode Condition="'$(ExpectedExitCode)' != ''">--expected-exit-code $(ExpectedExitCode)</_AppleExpectedExitCode> | ||
<_AfterBuildCommands> | ||
mv $XHARNESS_OUT/AOTBuild.binlog "$HELIX_WORKITEM_UPLOAD_ROOT" | ||
$(_AppleSignCommand) | ||
xharness apple test --app "$app" --output-directory "$output_directory" --target "$target" --timeout "$timeout" --xcode "$xcode_path" -v --launch-timeout "$launch_timeout" $(_ResetSimulatorSwitch) $(_SignalAppEndSwitch) -- </_AfterBuildCommands> | ||
xharness $(_AppleBuildCommand) --app "$app" --output-directory "$output_directory" --target "$target" --timeout "$timeout" --xcode "$xcode_path" -v --launch-timeout "$launch_timeout" $(_ResetSimulatorSwitch) $(_SignalAppEndSwitch) $(_AppleExpectedExitCode) -- </_AfterBuildCommands> | ||
|
||
<RunScriptCommand>$(_AOTBuildCommand) $(_AfterBuildCommands)</RunScriptCommand> | ||
</PropertyGroup> | ||
|
@@ -66,11 +71,23 @@ | |
Include="@(AppleAssembliesToBundle)" TargetDir="publish\%(AppleAssembliesToBundle.RecursiveDir)" /> | ||
<BundleFiles Include="@(AppleNativeFilesToBundle)" TargetDir="publish\%(AppleNativeFilesToBundle.RecursiveDir)" /> | ||
<BundleFiles Include="$(RuntimeConfigFilePath)" TargetDir="publish" /> | ||
|
||
<BundleFiles Include="$(MonoProjectRoot)\msbuild\apple\data\*" TargetDir="publish" /> | ||
<ExtraFiles Condition="'%(AppleAssembliesToBundle._IsNative)' == 'true'" | ||
Include="@(AppleAssembliesToBundle)" TargetDir="extraFiles\%(AppleAssembliesToBundle.RecursiveDir)" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(UseNativeAOTRuntime)' == 'true'"> | ||
<BundleFiles Include="$(MicrosoftNetCoreAppRuntimePackDir)/runtimes/$(TargetOS)-$(TargetArchitecture)/native/icudt.dat" | ||
TargetDir="publish" /> | ||
<BundleFiles Include="$(MicrosoftNetCoreAppRuntimePackDir)/runtimes/$(TargetOS)-$(TargetArchitecture)/native/libicudata.a" | ||
TargetDir="publish" /> | ||
<BundleFiles Include="$(MicrosoftNetCoreAppRuntimePackDir)/runtimes/$(TargetOS)-$(TargetArchitecture)/native/libicuuc.a" | ||
TargetDir="publish" /> | ||
<BundleFiles Include="$(MicrosoftNetCoreAppRuntimePackDir)/runtimes/$(TargetOS)-$(TargetArchitecture)/native/libicui18n.a" | ||
TargetDir="publish" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup Condition="'$(DebuggerSupport)' == 'true'"> | ||
<!-- Add any pdb files, if available --> | ||
<_BundlePdbFiles Include="$([System.IO.Path]::ChangeExtension('%(AppleAssembliesToBundle.Identity)', '.pdb'))" /> | ||
|
@@ -103,8 +120,10 @@ | |
<_ApplePropertyNames Include="HybridGlobalization" /> | ||
<_ApplePropertyNames Include="AssemblyName" /> | ||
<_ApplePropertyNames Include="MonoEnableLLVM" /> | ||
<_ApplePropertyNames Include="MainLibraryFileName" /> | ||
<_ApplePropertyNames Include="UseConsoleUITemplate" /> | ||
<_ApplePropertyNames Include="UseRuntimeComponents" /> | ||
<_ApplePropertyNames Include="IsRuntimeTests" /> | ||
<_ApplePropertyNames Include="IncludesTestRunner" /> | ||
|
||
<_ApplePropertiesToPass | ||
Include="$(%(_ApplePropertyNames.Identity))" | ||
|
@@ -138,8 +157,9 @@ | |
<WriteLinesToFile File="$(PublishDir)xunit-excludes.txt" Lines="$(XunitExcludesTxtFileContent)" Overwrite="true" /> | ||
|
||
<PropertyGroup> | ||
<IncludesTestRunner Condition="'$(IncludesTestRunner)' == ''">true</IncludesTestRunner> | ||
<Optimized Condition="'$(Configuration)' == 'Release'">true</Optimized> | ||
<MainLibraryFileName Condition="'$(MainLibraryFileName)' == '' and '$(IsRuntimeTests)' != 'true'">AppleTestRunner.dll</MainLibraryFileName> | ||
<MainLibraryFileName Condition="'$(MainLibraryFileName)' == '' and '$(IsRuntimeTests)' != 'true' and '$(IncludesTestRunner)' == 'true'">AppleTestRunner.dll</MainLibraryFileName> | ||
|
||
<AppleBuildDir>$(PublishDir)</AppleBuildDir> | ||
<AppleBundleDir>$(BundleDir)</AppleBundleDir> | ||
|
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.
We don't have the capacity to run these on every PR. Even when we kick off runtime-ioslike manually, there may be a capacity issue running both mono and nativeaot together.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.
Nevermind, my concern was all libraries tests (when we get to it) and I see it's running smoke tests only.