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

Move MacCatalyst to runtime-extra-platforms pipeline #64452

Merged

Conversation

MaximLipnin
Copy link
Contributor

@MaximLipnin MaximLipnin commented Jan 28, 2022

Since #63572 no longer happens in CI, we can promote MacCatalyst templates from the runtime-staging to the runtime-extra-platforms pipeline.

For now we will keep MacCatalyst arm64 legs in runtime-staging as they are not stable on OSX 11.00 queues (see #64452 (comment))

@ghost
Copy link

ghost commented Jan 28, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Since #63572 no longer happens in CI, we can promote MacCatalyst templates from the runtime-staging to the runtime-extra-platforms pipeline.

Author: MaximLipnin
Assignees: MaximLipnin
Labels:

area-Infrastructure-libraries

Milestone: -

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@steveisok
Copy link
Member

steveisok commented Jan 28, 2022

The plist timeout looks like it has been fixed.

Not sure if the MacCatalyst failures are totally solved. One of the functional tests failed under mysterious circumstances.

Steve Pfister added 3 commits January 28, 2022 18:07
… smoke tests to runtime.yml. Do the same to iOS_arm64 and tvOS_arm64 as they were missing previously.
@steveisok
Copy link
Member

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

buildConfig: Release
runtimeFlavor: mono
platforms:
- MacCatalyst_x64
Copy link
Member

Choose a reason for hiding this comment

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

Since the runtime-extra platforms leg doesn't run on every PR, should we add a mac catalyst leg to the main runtime pipeline or the dev-innerloop pipeline that DOES NOT RUN TESTS, just to protect the vertical build and to make sure we don't break developer inerloop scenarios?

@MaximLipnin
Copy link
Contributor Author

@premun Could you please look at MacCatalyst arm64 failure? It's the functional tests but they seem to fail randomly.
Some recent example: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-64452-merge-bb3ef8f27ca448e9aa/iOS.Simulator.Interpreter.Test/1/console.4367e664.log?sv=2019-07-07&se=2022-02-21T00%3A13%3A12Z&sr=c&sp=rl&sig=FOQyuT45d%2BVvzFoPbY41ZTMB1S8f%2B5y6qqSGfURaqyY%3D

+ sudo launchctl asuser 503 sudo -u helix-runner sh ./xharness-runner.apple.sh --target maccatalyst --command-timeout 2280 --timeout 00:30:00 --launch-timeout 00:30:00 --expected-exit-code 42 --app /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app --output-directory /tmp/helix/working/BDE009DC/w/ADB609AA/uploads
XHarness command issued: apple run --app /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app --output-directory /tmp/helix/working/BDE009DC/w/ADB609AA/uploads --target maccatalyst --timeout 00:30:00 --xcode /Applications/Xcode130.app -v --expected-exit-code 42
[16:14:49] info: Preparing run for maccatalyst
[16:14:49] info: Getting app bundle information from '/tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app'
[16:14:49] dbug: 
[16:14:49] dbug: Running /usr/libexec/PlistBuddy -c "Print CFBundleName" /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app/Contents/Info.plist
[16:14:49] dbug: Process PlistBuddy exited with 0
[16:14:49] dbug: 
[16:14:49] dbug: Running /usr/libexec/PlistBuddy -c "Print CFBundleIdentifier" /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app/Contents/Info.plist
[16:14:49] dbug: Process PlistBuddy exited with 0
[16:14:49] dbug: 
[16:14:49] dbug: Running /usr/libexec/PlistBuddy -c "Print UIRequiredDeviceCapabilities" /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app/Contents/Info.plist
[16:14:49] dbug: Process PlistBuddy exited with 1
[16:14:49] dbug: Property UIRequiredDeviceCapabilities not present in Info.plist, assuming 32-bit is not supported
[16:14:49] dbug: 
[16:14:49] dbug: Running /usr/libexec/PlistBuddy -c "Print CFBundleExecutable" /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app/Contents/Info.plist
[16:14:49] dbug: Process PlistBuddy exited with 0
[16:14:49] info: Starting 'iOS.Simulator.Interpreter.Test' on MacCatalyst
[16:14:49] dbug: *** Executing 'iOS.Simulator.Interpreter.Test' on MacCatalyst ***
[16:14:49] dbug: 
[16:14:49] dbug: Running chmod +x /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app/Contents/MacOS/iOS.Simulator.Interpreter.Test
[16:14:49] dbug: Process chmod exited with 0
[16:14:49] dbug: 
[16:14:49] dbug: Running /System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister -f /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app
[16:14:49] dbug: Process lsregister exited with 0
[16:14:49] dbug: 
[16:14:49] dbug: Running open -W /tmp/helix/working/BDE009DC/w/ADB609AA/e/iOS.Simulator.Interpreter.Test.app
[16:14:55] dbug: Process open exited with 0
[16:14:55] info: App run ended, no abnormal exit code detected (0 assumed)
[16:14:55] fail: Application has finished with exit code 0 but 42 was expected
[16:14:55] dbug: Saving diagnostics data to '/tmp/helix/working/BDE009DC/w/ADB609AA/e/diagnostics.json'
XHarness exit code: 71 (GENERAL_FAILURE)

@premun
Copy link
Member

premun commented Feb 1, 2022

So the issue is that on all Apple platforms (iOS, tvOS, MacCatalyst), we have problems detecting app's exit code. The original (Xamarin) way of testing these platforms is always via the XHarness TestRunner that is included in the apps.

For all scenarios where we don't have the test runner (HelloiOS, functional tests), where we only fire up the app and wait for it to quit, XHarness is scanning MacOS syslogs for the event where the app terminated with non-zero code and parse the exit code from there. Unfortunately, this is not 100% reliable and sometimes we just don't see this line in the logs. We have no power over flushing of this log and there might be other reasons like MacOS cycling the log away when it reaches some size and starting a new one.

We could implement some artifical waiting, though I it would be quite complicated. What I think would be better, since these tests only take few seconds, is to employ the DevWF retries and configure it to retry the run (can be on the same machine). I think this might help?

@MaximLipnin
Copy link
Contributor Author

What I think would be better, since these tests only take few seconds, is to employ the DevWF retries and configure it to retry the run (can be on the same machine). I think this might help?

That makes sense, thanks. What I need to do is just to update https://github.com/dotnet/runtime/blob/main/eng/test-configuration.json with an appropriate rule for the functional tests, right?

@premun
Copy link
Member

premun commented Feb 1, 2022

@MaximLipnin something like that, yeah. But to be honest, I am not an expert in this area so not sure if this file goes where it needs and also if we can scope it to some platforms only for example. I might delegate you to the First Responder channel where people will be able to help you.

@MaximLipnin
Copy link
Contributor Author

@premun Thanks for helping! BTW, there would also be an option like adding Console.WriteLine("exit-code=" + exitCode); to a test and extending the xharness parsing logic, but Alexander called it an "ugly hack" 😄

@premun
Copy link
Member

premun commented Feb 1, 2022

Well, the whole Apple platform is one giant hack to be honest 😅 We have already a switch for a similar flow called --signal-app-end back from when we had some issues with detecting app shutting down, so we could theoretically extend it. However I am not a fan of this solution as then we are testing something a bit different. We can definitely bring back the discussion. However, if we manage to utilize the test retries here and it will work, I think it's a preferred way.

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@steveisok
Copy link
Member

Doesn't look like retries happened on the MacCatalyst failure. @premun will we still see Attempt 1 and subsequent retries in the artifacts tab?

@MaximLipnin
Copy link
Contributor Author

Currently the test retry logic doesn't take into account our particular case 😢

Apart from it, another Alexander's suggestion: since xharness reads the whole /var/log/system.log, would it make sense to switch to calling the log command with a filter for the process ID?

@premun
Copy link
Member

premun commented Feb 2, 2022

Looking at the docs (Test Retry Documentation.md), it seems that the problem is that our work item doesn't return exit code 0 (point 4 in How to get on board). It seems that DevWF is looking for failed tests, not failed work items.

I tried the filtering before but that won't solve the problem as it only leaves out lines of log which is already missing what we need. The non-filtered log we are scanning is actually very short (~10 lines) as there's not much of system logging during the few seconds the app runs. For example for the last build from this PR it's here:

FWIW, the line we're looking for is this:

Feb 1 13:50:17 dci-mac-build-183 com.apple.xpc.launchd[1] (net.dot.iOS.Simulator.Aot-Llvm.Test.164872[75355]): Service exited with abnormal code: 42

@MaximLipnin
Copy link
Contributor Author

I fixed the wrong queue name in #65093.

The arm64 will have to go to 11.00 which might not pass as well as 10.15 does. Can we, for the time being, ignore that leg and move the rest? I will work on the exit codes and maybe we'll have a solution for it soon.

Updated to keep arm64 in runtime-staging for now

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

LGTM

@MaximLipnin
Copy link
Contributor Author

Do we need to wait for #65068 which is going to update the arcade dependency to the latest version?

@premun
Copy link
Member

premun commented Feb 10, 2022

@MaximLipnin there is a blocker and the dependency PR will take one more day to complete. However, we cannot merge this PR as it would break runtime due to some EOL errors. So we need to wait.

# Conflicts:
#	eng/Version.Details.xml
#	eng/Versions.props
#	global.json
@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…mitCalliNonBlittable test failing on MacCatalyst x64
@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin
Copy link
Contributor Author

Failures are unrelated, merging

@MaximLipnin MaximLipnin merged commit 6a69afc into dotnet:main Feb 15, 2022
@MaximLipnin MaximLipnin deleted the move_maccatalyst_to_extra_platfroms_pipeline branch February 15, 2022 12:08
MaximLipnin added a commit that referenced this pull request Feb 17, 2022
…e-extra-platforms pipeline (#65427)

It's a follow-up on #64452.

The change is to move MacCatalyst Arm64 legs (basic and sandbox-enabled) from runtime-staging to runtime-extra-platforms pipeline. Those legs will be available for manual per-PR run as we have enough devices for that.
@ghost ghost locked as resolved and limited conversation to collaborators Mar 17, 2022
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.

5 participants