Skip to content

Run tests on either macOS 12 or macOS 13 #5089

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.

Sign up for GitHub

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

Merged
auto-submit merged 4 commits into from
Oct 10, 2023
Merged

Run tests on either macOS 12 or macOS 13 #5089

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e76652d
update test scripts and ci.yaml for macOS 13
vashworth Oct 9, 2023
6fb70b7
create and delete simulator for pigeon tests, add some comments and n…
vashworth Oct 9, 2023
eef0513
Merge remote-tracking branch 'upstream/main' into macos_12_or_13
vashworth Oct 9, 2023
9d8d5c1
fix nits, run on macOS 12 or 13
vashworth Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
update test scripts and ci.yaml for macOS 13
  • Loading branch information
@vashworth
vashworth committed Oct 9, 2023
commit e76652dac543aa46de598ab277851dddf12403f1
4 changes: 2 additions & 2 deletions .ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ platform_properties:
[
{"dependency": "gems", "version": "v3.3.14"}
]
os: Mac-12
os: Mac-13
device_type: none
cpu: arm64
$flutter/osx_sdk : >-
Expand All @@ -77,7 +77,7 @@ platform_properties:
[
{"dependency": "gems", "version": "v3.3.14"}
]
os: Mac-12
os: Mac-13
device_type: none
cpu: x86
$flutter/osx_sdk : >-
Expand Down
12 changes: 12 additions & 0 deletions .ci/scripts/boot_simulator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash
# Copyright 2013 The Flutter Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
set -e

# The name here must match create_simulator.sh
readonly DEVICE_NAME=Flutter-iPhone

xcrun simctl boot "$DEVICE_NAME" || :
echo -e ""
xcrun simctl list
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Oct 9, 2023

Choose a reason for hiding this comment

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

Do we want to keep this echo?

Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

It's not needed, but I liked it - it puts a newline between the boot output and list output. Just gives it a little separation

14 changes: 14 additions & 0 deletions .ci/scripts/create_simulator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,19 @@ readonly DEVICE_NAME=Flutter-iPhone
readonly DEVICE=com.apple.CoreSimulator.SimDeviceType.iPhone-14
readonly OS=com.apple.CoreSimulator.SimRuntime.iOS-16-4

# Delete any existing devices named Flutter-iPhone. Having more than one may
# cause issues when builds target the device.
Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

For reference, here's an example of this problem in action: https://ci.chromium.org/ui/p/flutter/builders/try/Mac_arm64%20custom_package_tests%20master/7040/overview

Unclear if it's a problem new to macOS 13

echo -e "Deleting any existing devices names $DEVICE_NAME..."
RESULT=0
while [[ $RESULT == 0 ]]; do
xcrun simctl delete "$DEVICE_NAME" || RESULT=1
if [ $RESULT == 0 ]; then
echo -e "Deleted $DEVICE_NAME"
fi
done
echo -e ""

xcrun simctl list
echo -e "\nCreating $DEVICE_NAME $DEVICE $OS ...\n"
xcrun simctl create "$DEVICE_NAME" "$DEVICE" "$OS" | xargs xcrun simctl boot
xcrun simctl list
6 changes: 5 additions & 1 deletion .ci/targets/ios_platform_tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ tasks:
args: ["xcode-analyze", "--ios", "--ios-min-version=13.0"]
- name: native test
script: script/tool_runner.sh
args: ["native-test", "--ios", "--ios-destination", "platform=iOS Simulator,name=iPhone 14,OS=latest"]
args: ["native-test", "--ios", "--ios-destination", "platform=iOS Simulator,name=Flutter-iPhone,OS=16.4"]
Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

As we upgrade, there's no guarantee that there will be an iPhone 14 simulator. Better to use the simulator we create in the "create simulator" step (same for the pigeon test_suites.dart).

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Oct 9, 2023

Choose a reason for hiding this comment

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

Pigeon's test_suites is a weird case; we need it to run locally as well as in CI. So we don't want to hard-code it to a simulator that only exists in CI.

Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

Okay, so I updated Pigeon's test_suites to create and delete it's own simulator so that it'll work in CI and locally

FYI couple reasons I think that's the best approach:

  1. Due to new way Simulator runtimes are handled, when you use OS=latest or you omit the OS (which defaults to latest), it may use a newer simulator runtime that's not the default for the Xcode version. For example, if iOS 17 runtime is mounted, it'll use that instead of iOS 16.4 even if you're using Xcode 14.3. So even if there's an "iPhone 14" device with OS 16.4, it won't match against it since it's looking for iOS 17.
  2. There's no guarantee to be a Simulator named "iPhone 14" unless we create it beforehand (both in CI and locally)

Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

Also btw, I ran dart run tool/run_tests.dart in packages/packages/pigeon locally to make sure it passed locally

- name: boot simulator
# Ensure simulator is still booted
Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

While I was testing, sometimes the simulator would shutdown after "native test" and before "drive examples"

See https://luci-milo.appspot.com/ui/p/flutter/builders/try/Mac_arm64%20ios_platform_tests_shard_4%20master/7162/overview for example (I added logs to drive example, where you'll see the Flutter-iPhone simulator is shutdown)

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Oct 9, 2023

Choose a reason for hiding this comment

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

That's really weird. Is there any indication of why it's shutting down?

Copy link
Contributor Author

@vashworth vashworth Oct 9, 2023

Choose a reason for hiding this comment

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

No, the "native test" step shows the simulator is booted and works as expected. There's no logs that indicate it would be triggered to be shutdown. It also doesn't seem to happen every time.

script: .ci/scripts/boot_simulator.sh
infra_step: true # Note infra steps failing prevents "always" from running.
- name: drive examples
# `drive-examples` contains integration tests, which changes the UI of the application.
# This UI change sometimes affects `xctest`.
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/tool/shared/test_suites.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ Future<int> _runIOSPluginUnitTests(String testPluginPath) async {
return runXcodeBuild(
'$examplePath/ios',
sdk: 'iphonesimulator',
destination: 'platform=iOS Simulator,name=iPhone 14',
destination: 'platform=iOS Simulator,name=Flutter-iPhone,OS=16.4',
extraArguments: <String>['test'],
);
}
Expand Down